Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,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
Expand Down
59 changes: 50 additions & 9 deletions src/permission/auto-shell-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)));
}

Expand Down
78 changes: 78 additions & 0 deletions src/permission/classify-security.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
Loading