diff --git a/docs/bash-tool-runtime.md b/docs/bash-tool-runtime.md index 642fd96f2..1d8887179 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. +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. Interception behavior: diff --git a/docs/tools/bash.md b/docs/tools/bash.md index ddea53769..3ef14e670 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. 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 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. ### 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, 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 a single unquoted `|`), 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/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index b20591af8..6e83dc9c5 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed the bash interceptor blocking `grep`/`cat`/`find` used as a downstream pipeline stage (e.g. `printf 'x\n' | grep x`); a stage consuming piped stdin cannot be replaced by a path-based dedicated tool, so it is no longer matched, while standalone and first-stage searches stay intercepted ([#7496](https://github.com/can1357/oh-my-pi/issues/7496)). + ## [17.2.5] - 2026-08-03 ### Breaking Changes diff --git a/packages/coding-agent/src/tools/bash-interceptor.ts b/packages/coding-agent/src/tools/bash-interceptor.ts index 6d2e83613..baa6cfbdc 100644 --- a/packages/coding-agent/src/tools/bash-interceptor.ts +++ b/packages/coding-agent/src/tools/bash-interceptor.ts @@ -96,10 +96,14 @@ function withoutLeadingEnvironmentAssignments(command: string): string | null { function interceptionCandidates(command: string): string[] { const candidates = [command.trim()]; - const segments = extractFlatShellCommandSegments(command); - candidates.push(...segments.map(segment => segment.trim())); - for (const segment of segments) { - const withoutAssignments = withoutLeadingEnvironmentAssignments(segment); + for (const segment of extractFlatShellCommandSegments(command)) { + // A segment that consumes the previous stage's stdout via `|` reads piped + // stdin, which no path-based dedicated tool (read/grep/glob) — nor any + // other dedicated tool — can replace, so it is not an interception + // candidate. Standalone and first-stage commands still match. + if (segment.pipedStdin) continue; + candidates.push(segment.text); + const withoutAssignments = withoutLeadingEnvironmentAssignments(segment.text); if (withoutAssignments) candidates.push(withoutAssignments); } return candidates; diff --git a/packages/coding-agent/src/tools/shell-tokenize.ts b/packages/coding-agent/src/tools/shell-tokenize.ts index faa50bb72..d1e70e5c5 100644 --- a/packages/coding-agent/src/tools/shell-tokenize.ts +++ b/packages/coding-agent/src/tools/shell-tokenize.ts @@ -83,25 +83,44 @@ export function tokenizeShellSegments(command: string): string[][] { } /** - * Returns the original text of flat shell command segments. Unlike + * A flat shell command segment with the context needed to decide interception. + * + * @see extractFlatShellCommandSegments + */ +export interface FlatShellCommandSegment { + /** Original segment text with quoting and escaping preserved. */ + 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. + */ + pipedStdin: boolean; +} + +/** + * Returns the flat shell command segments with the original text of each. Unlike * `tokenizeShellSegments`, this preserves quoting and escaping so the results - * are safe to match against user-configured regular expressions. + * are safe to match against user-configured regular expressions, and flags + * segments that receive piped stdin. * * The extractor deliberately declines to split syntax whose execution context * cannot be determined with this small scanner (heredocs, command substitution, * backticks, grouping, and malformed quoting). Callers must still check the * complete input in that case. */ -export function extractFlatShellCommandSegments(command: string): string[] { - const segments: string[] = []; +export function extractFlatShellCommandSegments(command: string): FlatShellCommandSegment[] { + const segments: FlatShellCommandSegment[] = []; let segmentStart = 0; let inSingle = false; let inDouble = false; let atWordStart = true; + let currentPiped = false; const pushSegment = (end: number) => { const segment = command.slice(segmentStart, end).trim(); - if (segment.length > 0) segments.push(segment); + if (segment.length > 0) segments.push({ text: segment, pipedStdin: currentPiped }); }; for (let i = 0; i < command.length; i++) { @@ -160,6 +179,7 @@ export function extractFlatShellCommandSegments(command: string): string[] { i = newline; segmentStart = newline + 1; atWordStart = true; + currentPiped = false; continue; } const isRedirectionOperatorCharacter = @@ -170,7 +190,12 @@ export function extractFlatShellCommandSegments(command: string): string[] { : false; if ((ch === "\n" || ch === ";" || ch === "|" || ch === "&") && !isRedirectionOperatorCharacter) { pushSegment(i); - if ((ch === "|" || ch === "&") && command[i + 1] === ch) 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; 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 104e53bea..edcfb2074 100644 --- a/packages/coding-agent/test/tools/bash-interceptor.test.ts +++ b/packages/coding-agent/test/tools/bash-interceptor.test.ts @@ -77,13 +77,19 @@ describe("compound command interception", () => { "git add file && git commit -m message", "git add file; git commit -m message", "git add file || git commit -m message", - "git add file | git commit -m message", "git add file & git commit -m message", "git add file\ngit commit -m message", ])("blocks a later command after %s", command => { expect(checkBashInterception(command, ["commit"], rules).block).toBe(true); }); + it("does not intercept a downstream pipe stage that consumes piped stdin", () => { + // `git commit` after a single `|` reads the previous stage's stdout, so + // the dedicated tool cannot replace it. `||` still starts a fresh command. + expect(checkBashInterception("git add file | git commit -m message", ["commit"], rules).block).toBe(false); + expect(checkBashInterception("git add file || git commit -m message", ["commit"], rules).block).toBe(true); + }); + it("removes one or more leading environment assignments before matching", () => { expect( checkBashInterception('GIT_AUTHOR_EMAIL="a@example.com" git commit -m message', ["commit"], rules).block, @@ -203,6 +209,33 @@ describe("default echo/printf redirect rule", () => { }); }); +describe("default grep rule and pipeline stdin", () => { + const tools = ["grep"]; + + it("blocks standalone file searches", () => { + expect(checkBashInterception("grep pattern path", tools, DEFAULT_BASH_INTERCEPTOR_RULES).block).toBe(true); + expect(checkBashInterception("rg pattern src", tools, DEFAULT_BASH_INTERCEPTOR_RULES).block).toBe(true); + }); + + it("blocks a first-stage grep that produces pipeline input", () => { + expect(checkBashInterception("grep x file | wc -l", tools, DEFAULT_BASH_INTERCEPTOR_RULES).block).toBe(true); + }); + + it("does not block grep consuming pipeline stdin", () => { + expect(checkBashInterception("printf 'x\\n' | grep x", tools, DEFAULT_BASH_INTERCEPTOR_RULES).block).toBe(false); + expect( + checkBashInterception("tr -d '\\r' < input.log | grep -v '^ *foo'", tools, DEFAULT_BASH_INTERCEPTOR_RULES) + .block, + ).toBe(false); + }); + + it("still blocks a standalone grep sequenced after a pipeline", () => { + expect( + checkBashInterception("cat log | tr a b && grep err file", tools, DEFAULT_BASH_INTERCEPTOR_RULES).block, + ).toBe(true); + }); +}); + describe("default hub start rules", () => { const tools = ["hub"];