Skip to content

Commit bf1ac40

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 e86165f commit bf1ac40

9 files changed

Lines changed: 179 additions & 15 deletions

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

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

515-
// Peel bash/sh/zsh -c, xargs, env -S/--split-string, and transparent
515+
// Peel nested interpreters, xargs, env -S/--split-string, and transparent
516516
// prefixes so rules see the real payload. `stripQuoted` alone would delete
517517
// a quoted -c body and miss every rule. Content inside an -S payload is
518518
// 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 & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -477,10 +477,6 @@ export function inspectShellSecretReference(
477477
cwd: string = process.cwd(),
478478
dialect: ShellDialect = nativeShellDialect(process.platform),
479479
): ShellSecretInspection {
480-
if (dialect === "cmd") {
481-
return subjectReferencesSensitivePath(command, cwd, dialect);
482-
}
483-
484480
const expanded = expandShellSubjects(command);
485481
let opaque = expanded.opaque;
486482
for (const subject of expanded.subjects) {

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,4 +470,42 @@ describe("inspectShellSecretReference", () => {
470470
});
471471
}
472472
});
473+
474+
test("peels nested interpreters so quoted secret payloads are visible", () => {
475+
for (const command of [
476+
'fish -c "cat .envrc"',
477+
'fish -c "cat .env"',
478+
'busybox sh -c "cat .envrc"',
479+
'csh -c "cat .envrc"',
480+
'tcsh -c "cat .envrc"',
481+
'pwsh -c "cat .envrc"',
482+
]) {
483+
expect(inspectShellSecretReference(command)).toMatchObject({
484+
reference: expect.any(String),
485+
opaque: false,
486+
});
487+
}
488+
});
489+
490+
test("cmd dialect peels interpreter and cmd /c payloads", () => {
491+
expect(
492+
inspectShellSecretReference('bash -c "cat .envrc"', process.cwd(), "cmd"),
493+
).toMatchObject({ reference: ".envrc", opaque: false });
494+
expect(
495+
inspectShellSecretReference('cmd /c "type .envrc"', process.cwd(), "cmd"),
496+
).toMatchObject({ reference: ".envrc", opaque: false });
497+
});
498+
499+
test("keeps nested-interpreter template reads unsensitive", () => {
500+
for (const command of [
501+
'fish -c "cat .env.example"',
502+
'busybox sh -c "cat .env.template"',
503+
'bash -c "cat .env.sample"',
504+
]) {
505+
expect(inspectShellSecretReference(command)).toEqual({
506+
reference: undefined,
507+
opaque: false,
508+
});
509+
}
510+
});
473511
});

‎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
@@ -359,9 +359,24 @@ const ENV_ASSIGNMENT = /^\w+=/;
359359
const RM_WRAPPER = /^(sudo|command|env|exec|builtin|time|nice|nohup)$/;
360360
const RECURSIVE_FLAG = /^(--recursive|-[A-Za-z]*[rR][A-Za-z]*)$/;
361361

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

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

481516
function shellPayloadReferencesPositional(payload: string): boolean {
@@ -593,6 +628,7 @@ function peelShellDashC(
593628
tokens: string[],
594629
start: number,
595630
rawSegment: string,
631+
interpreter: string,
596632
): PeelOutcome {
597633
let i = start;
598634
while (i < tokens.length) {
@@ -602,7 +638,7 @@ function peelShellDashC(
602638
i++;
603639
break;
604640
}
605-
if (t === "-c" || t === "--command") {
641+
if (isInterpreterCommandSwitch(interpreter, t)) {
606642
const tokenPayload = tokens[i + 1];
607643
if (tokenPayload === undefined) return { kind: "opaque" };
608644
const optionOccurrence = tokens
@@ -955,7 +991,7 @@ function peelOnce(segment: string): PeelOutcome {
955991
const current = tokens[i];
956992
if (current === undefined)
957993
return strippedPrefix ? { kind: "opaque" } : { kind: "none" };
958-
const prog = programBasename(current);
994+
const prog = shellInterpreterName(current);
959995
if (SHELL_INTERPRETERS.has(prog)) {
960996
// A backtick or `$(` anywhere in the raw segment means the -c payload may
961997
// contain command substitution. tokenize() surfaces substitution content as
@@ -966,7 +1002,7 @@ function peelOnce(segment: string): PeelOutcome {
9661002
// wrapper as opaque rather than risk peeling a truncated, misleading payload.
9671003
if (segment.includes("`") || segment.includes("$("))
9681004
return { kind: "opaque" };
969-
const shellPeel = peelShellDashC(tokens, i + 1, segment);
1005+
const shellPeel = peelShellDashC(tokens, i + 1, segment, prog);
9701006
if (shellPeel.kind !== "none") return shellPeel;
9711007
// Interpreter without -c (e.g. `bash script.sh`) — not a peelable wrapper.
9721008
return { kind: "none" };
@@ -990,8 +1026,9 @@ export interface ShellExpandResult {
9901026
}
9911027

9921028
// Expand a shell command into subjects the auto-shell policy, hard-deny, and
993-
// recursive-rm checks should scan. Peels bash/sh/zsh/dash/ksh -c, xargs
994-
// utility tails, env -S/--split-string payloads, and transparent prefixes
1029+
// recursive-rm checks should scan. Peels nested interpreters (`bash`/`fish`/
1030+
// `cmd` `/c` and the rest of SHELL_INTERPRETERS), xargs utility tails, env
1031+
// -S/--split-string payloads, busybox applets, and transparent prefixes
9951032
// (env/nice/timeout/…), recursing with a depth cap so nested wrappers cannot
9961033
// hide a dangerous payload.
9971034
//

‎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)