diff --git a/TOOLING.md b/TOOLING.md index beba4529e..249d3df59 100644 --- a/TOOLING.md +++ b/TOOLING.md @@ -43,7 +43,7 @@ coverage. They do not replace positive and negative behavior tests. | `PathAccessPolicy.AddSessionRoots` | Only a Personal context receives shared session roots | 2 killed | `./scripts/run-path-access-mutations.sh` | | `ToolAccessPolicy.AuthorizeMcpInvocation` | Server and tool audience grants precede approval | 2 killed | `./scripts/run-tool-authorization-mutations.sh` | | `ToolAccessPolicy.AuthorizeShellInvocation` | A shell hard denial precedes approval | 1 killed | `./scripts/run-tool-authorization-mutations.sh` | -| Shell analysis, denial-only, tree effects, and reviewed-safe gates | Parser-proved regions and authored diagnostic syntax preserve hard denials; only bounded audited non-path values and consistent non-link-following tree facts can use reusable approval | 79 killed | `./scripts/run-shell-command-analysis-mutations.sh` | +| Shell analysis, denial-only, tree effects, and reviewed-safe gates | Parser-proved regions and authored diagnostic syntax preserve hard denials; only bounded audited non-path values and consistent non-link-following tree facts can use reusable approval | 81 killed | `./scripts/run-shell-command-analysis-mutations.sh` | | `ApprovalPatternMatching.EvaluateApprovalScope` | Folder grants require containment and reject link escape | 4 killed | `./scripts/run-approval-directory-mutations.sh` | | `ReminderManagerActor.HandleExecutionOutcomeAsync` | Only the current attempt can settle; the manager replies after settlement | 2 killed | `./scripts/run-reminder-execution-mutations.sh` | | `ActiveExecutionTracker.TryRemove` | Only the current owner can remove its guard; cleanup removes that guard | 2 killed | `./scripts/run-reminder-execution-mutations.sh` | @@ -189,12 +189,16 @@ Run the shell analysis gate: ./scripts/run-shell-command-analysis-mutations.sh ``` -The script tests 79 mutants across execution-region accounting, denial-only +The script tests 81 mutants across execution-region accounting, denial-only matching, tree traversal and root correspondence, bounded non-filesystem -values, candidate extraction, approval mode, path facts, and reviewed-safe -policy. The job fails unless every mutant dies. +values, bare status-parameter output, candidate extraction, approval mode, +path facts, and reviewed-safe policy. The job fails unless every mutant dies. -The final local run took about 6 minutes. CI allows 30 minutes for +Two status-parameter mutants test the rule that only bare `$?` can preserve +reusable candidates. The focused test rejects other unknown output data. +The new target took 49 seconds after package restore. + +The local run on 2026-09-18 took about 22 minutes. CI allows 30 minutes for hosted-runner variance and report upload. The report directory is `artifacts/stryker/shell-command-analysis`. diff --git a/docs/spec/SPEC-003-acl-policy-and-security-controls.md b/docs/spec/SPEC-003-acl-policy-and-security-controls.md index 6920409d3..b4b9a78f7 100644 --- a/docs/spec/SPEC-003-acl-policy-and-security-controls.md +++ b/docs/spec/SPEC-003-acl-policy-and-security-controls.md @@ -34,6 +34,13 @@ tool access, and ambient channel behavior. - policy engine exception -> deny result - missing grant for requested tool/data -> deny result +## Shell Approval Completeness + +Netclaw keeps static shell candidates reusable when an approval-exempt output command has a bare `$?` argument and no redirect. +The parser must still account for every executable region. Unknown paths, redirects, and unapproved verbs keep their normal gates. + +See the [shell approval contract](../../openspec/specs/tool-approval-gates/spec.md) for the complete authority rules. + ## Audit Events - policy allow/deny decision (with reason) diff --git a/openspec/changes/archive/2026-09-18-keep-pure-output-arguments-reusable/.openspec.yaml b/openspec/changes/archive/2026-09-18-keep-pure-output-arguments-reusable/.openspec.yaml new file mode 100644 index 000000000..f2cbbe6a6 --- /dev/null +++ b/openspec/changes/archive/2026-09-18-keep-pure-output-arguments-reusable/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-18 diff --git a/openspec/changes/archive/2026-09-18-keep-pure-output-arguments-reusable/design.md b/openspec/changes/archive/2026-09-18-keep-pure-output-arguments-reusable/design.md new file mode 100644 index 000000000..79a3d4e31 --- /dev/null +++ b/openspec/changes/archive/2026-09-18-keep-pure-output-arguments-reusable/design.md @@ -0,0 +1,53 @@ +## Context + +See [the proposal](proposal.md) and the [engineering glossary](../../../docs/spec/GLOSSARY.md). + +ShellSyntaxTree reports bare `$?` as unknown data because Bash can split an unquoted value. Netclaw then marks a complete static list complex before it extracts any candidates. + +The existing policy exempts a small set of output commands when they have no path or redirect effect. The parser still reports nested executable occurrences. + +## Goals / Non-Goals + +**Goals:** + +- Preserve normal grants for other static commands in the list. +- Keep all parser and path failures strict. + +**Non-Goals:** + +- Prove Bash initial variable state or change the process environment. +- Infer an executable's private argument grammar. +- Approve an unapproved verb or a redirect target. + +## Decisions + +Netclaw shell analysis owns the call-local exception. It checks parser facts before the coordinator creates approval candidates. + +The exception applies only to a bare `$?` argument that the parser classifies as an unknown `EnvVar` on a static output verb. The command must have no redirect. + +The exception does not change the argument's value. It only stops that value from invalidating the complete command list. + +The coordinator still applies hard denial, protected paths, path policy, grants, and reviewed-safe coverage in their current order. The approval actor keeps durable grants. + +Schematic flow: + +```text +parse complete source + -> reject unresolved syntax or unknown executable identity + -> classify each argument and redirect + -> ignore only eligible unknown output data for complexity + -> build every other command candidate + -> run the existing authorization gates +``` + +The alternative was to treat every unknown data value as safe. That would hide dynamic path and executable effects. Another alternative was to quote `$?` in agent guidance. That would leave the policy defect in place. + +## Risks / Trade-offs + +- A parser regression could hide an executable substitution. Tests must keep the child occurrence visible and require its authority. +- An output redirect could write a file. Tests must keep its path and approval checks active. +- A future output verb could have private side effects. The existing exemption set remains the only source for eligible verbs. The bare exit status cannot add an option. + +## Migration Plan + +This change adds no stored state or configuration. A rollback restores the prior one-time prompt behavior. diff --git a/openspec/changes/archive/2026-09-18-keep-pure-output-arguments-reusable/proposal.md b/openspec/changes/archive/2026-09-18-keep-pure-output-arguments-reusable/proposal.md new file mode 100644 index 000000000..640843474 --- /dev/null +++ b/openspec/changes/archive/2026-09-18-keep-pure-output-arguments-reusable/proposal.md @@ -0,0 +1,31 @@ +## Why + +Source PRD: [PRD-002 SEC-003](../../../docs/prd/PRD-002-gateway-security-envelope.md). + +A static shell list can lose all reusable approval candidates when `echo $?` has an unknown value. This creates a repeated one-time prompt after the operator approves the other verbs. + +## What Changes + +- Keep a complete, static shell list eligible for reusable grants when an approval-exempt output command has a bare `$?` argument. +- Keep executable substitutions, redirects, paths, unknown identities, and incomplete syntax under their current strict rules. +- Add a sanitized regression from the post-swap approval sample and negative authority cases. + +This change covers the narrow output case in the MVP. It does not change Bash initial-state proof, dynamic path authority, or command execution. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `tool-approval-gates`: Define when an unknown data value on an approval-exempt output command leaves other static candidates reusable. + +## Impact + +`ShellCommandAnalysis` and the shell approval tests change. The approval store, shell executor, public API, and configuration do not change. + +Security impact: The parser must still account for every executable region. Redirects and unknown paths must still require their usual authority. + +Operational impact: A command such as `git push; echo $?` can reuse a scoped `git push` grant. A command with an unapproved verb still prompts. diff --git a/openspec/changes/archive/2026-09-18-keep-pure-output-arguments-reusable/specs/tool-approval-gates/spec.md b/openspec/changes/archive/2026-09-18-keep-pure-output-arguments-reusable/specs/tool-approval-gates/spec.md new file mode 100644 index 000000000..4fcbef5d5 --- /dev/null +++ b/openspec/changes/archive/2026-09-18-keep-pure-output-arguments-reusable/specs/tool-approval-gates/spec.md @@ -0,0 +1,47 @@ +## ADDED Requirements + +### Requirement: Bare exit-status output preserves static shell approval candidates + +Netclaw SHALL keep other static shell candidates reusable when a complete approval-exempt output command has a bare `$?` argument. + +The exception SHALL require a static output verb, a complete parser tree, no redirect, and a parser-classified non-path status argument. + +Netclaw SHALL apply each other verb's normal [approval](../../../../../docs/spec/GLOSSARY.md#approval) and path scope. The exception SHALL grant no new verb or path authority. + +#### Scenario: An approved command precedes exit-status output + +- **GIVEN** a session grant covers `git push` in the current directory +- **WHEN** the agent calls `shell_execute` with `git push; echo $?` +- **THEN** Netclaw allows the call without another prompt +- **AND** Netclaw does not store an approval for `echo` + +#### Scenario: An unapproved command still needs consent + +- **GIVEN** no grant covers `git push` +- **WHEN** the agent calls `shell_execute` with `git push; echo $?` +- **THEN** Netclaw requests approval for `git push` +- **AND** Netclaw offers reusable scopes for that static candidate + +#### Scenario: An executable substitution keeps its authority gate + +- **WHEN** the agent calls `shell_execute` with `echo "$(touch /tmp/marker)"` +- **THEN** Netclaw does not exempt the `touch` occurrence +- **AND** the call does not run without the required approval + +#### Scenario: A redirect keeps its path gate + +- **WHEN** the agent calls `shell_execute` with `echo $? > /tmp/marker` +- **THEN** Netclaw does not treat `echo` as an approval-exempt output command +- **AND** the redirect target keeps its normal path and approval checks + +#### Scenario: An unknown path remains strict + +- **WHEN** the agent calls `shell_execute` with `grep "$TARGET"; echo $?` +- **THEN** the unknown `grep` path keeps the whole call on the one-time path +- **AND** the output exception does not create a reusable `grep` candidate + +#### Scenario: An unknown positional parameter remains strict + +- **WHEN** the agent calls `shell_execute` with `echo $@` +- **THEN** Netclaw keeps the call on the one-time path +- **AND** the output exception does not create a reusable candidate diff --git a/openspec/changes/archive/2026-09-18-keep-pure-output-arguments-reusable/tasks.md b/openspec/changes/archive/2026-09-18-keep-pure-output-arguments-reusable/tasks.md new file mode 100644 index 000000000..221d94f9c --- /dev/null +++ b/openspec/changes/archive/2026-09-18-keep-pure-output-arguments-reusable/tasks.md @@ -0,0 +1,16 @@ +## 1. Lock the Authorization Boundary + +- [x] 1.1 Add matcher tests for bare status output, substitution, redirect, and unknown path; verify the tests fail for the current false prompt. +- [x] 1.2 Add coordinator cases for a stored grant and an uncovered verb; verify candidate coverage and approval options. + +## 2. Implement and Document + +- [x] 2.1 Exempt only a parser-classified bare `$?` argument on a complete output command without redirects; verify focused tests pass. +- [x] 2.2 Update `docs/spec/SPEC-003-acl-policy-and-security-controls.md` with the bounded output rule; verify its link to the OpenSpec contract. + +## 3. Verify and Deliver + +- [x] 3.1 Run Security and Actors tests, strict OpenSpec validation, Slopwatch, and header checks; verify all pass. +- [x] 3.2 Review the focused shell mutation scope and run its gate if this boundary has a selected mutant; verify no unsafe survivor. +- [x] 3.3 Review the sanitized live case against the prior and new analyzer builds; verify the new result offers reusable grants without authority drift. +- [x] 3.4 Open a Netclaw pull request with the evidence and queue auto-merge subject to required CI; verify the queue state. diff --git a/openspec/specs/tool-approval-gates/spec.md b/openspec/specs/tool-approval-gates/spec.md index ac2daf0f9..f136833e3 100644 --- a/openspec/specs/tool-approval-gates/spec.md +++ b/openspec/specs/tool-approval-gates/spec.md @@ -2483,3 +2483,49 @@ A model-eval result SHALL NOT replace a failed or missing deterministic test. - **WHEN** no representative sanitized Windows agent behavior is available - **THEN** the suite does not invent a Windows model pattern - **AND** the missing behavioral case is recorded as future evidence work + +### Requirement: Bare exit-status output preserves static shell approval candidates + +Netclaw SHALL keep other static shell candidates reusable when a complete approval-exempt output command has a bare `$?` argument. + +The exception SHALL require a static output verb, a complete parser tree, no redirect, and a parser-classified non-path status argument. + +Netclaw SHALL apply each other verb's normal [approval](../../../docs/spec/GLOSSARY.md#approval) and path scope. The exception SHALL grant no new verb or path authority. + +#### Scenario: An approved command precedes exit-status output + +- **GIVEN** a session grant covers `git push` in the current directory +- **WHEN** the agent calls `shell_execute` with `git push; echo $?` +- **THEN** Netclaw allows the call without another prompt +- **AND** Netclaw does not store an approval for `echo` + +#### Scenario: An unapproved command still needs consent + +- **GIVEN** no grant covers `git push` +- **WHEN** the agent calls `shell_execute` with `git push; echo $?` +- **THEN** Netclaw requests approval for `git push` +- **AND** Netclaw offers reusable scopes for that static candidate + +#### Scenario: An executable substitution keeps its authority gate + +- **WHEN** the agent calls `shell_execute` with `echo "$(touch /tmp/marker)"` +- **THEN** Netclaw does not exempt the `touch` occurrence +- **AND** the call does not run without the required approval + +#### Scenario: A redirect keeps its path gate + +- **WHEN** the agent calls `shell_execute` with `echo $? > /tmp/marker` +- **THEN** Netclaw does not treat `echo` as an approval-exempt output command +- **AND** the redirect target keeps its normal path and approval checks + +#### Scenario: An unknown path remains strict + +- **WHEN** the agent calls `shell_execute` with `grep "$TARGET"; echo $?` +- **THEN** the unknown `grep` path keeps the whole call on the one-time path +- **AND** the output exception does not create a reusable `grep` candidate + +#### Scenario: An unknown positional parameter remains strict + +- **WHEN** the agent calls `shell_execute` with `echo $@` +- **THEN** Netclaw keeps the call on the one-time path +- **AND** the output exception does not create a reusable candidate diff --git a/scripts/run-shell-command-analysis-mutations.sh b/scripts/run-shell-command-analysis-mutations.sh index 8e1748d95..e399817e8 100755 --- a/scripts/run-shell-command-analysis-mutations.sh +++ b/scripts/run-shell-command-analysis-mutations.sh @@ -227,6 +227,21 @@ run_target \ "$output_path/integer-range" \ 11 +read -r status_start status_end < <( + find_span \ + "$analysis_file" \ + "private static bool IsUnknownOutputData(" \ + "argument.Argument.Raw == \"\$?\"" \ + "argument.Argument.Raw == \"\$?\"" +) +run_target \ + "stryker-shell-command-analysis.json" \ + "ShellCommandAnalysis.cs" \ + "$status_start" \ + "$status_end" \ + "$output_path/status-parameter" \ + 2 + matcher_file="$repo_root/src/Netclaw.Security/IToolApprovalMatcher.cs" read -r candidate_start candidate_end < <( find_span \ diff --git a/src/Netclaw.Actors.MutationTests/ShellCommandAnalysisMutationTests.cs b/src/Netclaw.Actors.MutationTests/ShellCommandAnalysisMutationTests.cs index 12882b051..43ccd2068 100644 --- a/src/Netclaw.Actors.MutationTests/ShellCommandAnalysisMutationTests.cs +++ b/src/Netclaw.Actors.MutationTests/ShellCommandAnalysisMutationTests.cs @@ -42,6 +42,32 @@ public void A_failed_directory_change_keeps_the_original_scope_after_a_sequence( .Order(StringComparer.Ordinal)); } + [Fact] + public void Bare_status_output_keeps_static_candidates_without_accepting_other_unknown_data_or_redirects() + { + var matcher = new ShellApprovalMatcher( + ShellExecutionEnvironment.CreateBash(ShellPlatform.Linux)); + + ShellApprovalAnalysis Analyze(string command) => matcher.AnalyzeInvocation( + new ToolName("shell_execute"), + new Dictionary + { + ["Command"] = command, + ["WorkingDirectory"] = "/work" + }); + + var status = Analyze("git push; echo $?"); + var positional = Analyze("git push; echo $@"); + var redirect = Analyze("git push; echo $? > /tmp/marker"); + + Assert.False(status.IsMessy); + Assert.Equal(["git push", "echo"], status.Candidates.Select(static candidate => candidate.Verb)); + Assert.True(positional.IsMessy); + Assert.Empty(positional.Candidates); + Assert.True(redirect.IsMessy); + Assert.Empty(redirect.Candidates); + } + [Fact] public void Known_and_unknown_execution_regions_keep_distinct_analysis_results() { diff --git a/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs b/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs index f0b746256..5e0e53bd2 100644 --- a/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs +++ b/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs @@ -1511,6 +1511,21 @@ public static class ShellApprovalCases Bash("echo done"), Approvals.None, ExpectedApproval.Allow(ToolAllowReason.ApprovalExemptShellCandidates)), + Case( + "unquoted-status-output-reuses-session-grant", + Bash("git push; echo $?"), + Approvals.Session("git push"), + ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "session:git push")), + Case( + "unquoted-status-output-prompts-for-unapproved-verb", + Bash("git push; echo $?"), + Approvals.None, + ExpectedApproval.Require(["git push"])), + Case( + "unquoted-status-output-redirect-remains-complex", + Bash("echo $? > /tmp/marker"), + Approvals.PersistentAnywhere("echo"), + ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)), Case( "control-flow-fails-closed", Bash("for f in *.txt; do cat \"$f\"; done"), 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 cec983019..5cd78b0fd 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 @@ -239,6 +239,9 @@ | printf-allows-without-grant | Bash | Personal | Project | Interactive | printf hello | none | Allowed | ApprovalExemptShellCandidates | none | Not applicable | | echo-redirect-prompts | Bash | Personal | Project | Interactive | echo hello > result.txt | none | RequiresApproval | approval required | echo | No | | echo-control-word-argument-allows | Bash | Personal | Project | Interactive | echo done | none | Allowed | ApprovalExemptShellCandidates | none | Not applicable | +| unquoted-status-output-reuses-session-grant | Bash | Personal | Project | Interactive | git push; echo $? | session[this-chat]:git push | Allowed | StoredApproval | none | Not applicable | +| unquoted-status-output-prompts-for-unapproved-verb | Bash | Personal | Project | Interactive | git push; echo $? | none | RequiresApproval | approval required | git push | No | +| unquoted-status-output-redirect-remains-complex | Bash | Personal | Project | Interactive | echo $? > /tmp/marker | persistent[anywhere]:echo | RequiresApproval | approval required | none | Yes | | control-flow-fails-closed | Bash | Personal | Project | Interactive | for f in *.txt; do cat "$f"; done | persistent[anywhere]:cat | RequiresApproval | approval required | none | Yes | | printf-variable-target-hidden-execution-fails-closed | Bash | Personal | Project | Interactive | printf -v'value[$(printf marker >&2)0]' '%s' data | persistent[anywhere]:printf | RequiresApproval | approval required | none | Yes | | recursive-builtin-eval-fails-closed | Bash | Personal | Project | Interactive | command -p -- builtin -- eval 'printf marker >&2' | persistent[anywhere]:command, persistent[anywhere]:builtin, persistent[anywhere]:eval, persistent[anywhere]:printf | RequiresApproval | approval required | none | Yes | diff --git a/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.cs b/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.cs index 69902bc94..809f68018 100644 --- a/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/ShellApprovalDispositionMatrixTests.cs @@ -82,6 +82,26 @@ public Task Noninteractive_candidate_can_use_an_explicit_persistent_grant() 1, "persistent:git status"))); + [SlopwatchSuppress("SW001", "The command uses a POSIX Bash status parameter.")] + [Fact(SkipUnless = nameof(IsPosix), Skip = "This case requires POSIX Bash semantics.")] + public async Task Unquoted_status_output_offers_reusable_grant_for_unapproved_verb() + { + await using var harness = await ShellApprovalHarness.CreateAsync( + ShellApprovalCases.Get("unquoted-status-output-prompts-for-unapproved-verb"), + fixture.ActorSystem, + TestContext.Current.CancellationToken); + + var decision = await harness.EvaluateDecisionAsync(TestContext.Current.CancellationToken); + + Assert.Equal(ToolAuthorizationOutcome.RequiresApproval, decision.Outcome); + var approval = Assert.IsType(decision.ApprovalContext); + Assert.False(approval.IsMessy); + Assert.Equal(["git push"], approval.CandidateVerbs); + Assert.Contains( + approval.Options, + option => option.Key.Value == ApprovalOptionKeys.ApproveSession); + } + [SlopwatchSuppress("SW001", "The observed compound uses POSIX Bash directory and pipeline semantics.")] [Fact(SkipUnless = nameof(IsPosix), Skip = "This case requires POSIX Bash semantics.")] public async Task Declared_project_does_not_resolve_an_inline_directory_pipeline() diff --git a/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs b/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs index baaeba01c..b2ac629cb 100644 --- a/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs +++ b/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs @@ -74,6 +74,35 @@ public void Bash_github_diagnostic_with_exit_status_is_reusable() analysis.Candidates.Select(static candidate => candidate.Verb)); } + [Fact] + public void Bash_unquoted_status_output_preserves_static_candidates() + { + var analysis = _matcher.AnalyzeInvocation( + new ToolName("shell_execute"), + Args("git status --short; echo $?", "/work")); + + Assert.False(analysis.IsMessy); + Assert.Equal( + ["git status", "echo"], + analysis.Candidates.Select(static candidate => candidate.Verb)); + } + + [Theory] + [InlineData("echo \"$(touch /tmp/marker)\"")] + [InlineData("echo $? > /tmp/marker")] + [InlineData("grep \"$TARGET\"; echo $?")] + [InlineData("echo $@")] + [InlineData("printf $@")] + public void Bash_output_exception_rejects_unknown_values_and_side_effects(string command) + { + var analysis = _matcher.AnalyzeInvocation( + new ToolName("shell_execute"), + Args(command, "/work")); + + Assert.True(analysis.IsMessy); + Assert.Empty(analysis.Candidates); + } + [Theory] [InlineData("head -c 20 /tmp/work/site.css | xxd | head -3")] [InlineData("rg -rn \"operation failed\" src/ tests/ | head -20; echo \"---\"; rg -rln \"upload\" src/ | head -20")] diff --git a/src/Netclaw.Security/ShellCommandAnalysis.cs b/src/Netclaw.Security/ShellCommandAnalysis.cs index 97dd91f36..471caca87 100644 --- a/src/Netclaw.Security/ShellCommandAnalysis.cs +++ b/src/Netclaw.Security/ShellCommandAnalysis.cs @@ -579,7 +579,8 @@ private bool CommandHasDynamicSyntax( !IsAccountedExecutionRegionArgument( argument, accountedRegionArguments) - && HasUnsupportedArgumentDomain(argument)) + && HasUnsupportedArgumentDomain(argument) + && !IsUnknownOutputData(command, argument)) // A glob in a directory segment can hide traversal or a symlink. // Only a leaf glob has a fixed directory scope. || command.Clause.Args.Any(arg => @@ -800,6 +801,24 @@ and not ShellValueDomain.FiniteSet }; } + private static bool IsUnknownOutputData( + CommandOccurrence command, + AnalyzedArgument argument) + { + // The parser proves the verb and every child command before this check. + // A bare status value cannot add an option or a path to an output command. + return command.Redirects.Count == 0 + && !command.Clause.Verb.IsDynamic + && command.Clause.Verb.Tokens.Count == 1 + && ShellTokenizer.SingleTokenSideEffectVerbs.Contains(command.Clause.Verb.Tokens[0]) + && argument.Argument.Kind == ArgKind.EnvVar + && !argument.Argument.IsPath + && argument.Argument.Raw == "$?" + && argument.Value is ShellValueDomain.Unknown + && argument.AuthoredFileSystemValue is ShellValueDomain.Unknown + && argument.AuthoredNonFileSystemValue is ShellValueDomain.Unknown; + } + private static bool HasUnresolvedRedirect(CommandOccurrence occurrence) => occurrence.Redirects.Any(redirect => HasUnresolvedRedirect(occurrence, redirect));