From 01d4436c763e512897330500f1409103081b7777 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 24 Apr 2026 12:50:38 +0000 Subject: [PATCH] improve: use SearchValues in GatewayUrlHelper; add SystemCapability coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 5 (Coding Improvements): - GatewayUrlHelper.s_authorityTerminators: char[] → SearchValues matching the SIMD-optimized pattern already used in ShellQuoting.cs - Span-based IndexOfAny avoids the start-offset overload Task 9 (Testing Improvements): - Add 10 new SystemCapability tests covering previously-untested paths: - system.run.prepare: array command, string command, plan structure, missing-command error - system.execApprovals.get: no-policy (disabled), with-policy (enabled + rules) - system.execApprovals.set: no-policy error, updates rules + ruleCount - system.run: blocked env var (PATH) → error; allowed env var passes through All 660 Shared tests pass (↑10), 122 Tray tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/OpenClaw.Shared/GatewayUrlHelper.cs | 13 +- .../OpenClaw.Shared.Tests/CapabilityTests.cs | 216 ++++++++++++++++++ 2 files changed, 223 insertions(+), 6 deletions(-) diff --git a/src/OpenClaw.Shared/GatewayUrlHelper.cs b/src/OpenClaw.Shared/GatewayUrlHelper.cs index 40bcd341..d9ab6d4a 100644 --- a/src/OpenClaw.Shared/GatewayUrlHelper.cs +++ b/src/OpenClaw.Shared/GatewayUrlHelper.cs @@ -1,4 +1,5 @@ using System; +using System.Buffers; namespace OpenClaw.Shared; @@ -6,7 +7,10 @@ public static class GatewayUrlHelper { public const string ValidationMessage = "Gateway URL must be a valid URL (ws://, wss://, http://, or https://)."; - private static readonly char[] s_authorityTerminators = { '/', '?', '#' }; + // SearchValues builds an optimized SIMD lookup structure at startup, + // consistent with the same pattern used in ShellQuoting.cs. + private static readonly SearchValues s_authorityTerminators = + SearchValues.Create("/?#"); public static bool IsValidGatewayUrl(string? gatewayUrl) => TryNormalizeWebSocketUrl(gatewayUrl, out _); @@ -143,11 +147,8 @@ private static string RemoveUserInfo(string url) } var authorityStart = schemeSeparator + 3; - var authorityEnd = url.IndexOfAny(s_authorityTerminators, authorityStart); - if (authorityEnd < 0) - { - authorityEnd = url.Length; - } + int relativeEnd = url.AsSpan(authorityStart).IndexOfAny(s_authorityTerminators); + var authorityEnd = relativeEnd < 0 ? url.Length : authorityStart + relativeEnd; var atIndex = url.IndexOf('@', authorityStart); if (atIndex < 0 || atIndex >= authorityEnd) diff --git a/tests/OpenClaw.Shared.Tests/CapabilityTests.cs b/tests/OpenClaw.Shared.Tests/CapabilityTests.cs index e32980ce..85beb679 100644 --- a/tests/OpenClaw.Shared.Tests/CapabilityTests.cs +++ b/tests/OpenClaw.Shared.Tests/CapabilityTests.cs @@ -306,6 +306,222 @@ public void ResolveExecutable_RejectsPathTraversal() Assert.Null(SystemCapability.ResolveExecutable("C:\\Windows\\cmd")); } + // --- system.run.prepare --- + + [Fact] + public async Task RunPrepare_ReturnsCommandText_ForArray() + { + var cap = new SystemCapability(NullLogger.Instance); + var req = new NodeInvokeRequest + { + Id = "p1", + Command = "system.run.prepare", + Args = Parse("""{"command":["echo","hello world"]}""") + }; + + var res = await cap.ExecuteAsync(req); + Assert.True(res.Ok); + var payload = JsonSerializer.Deserialize(JsonSerializer.Serialize(res.Payload)); + Assert.True(payload.TryGetProperty("cmdText", out var cmdText)); + Assert.Contains("echo", cmdText.GetString()); + } + + [Fact] + public async Task RunPrepare_ReturnsCommandText_ForString() + { + var cap = new SystemCapability(NullLogger.Instance); + var req = new NodeInvokeRequest + { + Id = "p2", + Command = "system.run.prepare", + Args = Parse("""{"command":"hostname","rawCommand":"hostname"}""") + }; + + var res = await cap.ExecuteAsync(req); + Assert.True(res.Ok); + var payload = JsonSerializer.Deserialize(JsonSerializer.Serialize(res.Payload)); + Assert.True(payload.TryGetProperty("cmdText", out var cmdText)); + Assert.Equal("hostname", cmdText.GetString()); + } + + [Fact] + public async Task RunPrepare_ReturnsPlan_WithArgvAndCwd() + { + var cap = new SystemCapability(NullLogger.Instance); + var req = new NodeInvokeRequest + { + Id = "p3", + Command = "system.run.prepare", + Args = Parse("""{"command":["ls","-la"],"cwd":"/tmp","agentId":"agent1","sessionKey":"sk1"}""") + }; + + var res = await cap.ExecuteAsync(req); + Assert.True(res.Ok); + var payload = JsonSerializer.Deserialize(JsonSerializer.Serialize(res.Payload)); + Assert.True(payload.TryGetProperty("plan", out var plan)); + Assert.True(plan.TryGetProperty("argv", out var argv)); + Assert.Equal(2, argv.GetArrayLength()); + Assert.True(plan.TryGetProperty("cwd", out var cwd)); + Assert.Equal("/tmp", cwd.GetString()); + Assert.True(plan.TryGetProperty("agentId", out var agentId)); + Assert.Equal("agent1", agentId.GetString()); + } + + [Fact] + public async Task RunPrepare_ReturnsError_WhenMissingCommand() + { + var cap = new SystemCapability(NullLogger.Instance); + var req = new NodeInvokeRequest + { + Id = "p4", + Command = "system.run.prepare", + Args = Parse("""{}""") + }; + + var res = await cap.ExecuteAsync(req); + Assert.False(res.Ok); + Assert.Contains("Missing command", res.Error); + } + + // --- system.execApprovals.get / set --- + + [Fact] + public async Task ExecApprovalsGet_WhenNoPolicyConfigured_ReturnsDisabled() + { + var cap = new SystemCapability(NullLogger.Instance); + var req = new NodeInvokeRequest + { + Id = "ea1", + Command = "system.execApprovals.get", + Args = Parse("""{}""") + }; + + var res = await cap.ExecuteAsync(req); + Assert.True(res.Ok); + var payload = JsonSerializer.Deserialize(JsonSerializer.Serialize(res.Payload)); + Assert.True(payload.TryGetProperty("enabled", out var enabled)); + Assert.False(enabled.GetBoolean()); + } + + [Fact] + public async Task ExecApprovalsGet_WhenPolicySet_ReturnsRules() + { + var tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(tempDir); + try + { + var cap = new SystemCapability(NullLogger.Instance); + var policy = new ExecApprovalPolicy(tempDir, NullLogger.Instance); + cap.SetApprovalPolicy(policy); + + var req = new NodeInvokeRequest + { + Id = "ea2", + Command = "system.execApprovals.get", + Args = Parse("""{}""") + }; + + var res = await cap.ExecuteAsync(req); + Assert.True(res.Ok); + var payload = JsonSerializer.Deserialize(JsonSerializer.Serialize(res.Payload)); + Assert.True(payload.TryGetProperty("enabled", out var enabled)); + Assert.True(enabled.GetBoolean()); + Assert.True(payload.TryGetProperty("rules", out _)); + } + finally + { + try { Directory.Delete(tempDir, true); } catch { } + } + } + + [Fact] + public async Task ExecApprovalsSet_WhenNoPolicyConfigured_ReturnsError() + { + var cap = new SystemCapability(NullLogger.Instance); + var req = new NodeInvokeRequest + { + Id = "ea3", + Command = "system.execApprovals.set", + Args = Parse("""{"rules":[]}""") + }; + + var res = await cap.ExecuteAsync(req); + Assert.False(res.Ok); + Assert.Contains("No exec policy configured", res.Error); + } + + [Fact] + public async Task ExecApprovalsSet_UpdatesRules() + { + var tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(tempDir); + try + { + var cap = new SystemCapability(NullLogger.Instance); + var policy = new ExecApprovalPolicy(tempDir, NullLogger.Instance); + cap.SetApprovalPolicy(policy); + + var req = new NodeInvokeRequest + { + Id = "ea4", + Command = "system.execApprovals.set", + Args = Parse("""{"rules":[{"pattern":"git *","action":"allow","description":"Allow git","enabled":true}],"defaultAction":"deny"}""") + }; + + var res = await cap.ExecuteAsync(req); + Assert.True(res.Ok); + var payload = JsonSerializer.Deserialize(JsonSerializer.Serialize(res.Payload)); + Assert.True(payload.TryGetProperty("updated", out var updated)); + Assert.True(updated.GetBoolean()); + Assert.True(payload.TryGetProperty("ruleCount", out var ruleCount)); + Assert.Equal(1, ruleCount.GetInt32()); + } + finally + { + try { Directory.Delete(tempDir, true); } catch { } + } + } + + // --- system.run with blocked env vars --- + + [Fact] + public async Task Run_BlockedEnvVar_ReturnsError() + { + var cap = new SystemCapability(NullLogger.Instance); + cap.SetCommandRunner(new FakeCommandRunner()); + + var req = new NodeInvokeRequest + { + Id = "e1", + Command = "system.run", + Args = Parse("""{"command":["echo","test"],"env":{"PATH":"C:\\evil"}}""") + }; + + var res = await cap.ExecuteAsync(req); + Assert.False(res.Ok); + Assert.Contains("PATH", res.Error); + } + + [Fact] + public async Task Run_AllowedEnvVar_Passes() + { + var cap = new SystemCapability(NullLogger.Instance); + var runner = new FakeCommandRunner(); + cap.SetCommandRunner(runner); + + var req = new NodeInvokeRequest + { + Id = "e2", + Command = "system.run", + Args = Parse("""{"command":["echo","test"],"env":{"MY_CUSTOM_VAR":"hello"}}""") + }; + + var res = await cap.ExecuteAsync(req); + Assert.True(res.Ok); + Assert.NotNull(runner.LastRequest!.Env); + Assert.True(runner.LastRequest.Env!.ContainsKey("MY_CUSTOM_VAR")); + } + private class FakeCommandRunner : ICommandRunner { public string Name => "fake";