diff --git a/IMPLEMENTATION_PLAN.md b/IMPLEMENTATION_PLAN.md index 26e1cdd48..4eb64db88 100644 --- a/IMPLEMENTATION_PLAN.md +++ b/IMPLEMENTATION_PLAN.md @@ -198,6 +198,8 @@ Done when: root from one-command `WorkingDirectory` scope, prevent redundant project switches, and preserve `cd` when directory mutation is the requested shell behavior. +- [x] Unknown non-path argument data does not make a structurally complete + shell command complex. Dynamic identities, paths, and redirects stay strict. - [x] Sanitized behavioral eval cases cover early project declaration, one-command typed scope, failed-path recovery, and deliberate inline `cd`. - [ ] Run the new behavioral eval cases against a configured model provider. diff --git a/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs b/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs index 288f813fe..9cc82818a 100644 --- a/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs +++ b/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs @@ -358,6 +358,14 @@ public static class ShellApprovalCases Approvals.None, ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)), + Case( + "safe-gh-run-diagnostic-exit-status-allows", + Bash( + "gh run view 123456 --repo example/project --log-failed --verbose 2>&1 " + + "| head -200; echo \"---EXIT $?---\""), + Approvals.None, + ExpectedApproval.Allow(ToolAllowReason.ApprovalExemptShellCandidates)), + Case( "native-project-path-operand-allows-safe-verb", Bash("git diff install-skills.sh"), 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 67fd64067..d8ddeee11 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 @@ -32,6 +32,7 @@ | mixed-safe-unsafe-compound-prompts | Bash | Personal | Project | Interactive | git status && git push | none | RequiresApproval | approval required | git push | No | | safe-pipe-unsafe-tail-prompts | Bash | Personal | Project | Interactive | git status \| git push | none | RequiresApproval | approval required | git push | No | | safe-pipeline-allows | Bash | Personal | Project | Interactive | git log \| head -20 | none | Allowed | SafeVerbInTrustedScope | none | Not applicable | +| safe-gh-run-diagnostic-exit-status-allows | Bash | Personal | Project | Interactive | gh run view 123456 --repo example/project --log-failed --verbose 2>&1 \| head -200; echo "---EXIT $?---" | none | Allowed | ApprovalExemptShellCandidates | none | Not applicable | | native-project-path-operand-allows-safe-verb | Bash | Personal | Project | Interactive | git diff install-skills.sh | none | Allowed | SafeVerbInTrustedScope | none | Not applicable | | native-external-path-operand-prompts | Bash | Personal | Project | Interactive | git diff /etc/passwd | none | RequiresApproval | approval required | git diff | No | | native-project-path-operand-reuses-grant | Bash | Personal | Project | Interactive | kubectl apply deployment.yaml | persistent[project]:kubectl apply | Allowed | StoredApproval | none | Not applicable | diff --git a/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs b/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs index 6bae876d3..5f515fc75 100644 --- a/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs +++ b/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs @@ -50,6 +50,23 @@ public void Bash_matcher_keeps_power_shell_as_one_external_approval_unit( Assert.Equal(expectedVerb, Assert.Single(analysis.Candidates).Verb); } + [Fact] + public void Bash_github_diagnostic_with_exit_status_is_reusable() + { + const string command = + "gh run view 123456 --repo example/project --log-failed --verbose 2>&1 " + + "| head -200; echo \"---EXIT $?---\""; + + var analysis = _matcher.AnalyzeInvocation( + new ToolName("shell_execute"), + Args(command, "/work")); + + Assert.False(analysis.IsMessy); + Assert.Equal( + ["gh run view", "head", "echo"], + analysis.Candidates.Select(static candidate => candidate.Verb)); + } + [Fact] public void Power_shell_matcher_uses_the_native_power_shell_grammar() { diff --git a/src/Netclaw.Security.Tests/ShellCommandAnalysisTests.cs b/src/Netclaw.Security.Tests/ShellCommandAnalysisTests.cs index b2af3e124..6fa1c7734 100644 --- a/src/Netclaw.Security.Tests/ShellCommandAnalysisTests.cs +++ b/src/Netclaw.Security.Tests/ShellCommandAnalysisTests.cs @@ -219,6 +219,35 @@ public void Power_shell_unknown_command_argument_region_stays_dynamic() Assert.True(analysis.HasDynamicSyntax, Describe(analysis)); } + [Theory] + [InlineData("echo \"---EXIT $?---\"")] + [InlineData("printf '%s' \"$?\"")] + [InlineData("status-report \"$?\"")] + [InlineData("status-report \"$@\"")] + public void Bash_unknown_non_path_data_keeps_static_structure(string command) + { + var analyzer = new ShellCommandAnalyzer(BashEnvironment); + var analysis = analyzer.Analyze(command, "/work"); + + Assert.Equal(ShellAnalysisFailure.None, analysis.Failure); + Assert.False(analysis.HasDynamicSyntax, Describe(analysis)); + } + + [Theory] + [InlineData("rm \"$1\"")] + [InlineData("echo ok > \"$1\"")] + [InlineData("\"$1\" --version")] + [InlineData("sh -c \"$1\"")] + public void Bash_unknown_authority_or_identity_stays_dynamic(string command) + { + var analyzer = new ShellCommandAnalyzer(BashEnvironment); + var analysis = analyzer.Analyze(command, "/work"); + + Assert.True( + analysis.Failure != ShellAnalysisFailure.None || analysis.HasDynamicSyntax, + Describe(analysis)); + } + [Fact] public void Power_shell_empty_command_argument_region_stays_dynamic() { diff --git a/src/Netclaw.Security/ShellCommandAnalysis.cs b/src/Netclaw.Security/ShellCommandAnalysis.cs index 5b09c0153..5b0c6d4bf 100644 --- a/src/Netclaw.Security/ShellCommandAnalysis.cs +++ b/src/Netclaw.Security/ShellCommandAnalysis.cs @@ -338,10 +338,6 @@ private bool CommandHasDynamicSyntax( command, arg, accountedRegionArguments)) - || command.Clause.Args.Any(static arg => - arg.Kind == ArgKind.EnvVar - && !arg.IsCwdAttribution - && string.IsNullOrWhiteSpace(arg.Resolved)) || command.Clause.Args.Any(static arg => arg.IsPath && arg.Kind != ArgKind.Glob @@ -431,12 +427,20 @@ private static bool HasUnsupportedWorkingDirectory(ShellValueDomain workingDirec _ => true }; - private static bool HasUnsupportedArgumentDomain(AnalyzedArgument argument) + private bool HasUnsupportedArgumentDomain(AnalyzedArgument argument) => argument.Value switch { // A raw authored glob has no one runtime value. Netclaw applies // its fixed covering-scope checks to the source Arg below. - ShellValueDomain.Unknown => argument.Argument.Kind != ArgKind.Glob, + // Bash parameter expansion happens after parsing. In a non-path + // argument it can change argv values or cardinality, but it cannot + // introduce shell operators, redirects, or command occurrences. + // Path slots, dynamic identities, and execution regions are + // classified separately and remain strict. + ShellValueDomain.Unknown => + argument.Argument.Kind != ArgKind.Glob + && (Environment.Grammar != ShellGrammar.Bash + || argument.Argument.Kind != ArgKind.EnvVar), ShellValueDomain.Exact => false, ShellValueDomain.FiniteSet finite => finite.Values.Count is < 2 or > 32 || finite.Values.Any(static value => value is null)