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
22 changes: 22 additions & 0 deletions src/permission/command.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 34 additions & 0 deletions src/permission/gate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 26 additions & 11 deletions src/shell/command-segments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 === "<";
}
Loading