diff --git a/Directory.Packages.props b/Directory.Packages.props index 810d06828..9aa565b9a 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -75,7 +75,7 @@ - + diff --git a/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs b/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs index 5757159f4..198545d45 100644 --- a/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs +++ b/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs @@ -309,65 +309,65 @@ public static class ShellApprovalCases Approvals.None, ExpectedApproval.Require(["git add", "git commit", "git push origin dev"])), Case( - "hard-deny-pipeline-tail-currently-prompts", + "hard-deny-pipeline-tail-blocks", Bash("echo safe | netclaw daemon stop"), Approvals.None, - ExpectedApproval.Require(["echo", "netclaw daemon stop"])), + ExpectedApproval.Deny("hard_deny_self_destructive")), 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", + "nested-shell-prompts-for-inner-command", Bash("bash -lc \"git push\""), Approvals.None, - ExpectedApproval.Require(["bash"])), + ExpectedApproval.Require(["git push"])), Case( - "nested-shell-inner-grant-currently-does-not-match", + "nested-shell-inner-grant-allows", Bash("bash -lc \"git push\""), Approvals.PersistentAnywhere("git push"), - ExpectedApproval.Require(["bash"])), + ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "persistent:git push")), Case( - "nested-shell-wrapper-grant-currently-allows", + "nested-shell-wrapper-grant-does-not-cover-inner-command", Bash("bash -lc \"git push\""), Approvals.PersistentAnywhere("bash"), - ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "persistent:bash")), + ExpectedApproval.Require(["git push"])), Case( "env-nested-shell-prompts", Bash("env bash -lc \"git push\""), Approvals.None, - ExpectedApproval.Require(["env bash"])), + ExpectedApproval.Require(["git push"])), Case( "timeout-nested-shell-prompts", Bash("timeout 5 bash -lc \"git push\""), Approvals.None, - ExpectedApproval.Require(["timeout"])), + ExpectedApproval.Require(["timeout", "git push"])), Case( "subshell-prompts", Bash("(git status && git push)"), Approvals.None, ExpectedApproval.Require(["git status", "git push"])), Case( - "command-substitution-currently-auto-allows", + "command-substitution-fails-closed", Bash("echo $(git push)"), Approvals.None, - ExpectedApproval.Allow(ToolAllowReason.ApprovalExemptShellCandidates)), + ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)), Case( - "dynamic-path-currently-auto-allows", + "dynamic-path-fails-closed", Bash("cat \"$FILE\""), Approvals.None, - ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)), + ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)), Case( - "dynamic-redirect-currently-prompts", + "dynamic-redirect-fails-closed", Bash("git status > \"$OUTPUT\""), Approvals.None, - ExpectedApproval.Require(["git status"])), + ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)), Case( - "background-list-currently-auto-allows", + "background-list-prompts-for-mutating-tail", Bash("git status & git push"), Approvals.None, - ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)), + ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)), Case( "unbalanced-quote-fails-closed", Bash("git push \"unterminated"), @@ -428,7 +428,7 @@ public static class ShellApprovalCases "heredoc-prompts", Bash("cat <<'EOF'\nhello\nEOF"), Approvals.None, - ExpectedApproval.Require([], approvalChecks: 0)), + ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)), Case( "echo-allows-without-grant", @@ -446,30 +446,30 @@ public static class ShellApprovalCases Approvals.None, ExpectedApproval.Require(["echo"])), Case( - "echo-done-fails-closed", + "echo-control-word-argument-allows", Bash("echo done"), Approvals.None, - ExpectedApproval.Require(["echo"], isMessy: true, approvalChecks: 0)), + ExpectedApproval.Allow(ToolAllowReason.ApprovalExemptShellCandidates)), Case( "control-flow-fails-closed", Bash("for f in *.txt; do cat \"$f\"; done"), Approvals.PersistentAnywhere("cat"), ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)), Case( - "process-substitution-currently-prompts-without-complex-flag", + "process-substitution-fails-closed", Bash("cat <(git push)"), Approvals.PersistentAnywhere("cat", "git push"), - ExpectedApproval.Require([], approvalChecks: 0)), + ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)), Case( - "arithmetic-expansion-currently-prompts-without-complex-flag", + "arithmetic-expansion-fails-closed", Bash("echo $((1 + 2))"), Approvals.None, - ExpectedApproval.Require([], approvalChecks: 0)), + ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)), Case( - "function-definition-currently-prompts-without-complex-flag", + "function-definition-fails-closed", Bash("deploy() { git push; }; deploy"), Approvals.PersistentAnywhere("git push"), - ExpectedApproval.Require([], approvalChecks: 0)), + ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)), Case( "inline-python-prompts-for-interpreter", Bash("python3 -c \"print('hello')\""), @@ -494,7 +494,7 @@ public static class ShellApprovalCases "inline-python-heredoc-fails-closed", Bash("python3 <<'PY'\nprint('hello')\nPY"), Approvals.PersistentAnywhere("python3"), - ExpectedApproval.Require([], approvalChecks: 0)), + ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)), Case( "empty-command-fails-closed", Bash(string.Empty), 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 a629768ff..22e085c84 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 @@ -27,18 +27,18 @@ | 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-pipeline-tail-blocks | Personal | Project | Interactive | echo safe \| netclaw daemon stop | none | Denied | hard_deny_self_destructive | none | Not applicable | | 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 | +| nested-shell-prompts-for-inner-command | Personal | Project | Interactive | bash -lc "git push" | none | RequiresApproval | approval required | git push | No | +| nested-shell-inner-grant-allows | Personal | Project | Interactive | bash -lc "git push" | persistent[anywhere]:git push | Allowed | StoredApproval | none | Not applicable | +| nested-shell-wrapper-grant-does-not-cover-inner-command | Personal | Project | Interactive | bash -lc "git push" | persistent[anywhere]:bash | RequiresApproval | approval required | git push | No | +| env-nested-shell-prompts | Personal | Project | Interactive | env bash -lc "git push" | none | RequiresApproval | approval required | git push | No | +| timeout-nested-shell-prompts | Personal | Project | Interactive | timeout 5 bash -lc "git push" | none | RequiresApproval | approval required | timeout, git push | 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 | -| dynamic-path-currently-auto-allows | Personal | Project | Interactive | cat "$FILE" | none | Allowed | SafeVerbInTrustedScope | none | Not applicable | -| dynamic-redirect-currently-prompts | Personal | Project | Interactive | git status > "$OUTPUT" | none | RequiresApproval | approval required | git status | No | -| background-list-currently-auto-allows | Personal | Project | Interactive | git status & git push | none | Allowed | SafeVerbInTrustedScope | none | Not applicable | +| command-substitution-fails-closed | Personal | Project | Interactive | echo $(git push) | none | RequiresApproval | approval required | none | Yes | +| dynamic-path-fails-closed | Personal | Project | Interactive | cat "$FILE" | none | RequiresApproval | approval required | none | Yes | +| dynamic-redirect-fails-closed | Personal | Project | Interactive | git status > "$OUTPUT" | none | RequiresApproval | approval required | none | Yes | +| background-list-prompts-for-mutating-tail | Personal | Project | Interactive | git status & git push | none | RequiresApproval | approval required | none | Yes | | 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 | @@ -49,20 +49,20 @@ | 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 | +| heredoc-prompts | Personal | Project | Interactive | cat <<'EOF'\nhello\nEOF | none | RequiresApproval | approval required | none | Yes | | 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 | +| echo-control-word-argument-allows | Personal | Project | Interactive | echo done | none | Allowed | ApprovalExemptShellCandidates | none | Not applicable | | control-flow-fails-closed | Personal | Project | Interactive | for f in *.txt; do cat "$f"; done | persistent[anywhere]:cat | RequiresApproval | approval required | none | Yes | -| process-substitution-currently-prompts-without-complex-flag | Personal | Project | Interactive | cat <(git push) | persistent[anywhere]:cat, persistent[anywhere]:git push | RequiresApproval | approval required | none | No | -| arithmetic-expansion-currently-prompts-without-complex-flag | Personal | Project | Interactive | echo $((1 + 2)) | none | RequiresApproval | approval required | none | No | -| function-definition-currently-prompts-without-complex-flag | Personal | Project | Interactive | deploy() { git push; }; deploy | persistent[anywhere]:git push | RequiresApproval | approval required | none | No | +| process-substitution-fails-closed | Personal | Project | Interactive | cat <(git push) | persistent[anywhere]:cat, persistent[anywhere]:git push | RequiresApproval | approval required | none | Yes | +| arithmetic-expansion-fails-closed | Personal | Project | Interactive | echo $((1 + 2)) | none | RequiresApproval | approval required | none | Yes | +| function-definition-fails-closed | Personal | Project | Interactive | deploy() { git push; }; deploy | persistent[anywhere]:git push | RequiresApproval | approval required | none | Yes | | inline-python-prompts-for-interpreter | Personal | Project | Interactive | python3 -c "print('hello')" | none | RequiresApproval | approval required | python3 | No | | inline-python-interpreter-grant-currently-allows | Personal | Project | Interactive | python3 -c "print('hello')" | persistent[anywhere]:python3 | Allowed | StoredApproval | none | Not applicable | | eval-prompts-for-interpreter | Personal | Project | Interactive | eval "$CODE" | none | RequiresApproval | approval required | eval | No | | eval-grant-currently-allows-dynamic-payload | Personal | Project | Interactive | eval "$CODE" | persistent[anywhere]:eval | Allowed | StoredApproval | none | Not applicable | -| inline-python-heredoc-fails-closed | Personal | Project | Interactive | python3 <<'PY'\nprint('hello')\nPY | persistent[anywhere]:python3 | RequiresApproval | approval required | none | No | +| inline-python-heredoc-fails-closed | Personal | Project | Interactive | python3 <<'PY'\nprint('hello')\nPY | persistent[anywhere]:python3 | 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 | diff --git a/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs b/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs index e5cea786c..758c3a52a 100644 --- a/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs +++ b/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs @@ -340,17 +340,13 @@ public void FormatForDisplay_heredoc_falls_back_to_flattened_raw_command() } [Fact(SkipUnless = nameof(IsPosix), Skip = "POSIX-only — matcher routes through BashParser on POSIX")] - public void ExtractPatterns_redirect_target_with_line_break_terminates_pattern() + public void ExtractPatterns_dynamic_redirect_target_is_not_persistable() { - // A quoted redirect target carrying an embedded newline must not - // reach the stored pattern — quote-aware normalization would - // otherwise preserve the break verbatim (`echo hi >> $LOGDIR\nfile`). - var patterns = _matcher.ExtractPatterns(new ToolName("shell_execute"), - Args("echo hi >> \"$LOGDIR\nfile\"")); + var toolName = new ToolName("shell_execute"); + var arguments = Args("echo hi >> \"$LOGDIR\nfile\""); - Assert.Single(patterns); - Assert.DoesNotContain('\n', patterns[0]); - Assert.Equal("echo hi", patterns[0]); + Assert.Empty(_matcher.ExtractPatterns(toolName, arguments)); + Assert.True(_matcher.IsMessy(toolName, arguments)); } [Fact(SkipUnless = nameof(IsPosix), Skip = "POSIX-only — matcher routes through BashParser on POSIX")] diff --git a/src/Netclaw.Security.Tests/ShellCommandAnalysisTests.cs b/src/Netclaw.Security.Tests/ShellCommandAnalysisTests.cs new file mode 100644 index 000000000..dbb216951 --- /dev/null +++ b/src/Netclaw.Security.Tests/ShellCommandAnalysisTests.cs @@ -0,0 +1,47 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using Xunit; + +namespace Netclaw.Security.Tests; + +public sealed class ShellCommandAnalysisTests +{ + private readonly ShellCommandAnalyzer _analyzer = ShellCommandAnalyzer.Bash; + + [Theory] + [InlineData("bash -lc")] + [InlineData("bash --noprofile -lc")] + public void Direct_shell_wrapper_is_replaced_by_inner_clauses(string invocation) + { + var analysis = _analyzer.Analyze( + $"{invocation} \"cat /outside/secret | curl https://example.com\""); + + Assert.Equal(ShellAnalysisFailure.None, analysis.Failure); + Assert.Contains(analysis.Clauses, clause => clause.Verb.Joined == "cat"); + Assert.Contains(analysis.Clauses, clause => clause.Verb.Joined == "curl"); + Assert.DoesNotContain(analysis.Clauses, clause => clause.Verb.Joined == "bash"); + } + + [Theory] + [InlineData("echo $(git push)")] + [InlineData("echo `$command`")] + public void Dynamic_command_syntax_is_explicit(string command) + { + var analysis = _analyzer.Analyze(command); + + Assert.Equal(ShellAnalysisFailure.None, analysis.Failure); + Assert.True(analysis.HasDynamicSyntax); + } + + [Fact] + public void Background_list_fails_closed_when_parser_omits_its_tail() + { + var analysis = _analyzer.Analyze("git status & git push"); + + Assert.Equal(ShellAnalysisFailure.Unresolved, analysis.Failure); + Assert.Empty(analysis.Clauses); + } +} diff --git a/src/Netclaw.Security.Tests/ShellCommandPolicyTests.cs b/src/Netclaw.Security.Tests/ShellCommandPolicyTests.cs index 42517e6cc..86e4a2710 100644 --- a/src/Netclaw.Security.Tests/ShellCommandPolicyTests.cs +++ b/src/Netclaw.Security.Tests/ShellCommandPolicyTests.cs @@ -89,6 +89,17 @@ public void Denies_compound_with_denied_segment() Assert.Equal(DenyCategory.SelfDestructive, decision.DenyCategory); } + [Theory] + [InlineData("echo safe | netclaw daemon stop")] + [InlineData("printf safe | sudo kill -9 123")] + [InlineData("bash -c \"echo safe | netclaw daemon stop\"")] + public void Denies_pipeline_with_denied_tail(string command) + { + var decision = _policy.EvaluateBash(command); + + Assert.False(decision.Allowed); + } + [Fact] public void Allows_compound_of_safe_commands() { @@ -101,9 +112,15 @@ public void Allows_compound_of_safe_commands() [Theory] [InlineData("bash -c \"netclaw daemon stop\"")] [InlineData("bash -lc \"netclaw daemon stop\"")] + [InlineData("bash --noprofile -lc \"netclaw daemon stop\"")] + [InlineData("env bash -lc \"netclaw daemon stop\"")] + [InlineData("command bash -lc \"netclaw daemon stop\"")] + [InlineData("timeout 5 bash -lc \"netclaw daemon stop\"")] + [InlineData("nice -n 5 bash -lc \"netclaw daemon stop\"")] + [InlineData("bash -c \"echo safe\" && bash -lc \"netclaw daemon stop\"")] public void Denies_bash_wrapping_denied_command(string command) { - var decision = _policy.Evaluate(command); + var decision = _policy.EvaluateBash(command); Assert.False(decision.Allowed); Assert.Equal(DenyCategory.SelfDestructive, decision.DenyCategory); } diff --git a/src/Netclaw.Security.Tests/ShellSyntaxTreeIntegrationTests.cs b/src/Netclaw.Security.Tests/ShellSyntaxTreeIntegrationTests.cs index a6ec7e6ad..aa50faa77 100644 --- a/src/Netclaw.Security.Tests/ShellSyntaxTreeIntegrationTests.cs +++ b/src/Netclaw.Security.Tests/ShellSyntaxTreeIntegrationTests.cs @@ -181,6 +181,20 @@ public void Dynamic_token_marked_for_skip() Assert.Null(argWithDynamic.Resolved); } + [Fact] + public void Command_string_wrapper_marks_inner_clause() + { + var parser = new BashParser(); + + var result = parser.Parse("bash -c \"git status\""); + + Assert.False(result.IsUnparseable); + var clause = Assert.Single(result.Clauses); + Assert.True(clause.IsCommandStringWrapped); + Assert.Equal("git status", clause.Verb.Joined); + Assert.Null(clause.Verb.CanonicalVerb); + } + [Fact] public void Leading_line_comment_is_stripped_from_clause_extraction() { @@ -272,7 +286,7 @@ public void Control_flow_keyword_opening_newline_clause_is_unparseable() { // A control-flow keyword that opens a newline-separated clause makes // the whole parse unparseable (control flow is unsupported in v0.1). - // ExtractCandidatesViaBashParser maps that to an empty candidate + // The Bash analysis maps that to an empty candidate // list, so the approval gate fails closed to a Once/Deny prompt. var parser = new BashParser(); diff --git a/src/Netclaw.Security/IToolApprovalMatcher.cs b/src/Netclaw.Security/IToolApprovalMatcher.cs index 35afa0163..74632ca6d 100644 --- a/src/Netclaw.Security/IToolApprovalMatcher.cs +++ b/src/Netclaw.Security/IToolApprovalMatcher.cs @@ -110,10 +110,7 @@ public sealed class ShellApprovalMatcher : IToolApprovalMatcher { public static readonly ShellApprovalMatcher Instance = new(); - // BashParser is immutable — Parse delegates to a pure static — so one - // shared instance is safe and avoids a per-evaluation allocation. The DI - // container registers it as a singleton for the same reason. - private static readonly ShellSyntaxTree.BashParser Parser = new(); + private static readonly ShellCommandAnalyzer Analyzer = ShellCommandAnalyzer.Bash; public string GetApprovalModeKey(ToolName toolName, IDictionary? arguments) => toolName.Value; @@ -137,7 +134,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, GetWorkingDirectory(arguments)); + return ExtractCandidatesViaBashAnalysis(command, GetWorkingDirectory(arguments)); var seen = new HashSet<(string, string?)>(); var candidates = new List(); @@ -196,38 +193,25 @@ public IReadOnlyList ExtractCandidates(ToolName toolName, IDi } /// - /// Parses , returning null when the parser - /// rejects it (unparseable, no clauses) or throws — a parser exception - /// must never take down the approval flow. Callers treat null as - /// "cannot decompose" and apply their own fail-safe: an empty - /// pattern/candidate list (messy semantics — Once/Deny prompt only) or - /// the flattened raw command for display. + /// Returns null when the parser cannot resolve the complete command. + /// Callers then offer only one-time approval or show the raw command. /// - private static ShellSyntaxTree.ParsedCommand? TryParseCommand( + private static ShellCommandAnalysis? TryAnalyzeCommand( string command, string? workingDirectory = null) { - try - { - 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 - { - return null; - } + var result = Analyzer.Analyze(command, workingDirectory); + return result.Failure == ShellAnalysisFailure.None && result.Clauses.Count > 0 + ? result + : null; } - private static IReadOnlyList ExtractCandidatesViaBashParser( + private static IReadOnlyList ExtractCandidatesViaBashAnalysis( string command, string? workingDirectory) { - var result = TryParseCommand(command, workingDirectory); - if (result is null) + var result = TryAnalyzeCommand(command, workingDirectory); + if (result is null || result.HasDynamicSyntax) return []; // The prompt groups a pipe as one approval unit. Authorization still @@ -247,8 +231,12 @@ private static IReadOnlyList ExtractCandidatesViaBashParser( // `git tag 0.4.2` matches. Mirrors the value-termination in // ReconstructClauseText so the gate candidate and the persisted // pattern normalize identically. - var verb = ShellTokenizer.ApplyVerbShortCircuit( - string.Join(" ", TrimTrailingValueTokens(clause.Verb.Tokens))); + if (clause.Verb.IsDynamic) + continue; + + var parsedVerb = clause.Verb.CanonicalVerb + ?? string.Join(" ", TrimTrailingValueTokens(clause.Verb.Tokens)); + var verb = ShellTokenizer.ApplyVerbShortCircuit(parsedVerb); if (string.IsNullOrEmpty(verb)) continue; @@ -352,18 +340,14 @@ private static IReadOnlyList ExtractCandidatesViaBashParser( /// commands — mirroring the legacy /// empty-result contract so the prompt builder offers only Once/Deny. /// - private static IReadOnlyList ExtractApprovalUnitsViaBashParser( + private static IReadOnlyList ExtractApprovalUnitsViaBashAnalysis( string command, string? workingDirectory) { - // Messy commands (control-flow keywords, unbalanced brackets) cannot - // be cleanly decomposed into approval units; mirror the legacy - // splitter, which returns no units for them. - if (ShellTokenizer.IsMessyCompoundCommand(command)) - return []; - - var result = TryParseCommand(command, workingDirectory); - if (result is null) + // The parser is the sole structural authority. Dynamic or unresolved + // syntax cannot produce a persistent approval unit. + var result = TryAnalyzeCommand(command, workingDirectory); + if (result is null || result.HasDynamicSyntax) return []; try @@ -560,17 +544,8 @@ public bool IsApproved( if (string.IsNullOrWhiteSpace(command)) return false; - // Messy commands cannot be auto-approved: the matcher cannot extract a - // candidate verb-chain to evaluate against the persisted store, so - // every messy invocation must round-trip through the user. The prompt - // builder offers only Once/Deny in this case (see IsMessy). - if (ShellTokenizer.IsMessyCompoundCommand(command)) - return false; - - // Fail-closed on a parser miss: BashParser swallows exceptions and - // unparseable results return an empty candidate list. Treating that - // as "approved" would silently auto-allow any command our parser - // regresses on. Force the gate instead. + // Empty candidates include parser failures and dynamic syntax. + // Both cases must return to the approval gate. var candidates = ExtractCandidates(toolName, arguments); if (candidates.Count == 0) return false; @@ -593,7 +568,17 @@ public bool IsApproved( } public bool IsMessy(ToolName toolName, IDictionary? arguments) - => ShellTokenizer.IsMessyCompoundCommand(GetCommand(arguments)); + { + var command = GetCommand(arguments); + if (string.IsNullOrWhiteSpace(command)) + return false; + + if (OperatingSystem.IsWindows()) + return ShellTokenizer.IsMessyCompoundCommand(command); + + var analysis = TryAnalyzeCommand(command, GetWorkingDirectory(arguments)); + return analysis is null || analysis.HasDynamicSyntax; + } public string FormatForDisplay(ToolName toolName, IDictionary? arguments) { @@ -636,7 +621,7 @@ public string FormatForDisplay(ToolName toolName, IDictionary? /// private static string BuildSanitizedDisplayViaParser(string command) { - var result = TryParseCommand(command); + var result = TryAnalyzeCommand(command); if (result is null) return command; diff --git a/src/Netclaw.Security/Netclaw.Security.csproj b/src/Netclaw.Security/Netclaw.Security.csproj index a598f1f6d..38dbb0a69 100644 --- a/src/Netclaw.Security/Netclaw.Security.csproj +++ b/src/Netclaw.Security/Netclaw.Security.csproj @@ -6,6 +6,10 @@ enable + + + + diff --git a/src/Netclaw.Security/ShellApprovalSemantics.cs b/src/Netclaw.Security/ShellApprovalSemantics.cs index 28cd1e1c8..f548d348e 100644 --- a/src/Netclaw.Security/ShellApprovalSemantics.cs +++ b/src/Netclaw.Security/ShellApprovalSemantics.cs @@ -350,8 +350,14 @@ public override IReadOnlyList ExtractInnerCommands(string command) if (!IsPosixShellInvoker(verb)) continue; - if (i + 1 < tokens.Count && IsShellCommandFlag(tokens[i + 1]) && i + 2 < tokens.Count) - results.Add(tokens[i + 2]); + for (var j = i + 1; j < tokens.Count - 1; j++) + { + if (!IsShellCommandFlag(tokens[j])) + continue; + + results.Add(tokens[j + 1]); + break; + } } return results; diff --git a/src/Netclaw.Security/ShellCommandAnalysis.cs b/src/Netclaw.Security/ShellCommandAnalysis.cs new file mode 100644 index 000000000..bb6c6c4f7 --- /dev/null +++ b/src/Netclaw.Security/ShellCommandAnalysis.cs @@ -0,0 +1,167 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using ShellSyntaxTree; + +namespace Netclaw.Security; + +/// +/// Parses Bash commands and expands nested shell command strings. +/// Approval and hard-deny policies share this analysis. +/// +internal sealed class ShellCommandAnalyzer +{ + private const int MaxWrapperDepth = 8; + + public static readonly ShellCommandAnalyzer Bash = new(); + + private ShellCommandAnalyzer() + { + } + + public ShellCommandAnalysis Analyze(string command, string? workingDirectory = null) + { + var clauses = new List(); + var failure = Analyze(command, workingDirectory, depth: 0, clauses); + return new ShellCommandAnalysis(clauses, failure); + } + + private static ShellAnalysisFailure Analyze( + string command, + string? workingDirectory, + int depth, + List clauses) + { + if (depth > MaxWrapperDepth) + return ShellAnalysisFailure.Unresolved; + + // ShellSyntaxTree 0.2 does not expose a background-list tail. + // Treat that parser boundary as unresolved so the tail cannot inherit + // the first command's safe-verb or stored-approval result. + if (ContainsBackgroundListOperator(command)) + return ShellAnalysisFailure.Unresolved; + + ParsedCommand parsed; + try + { + var parser = string.IsNullOrWhiteSpace(workingDirectory) + ? new BashParser() + : new BashParser(new BashParserOptions { WorkingDirectory = workingDirectory }); + parsed = parser.Parse(command); + } + catch + { + return ShellAnalysisFailure.Unresolved; + } + + if (parsed.IsUnparseable || parsed.Clauses.Count == 0) + return ShellAnalysisFailure.Unresolved; + + var innerCommands = PosixShellApprovalSemantics.Instance.ExtractInnerCommands(command); + var hasUnexpandedWrapper = parsed.Clauses.Any(IsUnexpandedWrapperClause); + if (innerCommands.Count == 0 || !hasUnexpandedWrapper) + { + clauses.AddRange(parsed.Clauses); + return ShellAnalysisFailure.None; + } + + // A direct shell wrapper adds no independent authority scope. + // Prefix wrappers such as env and timeout remain visible. + clauses.AddRange(parsed.Clauses.Where(clause => + !IsUnexpandedWrapperClause(clause) || HasShellInvokerInArguments(clause))); + + foreach (var innerCommand in innerCommands) + { + var failure = Analyze(innerCommand, workingDirectory, depth + 1, clauses); + if (failure != ShellAnalysisFailure.None) + return failure; + } + + return ShellAnalysisFailure.None; + } + + private static bool IsUnexpandedWrapperClause(Clause clause) + { + if (clause.Verb.Tokens.Count == 0 || clause.Args.Count == 0) + return false; + + if (!clause.Verb.Tokens.Any(IsShellInvokerToken) && !HasShellInvokerInArguments(clause)) + return false; + + return clause.Args.Any(static arg => + arg.Raw.Length > 1 + && arg.Raw[0] == '-' + && !arg.Raw.StartsWith("--", StringComparison.Ordinal) + && arg.Raw.AsSpan(1).IndexOf('c') >= 0); + } + + private static bool HasShellInvokerInArguments(Clause clause) + => clause.Args.Any(static arg => + arg.Kind != ArgKind.DynamicSkip && IsShellInvokerToken(arg.Raw)); + + private static bool IsShellInvokerToken(string token) + => PosixShellApprovalSemantics.IsPosixShellInvoker( + ShellTokenizer.TrimShellPunctuation(token)); + + private static bool ContainsBackgroundListOperator(string command) + { + char? quote = null; + var escaped = false; + + for (var i = 0; i < command.Length; i++) + { + var ch = command[i]; + if (escaped) + { + escaped = false; + continue; + } + + if (ch == '\\' && quote != '\'') + { + escaped = true; + continue; + } + + if (ch is '\'' or '"') + { + if (quote is null) + quote = ch; + else if (quote == ch) + quote = null; + + continue; + } + + if (quote is not null || ch != '&') + continue; + + var previous = i > 0 ? command[i - 1] : '\0'; + var next = i + 1 < command.Length ? command[i + 1] : '\0'; + if (previous is '&' or '>' || next is '&' or '>') + continue; + + return true; + } + + return false; + } +} + +internal enum ShellAnalysisFailure +{ + None, + Unresolved +} + +internal sealed record ShellCommandAnalysis( + IReadOnlyList Clauses, + ShellAnalysisFailure Failure) +{ + public bool HasDynamicSyntax => Clauses.Any(static clause => + clause.Verb.IsDynamic + || clause.Args.Any(static arg => arg.Kind == ArgKind.DynamicSkip) + || clause.Redirects.Any(static redirect => redirect.IsDynamicSkip)); +} diff --git a/src/Netclaw.Security/ShellCommandPolicy.cs b/src/Netclaw.Security/ShellCommandPolicy.cs index c5fb97018..0d3809e81 100644 --- a/src/Netclaw.Security/ShellCommandPolicy.cs +++ b/src/Netclaw.Security/ShellCommandPolicy.cs @@ -23,6 +23,7 @@ public sealed record ShellCommandDecision(bool Allowed, string? DenyReason = nul /// public sealed class ShellCommandPolicy { + private static readonly ShellCommandAnalyzer Analyzer = ShellCommandAnalyzer.Bash; private readonly IReadOnlyList _denyPatterns; private readonly IReadOnlyList _rawStringPatterns; @@ -122,8 +123,46 @@ public ShellCommandDecision Evaluate(string command) if (!rawDecision.Allowed) return rawDecision; - var segments = ShellTokenizer.GetAllCommandSegments(command); - foreach (var segment in segments) + if (OperatingSystem.IsWindows()) + return EvaluateLegacySegments(command); + + return EvaluateBashAnalysis(command); + } + + /// + /// Evaluates a Bash command with the same structural path that production + /// uses on Linux and macOS. + /// + internal ShellCommandDecision EvaluateBash(string command) + { + if (string.IsNullOrWhiteSpace(command)) + return ShellCommandDecision.Allow(); + + var rawDecision = EvaluateRawString(command); + return rawDecision.Allowed ? EvaluateBashAnalysis(command) : rawDecision; + } + + private ShellCommandDecision EvaluateBashAnalysis(string command) + { + var analysis = Analyzer.Analyze(command); + if (analysis.Failure == ShellAnalysisFailure.Unresolved || analysis.Clauses.Count == 0) + return EvaluateLegacySegments(command); + + foreach (var clause in analysis.Clauses) + { + var decision = EvaluateClause(clause); + if (!decision.Allowed) + return decision; + } + + return ShellCommandDecision.Allow(); + } + + private ShellCommandDecision EvaluateLegacySegments(string command) + { + // The approval matcher does not persist unresolved syntax. Keep the + // legacy scan here so known deny forms still fail at this boundary. + foreach (var segment in ShellTokenizer.GetAllCommandSegments(command)) { var decision = EvaluateSegment(segment); if (!decision.Allowed) @@ -159,6 +198,30 @@ private ShellCommandDecision EvaluateSegment(string segment) return ShellCommandDecision.Allow(); } + private ShellCommandDecision EvaluateClause(ShellSyntaxTree.Clause clause) + { + var tokens = new List( + clause.Verb.Tokens.Count + clause.Args.Count + clause.Redirects.Count); + tokens.AddRange(clause.Verb.Tokens); + tokens.AddRange(clause.Args + .Where(static arg => !arg.IsCwdAttribution) + .Select(static arg => arg.Raw)); + tokens.AddRange(clause.Redirects + .Where(static redirect => !string.IsNullOrEmpty(redirect.Target)) + .Select(static redirect => redirect.Target)); + + if (tokens.Count == 0) + return ShellCommandDecision.Allow(); + + foreach (var pattern in _denyPatterns) + { + if (pattern.Matches(tokens)) + return ShellCommandDecision.Deny(pattern.Reason, pattern.Category); + } + + return ShellCommandDecision.Allow(); + } + private static DenyPattern? ParseDenyPattern(string raw) { var tokens = ShellTokenizer.Tokenize(raw).ToList();