Merge PR #7553: fix(coding-agent): allow quoted metacharacters in bash patterns (@roboomp)
This commit is contained in:
@@ -18,6 +18,7 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed Codex `config.toml` discovery importing MCP servers with `enabled = false` ([#7538](https://github.com/can1357/oh-my-pi/issues/7538)).
|
||||
- Fixed `bash.patterns` allow rules rejecting simple commands when quoted arguments contained shell metacharacters such as Cargo benchmark regex filters ([#7552](https://github.com/can1357/oh-my-pi/issues/7552)).
|
||||
|
||||
## [17.2.6] - 2026-08-03
|
||||
|
||||
|
||||
@@ -58,7 +58,65 @@ export const BASH_DEFAULT_PREVIEW_LINES = DEFAULT_TERMINAL_PREVIEW_LINES;
|
||||
|
||||
const BASH_ENV_NAME_PATTERN = /^[A-Za-z_][A-Za-z0-9_]*$/;
|
||||
const DEFAULT_AUTO_BACKGROUND_THRESHOLD_MS = 60_000;
|
||||
const BASH_APPROVAL_SHELL_CONTROL_RE = /[\n\r;&|<>`$()]/u;
|
||||
const BASH_APPROVAL_SHELL_CONTROL_CHARS: Record<string, true> = {
|
||||
"\n": true,
|
||||
"\r": true,
|
||||
";": true,
|
||||
"&": true,
|
||||
"|": true,
|
||||
"<": true,
|
||||
">": true,
|
||||
"`": true,
|
||||
$: true,
|
||||
"(": true,
|
||||
")": true,
|
||||
};
|
||||
const BASH_APPROVAL_REINTERPRETED_ARGUMENT_RE = /(?:^|[ \t])(?:-[^-]*[ce]|--(?:command|eval))(?:[= \t]|$)/u;
|
||||
|
||||
function hasBashApprovalShellControl(command: string): boolean {
|
||||
let quote: "'" | '"' | undefined;
|
||||
let hasReinterpretableShellControl = false;
|
||||
for (let i = 0; i < command.length; i++) {
|
||||
const ch = command[i];
|
||||
if (quote === "'") {
|
||||
if (ch === "'") {
|
||||
quote = undefined;
|
||||
} else if (Object.hasOwn(BASH_APPROVAL_SHELL_CONTROL_CHARS, ch)) {
|
||||
hasReinterpretableShellControl = true;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (ch === "\\") {
|
||||
const escaped = command[i + 1];
|
||||
if (escaped && Object.hasOwn(BASH_APPROVAL_SHELL_CONTROL_CHARS, escaped)) {
|
||||
hasReinterpretableShellControl = true;
|
||||
}
|
||||
i++;
|
||||
continue;
|
||||
}
|
||||
if (quote === '"') {
|
||||
if (ch === '"') {
|
||||
quote = undefined;
|
||||
continue;
|
||||
}
|
||||
// Expansion is active inside double quotes even in the original line.
|
||||
if (ch === "`" || ch === "$") return true;
|
||||
// Other control characters are literal here but become executable if a
|
||||
// `-c`/`-e` option reinterprets the argument through another shell.
|
||||
if (Object.hasOwn(BASH_APPROVAL_SHELL_CONTROL_CHARS, ch)) hasReinterpretableShellControl = true;
|
||||
continue;
|
||||
}
|
||||
if (ch === "'" || ch === '"') {
|
||||
quote = ch;
|
||||
continue;
|
||||
}
|
||||
if (Object.hasOwn(BASH_APPROVAL_SHELL_CONTROL_CHARS, ch)) return true;
|
||||
}
|
||||
// Options such as `git -c alias.x='!...'` and `sh -c "..."` reinterpret
|
||||
// otherwise literal quoted or escaped arguments as executable code.
|
||||
return hasReinterpretableShellControl && BASH_APPROVAL_REINTERPRETED_ARGUMENT_RE.test(command);
|
||||
}
|
||||
|
||||
const BASH_PATTERN_APPROVAL_VALUES = new Set(["allow", "deny", "prompt"]);
|
||||
|
||||
/**
|
||||
@@ -218,7 +276,7 @@ function commandSegmentMatchesBashApprovalPattern(command: string, pattern: stri
|
||||
// `prompt` fire on any matching segment so they mean what they appear to.
|
||||
function bashApprovalRuleMatches(command: string, rule: BashApprovalPatternRule): boolean {
|
||||
if (rule.approval === "allow") {
|
||||
if (BASH_APPROVAL_SHELL_CONTROL_RE.test(command)) return false;
|
||||
if (hasBashApprovalShellControl(command)) return false;
|
||||
return commandMatchesBashApprovalPattern(command, rule.match);
|
||||
}
|
||||
return commandSegmentMatchesBashApprovalPattern(command, rule.match);
|
||||
|
||||
@@ -329,6 +329,9 @@ describe("tool-owned dynamic approval declarations", () => {
|
||||
"git $(rm file.txt)",
|
||||
"git `rm file.txt` status",
|
||||
"git status > /etc/passwd",
|
||||
"git -c alias.x='!touch /tmp/pwn; printf ok' x",
|
||||
'git -c alias.x="!touch /tmp/pwn; printf ok" x',
|
||||
"git -c alias.x=!touch\\ /tmp/pwn\\;\\ printf\\ ok x",
|
||||
"git status < seed",
|
||||
// Different binary resolution than the pattern names.
|
||||
"FOO=1 git status",
|
||||
@@ -347,6 +350,16 @@ describe("tool-owned dynamic approval declarations", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("allows literal shell metacharacters in quoted arguments", () => {
|
||||
const settingsOverrides = {
|
||||
"bash.patterns": [{ match: "cargo *", approval: "allow" }],
|
||||
};
|
||||
const command =
|
||||
"cargo bench --manifest-path layers/layer3/Cargo.toml --bench standardized_criterion -- --full '^layer3/write/file-wal/batch-(10|1000|10000)$'";
|
||||
|
||||
expect(bashApproval(command, settingsOverrides)).toEqual({ tier: "write", policy: "allow" });
|
||||
});
|
||||
|
||||
it("honors bash pattern rules in yolo mode", () => {
|
||||
const tool = createBashTool({
|
||||
"bash.patterns": [
|
||||
|
||||
Reference in New Issue
Block a user