Skip to content

Commit 588b094

Browse files
committed
fix(secret-guard): peel nested interpreters for secret reads
Quoted -c payloads behind fish, busybox, csh, pwsh, and cmd /c stayed one token, so auto mode default-allowed secret reads including .env.
1 parent d4f2ac5 commit 588b094

9 files changed

Lines changed: 190 additions & 21 deletions

‎src/permission/auto-shell-policy.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ export function autoShellRuleForCall(
513513
const command = call.arguments.command;
514514
if (typeof command !== "string") return undefined;
515515

516-
// Peel bash/sh/zsh -c, xargs, env -S/--split-string, and transparent
516+
// Peel nested interpreters, xargs, env -S/--split-string, and transparent
517517
// prefixes so rules see the real payload. `stripQuoted` alone would delete
518518
// a quoted -c body and miss every rule. Content inside an -S payload is
519519
// scanned here exactly as if written plainly — never a weaker tier.

‎src/permission/classify-security.test.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,49 @@ describe("isAutoAllowedShellCall — sensitive-path arguments", () => {
105105
});
106106
});
107107

108+
describe("nested interpreter secret reads", () => {
109+
const nestedSecrets = [
110+
'fish -c "cat .envrc"',
111+
'fish -c "cat .env"',
112+
'busybox sh -c "cat .envrc"',
113+
'csh -c "cat .envrc"',
114+
'tcsh -c "cat .envrc"',
115+
'pwsh -c "cat .envrc"',
116+
];
117+
118+
test("does not auto-allow secret reads behind nested interpreters", () => {
119+
for (const command of nestedSecrets) {
120+
expect(isAutoAllowedShellCall(shellCall(command))).toBe(false);
121+
expect(autoShellRuleForCall(shellCall(command))).toMatchObject({
122+
name: "sensitive-path",
123+
effect: "ask",
124+
});
125+
}
126+
});
127+
128+
test("auto mode does not allow nested-interpreter secret reads", async () => {
129+
for (const command of nestedSecrets) {
130+
const gate = createPermissionGate({
131+
approvals: [],
132+
interactive: false,
133+
skipPermissions: false,
134+
reactorGated: false,
135+
auto: true,
136+
});
137+
expect((await gate.evaluate(shellCall(command))).allowed).toBe(false);
138+
}
139+
});
140+
141+
test("templates behind nested interpreters stay unsensitive", () => {
142+
expect(
143+
autoShellRuleForCall(shellCall('fish -c "cat .env.example"'))?.name,
144+
).not.toBe("sensitive-path");
145+
expect(
146+
autoShellRuleForCall(shellCall('busybox sh -c "cat .env.sample"'))?.name,
147+
).not.toBe("sensitive-path");
148+
});
149+
});
150+
108151
describe("clustered shell command options", () => {
109152
test("classifies clustered command payloads like canonical command payloads", () => {
110153
for (const options of ["-c", "-lc", "-xec", "-cc", "-cache"]) {

‎src/permission/gate.test.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,12 @@ describe("expanded secret wrapper guards", () => {
146146
"ksh -Gc \"cat \\$'.envrc'\"",
147147
`bash -c "cat "'.envrc'`,
148148
`sh -cc "cat "'.flaskenv'`,
149+
'fish -c "cat .envrc"',
150+
'fish -c "cat .env"',
151+
'busybox sh -c "cat .envrc"',
152+
'csh -c "cat .envrc"',
153+
'tcsh -c "cat .envrc"',
154+
'pwsh -c "cat .envrc"',
149155
"cat $'notes\\cQ'",
150156
]) {
151157
const gate = createPermissionGate({
@@ -162,6 +168,23 @@ describe("expanded secret wrapper guards", () => {
162168
}
163169
});
164170

171+
test("star grants do not cover nested-interpreter secret reads", async () => {
172+
for (const command of [
173+
'fish -c "cat .envrc"',
174+
'fish -c "cat .env"',
175+
'busybox sh -c "cat .envrc"',
176+
]) {
177+
const gate = createPermissionGate({
178+
approvals: [{ tool: "run_shell", pattern: "*" }],
179+
interactive: false,
180+
skipPermissions: false,
181+
reactorGated: false,
182+
auto: true,
183+
});
184+
expect((await gate.evaluate(shellCall(command))).allowed).toBe(false);
185+
}
186+
});
187+
165188
test("ambiguous file-option clusters cannot use a broad grant", async () => {
166189
const gate = createPermissionGate({
167190
approvals: [{ tool: "run_shell", pattern: "*" }],

‎src/plugins/secret-guard-plugin.ts‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -542,15 +542,6 @@ export function inspectShellSecretReference(
542542
dialect: ShellDialect = nativeShellDialect(process.platform),
543543
): ShellSecretInspection {
544544
const resolvedCwd = cwd ?? process.cwd();
545-
if (dialect === "cmd") {
546-
return subjectReferencesSensitivePath(
547-
command,
548-
resolvedCwd,
549-
dialect,
550-
isExtraDenied,
551-
);
552-
}
553-
554545
const expanded = expandShellSubjects(command);
555546
let opaque = expanded.opaque;
556547
for (const subject of expanded.subjects) {

‎src/shell/literal-path-arguments.test.ts‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ const classificationCases: Record<
180180
"/usr/bin/env FILE=.flaskenv sh -c 'cat \"$FILE\"'",
181181
"env -i FILE=.envrc sh -c 'cat \"$FILE\"'",
182182
"command env FILE=.flaskenv sh -c 'cat \"$FILE\"'",
183+
"/tmp/env FILE=.envrc echo ok",
183184
"grep --file=.envrc needle",
184185
"env grep --file=.envrc needle",
185186
"env -i grep --file=.flaskenv needle",
@@ -250,7 +251,6 @@ const classificationCases: Record<
250251
"awk -Ff.envrc input.txt",
251252
"echo dd if=.flaskenv",
252253
"echo env FILE=.envrc",
253-
"/tmp/env FILE=.envrc echo ok",
254254
"echo 'env grep --file=.envrc'",
255255
"printf '%s' 'nice -n 5 grep --file=.envrc'",
256256
"echo '`cat .envrc`'",
@@ -480,4 +480,52 @@ describe("inspectShellSecretReference", () => {
480480
});
481481
}
482482
});
483+
484+
test("peels nested interpreters so quoted secret payloads are visible", () => {
485+
for (const command of [
486+
'fish -c "cat .envrc"',
487+
'fish -c "cat .env"',
488+
'busybox sh -c "cat .envrc"',
489+
'csh -c "cat .envrc"',
490+
'tcsh -c "cat .envrc"',
491+
'pwsh -c "cat .envrc"',
492+
]) {
493+
expect(inspectShellSecretReference(command)).toMatchObject({
494+
reference: expect.any(String),
495+
opaque: false,
496+
});
497+
}
498+
});
499+
500+
test("cmd dialect peels interpreter and cmd /c payloads", () => {
501+
expect(
502+
inspectShellSecretReference(
503+
'bash -c "cat .envrc"',
504+
process.cwd(),
505+
() => false,
506+
"cmd",
507+
),
508+
).toMatchObject({ reference: ".envrc", opaque: false });
509+
expect(
510+
inspectShellSecretReference(
511+
'cmd /c "type .envrc"',
512+
process.cwd(),
513+
() => false,
514+
"cmd",
515+
),
516+
).toMatchObject({ reference: ".envrc", opaque: false });
517+
});
518+
519+
test("keeps nested-interpreter template reads unsensitive", () => {
520+
for (const command of [
521+
'fish -c "cat .env.example"',
522+
'busybox sh -c "cat .env.template"',
523+
'bash -c "cat .env.sample"',
524+
]) {
525+
expect(inspectShellSecretReference(command)).toEqual({
526+
reference: undefined,
527+
opaque: false,
528+
});
529+
}
530+
});
483531
});

‎src/shell/run-shell-authz.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,24 @@ describe("clustered shell command options", () => {
194194
}
195195
});
196196

197+
test("peels nested interpreters including busybox applets and cmd /c", () => {
198+
expect(expandShellSubjects('fish -c "cat .envrc"').subjects).toContain(
199+
"cat .envrc",
200+
);
201+
expect(
202+
expandShellSubjects('busybox sh -c "cat .envrc"').subjects,
203+
).toContain("cat .envrc");
204+
expect(expandShellSubjects('csh -c "cat .envrc"').subjects).toContain(
205+
"cat .envrc",
206+
);
207+
expect(expandShellSubjects('pwsh -c "cat .envrc"').subjects).toContain(
208+
"cat .envrc",
209+
);
210+
expect(expandShellSubjects('cmd /c "type .envrc"').subjects).toContain(
211+
"type .envrc",
212+
);
213+
});
214+
197215
test("hard-denies complete adjacent-fragment payloads", () => {
198216
for (const command of [
199217
`bash -c "rm "'-rf /'`,

‎src/shell/run-shell-authz.ts‎

Lines changed: 46 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -360,9 +360,24 @@ const ENV_ASSIGNMENT = /^\w+=/;
360360
const RM_WRAPPER = /^(sudo|command|env|exec|builtin|time|nice|nohup)$/;
361361
const RECURSIVE_FLAG = /^(--recursive|-[A-Za-z]*[rR][A-Za-z]*)$/;
362362

363-
// Interpreters whose `-c` / `--command` payload is an independent shell subject.
364-
// Exported so tests and callers share one explicit list with the peeler.
365-
export const SHELL_INTERPRETERS = new Set(["bash", "sh", "zsh", "dash", "ksh"]);
363+
// Interpreters whose `-c` / `--command` / cmd `/c` payload is an independent
364+
// shell subject. Exported so tests and callers share one explicit list with
365+
// the peeler. Matching is basename-based and ignores Windows executable
366+
// suffixes (`cmd.exe` → `cmd`).
367+
export const SHELL_INTERPRETERS = new Set([
368+
"bash",
369+
"sh",
370+
"zsh",
371+
"dash",
372+
"ksh",
373+
"ash",
374+
"fish",
375+
"csh",
376+
"tcsh",
377+
"pwsh",
378+
"powershell",
379+
"cmd",
380+
]);
366381
// Max recursive peel depth for nested wrappers. Exported so the depth cap is a
367382
// named policy knob tests can assert against, not a magic number.
368383
export const MAX_PEEL_DEPTH = 4;
@@ -473,10 +488,30 @@ function isSafeShellPositional(token: string): boolean {
473488
return SAFE_REJOIN_TOKEN.test(token);
474489
}
475490

491+
const INTERPRETER_SUFFIX = /\.(?:exe|cmd|com|bat)$/i;
492+
const CMD_INTERPRETERS = new Set(["cmd"]);
493+
const PWSH_INTERPRETERS = new Set(["pwsh", "powershell"]);
494+
495+
function shellInterpreterName(token: string): string {
496+
return programBasename(token).replace(INTERPRETER_SUFFIX, "").toLowerCase();
497+
}
498+
499+
function isInterpreterCommandSwitch(
500+
interpreter: string,
501+
token: string,
502+
): boolean {
503+
if (token === "-c" || token === "--command") return true;
504+
if (CMD_INTERPRETERS.has(interpreter) && /^\/[ck]$/i.test(token)) return true;
505+
if (PWSH_INTERPRETERS.has(interpreter) && /^-command$/i.test(token))
506+
return true;
507+
return false;
508+
}
509+
476510
// `\bash` / `\sh` — tokenize artifact from peeling through an escaped quote.
477511
function isBackslashInterpreterToken(token: string): boolean {
478512
const base = programBasename(token);
479-
return base.startsWith("\\") && SHELL_INTERPRETERS.has(base.slice(1));
513+
if (!base.startsWith("\\")) return false;
514+
return SHELL_INTERPRETERS.has(shellInterpreterName(base.slice(1)));
480515
}
481516

482517
function shellPayloadReferencesPositional(payload: string): boolean {
@@ -594,6 +629,7 @@ function peelShellDashC(
594629
tokens: string[],
595630
start: number,
596631
rawSegment: string,
632+
interpreter: string,
597633
): PeelOutcome {
598634
let i = start;
599635
while (i < tokens.length) {
@@ -603,7 +639,7 @@ function peelShellDashC(
603639
i++;
604640
break;
605641
}
606-
if (t === "-c" || t === "--command") {
642+
if (isInterpreterCommandSwitch(interpreter, t)) {
607643
const tokenPayload = tokens[i + 1];
608644
if (tokenPayload === undefined) return { kind: "opaque" };
609645
const optionOccurrence = tokens
@@ -935,7 +971,7 @@ function peelOnce(segment: string): PeelOutcome {
935971
const current = tokens[i];
936972
if (current === undefined)
937973
return strippedPrefix ? { kind: "opaque" } : { kind: "none" };
938-
const prog = programBasename(current);
974+
const prog = shellInterpreterName(current);
939975
if (SHELL_INTERPRETERS.has(prog)) {
940976
// A backtick or `$(` anywhere in the raw segment means the -c payload may
941977
// contain command substitution. tokenize() surfaces substitution content as
@@ -946,7 +982,7 @@ function peelOnce(segment: string): PeelOutcome {
946982
// wrapper as opaque rather than risk peeling a truncated, misleading payload.
947983
if (segment.includes("`") || segment.includes("$("))
948984
return { kind: "opaque" };
949-
const shellPeel = peelShellDashC(tokens, i + 1, segment);
985+
const shellPeel = peelShellDashC(tokens, i + 1, segment, prog);
950986
if (shellPeel.kind !== "none") return shellPeel;
951987
// Interpreter without -c (e.g. `bash script.sh`) — not a peelable wrapper.
952988
return { kind: "none" };
@@ -979,8 +1015,9 @@ export interface ShellExpandResult {
9791015
}
9801016

9811017
// Expand a shell command into subjects the auto-shell policy, hard-deny, and
982-
// recursive-rm checks should scan. Peels bash/sh/zsh/dash/ksh -c, xargs
983-
// utility tails, env -S/--split-string payloads, and transparent prefixes
1018+
// recursive-rm checks should scan. Peels nested interpreters (`bash`/`fish`/
1019+
// `cmd` `/c` and the rest of SHELL_INTERPRETERS), xargs utility tails, env
1020+
// -S/--split-string payloads, busybox applets, and transparent prefixes
9841021
// (env/nice/timeout/…), recursing with a depth cap so nested wrappers cannot
9851022
// hide a dangerous payload.
9861023
//

‎src/shell/transparent-command.test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,12 @@ describe("peelTransparentCommand", () => {
166166
executableIndex: 2,
167167
wrapperIndexes: [0],
168168
},
169+
{
170+
name: "peels busybox so the applet is the executable",
171+
tokens: ["busybox", "sh", "-c", "cat .envrc"],
172+
executableIndex: 1,
173+
wrapperIndexes: [0],
174+
},
169175
];
170176

171177
for (const entry of cases) {

‎src/shell/transparent-command.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,10 @@ export function peelTransparentCommand(
391391
index = parsed.executableIndex;
392392
continue;
393393
}
394-
if (["builtin", "nohup"].includes(program)) {
394+
if (
395+
["builtin", "nohup", "busybox"].includes(program) ||
396+
program.toLowerCase() === "busybox.exe"
397+
) {
395398
wrapperIndexes.push(index);
396399
index++;
397400
continue;

0 commit comments

Comments
 (0)