diff --git a/src/permission/command.test.ts b/src/permission/command.test.ts index da41e3492..68e94b465 100644 --- a/src/permission/command.test.ts +++ b/src/permission/command.test.ts @@ -56,6 +56,28 @@ describe("splitChainedCommand heredocs", () => { }); }); +describe("splitChainedCommand lone-& bypass (CL-7781)", () => { + // A `&` with no trailing space still backgrounds the preceding command — + // treating it as a redirect token lets a second command hide behind a + // standing grant for the benign head. Only a redirect-bound `&` (after + // `>`/`<`, or opening `&>`/`&>>`) stays attached to its command. + const cases: { command: string; segments: string[] }[] = [ + { command: "a &b", segments: ["a", "b"] }, + { command: "a & b", segments: ["a", "b"] }, + { command: "a &>f", segments: ["a &>f"] }, + { command: "a 2>&1", segments: ["a 2>&1"] }, + { command: "a >&2", segments: ["a >&2"] }, + { command: "a <&-", segments: ["a <&-"] }, + { command: "a&&b", segments: ["a", "b"] }, + { command: "a &&b", segments: ["a", "b"] }, + ]; + for (const { command, segments } of cases) { + test(`splits ${JSON.stringify(command)} into ${segments.length} segment(s)`, () => { + expect(splitChainedCommand(command)).toEqual(segments); + }); + } +}); + describe("splitChainedCommand redirect and background fragments", () => { // A bare digit (or "-") after a chain separator is not, by itself, evidence // of a stray redirect remnant — it may be a genuine, distinct command. Only diff --git a/src/permission/gate.test.ts b/src/permission/gate.test.ts index 1b25bd575..1bcb68637 100644 --- a/src/permission/gate.test.ts +++ b/src/permission/gate.test.ts @@ -115,6 +115,40 @@ describe("preGrantGuardReason / isRequestCoveredByGrant guard parity", () => { }); }); +describe("lone-& bypass at the gate (CL-7781)", () => { + // A standing grant for a benign head must not auto-allow a payload hidden + // behind a `&` with no trailing space. Per-segment coverage means the + // hidden second segment has no matching grant and the request stays + // uncovered (the gate prompts) — same as the spaced form. + const cwd = mkdtempSync(join(tmpdir(), "gate-lone-amp-")); + const isRestricted = createPathRestriction( + cwd, + createWorktreeRootsProvider(cwd), + ).isRestricted; + const workspace = { resolvedCwd: cwd, roots: [] as string[] }; + const grant: Approval = { tool: "run_shell", pattern: "bun test *" }; + const covered = (subject: string): boolean => + isRequestCoveredByGrant( + { tool: "run_shell", action: "Run", subject, scopes: [], cwd }, + grant, + undefined, + isRestricted, + workspace, + ); + + test("unspaced &payload is not covered by a grant for the head", () => { + expect(covered("bun test x &touch pwn")).toBe(false); + }); + + test("spaced & payload is not covered by a grant for the head", () => { + expect(covered("bun test x & touch pwn")).toBe(false); + }); + + test("the benign head alone stays covered", () => { + expect(covered("bun test x")).toBe(true); + }); +}); + // Relative path tokens rebind to the request's process cwd before the gate's // restriction closure judges them, so a sub-agent worktree's relative targets // match what the shell will open. Absolute paths still pass through the diff --git a/src/shell/command-segments.ts b/src/shell/command-segments.ts index a4ec34353..2939d2fb1 100644 --- a/src/shell/command-segments.ts +++ b/src/shell/command-segments.ts @@ -113,12 +113,14 @@ export function splitChainedCommand(command: string): string[] { i++; continue; } - // `&` participates in a redirect when it opens a bash combined redirect - // (`&>file`) or duplicates a fd after `>`/`<` (`2>&1`, `<&-`). In those - // positions it is not a background operator and must not split the chain — - // otherwise `bun run build 2>&1` fragments into a real command and a stray - // `1`, and the operator gets a separate approval prompt for "1". - if (ch === "&" && isRedirectAmpersand(next)) { + // `&` is redirect-bound only by its neighbours: a preceding `>`/`<` + // (fd duplication: `2>&1`, `>&2`, `<&-`) or an immediately following `>` + // (combined redirect: `&>file`, `&>>file`). Any other `&` — including one + // with no trailing space (`a &b`) — is the background operator and must + // split the chain; otherwise `bun run build 2>&1` fragments into a real + // command and a stray `1`, and the operator gets a separate approval + // prompt for "1". + if (ch === "&" && isRedirectAmpersand(previousNonSpace(current), next)) { current += ch; continue; } @@ -215,9 +217,22 @@ function unwrapGroup(segment: string): string | null { return null; } -// `&` is the background operator when it stands alone as a word — followed by -// whitespace or end of input. Anywhere else it is part of a redirect token: -// `2>&1`, `<&-`, `&>file`. -function isRedirectAmpersand(next: string | undefined): boolean { - return !(next === undefined || next === " " || next === "\t"); +// The closest non-space character already scanned into the current segment, +// or undefined at the start of a segment. `&` consults this (not the +// following character) to decide whether it is redirect-bound. +function previousNonSpace(current: string): string | undefined { + const trimmed = current.trimEnd(); + return trimmed.length > 0 ? trimmed[trimmed.length - 1] : undefined; +} + +// `&` is redirect-bound only when the previous non-space character is `>` or +// `<` (fd duplication or close: `2>&1`, `>&2`, `<&-`), or when `&` is +// immediately followed by `>` (combined redirect: `&>file`, `&>>file`). +// Everything else is the background operator — a chain boundary. +function isRedirectAmpersand( + prev: string | undefined, + next: string | undefined, +): boolean { + if (next === ">") return true; + return prev === ">" || prev === "<"; }