Skip to content

Commit 3d26679

Browse files
committed
Treat unspaced & as a background operator unless redirect-bound
1 parent a61cd76 commit 3d26679

3 files changed

Lines changed: 82 additions & 11 deletions

File tree

src/permission/command.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,28 @@ describe("splitChainedCommand heredocs", () => {
5656
});
5757
});
5858

59+
describe("splitChainedCommand lone-& bypass (CL-7781)", () => {
60+
// A `&` with no trailing space still backgrounds the preceding command —
61+
// treating it as a redirect token lets a second command hide behind a
62+
// standing grant for the benign head. Only a redirect-bound `&` (after
63+
// `>`/`<`, or opening `&>`/`&>>`) stays attached to its command.
64+
const cases: { command: string; segments: string[] }[] = [
65+
{ command: "a &b", segments: ["a", "b"] },
66+
{ command: "a & b", segments: ["a", "b"] },
67+
{ command: "a &>f", segments: ["a &>f"] },
68+
{ command: "a 2>&1", segments: ["a 2>&1"] },
69+
{ command: "a >&2", segments: ["a >&2"] },
70+
{ command: "a <&-", segments: ["a <&-"] },
71+
{ command: "a&&b", segments: ["a", "b"] },
72+
{ command: "a &&b", segments: ["a", "b"] },
73+
];
74+
for (const { command, segments } of cases) {
75+
test(`splits ${JSON.stringify(command)} into ${segments.length} segment(s)`, () => {
76+
expect(splitChainedCommand(command)).toEqual(segments);
77+
});
78+
}
79+
});
80+
5981
describe("splitChainedCommand redirect and background fragments", () => {
6082
// A bare digit (or "-") after a chain separator is not, by itself, evidence
6183
// of a stray redirect remnant — it may be a genuine, distinct command. Only

src/permission/gate.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,40 @@ describe("preGrantGuardReason / isRequestCoveredByGrant guard parity", () => {
115115
});
116116
});
117117

118+
describe("lone-& bypass at the gate (CL-7781)", () => {
119+
// A standing grant for a benign head must not auto-allow a payload hidden
120+
// behind a `&` with no trailing space. Per-segment coverage means the
121+
// hidden second segment has no matching grant and the request stays
122+
// uncovered (the gate prompts) — same as the spaced form.
123+
const cwd = mkdtempSync(join(tmpdir(), "gate-lone-amp-"));
124+
const isRestricted = createPathRestriction(
125+
cwd,
126+
createWorktreeRootsProvider(cwd),
127+
).isRestricted;
128+
const workspace = { resolvedCwd: cwd, roots: [] as string[] };
129+
const grant: Approval = { tool: "run_shell", pattern: "bun test *" };
130+
const covered = (subject: string): boolean =>
131+
isRequestCoveredByGrant(
132+
{ tool: "run_shell", action: "Run", subject, scopes: [], cwd },
133+
grant,
134+
undefined,
135+
isRestricted,
136+
workspace,
137+
);
138+
139+
test("unspaced &payload is not covered by a grant for the head", () => {
140+
expect(covered("bun test x &touch pwn")).toBe(false);
141+
});
142+
143+
test("spaced & payload is not covered by a grant for the head", () => {
144+
expect(covered("bun test x & touch pwn")).toBe(false);
145+
});
146+
147+
test("the benign head alone stays covered", () => {
148+
expect(covered("bun test x")).toBe(true);
149+
});
150+
});
151+
118152
// Relative path tokens rebind to the request's process cwd before the gate's
119153
// restriction closure judges them, so a sub-agent worktree's relative targets
120154
// match what the shell will open. Absolute paths still pass through the

src/shell/command-segments.ts

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -113,12 +113,14 @@ export function splitChainedCommand(command: string): string[] {
113113
i++;
114114
continue;
115115
}
116-
// `&` participates in a redirect when it opens a bash combined redirect
117-
// (`&>file`) or duplicates a fd after `>`/`<` (`2>&1`, `<&-`). In those
118-
// positions it is not a background operator and must not split the chain —
119-
// otherwise `bun run build 2>&1` fragments into a real command and a stray
120-
// `1`, and the operator gets a separate approval prompt for "1".
121-
if (ch === "&" && isRedirectAmpersand(next)) {
116+
// `&` is redirect-bound only by its neighbours: a preceding `>`/`<`
117+
// (fd duplication: `2>&1`, `>&2`, `<&-`) or an immediately following `>`
118+
// (combined redirect: `&>file`, `&>>file`). Any other `&` — including one
119+
// with no trailing space (`a &b`) — is the background operator and must
120+
// split the chain; otherwise `bun run build 2>&1` fragments into a real
121+
// command and a stray `1`, and the operator gets a separate approval
122+
// prompt for "1".
123+
if (ch === "&" && isRedirectAmpersand(previousNonSpace(current), next)) {
122124
current += ch;
123125
continue;
124126
}
@@ -215,9 +217,22 @@ function unwrapGroup(segment: string): string | null {
215217
return null;
216218
}
217219

218-
// `&` is the background operator when it stands alone as a word — followed by
219-
// whitespace or end of input. Anywhere else it is part of a redirect token:
220-
// `2>&1`, `<&-`, `&>file`.
221-
function isRedirectAmpersand(next: string | undefined): boolean {
222-
return !(next === undefined || next === " " || next === "\t");
220+
// The closest non-space character already scanned into the current segment,
221+
// or undefined at the start of a segment. `&` consults this (not the
222+
// following character) to decide whether it is redirect-bound.
223+
function previousNonSpace(current: string): string | undefined {
224+
const trimmed = current.trimEnd();
225+
return trimmed.length > 0 ? trimmed[trimmed.length - 1] : undefined;
226+
}
227+
228+
// `&` is redirect-bound only when the previous non-space character is `>` or
229+
// `<` (fd duplication or close: `2>&1`, `>&2`, `<&-`), or when `&` is
230+
// immediately followed by `>` (combined redirect: `&>file`, `&>>file`).
231+
// Everything else is the background operator — a chain boundary.
232+
function isRedirectAmpersand(
233+
prev: string | undefined,
234+
next: string | undefined,
235+
): boolean {
236+
if (next === ">") return true;
237+
return prev === ">" || prev === "<";
223238
}

0 commit comments

Comments
 (0)