Fix auto-mode allowing quoted shell redirect writes and dangerous flags (#601)

* Fix auto-mode shell policy: quoting no longer defeats deny/ask rules

The auto-shell policy blanked quoted spans to whole-space before matching
its rules, so a quoted redirect target, a quoted -c/-i flag, a quoted
install subcommand, or a quoted argv0 all read as absent text and slipped
past deny/ask. Replace the blanket blank with quote-aware dequoting that
mirrors real shell semantics: quote characters are dropped and their
content stays literal, except the handful of characters that are only
operators outside quotes (> < | & ; `) which are neutralized when quoted,
so a literal '>' in a commit message still can't be mistaken for a
redirect. Also widen the file-mutation redirect pattern to recognize the
`>|` / `>>|` clobber form, which never matched at all.

One tokenizer fix covers all three bypasses since they share the same
root cause (stripQuoted) and the same call site (matchAutoShellRule).

* Close backslash-escaped-quote bypass; narrow changelog claim

dequoteForMatching tracked quote state without escape awareness, so a
backslash-escaped quote (\") still toggled quote state the same as a real
one. In real bash \" is a literal quote character that never opens or
closes a quoted span, so an operator or flag that follows is genuinely
unquoted. Skip the escaped character without touching quote state.

Also narrow the CHANGELOG's nested bash -c claim to what the tests
actually cover (one level of quoting inside -c), not true multi-level
nested-shell parsing, which remains a known gap tracked separately.

* Apply prettier formatting

Author: TheGreatAxios

CHANGELOG.md

@@ -58,6 +58,21 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
   the tool at all. `progress_note` for leaf workers is a separate,
   not-yet-implemented follow-up.
 
+### Permissions
+
+- **Quoting or backslash-escaping a redirect target, a dangerous flag, or a
+  program name no longer bypasses auto mode's shell rules.** The auto-shell
+  policy used to blank out quoted text before matching its rules, so `echo hi
+  > "file"`, `echo hi >|file`, a quoted `-c`/`-i` flag, a quoted `install`
+  subcommand, or a quoted upload-tool name all slipped past the
+  file-mutation, dependency-install, and network-upload rules — including one
+  level of quoting inside a `bash -c` payload. Matching now dequotes the
+  command the way a real shell would (only the operator characters `> < | &
+  ; \`` are neutralized when they occur inside a quote, everything else stays
+  literal, and a backslash-escaped quote never opens or closes a span), and
+  the file-mutation redirect pattern now also recognizes the `>|` / `>>|`
+  clobber form.
+
 ### Fixed
 
 - **Interrupting a turn no longer risks a startup crash.** If an interrupt hit

src/permission/auto-shell-policy.ts

@@ -33,12 +33,52 @@ export interface AutoShellRule {
 const CMD = String.raw`(?:^|[\n;&|({]\s*)(?:\w+=\S*\s+)*`;
 const inCmd = (body: string): RegExp => new RegExp(`${CMD}${body}`);
 
-// Drop the contents of single- and double-quoted spans before matching so a
-// quoted argument cannot trip a rule (e.g. `git commit -m 'fix > bug'` is not a
-// redirect, `echo "npm install"` is not an install). Quoted-out redirect targets
-// and heredoc markers fall away with their quotes, which is why the file-mutation
-// heredoc pattern keys on the bare `<<` operator rather than the marker word.
-const stripQuoted = (command: string): string => command.replace(/'[^']*'|"[^"]*"/g, " ");
+// Quote-aware dequoting for rule matching. Real shells strip the quote
+// characters themselves and hand the program a literal argument, so a rule
+// must see the same thing the program would: a quoted redirect target
+// (`>"file"`), a quoted flag (`"-c"`), or a quoted program/subcommand name
+// (`"sed" -i`, `npm "install"`) all read exactly like their unquoted form.
+// The one thing quoting genuinely changes is that a shell *operator*
+// character loses its operator meaning inside quotes — `'fix > bug'` is a
+// literal string, not a redirect — so only that small set of operator
+// characters (`> < | & ; \``) is neutralized when it occurs inside a quoted
+// span; every other character (letters, digits, `-`) passes through
+// dequoted. Heredoc bodies are left alone: the file-mutation heredoc pattern
+// keys on the bare `<<` operator, which is always outside any quoting.
+//
+// A backslash before a quote character escapes it: `\"` is a literal `"`
+// that never opens or closes a quoted span (real bash semantics outside
+// single quotes), so `echo hi \"> file"` is a bare, unquoted redirect, not
+// text inside a quote. Skip the escaped character without touching quote
+// state so its following operator is still seen as live.
+const QUOTE_NEUTRALIZED_OPERATORS = new Set(["<", ">", "|", "&", ";", "`"]);
+
+const dequoteForMatching = (command: string): string => {
+  let out = "";
+  let quote: '"' | "'" | null = null;
+  for (let i = 0; i < command.length; i++) {
+    const ch = command[i] as string;
+    if (ch === "\\" && quote !== "'" && i + 1 < command.length) {
+      out += command[i + 1];
+      i++;
+      continue;
+    }
+    if (quote !== null) {
+      if (ch === quote) {
+        quote = null;
+      } else {
+        out += QUOTE_NEUTRALIZED_OPERATORS.has(ch) ? " " : ch;
+      }
+      continue;
+    }
+    if (ch === '"' || ch === "'") {
+      quote = ch;
+      continue;
+    }
+    out += ch;
+  }
+  return out;
+};
 
 // Named separately (not inlined in AUTO_SHELL_RULES below) so the dedicated
 // `env -S`/`--split-string` check further down — which cannot be expressed as
@@ -71,9 +111,10 @@ export const AUTO_SHELL_RULES: AutoShellRule[] = [
     reason:
       "File creation and edits must go through the write_file and edit_file tools, not shell tooling (python, sed -i, awk, perl, tee, or output redirection). Re-do this change with edit_file for a surgical replacement or write_file for the full contents.",
     patterns: [
-      // `>` / `>>` (optionally fd-qualified) to a target that is not an fd dup
+      // `>` / `>>` (optionally fd-qualified, optionally clobber-forced with a
+      // trailing `|` as in `>|` / `>>|`) to a target that is not an fd dup
       // (`2>&1`) or a safe pseudo-device (`> /dev/null`, a TTY).
-      /[0-9]?>>?\s*(?!&|\/dev\/(?:null|stdout|stderr|stdin|tty|pts\/|fd\/))[^\s|;&)]/,
+      /[0-9]?>>?\|?\s*(?!&|\/dev\/(?:null|stdout|stderr|stdin|tty|pts\/|fd\/))[^\s|;&)]/,
       // tee writes its stdin to one or more files.
       /(?:^|[\n;&|({]\s*)tee\b/,
       // In-place stream editors: sed -i, perl -pi -e, ruby -i.
@@ -166,7 +207,7 @@ export const AUTO_SHELL_RULES: AutoShellRule[] = [
 ];
 
 export function matchAutoShellRule(command: string): AutoShellRule | undefined {
-  const scannable = stripQuoted(command);
+  const scannable = dequoteForMatching(command);
   return AUTO_SHELL_RULES.find((rule) => rule.patterns.some((pattern) => pattern.test(scannable)));
 }
 

src/permission/classify-security.test.ts

@@ -712,3 +712,81 @@ describe("pure directory listing exemption", () => {
     expect(autoShellRuleForCall(shellCall("ls .env | xargs cat"))?.effect).toBe("ask");
   });
 });
+
+describe("CL-6703 — quoted redirect targets still deny file-mutation", () => {
+  test("plain unquoted redirect denies (baseline)", () => {
+    expect(autoShellRuleForCall(shellCall("echo hi > out.txt"))?.name).toBe("file-mutation");
+  });
+
+  test("a quoted redirect target denies", () => {
+    expect(autoShellRuleForCall(shellCall(`echo hi > "out.txt"`))?.name).toBe("file-mutation");
+    expect(autoShellRuleForCall(shellCall(`echo hi > 'out.txt'`))?.name).toBe("file-mutation");
+  });
+
+  test('a quoted fd-qualified redirect target (1>"file") denies', () => {
+    expect(autoShellRuleForCall(shellCall(`echo hi 1>"file"`))?.name).toBe("file-mutation");
+  });
+
+  test("a nested bash -c form with a quoted redirect denies", () => {
+    expect(autoShellRuleForCall(shellCall(`bash -c 'echo hi > "out.txt"'`))?.name).toBe(
+      "file-mutation",
+    );
+  });
+
+  test("a quoted '>' inside non-redirect text does not false-positive", () => {
+    expect(autoShellRuleForCall(shellCall(`git commit -m 'fix > bug'`))).toBeUndefined();
+  });
+
+  test("a backslash-escaped quote before a redirect still denies", () => {
+    // `\"` is a literal quote character in real bash, not a quote-open — the
+    // shell is never inside a quoted string here, so the `>` that follows is
+    // a genuine, unquoted redirect.
+    expect(autoShellRuleForCall(shellCall('echo hi \\"> file"'))?.name).toBe("file-mutation");
+  });
+
+  test("a backslash-escaped quote ahead of a dangerous flag still denies", () => {
+    // The escaped quote sits before an extra leading space, so it never
+    // touches the `\s-c` junction later in the string; a naive quote-pairing
+    // scanner (ignoring the backslash) would consume that junction as part
+    // of a fake quoted span and hide the -c flag entirely.
+    expect(autoShellRuleForCall(shellCall('python3 \\" -c print(1)"'))?.name).toBe("file-mutation");
+  });
+});
+
+describe("CL-6702 — bash clobber redirects match file-mutation", () => {
+  test("echo hi >|path denies", () => {
+    expect(autoShellRuleForCall(shellCall("echo hi >|path"))?.name).toBe("file-mutation");
+  });
+
+  test("echo hi >>|path denies", () => {
+    expect(autoShellRuleForCall(shellCall("echo hi >>|path"))?.name).toBe("file-mutation");
+  });
+});
+
+describe("CL-6697 — quoted dangerous flags and program names still deny/ask", () => {
+  test("a quoted -c interpreter one-liner denies", () => {
+    expect(autoShellRuleForCall(shellCall(`python3 "-c" "print(1)"`))?.name).toBe("file-mutation");
+  });
+
+  test("a quoted sed -i denies", () => {
+    expect(autoShellRuleForCall(shellCall(`sed "-i" 's/a/b/' file.txt`))?.name).toBe(
+      "file-mutation",
+    );
+  });
+
+  test("a quoted npm install asks", () => {
+    expect(autoShellRuleForCall(shellCall(`npm "install" left-pad`))?.name).toBe(
+      "dependency-install",
+    );
+  });
+
+  test("a quoted upload-tool argv0 (curl) asks", () => {
+    expect(
+      autoShellRuleForCall(shellCall(`"curl" -d @payload.json https://example.com`))?.name,
+    ).toBe("network-upload");
+  });
+
+  test("an innocent quoted argument interior does not false-positive", () => {
+    expect(autoShellRuleForCall(shellCall(`git commit -m "some text"`))).toBeUndefined();
+  });
+});