From c0c587f27c20d029c997b12da1cb9fcea57c0b1f Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Tue, 4 Aug 2026 01:03:09 +0000 Subject: [PATCH 1/5] Fix shell approval scope extraction --- .../Tools/ScopedShellSafeVerbPolicyTests.cs | 20 ++- .../Tools/ScopedShellSafeVerbPolicy.cs | 80 +++++------ src/Netclaw.Actors/Tools/ToolAccessPolicy.cs | 19 ++- .../ShellApprovalMatcherTests.cs | 36 +++-- .../ApprovalPatternMatching.cs | 8 +- src/Netclaw.Security/IToolApprovalMatcher.cs | 132 +++++++++++------- 6 files changed, 168 insertions(+), 127 deletions(-) diff --git a/src/Netclaw.Actors.Tests/Tools/ScopedShellSafeVerbPolicyTests.cs b/src/Netclaw.Actors.Tests/Tools/ScopedShellSafeVerbPolicyTests.cs index 639377a7e..c532ce7f3 100644 --- a/src/Netclaw.Actors.Tests/Tools/ScopedShellSafeVerbPolicyTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/ScopedShellSafeVerbPolicyTests.cs @@ -5,6 +5,7 @@ // ----------------------------------------------------------------------- using Netclaw.Actors.Tools; using Netclaw.Configuration; +using Netclaw.Security; using Netclaw.Tools; using Xunit; @@ -59,6 +60,9 @@ private static void SafeDelete(string path) private static SafeVerbList VerbList(params string[] verbs) => SafeVerbList.FromVerbs(verbs); + private static IReadOnlyList Candidates(params string[] verbs) + => verbs.Select(verb => new ApprovalCandidate(verb, Directory: null)).ToList(); + private ToolInvocationContext PersonalContext(string? projectDir = null, string? sessionDir = null) => TestToolExecutionContext.CreateBound("session-1", sessionDir ?? _sessionDir, new TestToolExecutionContextOptions { @@ -153,7 +157,7 @@ public void All_short_circuit_returns_false_when_any_verb_is_unsafe() var policy = new ScopedShellSafeVerbPolicy(VerbList("grep", "cat")); var ctx = PersonalContext(projectDir: _projectDir); - Assert.False(policy.AllShortCircuit(["grep", "git push"], _projectDir, ctx)); + Assert.False(policy.AllShortCircuit(Candidates("grep", "git push"), _projectDir, ctx)); } [Fact] @@ -162,7 +166,7 @@ public void All_short_circuit_returns_true_when_every_verb_is_safe_and_in_space( var policy = new ScopedShellSafeVerbPolicy(VerbList("grep", "cat", "wc")); var ctx = PersonalContext(projectDir: _projectDir); - Assert.True(policy.AllShortCircuit(["grep", "cat", "wc"], _projectDir, ctx)); + Assert.True(policy.AllShortCircuit(Candidates("grep", "cat", "wc"), _projectDir, ctx)); } [Fact] @@ -206,6 +210,16 @@ public void New_safe_verb_chained_with_mutating_verb_still_prompts() var policy = new ScopedShellSafeVerbPolicy(VerbList("date")); var ctx = PersonalContext(projectDir: _projectDir); - Assert.False(policy.AllShortCircuit(["date", "git push origin main"], _projectDir, ctx)); + Assert.False(policy.AllShortCircuit(Candidates("date", "git push origin main"), _projectDir, ctx)); + } + + [Fact] + public void Candidate_path_outside_safe_spaces_falls_through_to_prompt() + { + var policy = new ScopedShellSafeVerbPolicy(VerbList("cat")); + var ctx = PersonalContext(projectDir: _projectDir); + var candidates = new[] { new ApprovalCandidate("cat", _outsideDir) }; + + Assert.False(policy.AllShortCircuit(candidates, _projectDir, ctx)); } } diff --git a/src/Netclaw.Actors/Tools/ScopedShellSafeVerbPolicy.cs b/src/Netclaw.Actors/Tools/ScopedShellSafeVerbPolicy.cs index b4195e2b6..8f3997c07 100644 --- a/src/Netclaw.Actors/Tools/ScopedShellSafeVerbPolicy.cs +++ b/src/Netclaw.Actors/Tools/ScopedShellSafeVerbPolicy.cs @@ -12,9 +12,9 @@ namespace Netclaw.Actors.Tools; /// /// Layer 1.5 of the shell approval pipeline (between the hard-deny list and /// the interactive approval gate): when both the candidate verb chain is on -/// the curated AND the candidate's cwd resolves -/// under one of the audience-aware safe-space roots, the policy short-circuits -/// to "approved" without prompting the user. +/// the supplied AND each effective directory is +/// under an audience-aware safe-space root, the policy grants access without +/// a prompt. /// /// Mirrors for the audience model and /// the symlink-segment guard. Personal and Team audiences get @@ -26,7 +26,7 @@ namespace Netclaw.Actors.Tools; /// The policy never relaxes the hard-deny list (layer 1) — that runs first /// in . It only relaxes the interactive /// approval gate (layer 2) for verbs that have been explicitly classified as -/// read-only by the bundled safe-verbs list and any user-additive override. +/// read-only by the supplied safe-verbs list. /// internal sealed class ScopedShellSafeVerbPolicy { @@ -38,35 +38,29 @@ public ScopedShellSafeVerbPolicy(SafeVerbList safeVerbs) } /// - /// Evaluates a candidate (verb, cwd) pair against the safe-verb policy. + /// Evaluates a candidate verb and cwd against the safe-verb policy. /// Returns true when the gate should short-circuit to allow with /// no user prompt; false when the candidate should fall through /// to the existing approval gate. /// public bool ShortCircuitsApproval(string candidateVerb, string? cwd, ToolInvocationContext context) - => AllShortCircuit([candidateVerb], cwd, context); + => AllShortCircuit([new ApprovalCandidate(candidateVerb, Directory: null)], cwd, context); /// - /// Returns true when every candidate verb in - /// is short-circuited by the safe-verb policy under the supplied - /// . Used by the gate to bypass the approval prompt - /// only when the entire compound is read-only-in-safe-space; any single - /// non-safe candidate falls the whole invocation through to the prompt. - /// Cwd-and-roots resolution runs once per call rather than per verb, - /// so an N-verb compound costs one path-normalize + one symlink-segment - /// scan instead of N. + /// Returns true when each candidate has a safe verb and a safe effective + /// directory. The candidate directory takes precedence over the cwd. /// - public bool AllShortCircuit(IReadOnlyList candidateVerbs, string? cwd, ToolInvocationContext context) + public bool AllShortCircuit( + IReadOnlyList candidates, + string? cwd, + ToolInvocationContext context) { - if (candidateVerbs.Count == 0) + if (candidates.Count == 0) return false; - if (string.IsNullOrWhiteSpace(cwd)) - return false; - - foreach (var verb in candidateVerbs) + foreach (var candidate in candidates) { - if (string.IsNullOrWhiteSpace(verb) || !_safeVerbs.Contains(verb)) + if (string.IsNullOrWhiteSpace(candidate.Verb) || !_safeVerbs.Contains(candidate.Verb)) return false; } @@ -74,33 +68,31 @@ public bool AllShortCircuit(IReadOnlyList candidateVerbs, string? cwd, T if (safeRoots.Count == 0) return false; - string fullCwd; - try - { - fullCwd = Path.GetFullPath(cwd); - } - catch (Exception ex) when (ex is ArgumentException or NotSupportedException or PathTooLongException) + foreach (var candidate in candidates) { - return false; - } + var effectiveDirectory = candidate.Directory ?? cwd; + if (string.IsNullOrWhiteSpace(effectiveDirectory)) + return false; - foreach (var root in safeRoots) - { - if (!PathUtility.IsWithinRoot(fullCwd, root)) - continue; - - // A planted symlink under a safe-space root could redirect the - // cwd into a path outside that root. Refuse the short-circuit if - // any segment of the cwd path is a reparse point — the user can - // still grant manually via the interactive prompt, where they - // will see the literal cwd they are authorizing. - if (PathUtility.ContainsSymlinkSegment(root, fullCwd)) - continue; - - return true; + string fullDirectory; + try + { + fullDirectory = Path.GetFullPath(effectiveDirectory); + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException or PathTooLongException) + { + return false; + } + + var isSafe = safeRoots.Any(root => + PathUtility.IsWithinRoot(fullDirectory, root) + && !PathUtility.ContainsSymlinkSegment(root, fullDirectory)); + + if (!isSafe) + return false; } - return false; + return true; } /// diff --git a/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs b/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs index 4d3aa1a5f..89faeb4f6 100644 --- a/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs +++ b/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs @@ -301,9 +301,9 @@ private ToolAccessDecision CheckApprovalGate( // - `patterns`: the exact blocked units shown to the user and reused by // approve-once retries. // - `candidates`: the (verb, directory) pairs evaluated against - // persisted ApprovalEntry records by the gate. The directory half is - // the path argument extracted from each clause when present, falling - // back to ToolExecutionContext.Cwd at evaluation time. + // persisted ApprovalEntry records by the gate. Candidates include + // path operands, redirect targets, and each pipeline clause. + // A null directory uses ToolExecutionContext.Cwd. // - `candidateVerbs`: the verb-only projection of `candidates`, kept // for renderers (Slack/Discord builders) that bullet-list verbs in // the prompt body. Button labels stay fixed; runtime values like @@ -333,12 +333,12 @@ private ToolAccessDecision CheckApprovalGate( // when the matcher could extract candidate verbs cleanly — messy // commands always prompt regardless of verb membership. Auto-allows // demonstrably read-only verbs (cat/ls/grep/find/git status/...) - // when the cwd is inside session_dir or project_dir. + // when every effective directory is inside session_dir or project_dir. if (_safeVerbPolicy is not null && isShell && !isMessy && candidateVerbs.Count > 0 - && _safeVerbPolicy.AllShortCircuit(candidateVerbs, context.Approval.Cwd, context.Invocation)) + && _safeVerbPolicy.AllShortCircuit(candidates, context.Approval.Cwd, context.Invocation)) { return ToolAccessDecision.Allow(ToolAllowReason.SafeVerbInTrustedScope); } @@ -646,12 +646,9 @@ public sealed record ToolApprovalContext( // Channel adapters use this to omit the persistent-grant buttons and // surface the "complex command" hint. bool IsMessy = false, - // Per-clause (verb, directory) pairs evaluated against the persisted - // ApprovalEntry store. The directory half is the path argument - // extracted from the clause when present, falling back to Cwd at - // match time. The persistence path reads this on ApprovedAlways so - // "Always here" stores per-clause folder-scoped grants from the - // actual paths the agent touched. + // Per-clause (verb, directory) pairs for the persisted ApprovalEntry store. + // The list includes path operands, redirect targets, and pipeline clauses. + // A null directory uses Cwd. ApprovedAlways stores these effective scopes. IReadOnlyList? Candidates = null); public sealed record ToolApprovalOption(ApprovalOptionKey Key, string Label); diff --git a/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs b/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs index 3f68c194c..e5cea786c 100644 --- a/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs +++ b/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs @@ -856,9 +856,8 @@ public void ExtractCandidates_side_effect_verbs_do_not_inherit_cd_attribution() // so cd attribution must NOT attach to them — both because the // attribution is semantically meaningless for these verbs and // because ApprovalPatternMatching.IsPureSideEffect treats them - // as unconditional pass when Directory is null (the redirect - // detector still kicks in if a literal `> /tmp/log` path arg - // is present on the clause). + // as an unconditional pass when Directory is null. A redirect + // produces an additional directory candidate. var candidates = _matcher.ExtractCandidates( new ToolName("shell_execute"), new Dictionary @@ -900,11 +899,10 @@ public void ExtractCandidates_normalizes_tilde_cd_to_absolute_path_so_clauses_sh } [Fact(SkipUnless = nameof(IsPosix), Skip = "POSIX-only path semantics")] - public void ExtractCandidates_collapses_pipe_chain_into_single_candidate() + public void ExtractCandidates_checks_each_clause_in_one_pipe_approval_unit() { - // Pipes stay inside one approval unit — approving cat /etc/hosts - // | wc -l shouldn't prompt twice. Compare with && which DOES - // produce independent units. + // The prompt keeps a pipeline in one approval unit. Authorization + // still checks each clause so an unsafe tail cannot hide. var candidates = _matcher.ExtractCandidates( new ToolName("shell_execute"), new Dictionary @@ -912,9 +910,27 @@ public void ExtractCandidates_collapses_pipe_chain_into_single_candidate() ["Command"] = "cat /etc/hosts | wc -l" }); - Assert.Single(candidates); - Assert.Equal("cat", candidates[0].Verb); - Assert.Equal("/etc/hosts", candidates[0].Directory); // no extension → no file-parent + Assert.Equal(2, candidates.Count); + Assert.Contains(candidates, candidate => + candidate.Verb == "cat" && candidate.Directory == "/etc/hosts"); + Assert.Contains(candidates, candidate => + candidate.Verb == "wc" && candidate.Directory is null); + } + + [Fact(SkipUnless = nameof(IsPosix), Skip = "POSIX-only path semantics")] + public void ExtractCandidates_uses_redirect_target_and_invocation_working_directory() + { + var workingDirectory = Path.Combine(Path.GetTempPath(), $"netclaw-redirect-{Guid.NewGuid():N}"); + var candidates = _matcher.ExtractCandidates( + new ToolName("shell_execute"), + new Dictionary + { + ["Command"] = "echo hello > result.txt", + ["WorkingDirectory"] = workingDirectory + }); + + Assert.Contains(candidates, candidate => + candidate.Verb == "echo" && candidate.Directory == workingDirectory); } [Fact] diff --git a/src/Netclaw.Security/ApprovalPatternMatching.cs b/src/Netclaw.Security/ApprovalPatternMatching.cs index b89df4f5a..6b3bc68f1 100644 --- a/src/Netclaw.Security/ApprovalPatternMatching.cs +++ b/src/Netclaw.Security/ApprovalPatternMatching.cs @@ -142,11 +142,9 @@ public static bool MatchesAny(string candidate, IEnumerable appro /// /// Returns true when this candidate is a pure side-effect clause that /// should not be persisted on Always-here/Always-anywhere clicks. The - /// rule is verb-in-skip-list AND no path argument. Redirect detection - /// (e.g. echo X > /tmp/log) is implicit: a redirect target - /// shows up as the candidate's directory via - /// , so a candidate - /// with a non-null Directory is never considered pure side effect. + /// rule is verb-in-skip-list AND no effective directory. The shell + /// candidate extractor emits a separate directory candidate for each + /// redirect target. Thus, echo X > /tmp/log is not exempt. /// /// /// The side-effect verb set diff --git a/src/Netclaw.Security/IToolApprovalMatcher.cs b/src/Netclaw.Security/IToolApprovalMatcher.cs index e23261422..35afa0163 100644 --- a/src/Netclaw.Security/IToolApprovalMatcher.cs +++ b/src/Netclaw.Security/IToolApprovalMatcher.cs @@ -14,10 +14,10 @@ namespace Netclaw.Security; /// /// One approval candidate extracted from a tool invocation. The verb is the /// command head plus subcommand chain (e.g., find, git status). -/// The directory is the first path-like positional argument with the -/// file-parent rule applied — when present it overrides the resolved cwd as -/// the candidate's effective directory in the approval matcher; when null -/// the matcher falls back to the spawned process's cwd. +/// The directory identifies a path operand, a redirect parent, or an inherited +/// shell directory. A null directory uses the spawned process cwd. +/// One shell clause can produce multiple candidates when it accesses multiple +/// authorization scopes. /// public sealed record ApprovalCandidate(string Verb, string? Directory); @@ -65,11 +65,9 @@ public interface IToolApprovalMatcher /// /// Returns the candidate (verb, directory) pairs for this tool - /// invocation. The directory half is the first path-like positional - /// argument extracted from each clause (with the file-parent rule - /// applied), or null when the clause has no path argument. The matcher - /// SHALL use this directory as the candidate's effective directory, - /// falling back to when null. + /// invocation. A shell clause can emit candidates for its path operand, + /// redirect targets, and inherited directory. A null directory uses + /// . /// IReadOnlyList ExtractCandidates(ToolName toolName, IDictionary? arguments); @@ -139,7 +137,7 @@ public IReadOnlyList ExtractPatterns(ToolName toolName, IDictionary ExtractCandidates(ToolName toolName, IDi // when the clause itself has no anchored path arg. Windows keeps // the legacy ShellTokenizer path — ShellSyntaxTree is bash-only. if (!OperatingSystem.IsWindows()) - return ExtractCandidatesViaBashParser(command); + return ExtractCandidatesViaBashParser(command, GetWorkingDirectory(arguments)); var seen = new HashSet<(string, string?)>(); var candidates = new List(); @@ -205,11 +203,17 @@ public IReadOnlyList ExtractCandidates(ToolName toolName, IDi /// pattern/candidate list (messy semantics — Once/Deny prompt only) or /// the flattened raw command for display. /// - private static ShellSyntaxTree.ParsedCommand? TryParseCommand(string command) + private static ShellSyntaxTree.ParsedCommand? TryParseCommand( + string command, + string? workingDirectory = null) { try { - var result = Parser.Parse(command); + var parser = string.IsNullOrWhiteSpace(workingDirectory) + ? Parser + : new ShellSyntaxTree.BashParser( + new ShellSyntaxTree.BashParserOptions { WorkingDirectory = workingDirectory }); + var result = parser.Parse(command); return result.IsUnparseable || result.Clauses.Count == 0 ? null : result; } catch @@ -218,31 +222,21 @@ public IReadOnlyList ExtractCandidates(ToolName toolName, IDi } } - private static IReadOnlyList ExtractCandidatesViaBashParser(string command) + private static IReadOnlyList ExtractCandidatesViaBashParser( + string command, + string? workingDirectory) { - var result = TryParseCommand(command); + var result = TryParseCommand(command, workingDirectory); if (result is null) return []; - // Group consecutive Pipe clauses into a single approval unit so - // `cat /etc/hosts | wc -l` stays one decision rather than two. - // AndIf / OrIf / Sequence and the leading None-operator clause each - // start a fresh group. + // The prompt groups a pipe as one approval unit. Authorization still + // checks each clause so an unsafe tail cannot hide behind a safe head. var seen = new HashSet<(string, string?)>(); var candidates = new List(); - ShellSyntaxTree.Clause? groupHead = null; foreach (var clause in result.Clauses) { - if (clause.Operator != ShellSyntaxTree.CompoundOperator.Pipe) - groupHead = clause; - - if (groupHead is null) - continue; - - if (!ReferenceEquals(clause, groupHead)) - continue; // pipe-tail clauses fold into the group head - // ShellSyntaxTree's greedy verb walk (SPEC §6.1) folds // lowercase-leading value tokens into the verb chain (`git tag // v0.4.2`, `git show aa211dcb`, `git checkout feature2`), while @@ -258,26 +252,24 @@ private static IReadOnlyList ExtractCandidatesViaBashParser(s if (string.IsNullOrEmpty(verb)) continue; - // Side-effect verbs (echo, printf, :, true, false) don't - // operate on the filesystem, so inheriting the cd target - // would (a) break ApprovalPatternMatching.IsPureSideEffect's - // null-directory invariant and (b) attach a misleading scope - // to a verb that ignores cwd. Their candidates remain - // directory-less; redirects (echo X > /tmp/log) still - // surface their target via the explicit-path scan above - // when BashParser exposes the redirect arg. var isSideEffectVerb = ShellTokenizer.SingleTokenSideEffectVerbs.Contains(verb); - var directory = ResolveClauseDirectory(clause, isSideEffectVerb); - var key = (verb.ToLowerInvariant(), directory); - if (seen.Add(key)) - candidates.Add(new ApprovalCandidate(verb, directory)); + foreach (var directory in ResolveClauseDirectories(clause, isSideEffectVerb)) + { + var key = (verb.ToLowerInvariant(), directory); + if (seen.Add(key)) + candidates.Add(new ApprovalCandidate(verb, directory)); + } } return candidates; } - private static string? ResolveClauseDirectory(ShellSyntaxTree.Clause clause, bool isSideEffectVerb) + private static IReadOnlyList ResolveClauseDirectories( + ShellSyntaxTree.Clause clause, + bool isSideEffectVerb) { + var directories = new List(); + // First explicit path arg wins — that's the candidate's own // operand, e.g. `dotnet test /home/user/repos/Foo`. Only the // anchored-path predicate from the legacy tokenizer counts (/, ~/, @@ -305,21 +297,51 @@ private static IReadOnlyList ExtractCandidatesViaBashParser(s if (ShellTokenizer.IsPathToken(raw)) { var canonical = !string.IsNullOrEmpty(arg.Resolved) ? arg.Resolved : raw; - return ShellTokenizer.ApplyFileParentRule(canonical); + directories.Add(ShellTokenizer.ApplyFileParentRule(canonical)); + break; } } - // Side-effect verbs ignore cd attribution — see caller's comment - // on why (null-directory invariant in IsPureSideEffect, and these - // verbs don't operate on the filesystem anyway). - if (isSideEffectVerb) + if (directories.Count == 0) + { + // A side-effect verb ignores cwd. Other verbs inherit a prior cd. + var cwdAttribution = isSideEffectVerb + ? null + : clause.Args.FirstOrDefault(a => a.IsCwdAttribution)?.Resolved; + directories.Add(cwdAttribution); + } + + foreach (var redirect in clause.Redirects) + { + var directory = ResolveRedirectDirectory(redirect); + if (directory is not null) + directories.Add(directory); + } + + return directories.Distinct(StringComparer.Ordinal).ToList(); + } + + private static string? ResolveRedirectDirectory(ShellSyntaxTree.Redirect redirect) + { + if (string.IsNullOrWhiteSpace(redirect.Target) + || IsHeredocRedirect(redirect) + || redirect.Target.StartsWith('&')) + { return null; + } - // No explicit path → inherit the cd-attributed cwd from any - // preceding `cd X` in this compound (or in a wrapping `bash -c - // "..."` invocation; the parser flattens that). - var cwdAttribution = clause.Args.FirstOrDefault(a => a.IsCwdAttribution); - return cwdAttribution?.Resolved; + // A dynamic target must still block an automatic side-effect grant. + if (redirect.IsDynamicSkip) + return redirect.Target; + + try + { + return Path.GetDirectoryName(redirect.Target) ?? redirect.Target; + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException or PathTooLongException) + { + return redirect.Target; + } } /// @@ -330,7 +352,9 @@ private static IReadOnlyList ExtractCandidatesViaBashParser(s /// commands — mirroring the legacy /// empty-result contract so the prompt builder offers only Once/Deny. /// - private static IReadOnlyList ExtractApprovalUnitsViaBashParser(string command) + private static IReadOnlyList ExtractApprovalUnitsViaBashParser( + string command, + string? workingDirectory) { // Messy commands (control-flow keywords, unbalanced brackets) cannot // be cleanly decomposed into approval units; mirror the legacy @@ -338,7 +362,7 @@ private static IReadOnlyList ExtractApprovalUnitsViaBashParser(string co if (ShellTokenizer.IsMessyCompoundCommand(command)) return []; - var result = TryParseCommand(command); + var result = TryParseCommand(command, workingDirectory); if (result is null) return []; From e5dd3f4f17f11f5851e8c5d3e469d9e084d417ac Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Tue, 4 Aug 2026 01:03:17 +0000 Subject: [PATCH 2/5] Add shell approval disposition matrix --- .../Tools/ShellApprovalCaseCatalog.cs | 563 ++++++++++++++++++ ...roval_cases_match_review_table.verified.md | 45 ++ .../ShellApprovalDispositionMatrixTests.cs | 51 ++ .../Tools/ShellApprovalHarness.cs | 321 ++++++++++ .../Tools/ToolApprovalGateTests.cs | 159 ----- 5 files changed, 980 insertions(+), 159 deletions(-) create mode 100644 src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs create mode 100644 src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.Shell_approval_cases_match_review_table.verified.md create mode 100644 src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.cs create mode 100644 src/Netclaw.Actors.Tests/Tools/ShellApprovalHarness.cs diff --git a/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs b/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs new file mode 100644 index 000000000..7447c0e63 --- /dev/null +++ b/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs @@ -0,0 +1,563 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using System.Collections.Frozen; +using Netclaw.Actors.Tools; +using Netclaw.Configuration; +using Xunit; + +namespace Netclaw.Actors.Tests.Tools; + +/// +/// Defines the explicit approval-policy shape that a matrix case installs. +/// The value tests the secure fallback without a policy object. +/// +internal enum ApprovalPolicyShape +{ + /// The audience profile has no explicit approval policy. + Missing, + + /// The shell tool requires approval unless another gate grants access. + Approval, + + /// The approval policy grants shell access without a stored approval. + Auto, + + /// The approval policy denies shell access. + Deny +} + +/// +/// Names a logical directory that the harness resolves inside its isolated test root. +/// This type prevents a case from embedding a harness-specific temporary path. +/// +internal enum ApprovalDirectoryShape +{ + /// The case supplies no directory. + None, + + /// The case uses the active project directory. + Project, + + /// The case uses the active session directory. + Session, + + /// The case uses a directory outside the project and session roots. + External +} + +/// +/// Identifies the store that owns a seeded approval. +/// The harness uses this value to select session memory or persistent storage. +/// +internal enum ApprovalSeedSource +{ + /// The approval exists only in an actor session. + Session, + + /// The approval survives creation of a new approval actor. + Persistent +} + +/// +/// Selects the session identity for a session-scoped approval seed. +/// This axis proves that a session approval cannot authorize another session. +/// +internal enum ApprovalSessionShape +{ + /// The seed uses the session that invokes the shell tool. + Invocation, + + /// The seed uses an unrelated session. + Other +} + +internal sealed record ShellApprovalPolicy( + ApprovalPolicyShape Approval, + ShellExecutionMode ShellMode = ShellExecutionMode.HostAllowed, + string? AdditionalSafeVerb = null) +{ + public string Display => AdditionalSafeVerb is null + ? $"{Approval}/{ShellMode}" + : $"{Approval}/{ShellMode}+{AdditionalSafeVerb}"; +} + +internal sealed record ShellApprovalInvocation( + string Command, + ApprovalDirectoryShape WorkingDirectory = ApprovalDirectoryShape.Project, + TrustAudience Audience = TrustAudience.Personal, + bool Interactive = true); + +internal sealed record ApprovalSeed( + ApprovalSeedSource Source, + string Pattern, + TrustAudience Audience, + ApprovalSessionShape Session, + ApprovalDirectoryShape Directory); + +internal sealed record ApprovalState(IReadOnlyList Seeds) +{ + public static ApprovalState Empty { get; } = new([]); + + public string Display => Seeds.Count == 0 + ? "none" + : string.Join(", ", Seeds.Select(DescribeSeed)); + + private static string DescribeSeed(ApprovalSeed seed) + { + var source = seed.Source.ToString().ToLowerInvariant(); + var scope = seed.Source switch + { + ApprovalSeedSource.Session => seed.Session == ApprovalSessionShape.Invocation + ? "this-chat" + : "other-chat", + ApprovalSeedSource.Persistent => seed.Directory == ApprovalDirectoryShape.None + ? "anywhere" + : seed.Directory.ToString().ToLowerInvariant(), + _ => throw new ArgumentOutOfRangeException(nameof(seed), seed.Source, "Unknown approval source.") + }; + var audience = seed.Audience == TrustAudience.Personal ? string.Empty : $",{seed.Audience}"; + return $"{source}[{scope}{audience}]:{seed.Pattern}"; + } +} + +internal static class Approvals +{ + public static ApprovalState None => ApprovalState.Empty; + + public static ApprovalState Session(params string[] patterns) + => CreateSession(ApprovalSessionShape.Invocation, TrustAudience.Personal, patterns); + + public static ApprovalState SessionForOtherSession(params string[] patterns) + => CreateSession(ApprovalSessionShape.Other, TrustAudience.Personal, patterns); + + public static ApprovalState PersistentAnywhere(params string[] patterns) + => CreatePersistent(TrustAudience.Personal, ApprovalDirectoryShape.None, patterns); + + public static ApprovalState PersistentHere(ApprovalDirectoryShape directory, params string[] patterns) + => CreatePersistent(TrustAudience.Personal, directory, patterns); + + public static ApprovalState PersistentForOtherAudience(params string[] patterns) + => CreatePersistent(TrustAudience.Team, ApprovalDirectoryShape.None, patterns); + + public static ApprovalState Combine(params ApprovalState[] states) + => new(states.SelectMany(state => state.Seeds).ToList()); + + private static ApprovalState CreateSession( + ApprovalSessionShape session, + TrustAudience audience, + IReadOnlyList patterns) + => new(patterns + .Select(pattern => new ApprovalSeed( + ApprovalSeedSource.Session, + pattern, + audience, + session, + ApprovalDirectoryShape.None)) + .ToList()); + + private static ApprovalState CreatePersistent( + TrustAudience audience, + ApprovalDirectoryShape directory, + IReadOnlyList patterns) + => new(patterns + .Select(pattern => new ApprovalSeed( + ApprovalSeedSource.Persistent, + pattern, + audience, + ApprovalSessionShape.Invocation, + directory)) + .ToList()); +} + +internal sealed record ExpectedApproval( + ToolAuthorizationOutcome Outcome, + ToolAllowReason? AllowReason, + string? DenyReason, + IReadOnlyList Candidates, + bool? IsMessy, + int ApprovalChecks, + IReadOnlyList ApprovalMatches) +{ + public static ExpectedApproval Allow( + ToolAllowReason reason, + int approvalChecks = 0, + params string[] approvalMatches) + => new( + ToolAuthorizationOutcome.Allowed, + reason, + null, + [], + null, + approvalChecks, + approvalMatches); + + public static ExpectedApproval Require( + IReadOnlyList candidates, + bool isMessy = false, + int approvalChecks = 1, + params string[] approvalMatches) + => new( + ToolAuthorizationOutcome.RequiresApproval, + null, + null, + candidates, + isMessy, + approvalChecks, + approvalMatches); + + public static ExpectedApproval Deny(string reason) + => new( + ToolAuthorizationOutcome.Denied, + null, + reason, + [], + null, + 0, + []); +} + +internal sealed record ShellApprovalCase( + string Id, + ShellApprovalPolicy Policy, + ShellApprovalInvocation Invocation, + ApprovalState Approvals, + ExpectedApproval Expected); + +public static class ShellApprovalCases +{ + private static readonly ShellApprovalPolicy MissingPolicy = new(ApprovalPolicyShape.Missing); + private static readonly ShellApprovalPolicy ApprovalPolicy = new(ApprovalPolicyShape.Approval); + private static readonly ShellApprovalPolicy AutoPolicy = new(ApprovalPolicyShape.Auto); + private static readonly ShellApprovalPolicy DenyPolicy = new(ApprovalPolicyShape.Deny); + + internal static IReadOnlyList All { get; } = + [ + Case( + "missing-policy-prompts", + MissingPolicy, + Bash("git push origin dev"), + Approvals.None, + ExpectedApproval.Require(["git push origin dev"])), + Case( + "exact-approval-prompts", + ApprovalPolicy, + Bash("git push origin dev"), + Approvals.None, + ExpectedApproval.Require(["git push origin dev"])), + Case( + "exact-auto-allows", + AutoPolicy, + Bash("git push origin dev"), + Approvals.None, + ExpectedApproval.Allow(ToolAllowReason.PolicyAuto)), + Case( + "exact-deny-denies", + DenyPolicy, + Bash("git push origin dev"), + Approvals.None, + ExpectedApproval.Deny("tool_denied_by_approval_policy")), + Case( + "missing-policy-persistent-grant-allows", + MissingPolicy, + Bash("git push origin dev"), + Approvals.PersistentAnywhere("git push origin dev"), + ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "persistent:git push origin dev")), + + Case( + "team-audience-denied", + ApprovalPolicy, + Bash("git push", audience: TrustAudience.Team), + Approvals.None, + ExpectedApproval.Deny("shell_requires_personal_context")), + Case( + "public-audience-denied", + ApprovalPolicy, + Bash("git push", audience: TrustAudience.Public), + Approvals.None, + ExpectedApproval.Deny("shell_requires_personal_context")), + Case( + "team-auto-still-denied", + AutoPolicy, + Bash("git push", audience: TrustAudience.Team), + Approvals.None, + ExpectedApproval.Deny("shell_requires_personal_context")), + Case( + "public-auto-still-denied", + AutoPolicy, + Bash("git push", audience: TrustAudience.Public), + Approvals.None, + ExpectedApproval.Deny("shell_requires_personal_context")), + + Case( + "shell-off-denies", + new ShellApprovalPolicy(ApprovalPolicyShape.Auto, ShellExecutionMode.Off), + Bash("git status"), + Approvals.None, + ExpectedApproval.Deny("shell_disabled")), + Case( + "sandbox-only-denies", + new ShellApprovalPolicy(ApprovalPolicyShape.Auto, ShellExecutionMode.SandboxOnly), + Bash("git status"), + Approvals.None, + ExpectedApproval.Deny("shell_requires_sandbox_backend")), + + Case( + "hard-deny-beats-approval", + ApprovalPolicy, + Bash("netclaw daemon stop"), + Approvals.None, + ExpectedApproval.Deny("hard_deny_self_destructive")), + Case( + "hard-deny-beats-auto", + AutoPolicy, + Bash("netclaw daemon stop"), + Approvals.None, + ExpectedApproval.Deny("hard_deny_self_destructive")), + Case( + "hard-deny-beats-stored-grant", + ApprovalPolicy, + Bash("netclaw daemon stop"), + Approvals.PersistentAnywhere("netclaw daemon stop"), + ExpectedApproval.Deny("hard_deny_self_destructive")), + Case( + "compound-hard-deny-denies", + AutoPolicy, + Bash("git status && netclaw daemon stop"), + Approvals.None, + ExpectedApproval.Deny("hard_deny_self_destructive")), + + Case( + "safe-verb-project-allows", + ApprovalPolicy, + Bash("git status"), + Approvals.None, + ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)), + Case( + "safe-verb-session-allows", + ApprovalPolicy, + Bash("git status", ApprovalDirectoryShape.Session), + Approvals.None, + ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)), + Case( + "safe-verb-external-prompts", + ApprovalPolicy, + Bash("git status", ApprovalDirectoryShape.External), + Approvals.None, + ExpectedApproval.Require(["git status"])), + Case( + "safe-verb-external-path-prompts", + ApprovalPolicy, + Bash("cat /etc/passwd"), + Approvals.None, + ExpectedApproval.Require(["cat"])), + Case( + "safe-verb-external-redirect-prompts", + ApprovalPolicy, + Bash("git status > /tmp/netclaw-approval-matrix.txt"), + Approvals.None, + ExpectedApproval.Require(["git status"])), + Case( + "mutating-verb-project-prompts", + ApprovalPolicy, + Bash("git push"), + Approvals.None, + ExpectedApproval.Require(["git push"])), + Case( + "all-safe-compound-allows", + ApprovalPolicy, + Bash("git status && git log"), + Approvals.None, + ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)), + Case( + "mixed-safe-unsafe-compound-prompts", + ApprovalPolicy, + Bash("git status && git push"), + Approvals.None, + ExpectedApproval.Require(["git status", "git push"])), + Case( + "safe-pipe-unsafe-tail-prompts", + ApprovalPolicy, + Bash("git status | git push"), + Approvals.None, + ExpectedApproval.Require(["git status", "git push"])), + Case( + "added-safe-verb-project-allows", + new ShellApprovalPolicy(ApprovalPolicyShape.Approval, AdditionalSafeVerb: "eza"), + Bash("eza"), + Approvals.None, + ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)), + + Case( + "echo-allows-without-grant", + ApprovalPolicy, + Bash("echo hello"), + Approvals.None, + ExpectedApproval.Allow(ToolAllowReason.ApprovalExemptShellCandidates)), + Case( + "printf-allows-without-grant", + ApprovalPolicy, + Bash("printf hello"), + Approvals.None, + ExpectedApproval.Allow(ToolAllowReason.ApprovalExemptShellCandidates)), + Case( + "echo-redirect-prompts", + ApprovalPolicy, + Bash("echo hello > result.txt"), + Approvals.None, + ExpectedApproval.Require(["echo"])), + Case( + "echo-done-fails-closed", + ApprovalPolicy, + Bash("echo done"), + Approvals.None, + ExpectedApproval.Require(["echo"], isMessy: true, approvalChecks: 0)), + Case( + "control-flow-fails-closed", + ApprovalPolicy, + Bash("for f in *.txt; do cat \"$f\"; done"), + Approvals.PersistentAnywhere("cat"), + ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)), + Case( + "empty-command-fails-closed", + ApprovalPolicy, + Bash(string.Empty), + Approvals.None, + ExpectedApproval.Require([], approvalChecks: 0)), + Case( + "whitespace-command-fails-closed", + ApprovalPolicy, + Bash(" "), + Approvals.None, + ExpectedApproval.Require([], approvalChecks: 0)), + + Case( + "session-grant-allows", + ApprovalPolicy, + Bash("git push"), + Approvals.Session("git push"), + ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "session:git push")), + Case( + "other-session-grant-prompts", + ApprovalPolicy, + Bash("git push"), + Approvals.SessionForOtherSession("git push"), + ExpectedApproval.Require(["git push"])), + Case( + "persistent-anywhere-allows", + ApprovalPolicy, + Bash("git push"), + Approvals.PersistentAnywhere("git push"), + ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "persistent:git push")), + Case( + "persistent-here-allows", + ApprovalPolicy, + Bash("git push"), + Approvals.PersistentHere(ApprovalDirectoryShape.Project, "git push"), + ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "persistent:git push")), + Case( + "persistent-here-directory-mismatch-prompts", + ApprovalPolicy, + Bash("git push", ApprovalDirectoryShape.External), + Approvals.PersistentHere(ApprovalDirectoryShape.Project, "git push"), + ExpectedApproval.Require(["git push"])), + Case( + "other-audience-grant-prompts", + ApprovalPolicy, + Bash("git push"), + Approvals.PersistentForOtherAudience("git push"), + ExpectedApproval.Require(["git push"])), + Case( + "mixed-session-persistent-compound-allows", + ApprovalPolicy, + Bash("git status && git push"), + Approvals.Combine( + Approvals.Session("git status"), + Approvals.PersistentAnywhere("git push")), + ExpectedApproval.Allow( + ToolAllowReason.StoredApproval, + 1, + "session:git status", + "persistent:git push")), + Case( + "partial-compound-grant-prompts", + ApprovalPolicy, + Bash("git status && git push"), + Approvals.PersistentAnywhere("git status"), + ExpectedApproval.Require( + ["git status", "git push"], + approvalMatches: ["persistent:git status"])), + + Case( + "noninteractive-unapproved-requires-approval", + ApprovalPolicy, + Bash("git push", interactive: false), + Approvals.None, + ExpectedApproval.Require(["git push"])), + Case( + "noninteractive-persistent-grant-allows", + ApprovalPolicy, + Bash("git push", interactive: false), + Approvals.PersistentAnywhere("git push"), + ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "persistent:git push")), + Case( + "noninteractive-exempt-allows", + ApprovalPolicy, + Bash("echo hello", interactive: false), + Approvals.None, + ExpectedApproval.Allow(ToolAllowReason.ApprovalExemptShellCandidates)) + ]; + + private static readonly FrozenDictionary CasesById = + All.ToFrozenDictionary(testCase => testCase.Id, StringComparer.Ordinal); + + public static IEnumerable> Rows => All.Select(testCase => + new TheoryDataRow(testCase.Id) + .WithTestDisplayName($"shell approval :: {testCase.Id}") + .WithTrait("Disposition", testCase.Expected.Outcome.ToString()) + .WithTrait("AllowReason", testCase.Expected.AllowReason?.ToString() ?? "NotAllowed")); + + internal static ShellApprovalCase Get(string id) => CasesById[id]; + + internal static string RenderReviewTable() + { + var lines = new List + { + "| ID | Policy | Audience | Cwd | Interaction | Command | Approval state | Result | Reason |", + "| --- | --- | --- | --- | --- | --- | --- | --- | --- |" + }; + + lines.AddRange(All.Select(testCase => + $"| {testCase.Id} | {testCase.Policy.Display} | " + + $"{testCase.Invocation.Audience} | {testCase.Invocation.WorkingDirectory} | " + + $"{(testCase.Invocation.Interactive ? "Interactive" : "Non-interactive")} | " + + $"{Escape(testCase.Invocation.Command)} | " + + $"{Escape(testCase.Approvals.Display)} | {testCase.Expected.Outcome} | " + + $"{testCase.Expected.AllowReason?.ToString() ?? testCase.Expected.DenyReason ?? "approval required"} |")); + + return string.Join(Environment.NewLine, lines) + Environment.NewLine; + } + + private static ShellApprovalCase Case( + string id, + ShellApprovalPolicy policy, + ShellApprovalInvocation invocation, + ApprovalState approvals, + ExpectedApproval expected) + => new(id, policy, invocation, approvals, expected); + + private static ShellApprovalInvocation Bash( + string command, + ApprovalDirectoryShape workingDirectory = ApprovalDirectoryShape.Project, + TrustAudience audience = TrustAudience.Personal, + bool interactive = true) + => new(command, workingDirectory, audience, interactive); + + private static string Escape(string value) + => value + .Replace("|", "\\|", StringComparison.Ordinal) + .Replace("\r", "\\r", StringComparison.Ordinal) + .Replace("\n", "\\n", StringComparison.Ordinal); +} diff --git a/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.Shell_approval_cases_match_review_table.verified.md b/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.Shell_approval_cases_match_review_table.verified.md new file mode 100644 index 000000000..46c74f679 --- /dev/null +++ b/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.Shell_approval_cases_match_review_table.verified.md @@ -0,0 +1,45 @@ +| ID | Policy | Audience | Cwd | Interaction | Command | Approval state | Result | Reason | +| --- | --- | --- | --- | --- | --- | --- | --- | --- | +| missing-policy-prompts | Missing/HostAllowed | Personal | Project | Interactive | git push origin dev | none | RequiresApproval | approval required | +| exact-approval-prompts | Approval/HostAllowed | Personal | Project | Interactive | git push origin dev | none | RequiresApproval | approval required | +| exact-auto-allows | Auto/HostAllowed | Personal | Project | Interactive | git push origin dev | none | Allowed | PolicyAuto | +| exact-deny-denies | Deny/HostAllowed | Personal | Project | Interactive | git push origin dev | none | Denied | tool_denied_by_approval_policy | +| missing-policy-persistent-grant-allows | Missing/HostAllowed | Personal | Project | Interactive | git push origin dev | persistent[anywhere]:git push origin dev | Allowed | StoredApproval | +| team-audience-denied | Approval/HostAllowed | Team | Project | Interactive | git push | none | Denied | shell_requires_personal_context | +| public-audience-denied | Approval/HostAllowed | Public | Project | Interactive | git push | none | Denied | shell_requires_personal_context | +| team-auto-still-denied | Auto/HostAllowed | Team | Project | Interactive | git push | none | Denied | shell_requires_personal_context | +| public-auto-still-denied | Auto/HostAllowed | Public | Project | Interactive | git push | none | Denied | shell_requires_personal_context | +| shell-off-denies | Auto/Off | Personal | Project | Interactive | git status | none | Denied | shell_disabled | +| sandbox-only-denies | Auto/SandboxOnly | Personal | Project | Interactive | git status | none | Denied | shell_requires_sandbox_backend | +| hard-deny-beats-approval | Approval/HostAllowed | Personal | Project | Interactive | netclaw daemon stop | none | Denied | hard_deny_self_destructive | +| hard-deny-beats-auto | Auto/HostAllowed | Personal | Project | Interactive | netclaw daemon stop | none | Denied | hard_deny_self_destructive | +| hard-deny-beats-stored-grant | Approval/HostAllowed | Personal | Project | Interactive | netclaw daemon stop | persistent[anywhere]:netclaw daemon stop | Denied | hard_deny_self_destructive | +| compound-hard-deny-denies | Auto/HostAllowed | Personal | Project | Interactive | git status && netclaw daemon stop | none | Denied | hard_deny_self_destructive | +| safe-verb-project-allows | Approval/HostAllowed | Personal | Project | Interactive | git status | none | Allowed | SafeVerbInTrustedScope | +| safe-verb-session-allows | Approval/HostAllowed | Personal | Session | Interactive | git status | none | Allowed | SafeVerbInTrustedScope | +| safe-verb-external-prompts | Approval/HostAllowed | Personal | External | Interactive | git status | none | RequiresApproval | approval required | +| safe-verb-external-path-prompts | Approval/HostAllowed | Personal | Project | Interactive | cat /etc/passwd | none | RequiresApproval | approval required | +| safe-verb-external-redirect-prompts | Approval/HostAllowed | Personal | Project | Interactive | git status > {TempPath}netclaw-approval-matrix.txt | none | RequiresApproval | approval required | +| mutating-verb-project-prompts | Approval/HostAllowed | Personal | Project | Interactive | git push | none | RequiresApproval | approval required | +| all-safe-compound-allows | Approval/HostAllowed | Personal | Project | Interactive | git status && git log | none | Allowed | SafeVerbInTrustedScope | +| mixed-safe-unsafe-compound-prompts | Approval/HostAllowed | Personal | Project | Interactive | git status && git push | none | RequiresApproval | approval required | +| safe-pipe-unsafe-tail-prompts | Approval/HostAllowed | Personal | Project | Interactive | git status \| git push | none | RequiresApproval | approval required | +| added-safe-verb-project-allows | Approval/HostAllowed+eza | Personal | Project | Interactive | eza | none | Allowed | SafeVerbInTrustedScope | +| echo-allows-without-grant | Approval/HostAllowed | Personal | Project | Interactive | echo hello | none | Allowed | ApprovalExemptShellCandidates | +| printf-allows-without-grant | Approval/HostAllowed | Personal | Project | Interactive | printf hello | none | Allowed | ApprovalExemptShellCandidates | +| echo-redirect-prompts | Approval/HostAllowed | Personal | Project | Interactive | echo hello > result.txt | none | RequiresApproval | approval required | +| echo-done-fails-closed | Approval/HostAllowed | Personal | Project | Interactive | echo done | none | RequiresApproval | approval required | +| control-flow-fails-closed | Approval/HostAllowed | Personal | Project | Interactive | for f in *.txt; do cat "$f"; done | persistent[anywhere]:cat | RequiresApproval | approval required | +| empty-command-fails-closed | Approval/HostAllowed | Personal | Project | Interactive | | none | RequiresApproval | approval required | +| whitespace-command-fails-closed | Approval/HostAllowed | Personal | Project | Interactive | | none | RequiresApproval | approval required | +| session-grant-allows | Approval/HostAllowed | Personal | Project | Interactive | git push | session[this-chat]:git push | Allowed | StoredApproval | +| other-session-grant-prompts | Approval/HostAllowed | Personal | Project | Interactive | git push | session[other-chat]:git push | RequiresApproval | approval required | +| persistent-anywhere-allows | Approval/HostAllowed | Personal | Project | Interactive | git push | persistent[anywhere]:git push | Allowed | StoredApproval | +| persistent-here-allows | Approval/HostAllowed | Personal | Project | Interactive | git push | persistent[project]:git push | Allowed | StoredApproval | +| persistent-here-directory-mismatch-prompts | Approval/HostAllowed | Personal | External | Interactive | git push | persistent[project]:git push | RequiresApproval | approval required | +| other-audience-grant-prompts | Approval/HostAllowed | Personal | Project | Interactive | git push | persistent[anywhere,Team]:git push | RequiresApproval | approval required | +| mixed-session-persistent-compound-allows | Approval/HostAllowed | Personal | Project | Interactive | git status && git push | session[this-chat]:git status, persistent[anywhere]:git push | Allowed | StoredApproval | +| partial-compound-grant-prompts | Approval/HostAllowed | Personal | Project | Interactive | git status && git push | persistent[anywhere]:git status | RequiresApproval | approval required | +| noninteractive-unapproved-requires-approval | Approval/HostAllowed | Personal | Project | Non-interactive | git push | none | RequiresApproval | approval required | +| noninteractive-persistent-grant-allows | Approval/HostAllowed | Personal | Project | Non-interactive | git push | persistent[anywhere]:git push | Allowed | StoredApproval | +| noninteractive-exempt-allows | Approval/HostAllowed | Personal | Project | Non-interactive | echo hello | none | Allowed | ApprovalExemptShellCandidates | diff --git a/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.cs b/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.cs new file mode 100644 index 000000000..150ef08ce --- /dev/null +++ b/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.cs @@ -0,0 +1,51 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using Xunit; + +namespace Netclaw.Actors.Tests.Tools; + +[Collection(ShellApprovalMatrixCollection.Name)] +public sealed class ShellApprovalDispositionMatrixTests(ShellApprovalMatrixFixture fixture) +{ + public static bool IsPosix => !OperatingSystem.IsWindows(); + + [SlopwatchSuppress("SW001", "This theory defines Bash authorization behavior. The Windows shell parser does not implement this contract.")] + [Theory(SkipUnless = nameof(IsPosix), Skip = "The first matrix defines Bash authorization behavior.")] + [MemberData(nameof(ShellApprovalCases.Rows), MemberType = typeof(ShellApprovalCases))] + public async Task Shell_approval_contract(string caseId) + { + var testCase = ShellApprovalCases.Get(caseId); + await using var harness = await ShellApprovalHarness.CreateAsync( + testCase, + fixture.ActorSystem, + TestContext.Current.CancellationToken); + + var observed = await harness.EvaluateAsync(TestContext.Current.CancellationToken); + + Assert.Equal(testCase.Expected.Outcome, observed.Outcome); + Assert.Equal(testCase.Expected.AllowReason, observed.AllowReason); + Assert.Equal(testCase.Expected.DenyReason, observed.DenyReason); + Assert.Equal(testCase.Expected.Candidates, observed.Candidates); + Assert.Equal(testCase.Expected.IsMessy, observed.IsMessy); + Assert.Equal(testCase.Expected.ApprovalChecks, harness.ApprovalService.CheckCount); + Assert.Equal(testCase.Expected.ApprovalMatches, observed.ApprovalMatches); + } + + [Fact] + public Task Shell_approval_cases_match_review_table() + => Verifier.Verify(ShellApprovalCases.RenderReviewTable(), extension: "md"); +} + +/// +/// Supplies source-level Slopwatch suppressions without a runtime package dependency. +/// +[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)] +internal sealed class SlopwatchSuppressAttribute(string ruleId, string reason) : Attribute +{ + public string RuleId { get; } = ruleId; + + public string Reason { get; } = reason; +} diff --git a/src/Netclaw.Actors.Tests/Tools/ShellApprovalHarness.cs b/src/Netclaw.Actors.Tests/Tools/ShellApprovalHarness.cs new file mode 100644 index 000000000..7598fd266 --- /dev/null +++ b/src/Netclaw.Actors.Tests/Tools/ShellApprovalHarness.cs @@ -0,0 +1,321 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using Akka.Actor; +using Akka.Hosting; +using Akka.Pattern; +using Microsoft.Extensions.AI; +using Netclaw.Actors.Hosting; +using Netclaw.Actors.Tools; +using Netclaw.Configuration; +using Netclaw.Security; +using Netclaw.Tests.Utilities; +using Netclaw.Tools; +using Xunit; + +namespace Netclaw.Actors.Tests.Tools; + +internal sealed record ObservedApproval( + ToolAuthorizationOutcome Outcome, + ToolAllowReason? AllowReason, + string? DenyReason, + IReadOnlyList Candidates, + bool? IsMessy, + IReadOnlyList ApprovalMatches); + +internal sealed class ShellApprovalHarness : IAsyncDisposable +{ + private const string InvocationSessionId = "signalr/approval-matrix"; + private const string OtherSessionId = "signalr/other-session"; + + private readonly string _rootDirectory; + private readonly IActorRef _approvalActor; + private readonly FunctionCallContent _toolCall; + private readonly ToolExecutionContext _context; + private readonly DispatchingToolExecutor _executor; + + private ShellApprovalHarness( + string rootDirectory, + IActorRef approvalActor, + FunctionCallContent toolCall, + ToolExecutionContext context, + DispatchingToolExecutor executor, + CountingApprovalService approvalService) + { + _rootDirectory = rootDirectory; + _approvalActor = approvalActor; + _toolCall = toolCall; + _context = context; + _executor = executor; + ApprovalService = approvalService; + } + + public CountingApprovalService ApprovalService { get; } + + public static async Task CreateAsync( + ShellApprovalCase testCase, + ActorSystem actorSystem, + CancellationToken ct) + { + var rootDirectory = Path.Combine( + Path.GetTempPath(), + "netclaw-approval-matrix", + Guid.NewGuid().ToString("N")); + var projectDirectory = Path.Combine(rootDirectory, "project"); + var sessionDirectory = Path.Combine(rootDirectory, "session"); + var externalDirectory = Path.Combine(rootDirectory, "external"); + Directory.CreateDirectory(projectDirectory); + Directory.CreateDirectory(sessionDirectory); + Directory.CreateDirectory(externalDirectory); + + var store = new ToolApprovalStore(Path.Combine(rootDirectory, "tool-approvals.json")); + var approvalActor = CreateApprovalActor(actorSystem, store); + var approvalService = CreateApprovalService(approvalActor); + + var persistentSeeds = testCase.Approvals.Seeds + .Where(seed => seed.Source == ApprovalSeedSource.Persistent) + .ToList(); + foreach (var seed in persistentSeeds) + { + await approvalService.RecordApprovalAsync( + (ToolApprovalSessionId)"seed/persistent", + seed.Audience, + new ToolName(ShellTool.ToolName), + [seed.Pattern], + persistent: true, + ResolveDirectory(seed.Directory, projectDirectory, sessionDirectory, externalDirectory), + ct); + } + + if (persistentSeeds.Count > 0) + { + await approvalActor.GracefulStop(TimeSpan.FromSeconds(5)); + approvalActor = CreateApprovalActor(actorSystem, store); + approvalService = CreateApprovalService(approvalActor); + } + + foreach (var seed in testCase.Approvals.Seeds.Where(seed => seed.Source == ApprovalSeedSource.Session)) + { + await approvalService.RecordApprovalAsync( + (ToolApprovalSessionId)ResolveSession(seed.Session), + seed.Audience, + new ToolName(ShellTool.ToolName), + [seed.Pattern], + persistent: false, + cwd: null, + ct); + } + + var countingApprovalService = new CountingApprovalService(approvalService); + var config = CreateConfig(testCase); + var registry = new ToolRegistry(); + registry.WithFirstPartyTools( + config, + new NetclawPaths(), + new ToolPathPolicy([]), + new ShellCommandPolicy()); + + var safeVerbs = testCase.Policy.AdditionalSafeVerb is null + ? SafeVerbLoader.Load() + : SafeVerbList.FromVerbs([testCase.Policy.AdditionalSafeVerb]); + var policy = new ToolAccessPolicy( + config, + new EffectivePolicyDefaults( + DeploymentPosture.Personal, + TrustAudience.Personal, + ShellExecutionMode.HostAllowed, + UsedStrictFallback: false), + shellCommandPolicy: new ShellCommandPolicy(), + shellTrustZonePolicy: new ShellTrustZonePolicy( + config, + new NetclawPaths(rootDirectory, Path.Combine(rootDirectory, "workspaces"))), + safeVerbs: safeVerbs); + var executor = new DispatchingToolExecutor(registry, policy, countingApprovalService); + + var workingDirectory = ResolveDirectory( + testCase.Invocation.WorkingDirectory, + projectDirectory, + sessionDirectory, + externalDirectory); + var arguments = workingDirectory is null + ? ToolInput.Create("Command", testCase.Invocation.Command) + : ToolInput.Create( + "Command", testCase.Invocation.Command, + "WorkingDirectory", workingDirectory); + var toolCall = new FunctionCallContent(testCase.Id, ShellTool.ToolName, arguments); + var context = TestToolExecutionContext.CreateBound( + InvocationSessionId, + sessionDirectory, + new TestToolExecutionContextOptions + { + Audience = testCase.Invocation.Audience, + ProjectDirectory = projectDirectory, + InteractiveApproval = TestToolExecutionContext.InteractiveApproval(testCase.Invocation.Interactive) + }); + + return new ShellApprovalHarness( + rootDirectory, + approvalActor, + toolCall, + context, + executor, + countingApprovalService); + } + + public async Task EvaluateAsync(CancellationToken ct) + { + var decision = await _executor.EvaluateAuthorizationAsync(_toolCall, _context, ct); + var approvalContext = decision.ApprovalContext; + + return new ObservedApproval( + decision.Outcome, + decision.AllowReason, + decision.DenyReason, + approvalContext?.CandidateVerbs ?? [], + approvalContext?.IsMessy, + decision.ApprovalMatches + .Select(match => $"{match.Source}:{match.Pattern}") + .ToList()); + } + + public async ValueTask DisposeAsync() + { + await _approvalActor.GracefulStop(TimeSpan.FromSeconds(5)); + if (Directory.Exists(_rootDirectory)) + Directory.Delete(_rootDirectory, recursive: true); + } + + private static ToolConfig CreateConfig(ShellApprovalCase testCase) + { + var config = new ToolConfig { ShellMode = testCase.Policy.ShellMode }; + var profile = ToolAudienceProfileDefaults.GetResolvedProfile( + config.AudienceProfiles, + testCase.Invocation.Audience); + + if (!profile.AllowedTools.Contains(ShellTool.ToolName, StringComparer.Ordinal)) + profile.AllowedTools.Add(ShellTool.ToolName); + + profile.ApprovalPolicy = testCase.Policy.Approval switch + { + ApprovalPolicyShape.Missing => null, + ApprovalPolicyShape.Approval => ExactPolicy(ToolApprovalMode.Approval), + ApprovalPolicyShape.Auto => ExactPolicy(ToolApprovalMode.Auto), + ApprovalPolicyShape.Deny => ExactPolicy(ToolApprovalMode.Deny), + _ => throw new ArgumentOutOfRangeException( + nameof(testCase), + testCase.Policy.Approval, + "Unknown approval policy shape.") + }; + + return config; + } + + private static ToolApprovalConfig ExactPolicy(ToolApprovalMode mode) + => new() + { + ToolOverrides = new Dictionary(StringComparer.Ordinal) + { + [ShellTool.ToolName] = mode + } + }; + + private static IActorRef CreateApprovalActor(ActorSystem actorSystem, ToolApprovalStore store) + => actorSystem.ActorOf( + ToolApprovalActor.CreateProps(store), + $"approval-matrix-{Guid.NewGuid():N}"); + + private static AkkaToolApprovalService CreateApprovalService(IActorRef actor) + => new(new StubRequiredActor(actor)); + + private static string ResolveSession(ApprovalSessionShape session) + => session switch + { + ApprovalSessionShape.Invocation => InvocationSessionId, + ApprovalSessionShape.Other => OtherSessionId, + _ => throw new ArgumentOutOfRangeException(nameof(session), session, "Unknown approval session shape.") + }; + + private static string? ResolveDirectory( + ApprovalDirectoryShape directory, + string projectDirectory, + string sessionDirectory, + string externalDirectory) + => directory switch + { + ApprovalDirectoryShape.None => null, + ApprovalDirectoryShape.Project => projectDirectory, + ApprovalDirectoryShape.Session => sessionDirectory, + ApprovalDirectoryShape.External => externalDirectory, + _ => throw new ArgumentOutOfRangeException(nameof(directory), directory, "Unknown approval directory shape.") + }; + + private sealed class StubRequiredActor(IActorRef actor) : IRequiredActor + { + public IActorRef ActorRef => actor; + + public Task GetAsync(CancellationToken cancellationToken = default) + => Task.FromResult(actor); + } +} + +internal sealed class CountingApprovalService(IToolApprovalService inner) : IToolApprovalService +{ + private int _checkCount; + + public int CheckCount => Volatile.Read(ref _checkCount); + + public async Task CheckApprovalAsync( + ToolApprovalSessionId? sessionId, + TrustAudience audience, + ToolName toolName, + IReadOnlyList candidates, + string? cwd, + CancellationToken ct = default) + { + Interlocked.Increment(ref _checkCount); + return await inner.CheckApprovalAsync(sessionId, audience, toolName, candidates, cwd, ct); + } + + public Task> GetUnapprovedPatternsAsync( + ToolApprovalSessionId? sessionId, + TrustAudience audience, + ToolName toolName, + IReadOnlyList patterns, + string? cwd, + CancellationToken ct = default) + => inner.GetUnapprovedPatternsAsync(sessionId, audience, toolName, patterns, cwd, ct); + + public Task RecordApprovalAsync( + ToolApprovalSessionId sessionId, + TrustAudience audience, + ToolName toolName, + IReadOnlyList patterns, + bool persistent, + string? cwd, + CancellationToken ct = default) + => inner.RecordApprovalAsync(sessionId, audience, toolName, patterns, persistent, cwd, ct); +} + +public sealed class ShellApprovalMatrixFixture : IAsyncLifetime +{ + public ActorSystem ActorSystem { get; private set; } = null!; + + public ValueTask InitializeAsync() + { + ActorSystem = ActorSystem.Create($"shell-approval-matrix-{Guid.NewGuid():N}"); + return ValueTask.CompletedTask; + } + + public async ValueTask DisposeAsync() + { + await ActorSystem.Terminate(); + } +} + +[CollectionDefinition(Name)] +public sealed class ShellApprovalMatrixCollection : ICollectionFixture +{ + public const string Name = "Shell approval matrix"; +} diff --git a/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs b/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs index 83d4201cc..64fb2d7f2 100644 --- a/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs @@ -46,134 +46,6 @@ private static INetclawTool ShellTool() return new ShellTool(config, new ToolPathPolicy([]), new ShellCommandPolicy()); } - [Fact] - public void Shell_in_approval_mode_returns_RequiresApproval_when_unapproved() - { - var policy = CreatePolicy(ToolApprovalMode.Approval); - var args = ToolInput.Create("Command", "git push origin main"); - - var decision = policy.AuthorizeInvocation(ShellTool(), PersonalContext(), args); - - Assert.True(decision.NeedsApproval); - Assert.NotNull(decision.ApprovalContext); - Assert.Equal("shell_execute", decision.ApprovalContext!.ToolName); - Assert.Contains("git push origin main", decision.ApprovalContext.Patterns); - } - - [Fact] - public void Shell_in_deny_mode_returns_deny() - { - var policy = CreatePolicy(ToolApprovalMode.Deny); - var args = ToolInput.Create("Command", "git push"); - - var decision = policy.AuthorizeInvocation(ShellTool(), PersonalContext(), args); - - Assert.False(decision.Allowed); - Assert.Equal("tool_denied_by_approval_policy", decision.DenyReason); - } - - [Fact] - public void Shell_in_auto_mode_allows_without_approval() - { - var policy = CreatePolicy(ToolApprovalMode.Auto); - var args = ToolInput.Create("Command", "git push"); - - var decision = policy.AuthorizeInvocation(ShellTool(), PersonalContext(), args); - - Assert.True(decision.Allowed); - Assert.False(decision.NeedsApproval); - Assert.Equal(ToolAllowReason.PolicyAuto, decision.AllowReason); - } - - [Fact] - public void Safe_verb_in_trusted_scope_reports_allow_reason() - { - var projectDirectory = Path.Combine( - Path.GetTempPath(), - $"netclaw-safe-reason-{Guid.NewGuid():N}"); - Directory.CreateDirectory(projectDirectory); - try - { - var config = new ToolConfig { ShellMode = ShellExecutionMode.HostAllowed }; - config.AudienceProfiles.Personal.ApprovalPolicy = new ToolApprovalConfig - { - ToolOverrides = new Dictionary(StringComparer.Ordinal) - { - ["shell_execute"] = ToolApprovalMode.Approval - } - }; - var policy = new ToolAccessPolicy( - config, - new EffectivePolicyDefaults( - DeploymentPosture.Personal, - TrustAudience.Personal, - ShellExecutionMode.HostAllowed, - UsedStrictFallback: false), - safeVerbs: SafeVerbList.FromVerbs(["git status"])); - var context = TestToolExecutionContext.CreateBound( - "signalr/thread-safe-reason", - null, - new TestToolExecutionContextOptions - { - Audience = TrustAudience.Personal, - ProjectDirectory = projectDirectory, - InteractiveApproval = TestToolExecutionContext.InteractiveApproval(true) - }); - - var decision = policy.AuthorizeInvocation( - ShellTool(), - context, - ToolInput.Create( - "Command", "git status", - "WorkingDirectory", projectDirectory)); - - Assert.True(decision.Allowed); - Assert.Equal(ToolAllowReason.SafeVerbInTrustedScope, decision.AllowReason); - } - finally - { - Directory.Delete(projectDirectory, recursive: true); - } - } - - [Fact] - public void Missing_personal_approval_policy_fails_closed_for_shell() - { - var config = new ToolConfig { ShellMode = ShellExecutionMode.HostAllowed }; - config.AudienceProfiles.Personal.ApprovalPolicy = null; - - var policy = new ToolAccessPolicy( - config, - new EffectivePolicyDefaults( - DeploymentPosture.Personal, - TrustAudience.Personal, - ShellExecutionMode.HostAllowed, - UsedStrictFallback: false)); - - var args = ToolInput.Create("Command", "git pull --ff-only"); - - var decision = policy.AuthorizeInvocation(ShellTool(), PersonalContext(), args); - - Assert.True(decision.NeedsApproval); - Assert.NotNull(decision.ApprovalContext); - Assert.Equal("shell_execute", decision.ApprovalContext!.ToolName); - } - - [Fact] - public void Unsupported_channel_returns_requires_approval_for_store_check() - { - var policy = CreatePolicy(ToolApprovalMode.Approval); - var args = ToolInput.Create("Command", "git push"); - - var decision = policy.AuthorizeInvocation(ShellTool(), PersonalContext(supportsApproval: false), args); - - // Non-interactive channels no longer auto-deny — they fall through to - // RequiresApproval so the executor can check the persistent approval store. - Assert.True(decision.NeedsApproval); - Assert.NotNull(decision.ApprovalContext); - Assert.Contains("git push", decision.ApprovalContext!.Patterns); - } - [Fact] public void Compound_command_surfaces_all_approval_patterns_for_service_filtering() { @@ -188,37 +60,6 @@ public void Compound_command_surfaces_all_approval_patterns_for_service_filterin Assert.Contains("git push", decision.ApprovalContext.Patterns); } - [Fact] - public void Hard_denied_shell_command_is_blocked_before_approval() - { - var config = new ToolConfig { ShellMode = ShellExecutionMode.HostAllowed }; - config.AudienceProfiles.Personal.ApprovalPolicy = new ToolApprovalConfig - { - ToolOverrides = new Dictionary(StringComparer.Ordinal) - { - ["shell_execute"] = ToolApprovalMode.Approval - } - }; - - var policy = new ToolAccessPolicy( - config, - new EffectivePolicyDefaults( - DeploymentPosture.Personal, - TrustAudience.Personal, - ShellExecutionMode.HostAllowed, - UsedStrictFallback: false), - new ShellCommandPolicy()); - - var decision = policy.AuthorizeInvocation( - ShellTool(), - PersonalContext(), - ToolInput.Create("Command", "netclaw daemon stop")); - - Assert.False(decision.Allowed); - Assert.False(decision.NeedsApproval); - Assert.Equal("hard_deny_self_destructive", decision.DenyReason); - } - private const string ControlPlaneRoot = "/home/user/.netclaw/config"; private static ToolAccessPolicy CreateFileWritePolicy(ToolApprovalConfig? approvalPolicy = null) From a0159db14ec0106baad06c50af4e8d1df97a3491 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Tue, 4 Aug 2026 02:22:25 +0000 Subject: [PATCH 3/5] Use the Personal install policy in approval tests --- .../Tools/ShellApprovalCaseCatalog.cs | 297 ++++++++++-------- ...roval_cases_match_review_table.verified.md | 112 ++++--- .../Tools/ShellApprovalHarness.cs | 40 +-- .../Tools/ToolApprovalGateTests.cs | 47 +++ .../Tui/Config/SecurityAccessViewModel.cs | 16 +- .../Steps/SecurityPostureStepViewModel.cs | 19 +- .../SecurityPolicyDefaultsTests.cs | 9 + .../ToolAudienceProfiles.cs | 21 ++ 8 files changed, 311 insertions(+), 250 deletions(-) diff --git a/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs b/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs index 7447c0e63..278b7683b 100644 --- a/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs +++ b/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs @@ -10,25 +10,6 @@ namespace Netclaw.Actors.Tests.Tools; -/// -/// Defines the explicit approval-policy shape that a matrix case installs. -/// The value tests the secure fallback without a policy object. -/// -internal enum ApprovalPolicyShape -{ - /// The audience profile has no explicit approval policy. - Missing, - - /// The shell tool requires approval unless another gate grants access. - Approval, - - /// The approval policy grants shell access without a stored approval. - Auto, - - /// The approval policy denies shell access. - Deny -} - /// /// Names a logical directory that the harness resolves inside its isolated test root. /// This type prevents a case from embedding a harness-specific temporary path. @@ -74,16 +55,6 @@ internal enum ApprovalSessionShape Other } -internal sealed record ShellApprovalPolicy( - ApprovalPolicyShape Approval, - ShellExecutionMode ShellMode = ShellExecutionMode.HostAllowed, - string? AdditionalSafeVerb = null) -{ - public string Display => AdditionalSafeVerb is null - ? $"{Approval}/{ShellMode}" - : $"{Approval}/{ShellMode}+{AdditionalSafeVerb}"; -} - internal sealed record ShellApprovalInvocation( string Command, ApprovalDirectoryShape WorkingDirectory = ApprovalDirectoryShape.Project, @@ -221,257 +192,297 @@ public static ExpectedApproval Deny(string reason) internal sealed record ShellApprovalCase( string Id, - ShellApprovalPolicy Policy, ShellApprovalInvocation Invocation, ApprovalState Approvals, ExpectedApproval Expected); public static class ShellApprovalCases { - private static readonly ShellApprovalPolicy MissingPolicy = new(ApprovalPolicyShape.Missing); - private static readonly ShellApprovalPolicy ApprovalPolicy = new(ApprovalPolicyShape.Approval); - private static readonly ShellApprovalPolicy AutoPolicy = new(ApprovalPolicyShape.Auto); - private static readonly ShellApprovalPolicy DenyPolicy = new(ApprovalPolicyShape.Deny); - internal static IReadOnlyList All { get; } = [ Case( - "missing-policy-prompts", - MissingPolicy, - Bash("git push origin dev"), - Approvals.None, - ExpectedApproval.Require(["git push origin dev"])), - Case( - "exact-approval-prompts", - ApprovalPolicy, + "mutating-command-prompts", Bash("git push origin dev"), Approvals.None, ExpectedApproval.Require(["git push origin dev"])), - Case( - "exact-auto-allows", - AutoPolicy, - Bash("git push origin dev"), - Approvals.None, - ExpectedApproval.Allow(ToolAllowReason.PolicyAuto)), - Case( - "exact-deny-denies", - DenyPolicy, - Bash("git push origin dev"), - Approvals.None, - ExpectedApproval.Deny("tool_denied_by_approval_policy")), - Case( - "missing-policy-persistent-grant-allows", - MissingPolicy, - Bash("git push origin dev"), - Approvals.PersistentAnywhere("git push origin dev"), - ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "persistent:git push origin dev")), Case( "team-audience-denied", - ApprovalPolicy, Bash("git push", audience: TrustAudience.Team), Approvals.None, - ExpectedApproval.Deny("shell_requires_personal_context")), + ExpectedApproval.Deny("tool_not_allowed_for_audience_profile")), Case( "public-audience-denied", - ApprovalPolicy, Bash("git push", audience: TrustAudience.Public), Approvals.None, - ExpectedApproval.Deny("shell_requires_personal_context")), - Case( - "team-auto-still-denied", - AutoPolicy, - Bash("git push", audience: TrustAudience.Team), - Approvals.None, - ExpectedApproval.Deny("shell_requires_personal_context")), - Case( - "public-auto-still-denied", - AutoPolicy, - Bash("git push", audience: TrustAudience.Public), - Approvals.None, - ExpectedApproval.Deny("shell_requires_personal_context")), - - Case( - "shell-off-denies", - new ShellApprovalPolicy(ApprovalPolicyShape.Auto, ShellExecutionMode.Off), - Bash("git status"), - Approvals.None, - ExpectedApproval.Deny("shell_disabled")), - Case( - "sandbox-only-denies", - new ShellApprovalPolicy(ApprovalPolicyShape.Auto, ShellExecutionMode.SandboxOnly), - Bash("git status"), - Approvals.None, - ExpectedApproval.Deny("shell_requires_sandbox_backend")), + ExpectedApproval.Deny("tool_not_allowed_for_audience_profile")), Case( - "hard-deny-beats-approval", - ApprovalPolicy, - Bash("netclaw daemon stop"), - Approvals.None, - ExpectedApproval.Deny("hard_deny_self_destructive")), - Case( - "hard-deny-beats-auto", - AutoPolicy, + "hard-deny-blocks", Bash("netclaw daemon stop"), Approvals.None, ExpectedApproval.Deny("hard_deny_self_destructive")), Case( "hard-deny-beats-stored-grant", - ApprovalPolicy, Bash("netclaw daemon stop"), Approvals.PersistentAnywhere("netclaw daemon stop"), ExpectedApproval.Deny("hard_deny_self_destructive")), Case( "compound-hard-deny-denies", - AutoPolicy, Bash("git status && netclaw daemon stop"), Approvals.None, ExpectedApproval.Deny("hard_deny_self_destructive")), Case( "safe-verb-project-allows", - ApprovalPolicy, Bash("git status"), Approvals.None, ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)), Case( "safe-verb-session-allows", - ApprovalPolicy, Bash("git status", ApprovalDirectoryShape.Session), Approvals.None, ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)), Case( "safe-verb-external-prompts", - ApprovalPolicy, Bash("git status", ApprovalDirectoryShape.External), Approvals.None, ExpectedApproval.Require(["git status"])), Case( "safe-verb-external-path-prompts", - ApprovalPolicy, Bash("cat /etc/passwd"), Approvals.None, ExpectedApproval.Require(["cat"])), Case( "safe-verb-external-redirect-prompts", - ApprovalPolicy, Bash("git status > /tmp/netclaw-approval-matrix.txt"), Approvals.None, ExpectedApproval.Require(["git status"])), Case( "mutating-verb-project-prompts", - ApprovalPolicy, Bash("git push"), Approvals.None, ExpectedApproval.Require(["git push"])), Case( "all-safe-compound-allows", - ApprovalPolicy, Bash("git status && git log"), Approvals.None, ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)), Case( "mixed-safe-unsafe-compound-prompts", - ApprovalPolicy, Bash("git status && git push"), Approvals.None, ExpectedApproval.Require(["git status", "git push"])), Case( "safe-pipe-unsafe-tail-prompts", - ApprovalPolicy, Bash("git status | git push"), Approvals.None, ExpectedApproval.Require(["git status", "git push"])), Case( - "added-safe-verb-project-allows", - new ShellApprovalPolicy(ApprovalPolicyShape.Approval, AdditionalSafeVerb: "eza"), - Bash("eza"), + "safe-pipeline-allows", + Bash("git log | head -20"), Approvals.None, ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)), + Case( + "semicolon-sequence-prompts", + Bash("git status; git push"), + Approvals.None, + ExpectedApproval.Require(["git status", "git push"])), + Case( + "newline-sequence-prompts", + Bash("git status\ngit push"), + Approvals.None, + ExpectedApproval.Require(["git status", "git push"])), + Case( + "or-chain-prompts", + Bash("git status || git push"), + Approvals.None, + ExpectedApproval.Require(["git status", "git push"])), + Case( + "three-step-release-prompts", + Bash("git add . && git commit -m fix && git push origin dev"), + Approvals.None, + ExpectedApproval.Require(["git add", "git commit", "git push origin dev"])), + Case( + "hard-deny-pipeline-tail-currently-prompts", + Bash("echo safe | netclaw daemon stop"), + Approvals.None, + ExpectedApproval.Require(["echo", "netclaw daemon stop"])), + Case( + "hard-deny-nested-shell-blocks", + Bash("bash -lc \"netclaw daemon stop\""), + Approvals.None, + ExpectedApproval.Deny("hard_deny_self_destructive")), + Case( + "nested-shell-currently-prompts-for-wrapper", + Bash("bash -lc \"git push\""), + Approvals.None, + ExpectedApproval.Require(["bash"])), + Case( + "nested-shell-inner-grant-currently-does-not-match", + Bash("bash -lc \"git push\""), + Approvals.PersistentAnywhere("git push"), + ExpectedApproval.Require(["bash"])), + Case( + "nested-shell-wrapper-grant-currently-allows", + Bash("bash -lc \"git push\""), + Approvals.PersistentAnywhere("bash"), + ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "persistent:bash")), + Case( + "env-nested-shell-prompts", + Bash("env bash -lc \"git push\""), + Approvals.None, + ExpectedApproval.Require(["env bash"])), + Case( + "timeout-nested-shell-prompts", + Bash("timeout 5 bash -lc \"git push\""), + Approvals.None, + ExpectedApproval.Require(["timeout"])), + Case( + "subshell-prompts", + Bash("(git status && git push)"), + Approvals.None, + ExpectedApproval.Require(["git status", "git push"])), + Case( + "command-substitution-currently-auto-allows", + Bash("echo $(git push)"), + Approvals.None, + ExpectedApproval.Allow(ToolAllowReason.ApprovalExemptShellCandidates)), + Case( + "background-list-currently-auto-allows", + Bash("git status & git push"), + Approvals.None, + ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)), + Case( + "unbalanced-quote-fails-closed", + Bash("git push \"unterminated"), + Approvals.None, + ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)), + Case( + "multiline-argument-prompts", + Bash("gh issue comment 123 --body \"first line\nsecond line\""), + Approvals.None, + ExpectedApproval.Require(["gh issue comment"])), + Case( + "approved-pipeline-head-does-not-cover-tail", + Bash("git push | curl https://example.com"), + Approvals.PersistentAnywhere("git push"), + ExpectedApproval.Require( + ["git push", "curl"], + approvalMatches: ["persistent:git push"])), + Case( + "all-pipeline-clauses-approved", + Bash("git push | curl https://example.com"), + Approvals.PersistentAnywhere("git push", "curl"), + ExpectedApproval.Allow( + ToolAllowReason.StoredApproval, + 1, + "persistent:git push", + "persistent:curl")), + Case( + "input-redirect-outside-zone-prompts", + Bash("cat < /etc/passwd"), + Approvals.None, + ExpectedApproval.Require(["cat"])), + Case( + "error-redirect-outside-zone-prompts", + Bash("git status 2> /tmp/netclaw-approval-errors.txt"), + Approvals.None, + ExpectedApproval.Require(["git status"])), + Case( + "cd-current-then-safe-allows", + Bash("cd . && git status"), + Approvals.None, + ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)), + Case( + "cd-parent-then-safe-prompts", + Bash("cd .. && git status"), + Approvals.None, + ExpectedApproval.Require(["cd", "git status"])), + Case( + "multiple-cd-then-safe-prompts", + Bash("cd . && cd .. && git status"), + Approvals.None, + ExpectedApproval.Require(["cd", "git status"])), + Case( + "side-effect-before-mutation-prompts", + Bash("echo ready && git push"), + Approvals.None, + ExpectedApproval.Require(["echo", "git push"])), + Case( + "heredoc-prompts", + Bash("cat <<'EOF'\nhello\nEOF"), + Approvals.None, + ExpectedApproval.Require([], approvalChecks: 0)), Case( "echo-allows-without-grant", - ApprovalPolicy, Bash("echo hello"), Approvals.None, ExpectedApproval.Allow(ToolAllowReason.ApprovalExemptShellCandidates)), Case( "printf-allows-without-grant", - ApprovalPolicy, Bash("printf hello"), Approvals.None, ExpectedApproval.Allow(ToolAllowReason.ApprovalExemptShellCandidates)), Case( "echo-redirect-prompts", - ApprovalPolicy, Bash("echo hello > result.txt"), Approvals.None, ExpectedApproval.Require(["echo"])), Case( "echo-done-fails-closed", - ApprovalPolicy, Bash("echo done"), Approvals.None, ExpectedApproval.Require(["echo"], isMessy: true, approvalChecks: 0)), Case( "control-flow-fails-closed", - ApprovalPolicy, Bash("for f in *.txt; do cat \"$f\"; done"), Approvals.PersistentAnywhere("cat"), ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)), Case( "empty-command-fails-closed", - ApprovalPolicy, Bash(string.Empty), Approvals.None, ExpectedApproval.Require([], approvalChecks: 0)), Case( "whitespace-command-fails-closed", - ApprovalPolicy, Bash(" "), Approvals.None, ExpectedApproval.Require([], approvalChecks: 0)), Case( "session-grant-allows", - ApprovalPolicy, Bash("git push"), Approvals.Session("git push"), ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "session:git push")), Case( "other-session-grant-prompts", - ApprovalPolicy, Bash("git push"), Approvals.SessionForOtherSession("git push"), ExpectedApproval.Require(["git push"])), Case( "persistent-anywhere-allows", - ApprovalPolicy, Bash("git push"), Approvals.PersistentAnywhere("git push"), ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "persistent:git push")), Case( "persistent-here-allows", - ApprovalPolicy, Bash("git push"), Approvals.PersistentHere(ApprovalDirectoryShape.Project, "git push"), ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "persistent:git push")), Case( "persistent-here-directory-mismatch-prompts", - ApprovalPolicy, Bash("git push", ApprovalDirectoryShape.External), Approvals.PersistentHere(ApprovalDirectoryShape.Project, "git push"), ExpectedApproval.Require(["git push"])), Case( "other-audience-grant-prompts", - ApprovalPolicy, Bash("git push"), Approvals.PersistentForOtherAudience("git push"), ExpectedApproval.Require(["git push"])), Case( "mixed-session-persistent-compound-allows", - ApprovalPolicy, Bash("git status && git push"), Approvals.Combine( Approvals.Session("git status"), @@ -483,7 +494,6 @@ public static class ShellApprovalCases "persistent:git push")), Case( "partial-compound-grant-prompts", - ApprovalPolicy, Bash("git status && git push"), Approvals.PersistentAnywhere("git status"), ExpectedApproval.Require( @@ -492,19 +502,16 @@ public static class ShellApprovalCases Case( "noninteractive-unapproved-requires-approval", - ApprovalPolicy, Bash("git push", interactive: false), Approvals.None, ExpectedApproval.Require(["git push"])), Case( "noninteractive-persistent-grant-allows", - ApprovalPolicy, Bash("git push", interactive: false), Approvals.PersistentAnywhere("git push"), ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "persistent:git push")), Case( "noninteractive-exempt-allows", - ApprovalPolicy, Bash("echo hello", interactive: false), Approvals.None, ExpectedApproval.Allow(ToolAllowReason.ApprovalExemptShellCandidates)) @@ -525,28 +532,33 @@ internal static string RenderReviewTable() { var lines = new List { - "| ID | Policy | Audience | Cwd | Interaction | Command | Approval state | Result | Reason |", - "| --- | --- | --- | --- | --- | --- | --- | --- | --- |" + "# Fresh Personal approval matrix", + string.Empty, + "`Tools.ShellMode`: `HostAllowed`", + string.Empty, + "`Personal.ApprovalPolicy.shell_execute`: `Approval`", + string.Empty, + "| ID | Audience | Cwd | Interaction | Command | Approval state | Result | Reason | Candidates | Complex |", + "| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- |" }; lines.AddRange(All.Select(testCase => - $"| {testCase.Id} | {testCase.Policy.Display} | " + - $"{testCase.Invocation.Audience} | {testCase.Invocation.WorkingDirectory} | " + + $"| {testCase.Id} | {testCase.Invocation.Audience} | {testCase.Invocation.WorkingDirectory} | " + $"{(testCase.Invocation.Interactive ? "Interactive" : "Non-interactive")} | " + $"{Escape(testCase.Invocation.Command)} | " + $"{Escape(testCase.Approvals.Display)} | {testCase.Expected.Outcome} | " + - $"{testCase.Expected.AllowReason?.ToString() ?? testCase.Expected.DenyReason ?? "approval required"} |")); + $"{testCase.Expected.AllowReason?.ToString() ?? testCase.Expected.DenyReason ?? "approval required"} | " + + $"{Escape(DisplayCandidates(testCase.Expected.Candidates))} | {DisplayComplexity(testCase.Expected.IsMessy)} |")); return string.Join(Environment.NewLine, lines) + Environment.NewLine; } private static ShellApprovalCase Case( string id, - ShellApprovalPolicy policy, ShellApprovalInvocation invocation, ApprovalState approvals, ExpectedApproval expected) - => new(id, policy, invocation, approvals, expected); + => new(id, invocation, approvals, expected); private static ShellApprovalInvocation Bash( string command, @@ -560,4 +572,15 @@ private static string Escape(string value) .Replace("|", "\\|", StringComparison.Ordinal) .Replace("\r", "\\r", StringComparison.Ordinal) .Replace("\n", "\\n", StringComparison.Ordinal); + + private static string DisplayCandidates(IReadOnlyList candidates) + => candidates.Count == 0 ? "none" : string.Join(", ", candidates); + + private static string DisplayComplexity(bool? isMessy) + => isMessy switch + { + true => "Yes", + false => "No", + null => "Not applicable" + }; } diff --git a/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.Shell_approval_cases_match_review_table.verified.md b/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.Shell_approval_cases_match_review_table.verified.md index 46c74f679..ad5bebc33 100644 --- a/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.Shell_approval_cases_match_review_table.verified.md +++ b/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.Shell_approval_cases_match_review_table.verified.md @@ -1,45 +1,67 @@ -| ID | Policy | Audience | Cwd | Interaction | Command | Approval state | Result | Reason | -| --- | --- | --- | --- | --- | --- | --- | --- | --- | -| missing-policy-prompts | Missing/HostAllowed | Personal | Project | Interactive | git push origin dev | none | RequiresApproval | approval required | -| exact-approval-prompts | Approval/HostAllowed | Personal | Project | Interactive | git push origin dev | none | RequiresApproval | approval required | -| exact-auto-allows | Auto/HostAllowed | Personal | Project | Interactive | git push origin dev | none | Allowed | PolicyAuto | -| exact-deny-denies | Deny/HostAllowed | Personal | Project | Interactive | git push origin dev | none | Denied | tool_denied_by_approval_policy | -| missing-policy-persistent-grant-allows | Missing/HostAllowed | Personal | Project | Interactive | git push origin dev | persistent[anywhere]:git push origin dev | Allowed | StoredApproval | -| team-audience-denied | Approval/HostAllowed | Team | Project | Interactive | git push | none | Denied | shell_requires_personal_context | -| public-audience-denied | Approval/HostAllowed | Public | Project | Interactive | git push | none | Denied | shell_requires_personal_context | -| team-auto-still-denied | Auto/HostAllowed | Team | Project | Interactive | git push | none | Denied | shell_requires_personal_context | -| public-auto-still-denied | Auto/HostAllowed | Public | Project | Interactive | git push | none | Denied | shell_requires_personal_context | -| shell-off-denies | Auto/Off | Personal | Project | Interactive | git status | none | Denied | shell_disabled | -| sandbox-only-denies | Auto/SandboxOnly | Personal | Project | Interactive | git status | none | Denied | shell_requires_sandbox_backend | -| hard-deny-beats-approval | Approval/HostAllowed | Personal | Project | Interactive | netclaw daemon stop | none | Denied | hard_deny_self_destructive | -| hard-deny-beats-auto | Auto/HostAllowed | Personal | Project | Interactive | netclaw daemon stop | none | Denied | hard_deny_self_destructive | -| hard-deny-beats-stored-grant | Approval/HostAllowed | Personal | Project | Interactive | netclaw daemon stop | persistent[anywhere]:netclaw daemon stop | Denied | hard_deny_self_destructive | -| compound-hard-deny-denies | Auto/HostAllowed | Personal | Project | Interactive | git status && netclaw daemon stop | none | Denied | hard_deny_self_destructive | -| safe-verb-project-allows | Approval/HostAllowed | Personal | Project | Interactive | git status | none | Allowed | SafeVerbInTrustedScope | -| safe-verb-session-allows | Approval/HostAllowed | Personal | Session | Interactive | git status | none | Allowed | SafeVerbInTrustedScope | -| safe-verb-external-prompts | Approval/HostAllowed | Personal | External | Interactive | git status | none | RequiresApproval | approval required | -| safe-verb-external-path-prompts | Approval/HostAllowed | Personal | Project | Interactive | cat /etc/passwd | none | RequiresApproval | approval required | -| safe-verb-external-redirect-prompts | Approval/HostAllowed | Personal | Project | Interactive | git status > {TempPath}netclaw-approval-matrix.txt | none | RequiresApproval | approval required | -| mutating-verb-project-prompts | Approval/HostAllowed | Personal | Project | Interactive | git push | none | RequiresApproval | approval required | -| all-safe-compound-allows | Approval/HostAllowed | Personal | Project | Interactive | git status && git log | none | Allowed | SafeVerbInTrustedScope | -| mixed-safe-unsafe-compound-prompts | Approval/HostAllowed | Personal | Project | Interactive | git status && git push | none | RequiresApproval | approval required | -| safe-pipe-unsafe-tail-prompts | Approval/HostAllowed | Personal | Project | Interactive | git status \| git push | none | RequiresApproval | approval required | -| added-safe-verb-project-allows | Approval/HostAllowed+eza | Personal | Project | Interactive | eza | none | Allowed | SafeVerbInTrustedScope | -| echo-allows-without-grant | Approval/HostAllowed | Personal | Project | Interactive | echo hello | none | Allowed | ApprovalExemptShellCandidates | -| printf-allows-without-grant | Approval/HostAllowed | Personal | Project | Interactive | printf hello | none | Allowed | ApprovalExemptShellCandidates | -| echo-redirect-prompts | Approval/HostAllowed | Personal | Project | Interactive | echo hello > result.txt | none | RequiresApproval | approval required | -| echo-done-fails-closed | Approval/HostAllowed | Personal | Project | Interactive | echo done | none | RequiresApproval | approval required | -| control-flow-fails-closed | Approval/HostAllowed | Personal | Project | Interactive | for f in *.txt; do cat "$f"; done | persistent[anywhere]:cat | RequiresApproval | approval required | -| empty-command-fails-closed | Approval/HostAllowed | Personal | Project | Interactive | | none | RequiresApproval | approval required | -| whitespace-command-fails-closed | Approval/HostAllowed | Personal | Project | Interactive | | none | RequiresApproval | approval required | -| session-grant-allows | Approval/HostAllowed | Personal | Project | Interactive | git push | session[this-chat]:git push | Allowed | StoredApproval | -| other-session-grant-prompts | Approval/HostAllowed | Personal | Project | Interactive | git push | session[other-chat]:git push | RequiresApproval | approval required | -| persistent-anywhere-allows | Approval/HostAllowed | Personal | Project | Interactive | git push | persistent[anywhere]:git push | Allowed | StoredApproval | -| persistent-here-allows | Approval/HostAllowed | Personal | Project | Interactive | git push | persistent[project]:git push | Allowed | StoredApproval | -| persistent-here-directory-mismatch-prompts | Approval/HostAllowed | Personal | External | Interactive | git push | persistent[project]:git push | RequiresApproval | approval required | -| other-audience-grant-prompts | Approval/HostAllowed | Personal | Project | Interactive | git push | persistent[anywhere,Team]:git push | RequiresApproval | approval required | -| mixed-session-persistent-compound-allows | Approval/HostAllowed | Personal | Project | Interactive | git status && git push | session[this-chat]:git status, persistent[anywhere]:git push | Allowed | StoredApproval | -| partial-compound-grant-prompts | Approval/HostAllowed | Personal | Project | Interactive | git status && git push | persistent[anywhere]:git status | RequiresApproval | approval required | -| noninteractive-unapproved-requires-approval | Approval/HostAllowed | Personal | Project | Non-interactive | git push | none | RequiresApproval | approval required | -| noninteractive-persistent-grant-allows | Approval/HostAllowed | Personal | Project | Non-interactive | git push | persistent[anywhere]:git push | Allowed | StoredApproval | -| noninteractive-exempt-allows | Approval/HostAllowed | Personal | Project | Non-interactive | echo hello | none | Allowed | ApprovalExemptShellCandidates | +# Fresh Personal approval matrix + +`Tools.ShellMode`: `HostAllowed` + +`Personal.ApprovalPolicy.shell_execute`: `Approval` + +| ID | Audience | Cwd | Interaction | Command | Approval state | Result | Reason | Candidates | Complex | +| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | +| mutating-command-prompts | Personal | Project | Interactive | git push origin dev | none | RequiresApproval | approval required | git push origin dev | No | +| team-audience-denied | Team | Project | Interactive | git push | none | Denied | tool_not_allowed_for_audience_profile | none | Not applicable | +| public-audience-denied | Public | Project | Interactive | git push | none | Denied | tool_not_allowed_for_audience_profile | none | Not applicable | +| hard-deny-blocks | Personal | Project | Interactive | netclaw daemon stop | none | Denied | hard_deny_self_destructive | none | Not applicable | +| hard-deny-beats-stored-grant | Personal | Project | Interactive | netclaw daemon stop | persistent[anywhere]:netclaw daemon stop | Denied | hard_deny_self_destructive | none | Not applicable | +| compound-hard-deny-denies | Personal | Project | Interactive | git status && netclaw daemon stop | none | Denied | hard_deny_self_destructive | none | Not applicable | +| safe-verb-project-allows | Personal | Project | Interactive | git status | none | Allowed | SafeVerbInTrustedScope | none | Not applicable | +| safe-verb-session-allows | Personal | Session | Interactive | git status | none | Allowed | SafeVerbInTrustedScope | none | Not applicable | +| safe-verb-external-prompts | Personal | External | Interactive | git status | none | RequiresApproval | approval required | git status | No | +| safe-verb-external-path-prompts | Personal | Project | Interactive | cat /etc/passwd | none | RequiresApproval | approval required | cat | No | +| safe-verb-external-redirect-prompts | Personal | Project | Interactive | git status > {TempPath}netclaw-approval-matrix.txt | none | RequiresApproval | approval required | git status | No | +| mutating-verb-project-prompts | Personal | Project | Interactive | git push | none | RequiresApproval | approval required | git push | No | +| all-safe-compound-allows | Personal | Project | Interactive | git status && git log | none | Allowed | SafeVerbInTrustedScope | none | Not applicable | +| mixed-safe-unsafe-compound-prompts | Personal | Project | Interactive | git status && git push | none | RequiresApproval | approval required | git status, git push | No | +| safe-pipe-unsafe-tail-prompts | Personal | Project | Interactive | git status \| git push | none | RequiresApproval | approval required | git status, git push | No | +| safe-pipeline-allows | Personal | Project | Interactive | git log \| head -20 | none | Allowed | SafeVerbInTrustedScope | none | Not applicable | +| semicolon-sequence-prompts | Personal | Project | Interactive | git status; git push | none | RequiresApproval | approval required | git status, git push | No | +| newline-sequence-prompts | Personal | Project | Interactive | git status\ngit push | none | RequiresApproval | approval required | git status, git push | No | +| or-chain-prompts | Personal | Project | Interactive | git status \|\| git push | none | RequiresApproval | approval required | git status, git push | No | +| three-step-release-prompts | Personal | Project | Interactive | git add . && git commit -m fix && git push origin dev | none | RequiresApproval | approval required | git add, git commit, git push origin dev | No | +| hard-deny-pipeline-tail-currently-prompts | Personal | Project | Interactive | echo safe \| netclaw daemon stop | none | RequiresApproval | approval required | echo, netclaw daemon stop | No | +| hard-deny-nested-shell-blocks | Personal | Project | Interactive | bash -lc "netclaw daemon stop" | none | Denied | hard_deny_self_destructive | none | Not applicable | +| nested-shell-currently-prompts-for-wrapper | Personal | Project | Interactive | bash -lc "git push" | none | RequiresApproval | approval required | bash | No | +| nested-shell-inner-grant-currently-does-not-match | Personal | Project | Interactive | bash -lc "git push" | persistent[anywhere]:git push | RequiresApproval | approval required | bash | No | +| nested-shell-wrapper-grant-currently-allows | Personal | Project | Interactive | bash -lc "git push" | persistent[anywhere]:bash | Allowed | StoredApproval | none | Not applicable | +| env-nested-shell-prompts | Personal | Project | Interactive | env bash -lc "git push" | none | RequiresApproval | approval required | env bash | No | +| timeout-nested-shell-prompts | Personal | Project | Interactive | timeout 5 bash -lc "git push" | none | RequiresApproval | approval required | timeout | No | +| subshell-prompts | Personal | Project | Interactive | (git status && git push) | none | RequiresApproval | approval required | git status, git push | No | +| command-substitution-currently-auto-allows | Personal | Project | Interactive | echo $(git push) | none | Allowed | ApprovalExemptShellCandidates | none | Not applicable | +| background-list-currently-auto-allows | Personal | Project | Interactive | git status & git push | none | Allowed | SafeVerbInTrustedScope | none | Not applicable | +| unbalanced-quote-fails-closed | Personal | Project | Interactive | git push "unterminated | none | RequiresApproval | approval required | none | Yes | +| multiline-argument-prompts | Personal | Project | Interactive | gh issue comment 123 --body "first line\nsecond line" | none | RequiresApproval | approval required | gh issue comment | No | +| approved-pipeline-head-does-not-cover-tail | Personal | Project | Interactive | git push \| curl https://example.com | persistent[anywhere]:git push | RequiresApproval | approval required | git push, curl | No | +| all-pipeline-clauses-approved | Personal | Project | Interactive | git push \| curl https://example.com | persistent[anywhere]:git push, persistent[anywhere]:curl | Allowed | StoredApproval | none | Not applicable | +| input-redirect-outside-zone-prompts | Personal | Project | Interactive | cat < /etc/passwd | none | RequiresApproval | approval required | cat | No | +| error-redirect-outside-zone-prompts | Personal | Project | Interactive | git status 2> {TempPath}netclaw-approval-errors.txt | none | RequiresApproval | approval required | git status | No | +| cd-current-then-safe-allows | Personal | Project | Interactive | cd . && git status | none | Allowed | SafeVerbInTrustedScope | none | Not applicable | +| cd-parent-then-safe-prompts | Personal | Project | Interactive | cd .. && git status | none | RequiresApproval | approval required | cd, git status | No | +| multiple-cd-then-safe-prompts | Personal | Project | Interactive | cd . && cd .. && git status | none | RequiresApproval | approval required | cd, git status | No | +| side-effect-before-mutation-prompts | Personal | Project | Interactive | echo ready && git push | none | RequiresApproval | approval required | echo, git push | No | +| heredoc-prompts | Personal | Project | Interactive | cat <<'EOF'\nhello\nEOF | none | RequiresApproval | approval required | none | No | +| echo-allows-without-grant | Personal | Project | Interactive | echo hello | none | Allowed | ApprovalExemptShellCandidates | none | Not applicable | +| printf-allows-without-grant | Personal | Project | Interactive | printf hello | none | Allowed | ApprovalExemptShellCandidates | none | Not applicable | +| echo-redirect-prompts | Personal | Project | Interactive | echo hello > result.txt | none | RequiresApproval | approval required | echo | No | +| echo-done-fails-closed | Personal | Project | Interactive | echo done | none | RequiresApproval | approval required | echo | Yes | +| control-flow-fails-closed | Personal | Project | Interactive | for f in *.txt; do cat "$f"; done | persistent[anywhere]:cat | RequiresApproval | approval required | none | Yes | +| empty-command-fails-closed | Personal | Project | Interactive | | none | RequiresApproval | approval required | none | No | +| whitespace-command-fails-closed | Personal | Project | Interactive | | none | RequiresApproval | approval required | none | No | +| session-grant-allows | Personal | Project | Interactive | git push | session[this-chat]:git push | Allowed | StoredApproval | none | Not applicable | +| other-session-grant-prompts | Personal | Project | Interactive | git push | session[other-chat]:git push | RequiresApproval | approval required | git push | No | +| persistent-anywhere-allows | Personal | Project | Interactive | git push | persistent[anywhere]:git push | Allowed | StoredApproval | none | Not applicable | +| persistent-here-allows | Personal | Project | Interactive | git push | persistent[project]:git push | Allowed | StoredApproval | none | Not applicable | +| persistent-here-directory-mismatch-prompts | Personal | External | Interactive | git push | persistent[project]:git push | RequiresApproval | approval required | git push | No | +| other-audience-grant-prompts | Personal | Project | Interactive | git push | persistent[anywhere,Team]:git push | RequiresApproval | approval required | git push | No | +| mixed-session-persistent-compound-allows | Personal | Project | Interactive | git status && git push | session[this-chat]:git status, persistent[anywhere]:git push | Allowed | StoredApproval | none | Not applicable | +| partial-compound-grant-prompts | Personal | Project | Interactive | git status && git push | persistent[anywhere]:git status | RequiresApproval | approval required | git status, git push | No | +| noninteractive-unapproved-requires-approval | Personal | Project | Non-interactive | git push | none | RequiresApproval | approval required | git push | No | +| noninteractive-persistent-grant-allows | Personal | Project | Non-interactive | git push | persistent[anywhere]:git push | Allowed | StoredApproval | none | Not applicable | +| noninteractive-exempt-allows | Personal | Project | Non-interactive | echo hello | none | Allowed | ApprovalExemptShellCandidates | none | Not applicable | diff --git a/src/Netclaw.Actors.Tests/Tools/ShellApprovalHarness.cs b/src/Netclaw.Actors.Tests/Tools/ShellApprovalHarness.cs index 7598fd266..cda7c00a2 100644 --- a/src/Netclaw.Actors.Tests/Tools/ShellApprovalHarness.cs +++ b/src/Netclaw.Actors.Tests/Tools/ShellApprovalHarness.cs @@ -109,7 +109,7 @@ await approvalService.RecordApprovalAsync( } var countingApprovalService = new CountingApprovalService(approvalService); - var config = CreateConfig(testCase); + var config = CreateConfig(); var registry = new ToolRegistry(); registry.WithFirstPartyTools( config, @@ -117,9 +117,6 @@ await approvalService.RecordApprovalAsync( new ToolPathPolicy([]), new ShellCommandPolicy()); - var safeVerbs = testCase.Policy.AdditionalSafeVerb is null - ? SafeVerbLoader.Load() - : SafeVerbList.FromVerbs([testCase.Policy.AdditionalSafeVerb]); var policy = new ToolAccessPolicy( config, new EffectivePolicyDefaults( @@ -131,7 +128,7 @@ await approvalService.RecordApprovalAsync( shellTrustZonePolicy: new ShellTrustZonePolicy( config, new NetclawPaths(rootDirectory, Path.Combine(rootDirectory, "workspaces"))), - safeVerbs: safeVerbs); + safeVerbs: SafeVerbLoader.Load()); var executor = new DispatchingToolExecutor(registry, policy, countingApprovalService); var workingDirectory = ResolveDirectory( @@ -187,38 +184,11 @@ public async ValueTask DisposeAsync() Directory.Delete(_rootDirectory, recursive: true); } - private static ToolConfig CreateConfig(ShellApprovalCase testCase) - { - var config = new ToolConfig { ShellMode = testCase.Policy.ShellMode }; - var profile = ToolAudienceProfileDefaults.GetResolvedProfile( - config.AudienceProfiles, - testCase.Invocation.Audience); - - if (!profile.AllowedTools.Contains(ShellTool.ToolName, StringComparer.Ordinal)) - profile.AllowedTools.Add(ShellTool.ToolName); - - profile.ApprovalPolicy = testCase.Policy.Approval switch - { - ApprovalPolicyShape.Missing => null, - ApprovalPolicyShape.Approval => ExactPolicy(ToolApprovalMode.Approval), - ApprovalPolicyShape.Auto => ExactPolicy(ToolApprovalMode.Auto), - ApprovalPolicyShape.Deny => ExactPolicy(ToolApprovalMode.Deny), - _ => throw new ArgumentOutOfRangeException( - nameof(testCase), - testCase.Policy.Approval, - "Unknown approval policy shape.") - }; - - return config; - } - - private static ToolApprovalConfig ExactPolicy(ToolApprovalMode mode) + private static ToolConfig CreateConfig() => new() { - ToolOverrides = new Dictionary(StringComparer.Ordinal) - { - [ShellTool.ToolName] = mode - } + ShellMode = ShellExecutionMode.HostAllowed, + AudienceProfiles = ToolAudienceProfileDefaults.CreateProfilesForPosture(DeploymentPosture.Personal) }; private static IActorRef CreateApprovalActor(ActorSystem actorSystem, ToolApprovalStore store) diff --git a/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs b/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs index 64fb2d7f2..0a573c6fe 100644 --- a/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs @@ -46,6 +46,53 @@ private static INetclawTool ShellTool() return new ShellTool(config, new ToolPathPolicy([]), new ShellCommandPolicy()); } + [Fact] + public void Shell_in_deny_mode_returns_deny() + { + var policy = CreatePolicy(ToolApprovalMode.Deny); + var args = ToolInput.Create("Command", "git push"); + + var decision = policy.AuthorizeInvocation(ShellTool(), PersonalContext(), args); + + Assert.False(decision.Allowed); + Assert.Equal("tool_denied_by_approval_policy", decision.DenyReason); + } + + [Fact] + public void Shell_in_auto_mode_allows_without_approval() + { + var policy = CreatePolicy(ToolApprovalMode.Auto); + var args = ToolInput.Create("Command", "git push"); + + var decision = policy.AuthorizeInvocation(ShellTool(), PersonalContext(), args); + + Assert.True(decision.Allowed); + Assert.False(decision.NeedsApproval); + Assert.Equal(ToolAllowReason.PolicyAuto, decision.AllowReason); + } + + [Fact] + public void Missing_personal_approval_policy_fails_closed_for_shell() + { + var config = new ToolConfig { ShellMode = ShellExecutionMode.HostAllowed }; + config.AudienceProfiles.Personal.ApprovalPolicy = null; + var policy = new ToolAccessPolicy( + config, + new EffectivePolicyDefaults( + DeploymentPosture.Personal, + TrustAudience.Personal, + ShellExecutionMode.HostAllowed, + UsedStrictFallback: false)); + + var decision = policy.AuthorizeInvocation( + ShellTool(), + PersonalContext(), + ToolInput.Create("Command", "git pull --ff-only")); + + Assert.True(decision.NeedsApproval); + Assert.Equal("shell_execute", decision.ApprovalContext!.ToolName); + } + [Fact] public void Compound_command_surfaces_all_approval_patterns_for_service_filtering() { diff --git a/src/Netclaw.Cli/Tui/Config/SecurityAccessViewModel.cs b/src/Netclaw.Cli/Tui/Config/SecurityAccessViewModel.cs index 086583c2a..9f15e1de7 100644 --- a/src/Netclaw.Cli/Tui/Config/SecurityAccessViewModel.cs +++ b/src/Netclaw.Cli/Tui/Config/SecurityAccessViewModel.cs @@ -781,21 +781,7 @@ private static string ReadExposureModeSummary(Dictionary config) } private static ToolAudienceProfiles BuildPostureProfiles(DeploymentPosture posture) - { - var profiles = ToolAudienceProfileDefaults.CreateProfiles(); - if (posture == DeploymentPosture.Personal) - { - profiles.Personal.ApprovalPolicy = new ToolApprovalConfig - { - ToolOverrides = new Dictionary(StringComparer.Ordinal) - { - [ToolAudienceProfileToolCatalog.ShellExecute] = ToolApprovalMode.Approval - } - }; - } - - return profiles; - } + => ToolAudienceProfileDefaults.CreateProfilesForPosture(posture); private static ToolAudienceProfile GetProfile(ToolAudienceProfiles profiles, TrustAudience audience) => audience switch diff --git a/src/Netclaw.Cli/Tui/Wizard/Steps/SecurityPostureStepViewModel.cs b/src/Netclaw.Cli/Tui/Wizard/Steps/SecurityPostureStepViewModel.cs index 6e723b743..9565af512 100644 --- a/src/Netclaw.Cli/Tui/Wizard/Steps/SecurityPostureStepViewModel.cs +++ b/src/Netclaw.Cli/Tui/Wizard/Steps/SecurityPostureStepViewModel.cs @@ -144,25 +144,8 @@ private static Dictionary BuildToolsDictionary(DeploymentPosture private static ShellExecutionMode ShellModeFor(DeploymentPosture posture) => posture == DeploymentPosture.Personal ? ShellExecutionMode.HostAllowed : ShellExecutionMode.Off; - // Personal posture gates shell behind an approval prompt by default; the operator can override - // this in config for unrestricted shell. Shared by the typed (ContributeConfig) and section - // (BuildContribution) emission paths so they cannot drift on this default-deny security default. private static ToolAudienceProfiles BuildAudienceProfiles(DeploymentPosture posture) - { - var profiles = ToolAudienceProfileDefaults.CreateProfiles(); - if (posture == DeploymentPosture.Personal) - { - profiles.Personal.ApprovalPolicy = new ToolApprovalConfig - { - ToolOverrides = new Dictionary(StringComparer.Ordinal) - { - ["shell_execute"] = ToolApprovalMode.Approval - } - }; - } - - return profiles; - } + => ToolAudienceProfileDefaults.CreateProfilesForPosture(posture); public void Dispose() { diff --git a/src/Netclaw.Configuration.Tests/SecurityPolicyDefaultsTests.cs b/src/Netclaw.Configuration.Tests/SecurityPolicyDefaultsTests.cs index 94fcec538..2322a856e 100644 --- a/src/Netclaw.Configuration.Tests/SecurityPolicyDefaultsTests.cs +++ b/src/Netclaw.Configuration.Tests/SecurityPolicyDefaultsTests.cs @@ -70,4 +70,13 @@ public void Tool_profile_defaults_allow_personal_all_mode() Assert.Equal(ToolFilesystemMode.All, defaults.Personal.WriteFiles.Mode); Assert.Equal(ToolFilesystemMode.All, defaults.Personal.AttachFiles.Mode); } + + [Fact] + public void Personal_posture_requires_shell_approval() + { + var profiles = ToolAudienceProfileDefaults.CreateProfilesForPosture(DeploymentPosture.Personal); + + var policy = Assert.IsType(profiles.Personal.ApprovalPolicy); + Assert.Equal(ToolApprovalMode.Approval, policy.ToolOverrides[ToolAudienceProfileToolCatalog.ShellExecute]); + } } diff --git a/src/Netclaw.Configuration/ToolAudienceProfiles.cs b/src/Netclaw.Configuration/ToolAudienceProfiles.cs index 9c6123c6c..6892cfd55 100644 --- a/src/Netclaw.Configuration/ToolAudienceProfiles.cs +++ b/src/Netclaw.Configuration/ToolAudienceProfiles.cs @@ -192,6 +192,27 @@ public static class ToolAudienceProfileDefaults GlobalReadRoots = [SkillsDirectoryToken, IdentityDirectoryToken, WorkspacesDirectoryToken] }; + /// + /// Creates the audience profiles that a new installation stores for the selected posture. + /// Personal installations require approval for shell commands unless another authorization gate permits the command. + /// + public static ToolAudienceProfiles CreateProfilesForPosture(DeploymentPosture posture) + { + var profiles = CreateProfiles(); + if (posture == DeploymentPosture.Personal) + { + profiles.Personal.ApprovalPolicy = new ToolApprovalConfig + { + ToolOverrides = new Dictionary(StringComparer.Ordinal) + { + [ToolAudienceProfileToolCatalog.ShellExecute] = ToolApprovalMode.Approval + } + }; + } + + return profiles; + } + // Audience tool grants are monotonic: Public ⊆ Team ⊆ Personal. Public is // the least-trusted, fail-closed audience — read, enumerate, and attach // only: no file-mutation tools and no outbound web tools (web_search / From c7b13f113261d1d7d90eb876a3bab240cc162237 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Tue, 4 Aug 2026 02:32:27 +0000 Subject: [PATCH 4/5] Use the host temp path in approval snapshots --- src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs b/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs index 278b7683b..803ee9b35 100644 --- a/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs +++ b/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs @@ -255,7 +255,7 @@ public static class ShellApprovalCases ExpectedApproval.Require(["cat"])), Case( "safe-verb-external-redirect-prompts", - Bash("git status > /tmp/netclaw-approval-matrix.txt"), + Bash($"git status > {TemporaryFile("netclaw-approval-matrix.txt")}"), Approvals.None, ExpectedApproval.Require(["git status"])), Case( @@ -386,7 +386,7 @@ public static class ShellApprovalCases ExpectedApproval.Require(["cat"])), Case( "error-redirect-outside-zone-prompts", - Bash("git status 2> /tmp/netclaw-approval-errors.txt"), + Bash($"git status 2> {TemporaryFile("netclaw-approval-errors.txt")}"), Approvals.None, ExpectedApproval.Require(["git status"])), Case( @@ -567,6 +567,9 @@ private static ShellApprovalInvocation Bash( bool interactive = true) => new(command, workingDirectory, audience, interactive); + private static string TemporaryFile(string fileName) + => Path.Join(Path.GetTempPath(), fileName); + private static string Escape(string value) => value .Replace("|", "\\|", StringComparison.Ordinal) From 69e5af23707943dce1c362085a4a9904de559c9a Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Tue, 4 Aug 2026 02:56:20 +0000 Subject: [PATCH 5/5] Name observed shell candidate verbs precisely --- .../Tools/ShellApprovalDispositionMatrixTests.cs | 2 +- src/Netclaw.Actors.Tests/Tools/ShellApprovalHarness.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.cs b/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.cs index 150ef08ce..ec8d21910 100644 --- a/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.cs @@ -28,7 +28,7 @@ public async Task Shell_approval_contract(string caseId) Assert.Equal(testCase.Expected.Outcome, observed.Outcome); Assert.Equal(testCase.Expected.AllowReason, observed.AllowReason); Assert.Equal(testCase.Expected.DenyReason, observed.DenyReason); - Assert.Equal(testCase.Expected.Candidates, observed.Candidates); + Assert.Equal(testCase.Expected.Candidates, observed.CandidateVerbs); Assert.Equal(testCase.Expected.IsMessy, observed.IsMessy); Assert.Equal(testCase.Expected.ApprovalChecks, harness.ApprovalService.CheckCount); Assert.Equal(testCase.Expected.ApprovalMatches, observed.ApprovalMatches); diff --git a/src/Netclaw.Actors.Tests/Tools/ShellApprovalHarness.cs b/src/Netclaw.Actors.Tests/Tools/ShellApprovalHarness.cs index cda7c00a2..37c91f376 100644 --- a/src/Netclaw.Actors.Tests/Tools/ShellApprovalHarness.cs +++ b/src/Netclaw.Actors.Tests/Tools/ShellApprovalHarness.cs @@ -21,7 +21,7 @@ internal sealed record ObservedApproval( ToolAuthorizationOutcome Outcome, ToolAllowReason? AllowReason, string? DenyReason, - IReadOnlyList Candidates, + IReadOnlyList CandidateVerbs, bool? IsMessy, IReadOnlyList ApprovalMatches);