From 430e4e386ba1bff9216c816b2dd513fd23573de3 Mon Sep 17 00:00:00 2001 From: roboomp Date: Mon, 3 Aug 2026 12:45:43 +0000 Subject: [PATCH] fix(tool): preserved piped stdin across continuations Retained pending pipeline state across blank and comment-only continuation lines, and parsed Bash's |& operator as a single pipe boundary. Added regression coverage for both forms and aligned the Bash interceptor docs. Fixes #7496 --- docs/bash-tool-runtime.md | 2 +- docs/tools/bash.md | 4 +-- .../coding-agent/src/tools/shell-tokenize.ts | 30 +++++++++++-------- .../test/tools/bash-interceptor.test.ts | 7 +++++ 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/docs/bash-tool-runtime.md b/docs/bash-tool-runtime.md index 1d8887179..235f3e276 100644 --- a/docs/bash-tool-runtime.md +++ b/docs/bash-tool-runtime.md @@ -32,7 +32,7 @@ There are no structured `head` or `tail` tool parameters in the current schema, ## 2) Optional interception (blocked-command path) -If `bashInterceptor.enabled` is true, `BashTool` loads rules from settings (`getBashInterceptorRules()`) and runs `checkBashInterception()` against the command — checking both the original and the cwd-normalized form (after a leading `cd … &&` is extracted) when they differ. Rule syntax is unchanged: each rule checks the complete input first, then raw flat command fragments separated by unquoted/unescaped `&&`, `||`, `;`, `|`, `&`, or newlines, then those fragments with leading `NAME=value` assignments removed. Fragments that receive piped stdin from a single unquoted `|` are excluded from the fragment candidates, because a stdin-consuming stage cannot be replaced by a path-based dedicated tool. +If `bashInterceptor.enabled` is true, `BashTool` loads rules from settings (`getBashInterceptorRules()`) and runs `checkBashInterception()` against the command — checking both the original and the cwd-normalized form (after a leading `cd … &&` is extracted) when they differ. Rule syntax is unchanged: each rule checks the complete input first, then raw flat command fragments separated by unquoted/unescaped `&&`, `||`, `;`, `|`, `|&`, `&`, or newlines, then those fragments with leading `NAME=value` assignments removed. Fragments that receive piped stdin from `|` or `|&` are excluded from the fragment candidates, including across blank/comment continuation lines, because a stdin-consuming stage cannot be replaced by a path-based dedicated tool. Interception behavior: diff --git a/docs/tools/bash.md b/docs/tools/bash.md index 3ef14e670..d66850598 100644 --- a/docs/tools/bash.md +++ b/docs/tools/bash.md @@ -110,7 +110,7 @@ git add file && git commit -m "message" GIT_AUTHOR_NAME=Dev git commit -m "message" ``` -An anchored rule such as `^\s*git\s+commit\b` can therefore match the `git commit` command in both examples. A stage that consumes another command's stdout through a single unquoted `|` (for example `grep x` in `printf 'x\n' | grep x`) is **not** treated as an interception candidate: it reads piped stdin, which the path-based dedicated tools cannot supply, so only a standalone or first-stage command is matched. Quoted, escaped, and commented text is not treated as a command. Heredocs, parameter expansion, command substitution, backticks, grouping, and malformed quoting retain only the complete-command check; the interceptor deliberately does not attempt to become a full shell parser. +An anchored rule such as `^\s*git\s+commit\b` can therefore match the `git commit` command in both examples. A stage that consumes another command's stdout through an unquoted `|` or `|&` (for example `grep x` in `printf 'x\n' | grep x`) is **not** treated as an interception candidate: it reads piped stdin, which the path-based dedicated tools cannot supply, so only a standalone or first-stage command is matched. Blank and comment-only continuation lines after the pipe preserve that context. Quoted, escaped, and commented text is not treated as a command. Heredocs, parameter expansion, command substitution, backticks, grouping, and malformed quoting retain only the complete-command check; the interceptor deliberately does not attempt to become a full shell parser. ### Interaction and selection guide @@ -127,7 +127,7 @@ Choose the setting by the desired outcome: 1. `BashTool.execute()` in `packages/coding-agent/src/tools/bash.ts` reads `command`, normalizes `env`, and defaults `timeout` to `300`. Commands execute exactly as written — there is no pre-execution rewrite pass. 2. If `cwd` is absent, it rewrites a leading `cd && ...` into the structured `cwd` field and strips that prefix from `command`. 3. If `async: true` is requested while `async.enabled` is off, it throws `ToolError` before any execution. -4. If `bashInterceptor.enabled` is on, `checkBashInterception()` runs against both the original command and the `cd`-stripped command. For each form, configured regexes still check the complete input first, then each flat command separated by unquoted/unescaped `&&`, `||`, `;`, `|`, `&`, or newlines (excluding stages that consume piped stdin from a single unquoted `|`), followed by versions of those fragments without leading `NAME=value` assignments. A matching enabled rule throws before URL expansion or execution. +4. If `bashInterceptor.enabled` is on, `checkBashInterception()` runs against both the original command and the `cd`-stripped command. For each form, configured regexes still check the complete input first, then each flat command separated by unquoted/unescaped `&&`, `||`, `;`, `|`, `|&`, `&`, or newlines (excluding stages that consume piped stdin from `|` or `|&`, including across blank/comment continuations), followed by versions of those fragments without leading `NAME=value` assignments. A matching enabled rule throws before URL expansion or execution. 5. `expandInternalUrls()` rewrites supported internal URLs inside `command`, each `env` value, and protocol-looking `cwd` values. Command replacements are shell-escaped; `env` and `cwd` replacements use raw filesystem/string values because they are not interpolated into shell text. 6. `resolveToCwd()` resolves `cwd` against `session.cwd`; `fs.stat()` verifies that the target exists and is a directory. 7. `clampTimeout("bash", requestedTimeoutSec)` enforces `TOOL_TIMEOUTS.bash` (`default: 300`, `min: 1`, `max: 3600`). When clamped, `#buildCompletedResult()` / `#buildBackgroundStartResult()` append a notice line. diff --git a/packages/coding-agent/src/tools/shell-tokenize.ts b/packages/coding-agent/src/tools/shell-tokenize.ts index d1e70e5c5..f93f3e276 100644 --- a/packages/coding-agent/src/tools/shell-tokenize.ts +++ b/packages/coding-agent/src/tools/shell-tokenize.ts @@ -92,9 +92,10 @@ export interface FlatShellCommandSegment { text: string; /** * True when this segment consumes the previous stage's stdout via an - * unquoted `|`. Such a stage reads piped stdin, so path-based dedicated - * tools (read/grep/glob) cannot replace it. `||`, `;`, `&`, `&&`, and - * newlines start an independent command and leave this false. + * unquoted `|` or `|&`. Blank and comment-only continuation lines preserve + * the pending pipe state. Such a stage reads piped stdin, so path-based + * dedicated tools (read/grep/glob) cannot replace it. `||`, `;`, `&`, and + * `&&` start an independent command and leave this false. */ pipedStdin: boolean; } @@ -118,9 +119,11 @@ export function extractFlatShellCommandSegments(command: string): FlatShellComma let atWordStart = true; let currentPiped = false; - const pushSegment = (end: number) => { + const pushSegment = (end: number): boolean => { const segment = command.slice(segmentStart, end).trim(); - if (segment.length > 0) segments.push({ text: segment, pipedStdin: currentPiped }); + if (segment.length === 0) return false; + segments.push({ text: segment, pipedStdin: currentPiped }); + return true; }; for (let i = 0; i < command.length; i++) { @@ -173,13 +176,14 @@ export function extractFlatShellCommandSegments(command: string): FlatShellComma return []; } if (ch === "#" && atWordStart) { - pushSegment(i); + const pushed = pushSegment(i); const newline = command.indexOf("\n", i + 1); if (newline === -1) return segments; i = newline; segmentStart = newline + 1; atWordStart = true; - currentPiped = false; + // Preserve a pending pipe through a comment-only continuation. + if (pushed) currentPiped = false; continue; } const isRedirectionOperatorCharacter = @@ -189,13 +193,13 @@ export function extractFlatShellCommandSegments(command: string): FlatShellComma ? command[i - 1] === ">" || command[i - 1] === "<" || command[i + 1] === ">" : false; if ((ch === "\n" || ch === ";" || ch === "|" || ch === "&") && !isRedirectionOperatorCharacter) { - pushSegment(i); + const pushed = pushSegment(i); const doubled = (ch === "|" || ch === "&") && command[i + 1] === ch; - if (doubled) i++; - // A single unquoted `|` pipes the previous stage's stdout into the - // next segment; `||`, `;`, `&`, `&&`, and newlines start a fresh - // command whose stdin is not the previous stage's output. - currentPiped = ch === "|" && !doubled; + const pipeStderr = ch === "|" && command[i + 1] === "&"; + if (doubled || pipeStderr) i++; + // `|` and `|&` pipe into the next segment. Blank continuation + // lines preserve that pending state; all other operators reset it. + if (pushed || ch !== "\n") currentPiped = ch === "|" && !doubled; segmentStart = i + 1; atWordStart = true; continue; diff --git a/packages/coding-agent/test/tools/bash-interceptor.test.ts b/packages/coding-agent/test/tools/bash-interceptor.test.ts index edcfb2074..924aacd6d 100644 --- a/packages/coding-agent/test/tools/bash-interceptor.test.ts +++ b/packages/coding-agent/test/tools/bash-interceptor.test.ts @@ -227,6 +227,13 @@ describe("default grep rule and pipeline stdin", () => { checkBashInterception("tr -d '\\r' < input.log | grep -v '^ *foo'", tools, DEFAULT_BASH_INTERCEPTOR_RULES) .block, ).toBe(false); + expect(checkBashInterception("printf 'x\\n' |\n grep x", tools, DEFAULT_BASH_INTERCEPTOR_RULES).block).toBe( + false, + ); + expect( + checkBashInterception("printf 'x\\n' |\n # filter\n grep x", tools, DEFAULT_BASH_INTERCEPTOR_RULES).block, + ).toBe(false); + expect(checkBashInterception("printf 'x\\n' |& grep x", tools, DEFAULT_BASH_INTERCEPTOR_RULES).block).toBe(false); }); it("still blocks a standalone grep sequenced after a pipeline", () => {