Merge PR #7497: fix(tool): exempt piped-stdin stages from bash interceptor (@roboomp)
This commit is contained in:
@@ -37,7 +37,7 @@ The bash tool has the `exec` approval tier. `bash.patterns` rules can explicitly
|
||||
|
||||
## 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 `|` 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:
|
||||
|
||||
|
||||
+2
-2
@@ -111,7 +111,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 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`, validates `env`, and defaults `timeout` to `300`.
|
||||
2. If `cwd` is absent, it rewrites a leading `cd <path> && ...` 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 `|` 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. `timeout: 0` disables the deadline. Otherwise `clampTimeout("bash", requestedTimeoutSec, tools.maxTimeout)` applies a positive global ceiling (when configured), then `TOOL_TIMEOUTS.bash` (`min: 1`, `max: 3600`). When clamped, `#buildCompletedResult()` / `#buildBackgroundStartResult()` append a notice line.
|
||||
|
||||
@@ -45,6 +45,9 @@
|
||||
- Fixed issues with `/btw` branch promotion where branches could park behind active turns, cut from outdated session leaves, or leave rejected branch keys indistinguishable from composer input.
|
||||
- Fixed database bloat by ensuring archived main and nested session rows are properly cleaned up from `stats.db` during garbage collection.
|
||||
- Fixed startup hanging during local model discovery when a timed-out transport left its request pending, which blocked the CLI before OAuth login could finish ([#7482](https://github.com/can1357/oh-my-pi/issues/7482)).
|
||||
### 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
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -83,25 +83,47 @@ 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 `|` 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;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 pushSegment = (end: number): boolean => {
|
||||
const segment = command.slice(segmentStart, end).trim();
|
||||
if (segment.length > 0) segments.push(segment);
|
||||
if (segment.length === 0) return false;
|
||||
segments.push({ text: segment, pipedStdin: currentPiped });
|
||||
return true;
|
||||
};
|
||||
|
||||
for (let i = 0; i < command.length; i++) {
|
||||
@@ -154,12 +176,14 @@ export function extractFlatShellCommandSegments(command: string): string[] {
|
||||
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;
|
||||
// Preserve a pending pipe through a comment-only continuation.
|
||||
if (pushed) currentPiped = false;
|
||||
continue;
|
||||
}
|
||||
const isRedirectionOperatorCharacter =
|
||||
@@ -169,8 +193,13 @@ export function extractFlatShellCommandSegments(command: string): string[] {
|
||||
? command[i - 1] === ">" || command[i - 1] === "<" || command[i + 1] === ">"
|
||||
: false;
|
||||
if ((ch === "\n" || ch === ";" || ch === "|" || ch === "&") && !isRedirectionOperatorCharacter) {
|
||||
pushSegment(i);
|
||||
if ((ch === "|" || ch === "&") && command[i + 1] === ch) i++;
|
||||
const pushed = pushSegment(i);
|
||||
const doubled = (ch === "|" || ch === "&") && command[i + 1] === ch;
|
||||
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;
|
||||
|
||||
@@ -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,40 @@ 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);
|
||||
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", () => {
|
||||
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"];
|
||||
|
||||
|
||||
Reference in New Issue
Block a user