diff --git a/src/OpenClaw.Shared/Capabilities/SystemCapability.cs b/src/OpenClaw.Shared/Capabilities/SystemCapability.cs index 91d0e1af..c6d55ef5 100644 --- a/src/OpenClaw.Shared/Capabilities/SystemCapability.cs +++ b/src/OpenClaw.Shared/Capabilities/SystemCapability.cs @@ -275,9 +275,13 @@ private async Task HandleRunAsync(NodeInvokeRequest request) } Logger.Info($"[system.run] corr={correlationId} decision={v2Result.Code} reason={v2Result.Reason}"); + if (v2Result.Code == ExecApprovalV2Code.Allowed) + { + // V2 coordinator already evaluated policy — execute directly. + Logger.Info($"[system.run] corr={correlationId} v2-approved executing"); + return await ExecuteRunRequestAsync(request, correlationId); + } // Rail 1: no silent fallback to legacy regardless of result code. - // In PR1 only ExecApprovalV2NullHandler exists (always unavailable); the real - // coordinator that can produce an allow decision is wired in PR7/PR8. return Error($"exec-approvals-v2: {v2Result.Code} ({v2Result.Reason})"); } @@ -288,13 +292,83 @@ private async Task HandleRunAsync(NodeInvokeRequest request) { return Error("Command execution not available"); } - + + // Parse argv and parameters from raw request. + var legacyParsed = ParseRunRequest(request); + if (legacyParsed.Error != null) + return Error(legacyParsed.Error); + + var (legacyCommand, legacyArgs, legacyShell, legacyCwd, legacyTimeoutMs, legacyEnv) = legacyParsed; + + var fullCommand = legacyArgs != null + ? FormatExecCommand([legacyCommand!, ..legacyArgs]) + : legacyCommand; + + Logger.Info($"system.run: {fullCommand} (shell={legacyShell ?? "auto"}, timeout={legacyTimeoutMs}ms)"); + + // Check exec approval policy + if (_approvalPolicy != null) + { + var approval = _approvalPolicy.Evaluate(fullCommand!, legacyShell); + if (!await EnsureApprovedAsync(fullCommand!, legacyShell, approval)) + { + Logger.Warn($"system.run DENIED: {fullCommand} ({approval.Reason})"); + return Error($"Command denied by exec policy: {approval.Reason}"); + } + + var parseResult = ExecShellWrapperParser.Expand(fullCommand!, legacyShell); + if (!string.IsNullOrWhiteSpace(parseResult.Error)) + { + Logger.Warn($"system.run DENIED: {fullCommand} ({parseResult.Error})"); + return Error($"Command denied by exec policy: {parseResult.Error}"); + } + + foreach (var target in parseResult.Targets) + { + var innerApproval = _approvalPolicy.Evaluate(target.Command, target.Shell); + if (!await EnsureApprovedAsync(target.Command, target.Shell, innerApproval)) + { + Logger.Warn($"system.run DENIED: {target.Command} ({innerApproval.Reason})"); + return Error($"Command denied by exec policy: {innerApproval.Reason}"); + } + } + } + + return await RunCommandAsync(legacyCommand!, legacyArgs, legacyShell, legacyCwd, legacyTimeoutMs, legacyEnv); + } + + /// + /// Executes a V2-approved request without re-checking policy. + /// Called by the V2 routing path after the handler returns . + /// + private async Task ExecuteRunRequestAsync(NodeInvokeRequest request, string correlationId) + { + if (_commandRunner == null) + return Error("Command execution not available"); + + var parsed = ParseRunRequest(request); + if (parsed.Error != null) + return Error(parsed.Error); + + var (command, args, shell, cwd, timeoutMs, env) = parsed; + var fullCommand = args != null ? FormatExecCommand([command!, ..args]) : command; + Logger.Info($"[system.run] corr={correlationId} v2-execute cmd={fullCommand} shell={shell ?? "auto"} timeout={timeoutMs}ms"); + + return await RunCommandAsync(command!, args, shell, cwd, timeoutMs, env); + } + + /// + /// Parses the raw NodeInvokeRequest fields common to both the legacy and V2 execution paths. + /// Returns a with set on failure. + /// + private ParsedRunRequest ParseRunRequest(NodeInvokeRequest request) + { // Per OpenClaw spec, "command" is an argv array (e.g. ["echo","Hello"]). // Also accept a plain string for backward compatibility. var argv = TryParseArgv(request.Args); string? command = argv?[0]; string[]? args = argv?.Length > 1 ? argv[1..] : null; - + // When command is a string, also check for separate "args" array if (argv?.Length == 1 && request.Args.TryGetProperty("args", out var argsEl) && argsEl.ValueKind == System.Text.Json.JsonValueKind.Array) @@ -308,12 +382,10 @@ private async Task HandleRunAsync(NodeInvokeRequest request) if (list.Count > 0) args = list.ToArray(); } - + if (string.IsNullOrWhiteSpace(command)) - { - return Error("Missing command parameter"); - } - + return ParsedRunRequest.Fail("Missing command parameter"); + var shell = GetStringArg(request.Args, "shell"); var cwd = GetStringArg(request.Args, "cwd"); var timeoutMs = GetIntArg(request.Args, "timeoutMs", @@ -325,7 +397,7 @@ private async Task HandleRunAsync(NodeInvokeRequest request) // from accidentally outliving the tray. if (timeoutMs <= 0) timeoutMs = DefaultRunTimeoutMs; if (timeoutMs > MaxRunTimeoutMs) timeoutMs = MaxRunTimeoutMs; - + // Parse env dict if present Dictionary? env = null; if (request.Args.ValueKind != System.Text.Json.JsonValueKind.Undefined && @@ -347,51 +419,24 @@ private async Task HandleRunAsync(NodeInvokeRequest request) Array.Sort(blockedNames, StringComparer.OrdinalIgnoreCase); var blockedList = string.Join(", ", blockedNames); Logger.Warn($"system.run DENIED: blocked environment overrides [{blockedList}]"); - return Error($"Unsafe environment variable override blocked: {blockedList}"); + return ParsedRunRequest.Fail($"Unsafe environment variable override blocked: {blockedList}"); } env = envResult.Allowed; - - // Build the full command string for policy evaluation and logging. - // When command arrives as an argv array, we must evaluate the entire - // command line — not just argv[0] — so policy rules like "rm *" correctly - // match "rm -rf /". - var fullCommand = args != null - ? FormatExecCommand([command!, ..args]) - : command; - - Logger.Info($"system.run: {fullCommand} (shell={shell ?? "auto"}, timeout={timeoutMs}ms)"); - - // Check exec approval policy - if (_approvalPolicy != null) - { - var approval = _approvalPolicy.Evaluate(fullCommand, shell); - if (!await EnsureApprovedAsync(fullCommand, shell, approval)) - { - Logger.Warn($"system.run DENIED: {fullCommand} ({approval.Reason})"); - return Error($"Command denied by exec policy: {approval.Reason}"); - } - var parseResult = ExecShellWrapperParser.Expand(fullCommand, shell); - if (!string.IsNullOrWhiteSpace(parseResult.Error)) - { - Logger.Warn($"system.run DENIED: {fullCommand} ({parseResult.Error})"); - return Error($"Command denied by exec policy: {parseResult.Error}"); - } + return ParsedRunRequest.Ok(command!, args, shell, cwd, timeoutMs, env); + } - foreach (var target in parseResult.Targets) - { - var innerApproval = _approvalPolicy.Evaluate(target.Command, target.Shell); - if (!await EnsureApprovedAsync(target.Command, target.Shell, innerApproval)) - { - Logger.Warn($"system.run DENIED: {target.Command} ({innerApproval.Reason})"); - return Error($"Command denied by exec policy: {innerApproval.Reason}"); - } - } - } - + private async Task RunCommandAsync( + string command, + string[]? args, + string? shell, + string? cwd, + int timeoutMs, + Dictionary? env) + { try { - var result = await _commandRunner.RunAsync(new CommandRequest + var result = await _commandRunner!.RunAsync(new CommandRequest { Command = command, Args = args, @@ -400,7 +445,7 @@ private async Task HandleRunAsync(NodeInvokeRequest request) TimeoutMs = timeoutMs, Env = env }); - + return Success(new { stdout = result.Stdout, @@ -417,6 +462,35 @@ private async Task HandleRunAsync(NodeInvokeRequest request) } } + /// + /// Parsed, sanitized system.run parameters — shared between legacy and V2 execution paths. + /// + private sealed class ParsedRunRequest + { + public string? Command { get; private init; } + public string[]? Args { get; private init; } + public string? Shell { get; private init; } + public string? Cwd { get; private init; } + public int TimeoutMs { get; private init; } + public Dictionary? Env { get; private init; } + public string? Error { get; private init; } + + public void Deconstruct( + out string? command, out string[]? args, out string? shell, + out string? cwd, out int timeoutMs, out Dictionary? env) + { + command = Command; args = Args; shell = Shell; + cwd = Cwd; timeoutMs = TimeoutMs; env = Env; + } + + public static ParsedRunRequest Ok( + string command, string[]? args, string? shell, + string? cwd, int timeoutMs, Dictionary? env) + => new() { Command = command, Args = args, Shell = shell, Cwd = cwd, TimeoutMs = timeoutMs, Env = env }; + + public static ParsedRunRequest Fail(string error) => new() { Error = error }; + } + private async Task EnsureApprovedAsync( string command, string? shell, diff --git a/src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2PolicyHandler.cs b/src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2PolicyHandler.cs new file mode 100644 index 00000000..3cce44bc --- /dev/null +++ b/src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2PolicyHandler.cs @@ -0,0 +1,73 @@ +using System.Threading.Tasks; + +namespace OpenClaw.Shared.ExecApprovals; + +/// +/// V2 exec approval handler: validates input and evaluates the configured +/// ExecApprovalPolicy to decide Allow / SecurityDeny / AllowlistMiss. +/// +/// This is the "PR7 coordinator" referred to in SystemCapability comments. +/// When installed via SetV2Handler and a command is approved, SystemCapability +/// executes it via the command runner already wired on the legacy path. +/// +/// Shell-wrapper expansion mirrors the legacy path: after the top-level policy +/// check passes, each shell-wrapped sub-command is also evaluated so that +/// `cmd /c "rm /"` is not approved by a rule for `cmd`. +/// +public sealed class ExecApprovalV2PolicyHandler : IExecApprovalV2Handler +{ + private readonly ExecApprovalPolicy _policy; + private readonly IOpenClawLogger _logger; + + public ExecApprovalV2PolicyHandler(ExecApprovalPolicy policy, IOpenClawLogger logger) + { + _policy = policy; + _logger = logger; + } + + public Task HandleAsync(NodeInvokeRequest request, string correlationId) + { + // Step 1: Structural input validation. + var validation = ExecApprovalV2InputValidator.Validate(request); + if (!validation.IsValid) + { + _logger.Info($"[exec-v2] corr={correlationId} validation-failed reason={validation.Error!.Reason}"); + return Task.FromResult(validation.Error!); + } + + var validated = validation.Request!; + var commandString = ShellQuoting.FormatExecCommand(validated.Argv); + + // Step 2: Top-level policy check. + var topResult = _policy.Evaluate(commandString, validated.Shell); + _logger.Info($"[exec-v2] corr={correlationId} policy={topResult.Action} pattern={topResult.MatchedPattern ?? "(default)"}"); + + if (topResult.Action == ExecApprovalAction.Deny) + return Task.FromResult(ExecApprovalV2Result.SecurityDeny(topResult.Reason ?? "policy-deny")); + + if (topResult.Action == ExecApprovalAction.Prompt) + return Task.FromResult(ExecApprovalV2Result.AllowlistMiss("prompt-required")); + + // Step 3: Shell-wrapper expansion — ensure wrapped sub-commands also pass policy. + var parseResult = ExecShellWrapperParser.Expand(commandString, validated.Shell); + if (!string.IsNullOrWhiteSpace(parseResult.Error)) + { + _logger.Warn($"[exec-v2] corr={correlationId} shell-parse-denied reason={parseResult.Error}"); + return Task.FromResult(ExecApprovalV2Result.SecurityDeny(parseResult.Error)); + } + + foreach (var target in parseResult.Targets) + { + var innerResult = _policy.Evaluate(target.Command, target.Shell); + if (innerResult.Action == ExecApprovalAction.Deny) + { + _logger.Warn($"[exec-v2] corr={correlationId} inner-policy-deny cmd={target.Command}"); + return Task.FromResult(ExecApprovalV2Result.SecurityDeny(innerResult.Reason ?? "inner-policy-deny")); + } + if (innerResult.Action == ExecApprovalAction.Prompt) + return Task.FromResult(ExecApprovalV2Result.AllowlistMiss("inner-prompt-required")); + } + + return Task.FromResult(ExecApprovalV2Result.Allowed()); + } +} diff --git a/src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2Result.cs b/src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2Result.cs index b175d515..f439f477 100644 --- a/src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2Result.cs +++ b/src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2Result.cs @@ -6,6 +6,7 @@ namespace OpenClaw.Shared.ExecApprovals; public enum ExecApprovalV2Code { Unavailable, + Allowed, SecurityDeny, AllowlistMiss, UserDenied, @@ -31,6 +32,9 @@ private ExecApprovalV2Result(ExecApprovalV2Code code, string reason) public static ExecApprovalV2Result Unavailable(string reason = "Handler not available") => new(ExecApprovalV2Code.Unavailable, reason); + public static ExecApprovalV2Result Allowed(string reason = "policy-allow") + => new(ExecApprovalV2Code.Allowed, reason); + public static ExecApprovalV2Result SecurityDeny(string reason) => new(ExecApprovalV2Code.SecurityDeny, reason); diff --git a/tests/OpenClaw.Shared.Tests/ExecApprovalV2PolicyHandlerTests.cs b/tests/OpenClaw.Shared.Tests/ExecApprovalV2PolicyHandlerTests.cs new file mode 100644 index 00000000..27f379ec --- /dev/null +++ b/tests/OpenClaw.Shared.Tests/ExecApprovalV2PolicyHandlerTests.cs @@ -0,0 +1,280 @@ +using System; +using System.IO; +using System.Text.Json; +using System.Threading.Tasks; +using Xunit; +using OpenClaw.Shared; +using OpenClaw.Shared.Capabilities; +using OpenClaw.Shared.ExecApprovals; + +namespace OpenClaw.Shared.Tests; + +/// +/// Tests for ExecApprovalV2PolicyHandler: the PR7 coordinator that bridges +/// V2 input validation and ExecApprovalPolicy evaluation. +/// +public class ExecApprovalV2PolicyHandlerTests +{ + private static JsonElement Parse(string json) + { + using var doc = JsonDocument.Parse(json); + return doc.RootElement.Clone(); + } + + private static NodeInvokeRequest RunRequest(string commandJson = """{"command":["echo","hello"]}""") + => new() { Id = "r1", Command = "system.run", Args = Parse(commandJson) }; + + private (ExecApprovalV2PolicyHandler handler, string tempDir) MakeHandler( + Action? configure = null) + { + var tempDir = Path.Combine(Path.GetTempPath(), $"v2policy-{Guid.NewGuid():N}"); + Directory.CreateDirectory(tempDir); + var policy = new ExecApprovalPolicy(tempDir, NullLogger.Instance); + configure?.Invoke(policy); + return (new ExecApprovalV2PolicyHandler(policy, NullLogger.Instance), tempDir); + } + + // ------------------------------------------------------------------------- + // 1. Allow rules → Allowed + // ------------------------------------------------------------------------- + + [Fact] + public async Task AllowRule_ReturnsAllowed() + { + var (handler, dir) = MakeHandler(p => + p.SetRules( + [new ExecApprovalRule { Pattern = "echo *", Action = ExecApprovalAction.Allow }], + ExecApprovalAction.Deny)); + try + { + var result = await handler.HandleAsync(RunRequest(), "corr01"); + Assert.Equal(ExecApprovalV2Code.Allowed, result.Code); + } + finally { TryDelete(dir); } + } + + [Fact] + public async Task AllowRuleNoArgs_ReturnsAllowed() + { + var (handler, dir) = MakeHandler(p => + p.SetRules( + [new ExecApprovalRule { Pattern = "echo*", Action = ExecApprovalAction.Allow }], + ExecApprovalAction.Deny)); + try + { + // echo with no args + var result = await handler.HandleAsync(RunRequest("""{"command":["echo"]}"""), "corr02"); + Assert.Equal(ExecApprovalV2Code.Allowed, result.Code); + } + finally { TryDelete(dir); } + } + + // ------------------------------------------------------------------------- + // 2. Deny rules → SecurityDeny + // ------------------------------------------------------------------------- + + [Fact] + public async Task DenyRule_ReturnsSecurityDeny() + { + var (handler, dir) = MakeHandler(p => + p.SetRules( + [new ExecApprovalRule { Pattern = "*", Action = ExecApprovalAction.Deny }], + ExecApprovalAction.Deny)); + try + { + var result = await handler.HandleAsync(RunRequest(), "corr03"); + Assert.Equal(ExecApprovalV2Code.SecurityDeny, result.Code); + } + finally { TryDelete(dir); } + } + + [Fact] + public async Task DenyRule_ReasonPreserved() + { + var (handler, dir) = MakeHandler(p => + p.SetRules( + [new ExecApprovalRule { Pattern = "*", Action = ExecApprovalAction.Deny, Description = "blocked by test" }], + ExecApprovalAction.Deny)); + try + { + var result = await handler.HandleAsync(RunRequest(), "corr04"); + Assert.Equal(ExecApprovalV2Code.SecurityDeny, result.Code); + Assert.NotEmpty(result.Reason); + } + finally { TryDelete(dir); } + } + + // ------------------------------------------------------------------------- + // 3. Prompt rules → AllowlistMiss + // ------------------------------------------------------------------------- + + [Fact] + public async Task PromptRule_ReturnsAllowlistMiss() + { + var (handler, dir) = MakeHandler(p => + p.SetRules( + [new ExecApprovalRule { Pattern = "*", Action = ExecApprovalAction.Prompt }], + ExecApprovalAction.Deny)); + try + { + var result = await handler.HandleAsync(RunRequest(), "corr05"); + Assert.Equal(ExecApprovalV2Code.AllowlistMiss, result.Code); + } + finally { TryDelete(dir); } + } + + // ------------------------------------------------------------------------- + // 4. Input validation failures → ValidationFailed (not SecurityDeny) + // ------------------------------------------------------------------------- + + [Fact] + public async Task MissingCommand_ReturnsValidationFailed() + { + var (handler, dir) = MakeHandler(p => + p.SetRules( + [new ExecApprovalRule { Pattern = "*", Action = ExecApprovalAction.Allow }], + ExecApprovalAction.Allow)); + try + { + var req = new NodeInvokeRequest { Id = "r1", Command = "system.run", Args = Parse("""{"timeout":5000}""") }; + var result = await handler.HandleAsync(req, "corr06"); + Assert.Equal(ExecApprovalV2Code.ValidationFailed, result.Code); + } + finally { TryDelete(dir); } + } + + [Fact] + public async Task MalformedCommand_ReturnsValidationFailed() + { + var (handler, dir) = MakeHandler(); + try + { + var req = new NodeInvokeRequest { Id = "r1", Command = "system.run", Args = Parse("""{"command":42}""") }; + var result = await handler.HandleAsync(req, "corr07"); + Assert.Equal(ExecApprovalV2Code.ValidationFailed, result.Code); + } + finally { TryDelete(dir); } + } + + // ------------------------------------------------------------------------- + // 5. Shell-wrapper inner command also evaluated + // ------------------------------------------------------------------------- + + [Fact] + public async Task ShellWrapper_InnerDeniedCommand_ReturnsSecurityDeny() + { + // Allow cmd.exe but deny rmdir. A wrapped "cmd /c rmdir /s /q C:\" must be denied. + var (handler, dir) = MakeHandler(p => + p.SetRules( + [ + new ExecApprovalRule { Pattern = "cmd*", Action = ExecApprovalAction.Allow }, + new ExecApprovalRule { Pattern = "rmdir*", Action = ExecApprovalAction.Deny }, + ], + ExecApprovalAction.Deny)); + try + { + var req = RunRequest("""{"command":["cmd","/c","rmdir /s /q C:\\"]}"""); + var result = await handler.HandleAsync(req, "corr08"); + Assert.Equal(ExecApprovalV2Code.SecurityDeny, result.Code); + } + finally { TryDelete(dir); } + } + + // ------------------------------------------------------------------------- + // 6. Handler never throws (defensive) + // ------------------------------------------------------------------------- + + [Fact] + public async Task Handler_DoesNotThrow_OnValidRequest() + { + var (handler, dir) = MakeHandler(); + try + { + var ex = await Record.ExceptionAsync(() => handler.HandleAsync(RunRequest(), "corr09")); + Assert.Null(ex); + } + finally { TryDelete(dir); } + } + + // ------------------------------------------------------------------------- + // 7. Integration: policy handler + SystemCapability → execution + // ------------------------------------------------------------------------- + + [Fact] + public async Task Integration_PolicyAllow_V2PathExecutesCommand() + { + var tempDir = Path.Combine(Path.GetTempPath(), $"v2int-{Guid.NewGuid():N}"); + Directory.CreateDirectory(tempDir); + try + { + var policy = new ExecApprovalPolicy(tempDir, NullLogger.Instance); + policy.SetRules( + [new ExecApprovalRule { Pattern = "echo*", Action = ExecApprovalAction.Allow }], + ExecApprovalAction.Deny); + + var handler = new ExecApprovalV2PolicyHandler(policy, NullLogger.Instance); + var runner = new FakeRunner(); + var cap = new SystemCapability(NullLogger.Instance); + cap.SetCommandRunner(runner); + cap.SetV2Handler(handler); + + var res = await cap.ExecuteAsync(RunRequest()); + + Assert.True(res.Ok, $"Expected Ok: {res.Error}"); + Assert.NotNull(runner.LastRequest); + } + finally { TryDelete(tempDir); } + } + + [Fact] + public async Task Integration_PolicyDeny_V2PathReturnsError() + { + var tempDir = Path.Combine(Path.GetTempPath(), $"v2int-{Guid.NewGuid():N}"); + Directory.CreateDirectory(tempDir); + try + { + var policy = new ExecApprovalPolicy(tempDir, NullLogger.Instance); + policy.SetRules( + [new ExecApprovalRule { Pattern = "*", Action = ExecApprovalAction.Deny }], + ExecApprovalAction.Deny); + + var handler = new ExecApprovalV2PolicyHandler(policy, NullLogger.Instance); + var runner = new FakeRunner(); + var cap = new SystemCapability(NullLogger.Instance); + cap.SetCommandRunner(runner); + cap.SetV2Handler(handler); + + var res = await cap.ExecuteAsync(RunRequest()); + + Assert.False(res.Ok); + Assert.Null(runner.LastRequest); // runner not called + } + finally { TryDelete(tempDir); } + } + + private static void TryDelete(string path) + { + try { Directory.Delete(path, recursive: true); } catch { } + } + + private sealed class FakeRunner : ICommandRunner + { + public string Name => "fake"; + public CommandRequest? LastRequest { get; private set; } + + public Task RunAsync(CommandRequest request, System.Threading.CancellationToken ct = default) + { + LastRequest = request; + return Task.FromResult(new CommandResult { Stdout = "ok", ExitCode = 0 }); + } + } + + private sealed class NullLogger : IOpenClawLogger + { + public static readonly NullLogger Instance = new(); + public void Info(string message) { } + public void Debug(string message) { } + public void Warn(string message) { } + public void Error(string message, Exception? ex = null) { } + } +} diff --git a/tests/OpenClaw.Shared.Tests/ExecApprovalV2RoutingTests.cs b/tests/OpenClaw.Shared.Tests/ExecApprovalV2RoutingTests.cs index 6fc2e2b0..ecdef635 100644 --- a/tests/OpenClaw.Shared.Tests/ExecApprovalV2RoutingTests.cs +++ b/tests/OpenClaw.Shared.Tests/ExecApprovalV2RoutingTests.cs @@ -31,21 +31,23 @@ private static NodeInvokeRequest RunRequest(string id = "r1") // ------------------------------------------------------------------------- [Fact] - public void V2Result_AllSixCodesConstructible() + public void V2Result_AllCodesConstructible() { var r1 = ExecApprovalV2Result.Unavailable("test"); - var r2 = ExecApprovalV2Result.SecurityDeny("test"); - var r3 = ExecApprovalV2Result.AllowlistMiss("test"); - var r4 = ExecApprovalV2Result.UserDenied("test"); - var r5 = ExecApprovalV2Result.ValidationFailed("test"); - var r6 = ExecApprovalV2Result.ResolutionFailed("test"); + var r2 = ExecApprovalV2Result.Allowed("test"); + var r3 = ExecApprovalV2Result.SecurityDeny("test"); + var r4 = ExecApprovalV2Result.AllowlistMiss("test"); + var r5 = ExecApprovalV2Result.UserDenied("test"); + var r6 = ExecApprovalV2Result.ValidationFailed("test"); + var r7 = ExecApprovalV2Result.ResolutionFailed("test"); Assert.Equal(ExecApprovalV2Code.Unavailable, r1.Code); - Assert.Equal(ExecApprovalV2Code.SecurityDeny, r2.Code); - Assert.Equal(ExecApprovalV2Code.AllowlistMiss, r3.Code); - Assert.Equal(ExecApprovalV2Code.UserDenied, r4.Code); - Assert.Equal(ExecApprovalV2Code.ValidationFailed, r5.Code); - Assert.Equal(ExecApprovalV2Code.ResolutionFailed, r6.Code); + Assert.Equal(ExecApprovalV2Code.Allowed, r2.Code); + Assert.Equal(ExecApprovalV2Code.SecurityDeny, r3.Code); + Assert.Equal(ExecApprovalV2Code.AllowlistMiss, r4.Code); + Assert.Equal(ExecApprovalV2Code.UserDenied, r5.Code); + Assert.Equal(ExecApprovalV2Code.ValidationFailed, r6.Code); + Assert.Equal(ExecApprovalV2Code.ResolutionFailed, r7.Code); } [Fact] @@ -369,15 +371,75 @@ public void ProductionWiring_SetV2Handler_NotCalledInSrc() Assert.NotNull(repoRoot); var srcDir = Path.Combine(repoRoot, "src"); + // Check for actual call-site invocations (dot-prefix + open-paren), not comments. var violations = Directory .GetFiles(srcDir, "*.cs", SearchOption.AllDirectories) .Where(f => !f.EndsWith("SystemCapability.cs", StringComparison.OrdinalIgnoreCase)) - .Where(f => File.ReadAllText(f).Contains("SetV2Handler", StringComparison.Ordinal)) + .Where(f => File.ReadAllText(f).Contains(".SetV2Handler(", StringComparison.Ordinal)) .ToList(); Assert.Empty(violations); } + // ------------------------------------------------------------------------- + // I-4. Allowed result causes command runner to be called (PR7) + // ------------------------------------------------------------------------- + + [Fact] + public async Task V2Path_AllowedResult_ExecutesCommandRunner() + { + var runner = new FakeRunner(); + var cap = new SystemCapability(NullLogger.Instance); + cap.SetCommandRunner(runner); + cap.SetV2Handler(new FixedResultHandler(ExecApprovalV2Result.Allowed())); + + var res = await cap.ExecuteAsync(RunRequest()); + + Assert.True(res.Ok, $"Expected Ok but got error: {res.Error}"); + Assert.NotNull(runner.LastRequest); // command runner was called + // RunRequest() uses {"command":"echo hello"} string form → parsed as single argv element + Assert.Equal("echo hello", runner.LastRequest!.Command); + } + + [Fact] + public async Task V2Path_AllowedResult_DoesNotReturnHandlerError() + { + var cap = new SystemCapability(NullLogger.Instance); + cap.SetCommandRunner(new FakeRunner()); + cap.SetV2Handler(new FixedResultHandler(ExecApprovalV2Result.Allowed())); + + var res = await cap.ExecuteAsync(RunRequest()); + + Assert.True(res.Ok); + Assert.Null(res.Error); + } + + [Fact] + public async Task V2Path_AllowedResult_LogsV2ApprovedExecuting() + { + var logger = new CapturingLogger(); + var cap = new SystemCapability(logger); + cap.SetCommandRunner(new FakeRunner()); + cap.SetV2Handler(new FixedResultHandler(ExecApprovalV2Result.Allowed())); + + await cap.ExecuteAsync(RunRequest()); + + Assert.True(logger.HasInfoContaining("v2-approved"), "v2-approved not logged on allow path"); + } + + [Fact] + public async Task V2Path_AllowedResult_NullRunner_ReturnsError() + { + // V2 allowed but no runner configured — execution must fail gracefully. + var cap = new SystemCapability(NullLogger.Instance); + cap.SetV2Handler(new FixedResultHandler(ExecApprovalV2Result.Allowed())); + + var res = await cap.ExecuteAsync(RunRequest()); + + Assert.False(res.Ok); + Assert.Contains("not available", res.Error!, StringComparison.OrdinalIgnoreCase); + } + private static string? FindRepoRoot() { var dir = new DirectoryInfo(AppContext.BaseDirectory);