Skip to content

Commit 41672f1

Browse files
committed
Harden nested bash auto-shell peels
1 parent 4412ba3 commit 41672f1

2 files changed

Lines changed: 32 additions & 4 deletions

File tree

src/permission/classify-security.test.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -802,9 +802,24 @@ describe("CL-6988 — nested / escaped interpreter peels do not auto-allow", ()
802802
});
803803

804804
test("a double-nested alternating-quote bash -c redirect still denies", () => {
805+
expect(autoShellRuleForCall(shellCall(`bash -c "bash -c 'echo hi > out.txt'"`))?.name).toBe(
806+
"file-mutation",
807+
);
808+
});
809+
810+
test("bash -O/-o option values before -c do not hide dependency installs", () => {
811+
expect(autoShellRuleForCall(shellCall(`bash -O extglob -c 'npm install left-pad'`))?.name).toBe(
812+
"dependency-install",
813+
);
805814
expect(
806-
autoShellRuleForCall(shellCall(`bash -c "bash -c 'echo hi > out.txt'"`))?.name,
807-
).toBe("file-mutation");
815+
autoShellRuleForCall(shellCall(`bash -o pipefail -c 'npm install left-pad'`))?.name,
816+
).toBe("dependency-install");
817+
});
818+
819+
test("bash -c positional argv execution does not auto-allow dependency installs", () => {
820+
const cmd = `bash -c '$0 $1 $2' npm install left-pad`;
821+
expect(isAutoAllowedShellCall(shellCall(cmd))).toBe(false);
822+
expect(autoShellRuleForCall(shellCall(cmd))?.name).toBe("opaque-wrapper");
808823
});
809824

810825
test("an escaped triple-nested bash -c redirect does not auto-allow", () => {
@@ -820,10 +835,10 @@ describe("CL-6988 — nested / escaped interpreter peels do not auto-allow", ()
820835

821836
test("quote-broken deep nesting that degrades to a bare interpreter asks", () => {
822837
// Alternating quotes collide by depth 4 and peel used to land on bare `bash`.
823-
const deep = `bash -c "bash -c 'bash -c \"bash -c 'echo hi > out.txt'\"'"`;
838+
const deep = String.raw`bash -c "bash -c 'bash -c \"bash -c 'echo hi > out.txt'\"'"`;
824839
expect(isAutoAllowedShellCall(shellCall(deep))).toBe(false);
825840
const rule = autoShellRuleForCall(shellCall(deep));
826841
expect(rule).toBeDefined();
827-
expect(rule!.effect === "ask" || rule!.effect === "deny").toBe(true);
842+
expect(rule?.effect === "ask" || rule?.effect === "deny").toBe(true);
828843
});
829844
});

src/shell/run-shell-authz.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,17 +391,26 @@ function isBackslashInterpreterToken(token: string): boolean {
391391
return base.startsWith("\\") && SHELL_INTERPRETERS.has(base.slice(1));
392392
}
393393

394+
function shellPayloadReferencesPositional(payload: string): boolean {
395+
return /\$(?:[0-9@*#]|\{(?:[0-9]+|[@*#])\})/.test(payload);
396+
}
397+
394398
function nestedInterpreterPayloadOpaque(payload: string, rest: readonly string[]): boolean {
395399
if (isBackslashInterpreterToken(payload)) return true;
396400
const first = tokenize(payload)[0];
397401
if (first !== undefined && isBackslashInterpreterToken(first)) return true;
402+
// The payload can execute trailing argv through $0/$1/... substitution, so
403+
// the payload alone is not a faithful subject for dependency-install policy.
404+
if (rest.length > 0 && shellPayloadReferencesPositional(payload)) return true;
398405
// Trailing tokens after the -c payload: allow only plain positionals.
399406
// `-c`, redirects, backslashes, or flags mean the quoted body was split and
400407
// the truncated payload must not be trusted on its own under auto.
401408
if (rest.length > 0 && !rest.every(isSafeShellPositional)) return true;
402409
return false;
403410
}
404411

412+
const SHELL_SEPARATE_VALUE_FLAGS = new Set(["-O", "-o"]);
413+
405414
function peelShellDashC(tokens: string[], start: number): PeelOutcome {
406415
let i = start;
407416
while (i < tokens.length) {
@@ -424,6 +433,10 @@ function peelShellDashC(tokens: string[], start: number): PeelOutcome {
424433
if (nestedInterpreterPayloadOpaque(payload, [])) return { kind: "opaque" };
425434
return { kind: "inner", command: payload };
426435
}
436+
if (SHELL_SEPARATE_VALUE_FLAGS.has(t)) {
437+
i += 2;
438+
continue;
439+
}
427440
// Clustered short flags that include `c` (`-lc`, `-ic`, …): `c` takes the
428441
// next token as the command string, matching bash/sh/zsh.
429442
if (/^-[A-Za-z]*c[A-Za-z]*$/.test(t)) {

0 commit comments

Comments
 (0)