From bf7ae89cf2cd6b26941acac3dc76276410e70944 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Sun, 9 Aug 2026 07:43:38 +0000 Subject: [PATCH] Adopt ShellSyntaxTree alpha.1 --- Directory.Packages.props | 2 +- IMPLEMENTATION_PLAN.md | 11 +- .../adopt-shellsyntax-alpha1/.openspec.yaml | 2 + .../adopt-shellsyntax-alpha1/design.md | 105 +++++++ .../adopt-shellsyntax-alpha1/proposal.md | 45 +++ .../specs/tool-approval-gates/spec.md | 195 +++++++++++++ .../changes/adopt-shellsyntax-alpha1/tasks.md | 25 ++ .../Tools/ShellApprovalCaseCatalog.cs | 83 +++++- ...roval_cases_match_review_table.verified.md | 21 +- .../ShellApprovalMatcherTests.cs | 32 ++- .../ShellCommandAnalysisTests.cs | 267 +++++++++++++++++- .../ShellSyntaxTreeIntegrationTests.cs | 18 +- src/Netclaw.Security/IToolApprovalMatcher.cs | 40 +-- src/Netclaw.Security/ShellCommandAnalysis.cs | 110 +++++++- 14 files changed, 900 insertions(+), 56 deletions(-) create mode 100644 openspec/changes/adopt-shellsyntax-alpha1/.openspec.yaml create mode 100644 openspec/changes/adopt-shellsyntax-alpha1/design.md create mode 100644 openspec/changes/adopt-shellsyntax-alpha1/proposal.md create mode 100644 openspec/changes/adopt-shellsyntax-alpha1/specs/tool-approval-gates/spec.md create mode 100644 openspec/changes/adopt-shellsyntax-alpha1/tasks.md diff --git a/Directory.Packages.props b/Directory.Packages.props index b052bf77a..5a3c3f73d 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -75,7 +75,7 @@ - + diff --git a/IMPLEMENTATION_PLAN.md b/IMPLEMENTATION_PLAN.md index 2bf8aea59..1be91783a 100644 --- a/IMPLEMENTATION_PLAN.md +++ b/IMPLEMENTATION_PLAN.md @@ -141,8 +141,15 @@ Done when: explicit Bash redirect facts for the existing grammar. - [x] Unknown occurrences, cwd facts, wrappers, and redirects stay prompt-only. Static descriptor redirects no longer appear dynamic. -- [ ] Netclaw interprets bounded loop arguments and promotes the new mutation - cases after the next ShellSyntaxTree prerelease. +- [x] Netclaw consumes ShellSyntaxTree `0.3.0-alpha.1` and promotes Bash + command-resolution mutation and reserved execution forms into the strict + 181-case review matrix. +- [x] A constrained stdin grammar allows a complete literal heredoc or bounded + here string only for argument-free `cat`. Unknown data, expanding heredocs, + arguments, wrappers, interpreters, and stored grants stay strict. +- [ ] Netclaw interprets bounded loop arguments only after the executor can + prove the Bash initial variable state. The inherited shell state remains + fail closed because an ambient nameref can change assignment semantics. ### Priority: Simplify Tool Execution Context Architecture diff --git a/openspec/changes/adopt-shellsyntax-alpha1/.openspec.yaml b/openspec/changes/adopt-shellsyntax-alpha1/.openspec.yaml new file mode 100644 index 000000000..d77f64e53 --- /dev/null +++ b/openspec/changes/adopt-shellsyntax-alpha1/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-09 diff --git a/openspec/changes/adopt-shellsyntax-alpha1/design.md b/openspec/changes/adopt-shellsyntax-alpha1/design.md new file mode 100644 index 000000000..91adb10bd --- /dev/null +++ b/openspec/changes/adopt-shellsyntax-alpha1/design.md @@ -0,0 +1,105 @@ +## Context + +Netclaw uses ShellSyntaxTree facts before it reuses a shell approval. The first +v0.3 alpha lacks the later Bash redirect and command-resolution behavior. + +The current consumer marks all heredocs and here strings unresolved. This rule +is safe, but it causes prompts for complete data sent to a non-interpreter. + +The shell gate runs before actor dispatch. This change does not alter actor +messages, actor ownership, persisted grants, or recovery behavior. + +## Goals / Non-Goals + +**Goals:** + +- Adopt ShellSyntaxTree `0.3.0-alpha.1`. +- Allow complete bounded stdin data for one constrained receiver grammar. +- Keep unknown and unsupported shell forms strict. +- Preserve complete command text in approval displays. +- Pin each decision in the review matrix. + +**Non-Goals:** + +- Add a general stdin receiver catalog. +- Change grant storage or approval button behavior. +- Migrate PowerShell shell analysis. +- Add a safe `sed` grammar. + +## Decisions + +### Use the typed redirect operation + +The consumer will use `RedirectOperation.HereDocument` and `HereString`. It will +not infer these forms from a compatibility redirect target. + +The display formatter will encode raw line breaks with a visible `⏎` marker +for both operations. This choice preserves `<<` or `<<<`, the data, and each +authored boundary in a single-line prompt. + +Alternative: reconstruct these redirects from `Clause.Redirects`. That model +cannot preserve the v0.3 redirect operation and can misstate `<<<` as `<`. + +### Start with one data-only receiver grammar + +The safe grammar will require all these facts: + +- The occurrence is complete. +- The verb chain contains only `cat`. +- The command has no authored arguments. +- The redirect uses the default stdin source or explicit descriptor zero. +- The redirect is complete and is not path-relevant. +- A heredoc is complete, literal, and has complete body provenance. +- A here-string target domain is `Exact` or `FiniteSet`. + +A heredoc uses its expansion mode and authored body facts. A here string uses +its target domain. Every expanding heredoc, other receiver, or unproved value +stays unresolved and prompts. + +Netclaw will keep its established transparent analysis for a direct shell +dispatch such as `bash -c`. A receiver wrapper such as `command cat` does not +match the one-token receiver grammar and stays unresolved. The parser clears +outer source spans after a safe command-string decode. Netclaw accepts paired +unavailable spans only on those decoded occurrences and still requires the raw +delimiter, raw body, literal mode, and complete facts. + +Alternative: allow all safe verbs. That choice is unsafe because some programs +interpret stdin as code, options, or a policy-sensitive language. + +### Keep every other redirect decision unchanged + +The consumer will still evaluate file redirects, descriptor redirects, hard +deny rules, protected paths, and every command occurrence. An output redirect +on the same `cat` command still receives its independent path decision. + +### Use the review matrix as the downstream contract + +The matrix will cover exact data, unknown data, interpreter receivers, stored +grants, command-resolution mutation, and reserved execution syntax. Focused +tests will pin display text and parser integration. + +## Risks / Trade-offs + +- [Risk] The `cat` grammar excludes useful receivers. -> The narrow boundary + prevents an unsafe receiver classification. Later slices can add proved + receivers. +- [Risk] A future package changes an enum or value-domain shape. -> Unknown + enum values and incomplete facts stay strict. +- [Risk] A parser failure removes approval candidates. -> Netclaw offers only + one-shot approval and deny. It does not persist a grant. +- [Risk] A data-only stdin redirect also has a file output. -> Netclaw evaluates + the file redirect separately. + +## Migration Plan + +1. Update the central package version. +2. Add focused analysis and display tests. +3. Add the review matrix cases. +4. Run the security and actor test suites. + +Rollback restores the previous package and consumer rule. No persisted state +or configuration needs conversion. + +## Open Questions + +None. diff --git a/openspec/changes/adopt-shellsyntax-alpha1/proposal.md b/openspec/changes/adopt-shellsyntax-alpha1/proposal.md new file mode 100644 index 000000000..efaed9386 --- /dev/null +++ b/openspec/changes/adopt-shellsyntax-alpha1/proposal.md @@ -0,0 +1,45 @@ +## Why + +PRD-002 and PRD-006 require default-deny shell approval with useful approval +reuse. Netclaw must adopt ShellSyntaxTree `0.3.0-alpha.1` because the first alpha +predates Bash command-resolution hardening, heredoc facts, and here-string facts. + +## What Changes + +- Update Netclaw from ShellSyntaxTree `0.3.0-alpha` to `0.3.0-alpha.1`. +- Correct the approval display contract for the v0.3 occurrence and redirect + model. Raw command text remains the fallback when facts are incomplete. +- Add strict matrix cases for Bash command-resolution mutation and reserved + execution forms. +- Add a narrow data-only stdin grammar for argument-free `cat` with a complete + literal heredoc or exact and finite here-string data. +- Add allow and prompt cases for complete and unknown Bash stdin data. +- Keep hard-deny, protected-path, dynamic-value, and incomplete-parse behavior + fail closed. + +Transparent direct shell dispatch remains subject to complete recursive +analysis. Receiver wrappers such as `command cat` stay strict. + +In scope: Bash approval analysis, the package reference, the approval matrix, +and the canonical approval specification. + +Out of scope: PowerShell migration, stable ShellSyntaxTree v0.3, new grant +shapes, broad stdin interpreter grammars, and the deferred safe `sed` grammar. + +## Capabilities + +### New Capabilities + +- None. + +### Modified Capabilities + +- `tool-approval-gates`: Align approval display and reuse rules with complete + v0.3 command-occurrence and redirect facts. + +## Impact + +- Code: Bash shell analysis and its package dependency. +- Tests: the review matrix and focused security tests. +- Security: unknown, incomplete, mutating, and unsupported forms stay strict. +- Operations: no configuration or migration changes are required. diff --git a/openspec/changes/adopt-shellsyntax-alpha1/specs/tool-approval-gates/spec.md b/openspec/changes/adopt-shellsyntax-alpha1/specs/tool-approval-gates/spec.md new file mode 100644 index 000000000..0086d657b --- /dev/null +++ b/openspec/changes/adopt-shellsyntax-alpha1/specs/tool-approval-gates/spec.md @@ -0,0 +1,195 @@ +## ADDED Requirements + +### Requirement: Bounded Bash stdin data has a constrained receiver grammar + +Netclaw SHALL treat Bash heredoc and here-string data as resolved only when all +required receiver and data facts are complete. The initial receiver grammar +SHALL accept only argument-free `cat`. It SHALL require a complete literal +heredoc or an exact or finite here-string target. + +The grammar SHALL use the heredoc expansion mode and authored body provenance. +It SHALL use `RedirectAnalysis.Target` for here strings. It SHALL reject +expanding heredocs, unknown domains, incomplete redirects, path-relevant +redirects, non-stdin source descriptors, authored arguments, receiver wrappers, +and every other receiver. A complete direct shell dispatch MAY expose its inner +receiver through Netclaw's established recursive analysis. + +Netclaw SHALL evaluate every other redirect on the occurrence independently. +Stored approval SHALL NOT bypass an unresolved stdin redirect. + +#### Scenario: Exact here string to cat can use the trusted scope + +- **GIVEN** an argument-free `cat` command in a trusted project directory +- **WHEN** its complete here string has exact data +- **THEN** the stdin redirect does not require a separate approval +- **AND** the normal safe-verb and path rules decide the command + +#### Scenario: Literal heredoc to cat can use the trusted scope + +- **GIVEN** an argument-free `cat` command in a trusted project directory +- **WHEN** its complete literal heredoc has complete authored body provenance +- **THEN** the stdin redirect does not require a separate approval +- **AND** the normal safe-verb and path rules decide the command + +#### Scenario: Unknown here-string data stays strict + +- **GIVEN** `cat <<< "$value"` in a trusted project directory +- **WHEN** the parser cannot prove the data value +- **THEN** Netclaw requires one-shot approval or deny +- **AND** Netclaw offers no persistent approval candidate + +#### Scenario: Interpreter stdin stays strict + +- **GIVEN** an interpreter receives a complete literal heredoc or here string +- **WHEN** Netclaw evaluates the redirect +- **THEN** Netclaw requires one-shot approval or deny +- **AND** an existing interpreter grant does not bypass the stdin decision + +### Requirement: Bash command-resolution mutation stays strict + +Netclaw SHALL use the pinned ShellSyntaxTree result as the structural authority. +An unparseable command-resolution mutation or reserved execution form SHALL +produce no persistent approval candidate. + +This rule SHALL cover unsupported `exec`, mutating `hash`, alias changes, +shell-option changes, builtin-enable changes, `time`, negation, coprocesses, and +current-shell brace groups. + +#### Scenario: Command-resolution mutation cannot reuse a grant + +- **GIVEN** a command changes command resolution before another occurrence +- **AND** stored grants cover each visible command name +- **WHEN** ShellSyntaxTree marks the full command unparseable +- **THEN** Netclaw requires one-shot approval or deny +- **AND** Netclaw offers no persistent approval candidate + +#### Scenario: Reserved execution form cannot flatten into a safe command + +- **GIVEN** an unsupported reserved execution form contains a safe verb +- **WHEN** ShellSyntaxTree marks the full command unparseable +- **THEN** Netclaw does not authorize the visible safe verb +- **AND** Netclaw offers no persistent approval candidate + +## MODIFIED Requirements + +### Requirement: Five-button approval prompt with verb-and-directory framing + +When the approval gate prompts the user, the prompt SHALL render five +buttons in one row: `Once`, `This chat`, `Always here`, `Always anywhere`, +`Deny`. The buttons `Always anywhere` and `Deny` SHALL be styled as +danger (Slack `style: "danger"`, Discord `ButtonStyle.Danger`). All +button labels SHALL fit within Slack's 76-character and Discord's +80-character button-text caps. + +The prompt body SHALL show the cwd in the header +(`Approve in ?`) and the extracted verb chains as a bulleted list. +Single-verb commands MAY collapse the list into the header +(`Approve in ?`). The body SHALL NOT render separate +"Patterns" or "Directory Roots" sections. + +The display text for a shell command SHALL be single-line. A command +containing embedded line breaks (LF or CR) SHALL be reconstructed from +its parse tree: statement separators render as explicit operators (`;`, +`&&`, `||`, `|`) and each multi-line argument or redirect target is +replaced with a `(N lines, M chars)` size summary instead of its +verbatim content (issue #1402) — channel renderers embed the display +text in single-line code fences, and dumping a multi-line quoted blob +verbatim corrupts the prompt layout. When the parser cannot decompose +the command, line breaks SHALL be flattened to spaces. + +Commands with heredocs, here strings, or subshell groups SHALL NOT use the +compatibility-clause reconstruction. The formatter SHALL detect heredocs and +here strings from their typed v0.3 redirect operations. It SHALL encode each +raw line break as a visible `⏎` marker so the display keeps each redirect +operator, data body, and execution boundary. Subshell groups SHALL use the same +fallback because a flat clause sequence cannot preserve their grouping. + +Button semantics: + +- `Once` SHALL run the command this one time and persist nothing. +- `This chat` SHALL allow the extracted verbs in the prompt's directory + for the rest of the session, stored in session-scoped memory only. +- `Always here` SHALL persist `(verb, prompt's directory)` entries to + `tool-approvals.json` for each extracted verb. +- `Always anywhere` SHALL persist `(verb, null)` entries for each extracted + verb — the global wildcard. +- `Deny` SHALL refuse this call only. Denying a verb SHALL NOT ban it + for future invocations. + +#### Scenario: Compound command shows verbs as bullets + +- **GIVEN** the agent invokes `shell_execute` with command + `cd ~/repos/foo && git remote -v && git rev-parse HEAD` + and cwd `~/repos/foo/` +- **WHEN** the approval prompt is rendered on Slack +- **THEN** the body header reads `Approve in ~/repos/foo/ ?` +- **AND** the verbs `cd`, `git remote`, `git rev-parse` appear as bullets +- **AND** the action row contains five buttons +- **AND** `Always anywhere` and `Deny` are styled as danger + +#### Scenario: Always here persists folder-scoped entries + +- **GIVEN** an approval prompt for verbs `git remote`, `git rev-parse` + in cwd `~/repos/foo/` +- **WHEN** the user clicks `Always here` +- **THEN** `tool-approvals.json` gains entries + `{"verb": "git remote", "directory": "~/repos/foo/"}` and + `{"verb": "git rev-parse", "directory": "~/repos/foo/"}` +- **AND** the resolution message reads + `Saved: git remote, git rev-parse in ~/repos/foo/` + +#### Scenario: Always anywhere persists global entries + +- **GIVEN** an approval prompt for verb `freshdesk` in cwd `~/.netclaw/sessions//` +- **WHEN** the user clicks `Always anywhere` +- **THEN** `tool-approvals.json` gains entry + `{"verb": "freshdesk", "directory": null}` +- **AND** the resolution message reads `Saved: freshdesk anywhere` + +#### Scenario: This chat persists session-scoped only + +- **GIVEN** an approval prompt for verb `jsonlint` in cwd `~/repos/foo/` +- **WHEN** the user clicks `This chat` +- **THEN** session-scoped memory records `(jsonlint, ~/repos/foo/)` +- **AND** `tool-approvals.json` is NOT modified +- **AND** a new session prompts again + +#### Scenario: Deny refuses only the current call + +- **GIVEN** an approval prompt for verb `git push` +- **WHEN** the user clicks `Deny` +- **THEN** the current call is refused +- **AND** `tool-approvals.json` is NOT modified +- **AND** a later `git push` call still prompts + +#### Scenario: Multi-line quoted argument summarized in display text + +- **GIVEN** the agent invokes `shell_execute` with command + `freshdesk ticket reply 605 --message "Hi,⏎We've rolled out a fix. Please verify."` + where the quoted argument spans two lines +- **WHEN** the approval prompt is rendered +- **THEN** the display text reads + `freshdesk ticket reply 605 --message (2 lines, 42 chars)` +- **AND** the display text contains no newline characters + +#### Scenario: Heredoc display keeps the full raw command + +- **GIVEN** a multi-line command contains a complete heredoc +- **WHEN** the approval prompt formats the command +- **THEN** the single-line display keeps the `<<` operator and body text +- **AND** the formatter does not rebuild the command from compatibility clauses + +#### Scenario: Here-string display keeps the authored operator + +- **GIVEN** a multi-line command contains a complete `<<<` redirect +- **WHEN** the approval prompt formats the command +- **THEN** the single-line display keeps the `<<<` operator and data text +- **AND** each authored line break renders as a visible `⏎` marker +- **AND** the formatter does not replace `<<<` with `<` + +#### Scenario: Heredoc display keeps a following command boundary + +- **GIVEN** a complete heredoc is followed by another command after its terminator +- **WHEN** the approval prompt formats the command +- **THEN** the single-line display places a visible `⏎` marker between the terminator and the following command +- **AND** the following command does not appear to be part of the heredoc body diff --git a/openspec/changes/adopt-shellsyntax-alpha1/tasks.md b/openspec/changes/adopt-shellsyntax-alpha1/tasks.md new file mode 100644 index 000000000..e24a296f1 --- /dev/null +++ b/openspec/changes/adopt-shellsyntax-alpha1/tasks.md @@ -0,0 +1,25 @@ +## 1. Package and display contract + +- [x] 1.1 Update the central ShellSyntaxTree version to `0.3.0-alpha.1`. +- [x] 1.2 Use typed heredoc and here-string operations for raw display fallback. +- [x] 1.3 Add focused display tests for full heredoc and `<<<` disclosure. + +## 2. Constrained stdin grammar + +- [x] 2.1 Accept only argument-free `cat` with a literal heredoc or bounded here string. +- [x] 2.2 Keep unknown data, other receivers, arguments, wrappers, and incomplete facts strict. +- [x] 2.3 Add focused analysis and policy tests for each stdin boundary. + +## 3. Approval review matrix + +- [x] 3.1 Add strict cases for Bash command-resolution mutation. +- [x] 3.2 Add strict cases for unsupported reserved execution forms. +- [x] 3.3 Add allow and prompt cases for heredoc and here-string data. +- [x] 3.4 Update and inspect the review-table snapshot. + +## 4. Verification and tracking + +- [x] 4.1 Run the focused Netclaw.Security test suite. +- [x] 4.2 Run the actor approval-matrix test suite. +- [x] 4.3 Run Slopwatch, header verification, and strict OpenSpec validation. +- [x] 4.4 Update `IMPLEMENTATION_PLAN.md` with the delivered package and matrix evidence. diff --git a/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs b/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs index 0f2ab7896..f485796fe 100644 --- a/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs +++ b/src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs @@ -635,9 +635,39 @@ public static class ShellApprovalCases Approvals.None, ExpectedApproval.Require(["git push"])), Case( - "heredoc-prompts", + "literal-heredoc-cat-allows", Bash("cat <<'EOF'\nhello\nEOF"), Approvals.None, + ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)), + Case( + "expanding-heredoc-cat-prompts", + Bash("cat <> \"$LOGDIR\nfile\"")); Assert.DoesNotContain('\n', display); - Assert.Equal("echo hi >> (2 lines, 12 chars)", display); + Assert.Equal("echo hi >> \"$LOGDIR file\"", display); } [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 index 111c02402..8c7a3cef7 100644 --- a/src/Netclaw.Security.Tests/ShellCommandAnalysisTests.cs +++ b/src/Netclaw.Security.Tests/ShellCommandAnalysisTests.cs @@ -144,8 +144,9 @@ public void Dynamic_file_descriptor_redirect_stays_dynamic(string command) { var analysis = _analyzer.Analyze(command); - Assert.Equal(ShellAnalysisFailure.None, analysis.Failure); - Assert.True(analysis.HasDynamicSyntax); + Assert.True( + analysis.Failure == ShellAnalysisFailure.Unresolved + || analysis.HasDynamicSyntax); } [Fact] @@ -171,14 +172,101 @@ public void Exact_file_redirect_has_bounded_path_target() Assert.True(redirect.IsPathRelevant); } + [Theory] + [InlineData("cat <<'EOF'\nbody\nEOF", RedirectOperation.HereDocument)] + [InlineData("cat <<< \"body\"", RedirectOperation.HereString)] + [InlineData("cat 0<<< \"body\"", RedirectOperation.HereString)] + public void Bounded_data_only_stdin_is_not_dynamic( + string command, + RedirectOperation expectedOperation) + { + var analysis = _analyzer.Analyze(command); + + Assert.Equal(ShellAnalysisFailure.None, analysis.Failure); + Assert.False(analysis.HasDynamicSyntax); + var redirect = Assert.Single(Assert.Single(analysis.Commands).Redirects); + Assert.Equal(expectedOperation, redirect.Operation); + Assert.False(redirect.IsPathRelevant); + Assert.True(redirect.IsComplete); + } + + [Fact] + public void Finite_here_string_data_for_argument_free_cat_is_not_dynamic() + { + var occurrence = new CommandOccurrence + { + Clause = new Clause + { + Verb = new VerbChain { Tokens = ["cat"] } + }, + ImmediateRole = CommandOccurrenceRole.Ordinary, + Redirects = + [ + new RedirectAnalysis + { + Source = new RedirectSource { Kind = RedirectSourceKind.Default }, + Operation = RedirectOperation.HereString, + Target = new ShellValueDomain + { + Kind = ShellValueDomainKind.FiniteSet, + Values = ["alpha\n", "beta\n"] + }, + IsComplete = true + } + ], + IsComplete = true + }; + var analysis = new ShellCommandAnalysis([occurrence], ShellAnalysisFailure.None); + + Assert.False(analysis.HasDynamicSyntax); + } + [Fact] - public void Heredoc_stays_approval_sensitive() + public void Transparent_shell_dispatch_keeps_bounded_inner_cat() { - var analysis = _analyzer.Analyze("cat < MalformedStdinRedirects => new() + { + HereDocumentRedirect(new HereDocumentAnalysis + { + Delimiter = new ShellSourceFragment { Raw = "EOF" }, + Body = new ShellSourceFragment { Raw = "body\n" }, + ExpansionMode = HereDocumentExpansionMode.Literal, + IsComplete = false + }), + HereDocumentRedirect(new HereDocumentAnalysis + { + Delimiter = new ShellSourceFragment { Raw = "EOF" }, + Body = new ShellSourceFragment { Raw = "$value\n" }, + ExpansionMode = HereDocumentExpansionMode.Expand, + IsComplete = true + }), + HereDocumentRedirect(new HereDocumentAnalysis + { + Delimiter = new ShellSourceFragment { Raw = "EOF" }, + Body = new ShellSourceFragment { Raw = "body\n" }, + ExpansionMode = (HereDocumentExpansionMode)999, + IsComplete = true + }), + HereDocumentRedirect(new HereDocumentAnalysis + { + ExpansionMode = HereDocumentExpansionMode.Literal, + IsComplete = true + }), + HereDocumentRedirect(new HereDocumentAnalysis + { + Delimiter = null!, + Body = new ShellSourceFragment { Raw = "body\n" }, + ExpansionMode = HereDocumentExpansionMode.Literal, + IsComplete = true + }), + HereDocumentRedirect(new HereDocumentAnalysis + { + Delimiter = new ShellSourceFragment { Raw = "EOF" }, + Body = null!, + ExpansionMode = HereDocumentExpansionMode.Literal, + IsComplete = true + }), + HereDocumentRedirect(new HereDocumentAnalysis + { + Delimiter = new ShellSourceFragment + { + Raw = "EOF", + SourceStart = 1 + }, + Body = new ShellSourceFragment { Raw = "body\n" }, + ExpansionMode = HereDocumentExpansionMode.Literal, + IsComplete = true + }), + HereDocumentRedirect(null), + HereDocumentRedirect( + new HereDocumentAnalysis + { + Delimiter = new ShellSourceFragment { Raw = "EOF" }, + Body = new ShellSourceFragment { Raw = "body\n" }, + ExpansionMode = HereDocumentExpansionMode.Literal, + IsComplete = true + }, + target: new ShellValueDomain + { + Kind = ShellValueDomainKind.Exact, + Values = ["body\n"] + }), + HereStringRedirect(ShellValueDomain.Unknown), + HereStringRedirect(null!), + HereStringRedirect(new ShellValueDomain + { + Kind = ShellValueDomainKind.Exact, + Values = ["one", "two"] + }), + HereStringRedirect(new ShellValueDomain + { + Kind = ShellValueDomainKind.Exact, + Values = [null!] + }), + HereStringRedirect(new ShellValueDomain + { + Kind = ShellValueDomainKind.FiniteSet, + Values = ["one"] + }), + HereStringRedirect(new ShellValueDomain + { + Kind = ShellValueDomainKind.FiniteSet, + Values = ["same", "same"] + }), + HereStringRedirect(new ShellValueDomain + { + Kind = ShellValueDomainKind.FiniteSet, + Values = Enumerable.Range(0, 33).Select(static index => index.ToString()).ToArray() + }), + HereStringRedirect(new ShellValueDomain + { + Kind = ShellValueDomainKind.Pattern, + Pattern = "*", + CoveringDirectory = "/work" + }), + HereStringRedirect( + new ShellValueDomain + { + Kind = ShellValueDomainKind.Exact, + Values = ["body\n"] + }, + source: new RedirectSource + { + Kind = RedirectSourceKind.Descriptor, + Descriptor = 2 + }), + HereStringRedirect( + new ShellValueDomain + { + Kind = ShellValueDomainKind.Exact, + Values = ["body\n"] + }, + isPathRelevant: true), + HereStringRedirect( + new ShellValueDomain + { + Kind = ShellValueDomainKind.Exact, + Values = ["body\n"] + }, + targetDescriptor: 0), + HereStringRedirect( + new ShellValueDomain + { + Kind = ShellValueDomainKind.Exact, + Values = ["body\n"] + }, + hereDocument: new HereDocumentAnalysis + { + Delimiter = new ShellSourceFragment { Raw = "EOF" }, + Body = new ShellSourceFragment { Raw = "body\n" }, + ExpansionMode = HereDocumentExpansionMode.Literal, + IsComplete = true + }) + }; + + private static RedirectAnalysis HereDocumentRedirect( + HereDocumentAnalysis? hereDocument, + ShellValueDomain? target = null) + => new() + { + Source = new RedirectSource { Kind = RedirectSourceKind.Default }, + Operation = RedirectOperation.HereDocument, + Target = target ?? ShellValueDomain.Unknown, + HereDocument = hereDocument, + IsComplete = true + }; + + private static RedirectAnalysis HereStringRedirect( + ShellValueDomain target, + RedirectSource? source = null, + bool isPathRelevant = false, + HereDocumentAnalysis? hereDocument = null, + int? targetDescriptor = null) + => new() + { + Source = source ?? new RedirectSource { Kind = RedirectSourceKind.Default }, + Operation = RedirectOperation.HereString, + Target = target, + TargetDescriptor = targetDescriptor, + HereDocument = hereDocument, + IsPathRelevant = isPathRelevant, + IsComplete = true + }; } diff --git a/src/Netclaw.Security.Tests/ShellSyntaxTreeIntegrationTests.cs b/src/Netclaw.Security.Tests/ShellSyntaxTreeIntegrationTests.cs index 852300ecb..034dff80f 100644 --- a/src/Netclaw.Security.Tests/ShellSyntaxTreeIntegrationTests.cs +++ b/src/Netclaw.Security.Tests/ShellSyntaxTreeIntegrationTests.cs @@ -168,22 +168,18 @@ public void Unparseable_input_sets_flag_without_throwing() } [Fact] - public void Dynamic_token_marked_for_skip() + public void Unknown_variable_state_is_unparseable() { - // Unresolved env var must be flagged so consumers don't extract - // a literal "$UNRESOLVED/foo" as a path candidate. + // Netclaw cannot prove the inherited Bash variable attributes. + // Alpha.1 rejects the full command before a nameref can hide + // execution inside the path expression. var parser = new BashParser(); var result = parser.Parse("rm $UNRESOLVED/foo"); - Assert.False(result.IsUnparseable); - Assert.Single(result.Clauses); - - var argWithDynamic = result.Clauses[0].Args - .FirstOrDefault(a => a.Raw.Contains("$UNRESOLVED")); - Assert.NotNull(argWithDynamic); - Assert.Equal(ArgKind.DynamicSkip, argWithDynamic.Kind); - Assert.Null(argWithDynamic.Resolved); + Assert.True(result.IsUnparseable); + Assert.Empty(result.Clauses); + Assert.Empty(result.Commands); } [Fact] diff --git a/src/Netclaw.Security/IToolApprovalMatcher.cs b/src/Netclaw.Security/IToolApprovalMatcher.cs index 674e1e9e6..f318bb278 100644 --- a/src/Netclaw.Security/IToolApprovalMatcher.cs +++ b/src/Netclaw.Security/IToolApprovalMatcher.cs @@ -929,8 +929,10 @@ public string FormatForDisplay(ToolName toolName, IDictionary? // inline script) dumped verbatim corrupts the approval prompt. On // POSIX, rebuild a one-line view from the parse tree with multi-line // args summarized by size; Windows flattens — ShellSyntaxTree is - // bash-only. The trailing ReplaceLineEndings catches line breaks the - // reconstruction can leak and collapses CRLF to a single space. + // bash-only. Heredoc and here-string fallbacks encode line breaks + // before the trailing replacement so command boundaries stay visible. + // The trailing replacement catches any other leaked line breaks and + // collapses CRLF to a single space. var display = OperatingSystem.IsWindows() ? command : BuildSanitizedDisplayViaParser(command); @@ -943,15 +945,14 @@ public string FormatForDisplay(ToolName toolName, IDictionary? /// parse tree: statement separators render as explicit operators and any /// multi-line argument is replaced with a (N lines, M chars) /// summary — the operator approving the command needs its shape, not the - /// full content (issue #1402). Returns the raw command (for the caller - /// to flatten) when the parser cannot decompose it, when the command - /// contains a heredoc — the parser drops heredoc bodies entirely (only - /// the <<EOF marker survives as a redirect target), so a - /// tree reconstruction would silently omit executable content the - /// approver must see — or when it contains a subshell, whose grouping + /// full content (issue #1402). Returns the raw command when the parser + /// cannot decompose it. A heredoc or here string uses a raw view with + /// visible line-break markers because the compatibility redirect cannot + /// preserve the v0.3 operation and data facts. A subshell uses the raw + /// fallback because its grouping /// does not survive the flat clause list, so a reconstruction would /// misstate which statements a pipe or && guard applies - /// to. The flattened raw command is ugly but fully disclosed. + /// to. The raw fallback is ugly but fully disclosed. /// private static string BuildSanitizedDisplayViaParser(string command) { @@ -959,8 +960,13 @@ private static string BuildSanitizedDisplayViaParser(string command) if (result is null) return command; - if (result.Commands.Any(c => - c.Clause.IsSubshell || c.Clause.Redirects.Any(IsHeredocRedirect))) + if (result.Commands.Any(static occurrence => + occurrence.Redirects.Any(IsRawDisplayRedirect))) + { + return command.ReplaceLineEndings(" ⏎ "); + } + + if (result.Commands.Any(static occurrence => occurrence.Clause.IsSubshell)) return command; try @@ -1014,14 +1020,14 @@ private static string BuildSanitizedDisplayViaParser(string command) } /// - /// True when a parsed redirect is a heredoc marker (<<EOF, - /// <<-EOF). The parser keeps only the marker as the redirect - /// target and drops the body from the tree, so heredoc-bearing commands - /// must not be display-reconstructed — see + /// True when the v0.3 redirect operation carries shell-fed data that the + /// compatibility clause cannot reconstruct without changing its meaning. + /// See /// . /// - private static bool IsHeredocRedirect(ShellSyntaxTree.Redirect redirect) - => redirect.Target?.StartsWith("<<", StringComparison.Ordinal) == true; + private static bool IsRawDisplayRedirect(ShellSyntaxTree.RedirectAnalysis redirect) + => redirect.Operation is ShellSyntaxTree.RedirectOperation.HereDocument + or ShellSyntaxTree.RedirectOperation.HereString; /// /// Size summary shown in place of a multi-line argument. Outer quotes diff --git a/src/Netclaw.Security/ShellCommandAnalysis.cs b/src/Netclaw.Security/ShellCommandAnalysis.cs index febab099a..a6c6c4662 100644 --- a/src/Netclaw.Security/ShellCommandAnalysis.cs +++ b/src/Netclaw.Security/ShellCommandAnalysis.cs @@ -288,7 +288,7 @@ internal sealed record ShellCommandAnalysis( // 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(ShellGlobPath.HasUnresolvedDescendantScope) - || command.Redirects.Any(HasUnresolvedRedirect)); + || HasUnresolvedRedirect(command)); private static bool HasUnsupportedWorkingDirectory(ShellValueDomain workingDirectory) { @@ -308,9 +308,16 @@ private static bool HasUnsupportedWorkingDirectory(ShellValueDomain workingDirec }; } - private static bool HasUnresolvedRedirect(RedirectAnalysis redirect) + private static bool HasUnresolvedRedirect(CommandOccurrence occurrence) + => occurrence.Redirects.Any(redirect => HasUnresolvedRedirect(occurrence, redirect)); + + private static bool HasUnresolvedRedirect( + CommandOccurrence occurrence, + RedirectAnalysis redirect) { - if (!redirect.IsComplete + if (redirect.Source is null + || redirect.Target is null + || !redirect.IsComplete || !Enum.IsDefined(redirect.Source.Kind) || redirect.Source.Kind == RedirectSourceKind.Unknown || !Enum.IsDefined(redirect.Operation) @@ -332,9 +339,8 @@ private static bool HasUnresolvedRedirect(RedirectAnalysis redirect) return redirect.Operation switch { - // Netclaw keeps shell-fed stdin approval-sensitive. The parser can - // prove its syntax without proving executable-specific data use. - RedirectOperation.HereDocument or RedirectOperation.HereString => true, + RedirectOperation.HereDocument or RedirectOperation.HereString => + !HasBoundedDataOnlyStdin(occurrence, redirect), RedirectOperation.DescriptorDuplicate or RedirectOperation.DescriptorMove => redirect.IsPathRelevant || redirect.TargetDescriptor is < 0 or null, RedirectOperation.DescriptorClose => @@ -349,6 +355,98 @@ or RedirectOperation.CombinedOutput }; } + private static bool HasBoundedDataOnlyStdin( + CommandOccurrence occurrence, + RedirectAnalysis redirect) + { + var clause = occurrence.Clause; + if (!IsStandardInputSource(redirect.Source) + || redirect.IsPathRelevant + || clause.Verb.Tokens.Count != 1 + || !string.Equals( + ShellTokenizer.TrimShellPunctuation(clause.Verb.Tokens[0]), + "cat", + StringComparison.Ordinal) + || clause.Args.Any(static arg => !arg.IsCwdAttribution)) + { + return false; + } + + return redirect.Operation switch + { + RedirectOperation.HereDocument when redirect.HereDocument is not null => + redirect.TargetDescriptor is null + && HasCanonicalUnknownData(redirect.Target) + && HasLiteralHereDocument( + redirect.HereDocument, + clause.IsCommandStringWrapped), + RedirectOperation.HereString when redirect.HereDocument is null => + redirect.TargetDescriptor is null + && HasBoundedData(redirect.Target), + _ => false + }; + } + + private static bool IsStandardInputSource(RedirectSource source) + => source.Kind == RedirectSourceKind.Default + || source is { Kind: RedirectSourceKind.Descriptor, Descriptor: 0 }; + + private static bool HasLiteralHereDocument( + HereDocumentAnalysis hereDocument, + bool allowUnavailableSourceSpans) + => hereDocument.Delimiter is not null + && hereDocument.Body is not null + && hereDocument.IsComplete + && Enum.IsDefined(hereDocument.ExpansionMode) + && hereDocument.ExpansionMode == HereDocumentExpansionMode.Literal + && HasValidSourceFragment( + hereDocument.Delimiter, + allowUnavailableSourceSpans) + && HasValidSourceFragment( + hereDocument.Body, + allowUnavailableSourceSpans); + + private static bool HasValidSourceFragment( + ShellSourceFragment fragment, + bool allowUnavailableSourceSpan) + { + if (fragment.Raw is null) + return false; + + if (fragment.SourceStart is null && fragment.SourceLength is null) + return allowUnavailableSourceSpan; + + return fragment.SourceStart >= 0 && fragment.SourceLength >= 0; + } + + private static bool HasCanonicalUnknownData(ShellValueDomain data) + => data is not null + && data.Kind == ShellValueDomainKind.Unknown + && data.Values.Count == 0 + && data.Pattern is null + && data.CoveringDirectory is null; + + private static bool HasBoundedData(ShellValueDomain data) + { + if (data is null + || !Enum.IsDefined(data.Kind) + || data.Pattern is not null + || data.CoveringDirectory is not null) + { + return false; + } + + return data.Kind switch + { + ShellValueDomainKind.Exact => data.Values.Count == 1 + && data.Values[0] is not null, + ShellValueDomainKind.FiniteSet => data.Values.Count is >= 2 and <= 32 + && data.Values.All(static value => value is not null) + && data.Values.Distinct(StringComparer.Ordinal).Count() == data.Values.Count, + _ => false + }; + } + private static bool HasBoundedPathTarget(ShellValueDomain target) => target.Kind switch {