Skip to content

Commit c7caf22

Browse files
fix(secret-guard): protect envrc and flaskenv files (#1167)
* fix(secret-guard): protect envrc and flaskenv files * 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 21e9a1a commit c7caf22

18 files changed

Lines changed: 1975 additions & 142 deletions

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

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ import {
33
commandHasRecursiveRm,
44
expandShellSubjects,
55
} from "../shell/run-shell-authz.js";
6-
import { commandReferencesSensitivePath } from "../plugins/secret-guard-plugin.js";
6+
import {
7+
inspectShellSecretReference,
8+
shellSecretInspectionRequiresApproval,
9+
} from "../plugins/secret-guard-plugin.js";
710
import {
811
commandHasUnboundedDirectoryListing,
912
commandTargetsRestricted,
@@ -510,7 +513,7 @@ export function autoShellRuleForCall(
510513
const command = call.arguments.command;
511514
if (typeof command !== "string") return undefined;
512515

513-
// Peel bash/sh/zsh -c, xargs, env -S/--split-string, and transparent
516+
// Peel nested interpreters, xargs, env -S/--split-string, and transparent
514517
// prefixes so rules see the real payload. `stripQuoted` alone would delete
515518
// a quoted -c body and miss every rule. Content inside an -S payload is
516519
// scanned here exactly as if written plainly — never a weaker tier.
@@ -533,11 +536,16 @@ export function autoShellRuleForCall(
533536
matched = preferRule(matched, ENV_ASSIGNMENT_ASK_RULE);
534537
}
535538

536-
for (const subject of subjects) {
537-
if (
538-
commandReferencesSensitivePath(subject, cwd, isExtraDenied) !== undefined
539-
)
540-
return SENSITIVE_PATH_ASK_RULE;
539+
const secretInspection = inspectShellSecretReference(
540+
command,
541+
cwd,
542+
isExtraDenied,
543+
);
544+
if (
545+
shellSecretInspectionRequiresApproval(secretInspection) &&
546+
(secretInspection.reference !== undefined || !opaque)
547+
) {
548+
return SENSITIVE_PATH_ASK_RULE;
541549
}
542550

543551
// Even inside the workspace: unbounded listing must ask so auto mode cannot OOM.

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

Lines changed: 96 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,92 @@ describe("isAutoAllowedShellCall — sensitive-path arguments", () => {
5959
expect(isAutoAllowedShellCall(shellCall("cat .git-credentials"))).toBe(
6060
false,
6161
);
62+
expect(
63+
isAutoAllowedShellCall(
64+
shellCall("env FILE=.envrc sh -c 'cat \"$FILE\"'"),
65+
),
66+
).toBe(false);
67+
expect(isAutoAllowedShellCall(shellCall("sed -Enf.flaskenv input"))).toBe(
68+
false,
69+
);
70+
expect(
71+
isAutoAllowedShellCall(shellCall("sed --fil=.envrc input.txt")),
72+
).toBe(false);
73+
});
74+
75+
test("aliases, clusters, control prefixes, and ambiguity cannot auto-allow", () => {
76+
for (const command of [
77+
"egrep -Jf.envrc needle",
78+
"grep -2f.flaskenv needle",
79+
"sed -anf.envrc input.txt",
80+
"{ awk -f.flaskenv input.txt; }",
81+
"! grep -Tf.envrc needle",
82+
"grep -Xf.envrc needle",
83+
"grep -uf.envrc needle",
84+
"cat $'.envrc'",
85+
"bash -c \"cat \\$'.envrc'\"",
86+
"bash -lc \"cat \\$'.envrc'\"",
87+
"bash -lc \"cat \\$'.flaskenv'\"",
88+
`bash -c "cat "'.envrc'`,
89+
`sh -cc "cat "'.flaskenv'`,
90+
"cat $'notes\\cQ'",
91+
]) {
92+
expect(isAutoAllowedShellCall(shellCall(command))).toBe(false);
93+
}
6294
});
6395

6496
test("still auto-allows reads of ordinary files", () => {
6597
expect(isAutoAllowedShellCall(shellCall("cat src/index.ts"))).toBe(true);
6698
expect(isAutoAllowedShellCall(shellCall("cat .env.example"))).toBe(true);
99+
expect(isAutoAllowedShellCall(shellCall("echo grep --file=.envrc"))).toBe(
100+
true,
101+
);
102+
expect(isAutoAllowedShellCall(shellCall("echo dd if=.flaskenv"))).toBe(
103+
true,
104+
);
105+
});
106+
});
107+
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");
67148
});
68149
});
69150

@@ -428,13 +509,23 @@ describe("sensitive-path shell commands require approval, not a hard deny", () =
428509
});
429510

430511
test("auto mode forces ask for shell commands that reference secret files", () => {
431-
const rule = autoShellRuleForCall(
432-
shellCall("bun --env-file=.env.staging run publish.ts"),
433-
);
512+
const rule = autoShellRuleForCall(shellCall("cat .envrc"));
434513
expect(rule?.name).toBe("sensitive-path");
435514
expect(rule?.effect).toBe("ask");
436515
});
437516

517+
test("auto mode asks for clustered bash secret reads", () => {
518+
for (const command of [
519+
"bash -lc \"cat \\$'.envrc'\"",
520+
"bash -lc \"cat \\$'.flaskenv'\"",
521+
]) {
522+
expect(autoShellRuleForCall(shellCall(command))).toMatchObject({
523+
name: "sensitive-path",
524+
effect: "ask",
525+
});
526+
}
527+
});
528+
438529
test("operator approval lets a sensitive-path shell command through the gate", async () => {
439530
let asked = 0;
440531
const gate = createPermissionGate({
@@ -484,7 +575,7 @@ describe("sensitive-path shell commands require approval, not a hard deny", () =
484575
skipPermissions: false,
485576
reactorGated: false,
486577
});
487-
const verdict = await gate.evaluate(shellCall("cat .env"));
578+
const verdict = await gate.evaluate(shellCall("cat .flaskenv"));
488579
expect(verdict.allowed).toBe(true);
489580
expect(asked).toBe(1);
490581
});
@@ -501,7 +592,7 @@ describe("sensitive-path shell commands require approval, not a hard deny", () =
501592
skipPermissions: false,
502593
reactorGated: false,
503594
});
504-
const verdict = await gate.evaluate(shellCall("cat README.md"));
595+
const verdict = await gate.evaluate(shellCall("cat ordinary=.envrc"));
505596
expect(verdict.allowed).toBe(true);
506597
expect(asked).toBe(0);
507598
});

‎src/permission/classify.ts‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ import {
1414
} from "../mcp/tool-name.js";
1515
import type { McpToolPermissionRegistry } from "../mcp/tool-permissions.js";
1616
import {
17-
commandReferencesSensitivePath,
17+
inspectShellSecretReference,
1818
isSensitiveShellToken,
19+
shellSecretInspectionRequiresApproval,
1920
PURE_DIRECTORY_LISTING_PROGRAMS,
2021
} from "../plugins/secret-guard-plugin.js";
2122
import {
@@ -440,7 +441,12 @@ function isAutoAllowedSegment(
440441
const trimmed = segment.trim();
441442
if (trimmed.length === 0) return false;
442443
if (isShellCommentOnly(trimmed) || isShellNoOp(trimmed)) return true;
443-
if (commandReferencesSensitivePath(trimmed, cwd, isExtraDenied)) return false;
444+
if (
445+
shellSecretInspectionRequiresApproval(
446+
inspectShellSecretReference(trimmed, cwd, isExtraDenied),
447+
)
448+
)
449+
return false;
444450
// Same metacharacter gate as isAutoAllowedShellCommand: this classifier also
445451
// runs standalone per pipeline/chain segment (see isAutoAllowedShellSegment),
446452
// so a segment carrying its own command substitution or redirect must not
@@ -502,7 +508,12 @@ export function isAutoAllowedShellCommand(
502508
(isShellCommentOnly(trimmed) || isShellNoOp(trimmed))
503509
)
504510
return true;
505-
if (commandReferencesSensitivePath(trimmed, cwd, isExtraDenied)) return false;
511+
if (
512+
shellSecretInspectionRequiresApproval(
513+
inspectShellSecretReference(trimmed, cwd, isExtraDenied),
514+
)
515+
)
516+
return false;
506517
// Never auto-allow a command the authz layer would hard-deny at execution.
507518
if (runShellAuthzBlockReason(trimmed) !== undefined) return false;
508519
// Reject anything with metacharacters that compose or redirect (& ; < > ` $ etc).

‎src/permission/critique-grep-file-env.test.ts‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { describe, test, expect } from "bun:test";
22
import { isAutoAllowedShellCall } from "./classify.js";
3-
import { commandReferencesSensitivePath } from "../plugins/secret-guard-plugin.js";
43
import { createPermissionGate } from "./gate.js";
54

65
const shellCall = (command: string) => ({
@@ -16,10 +15,6 @@ describe("critique permission lane", () => {
1615
).toBe(false);
1716
});
1817

19-
test("commandReferencesSensitivePath still sees glued flag .env", () => {
20-
expect(commandReferencesSensitivePath("grep --file=.env foo")).toBe(".env");
21-
});
22-
2318
test("interactive gate must prompt for grep --file=.env, not auto-allow", async () => {
2419
let asked = 0;
2520
const gate = createPermissionGate({
@@ -38,6 +33,27 @@ describe("critique permission lane", () => {
3833
expect(asked).toBe(1);
3934
});
4035

36+
test("a stored grant cannot bypass an expanded sensitive operand", async () => {
37+
let asked = 0;
38+
const gate = createPermissionGate({
39+
approvals: [{ tool: "run_shell", pattern: "*" }],
40+
requestApproval: async () => {
41+
asked++;
42+
return { allow: false };
43+
},
44+
interactive: true,
45+
skipPermissions: false,
46+
reactorGated: false,
47+
auto: false,
48+
});
49+
50+
const verdict = await gate.evaluate(
51+
shellCall("xargs grep --file=.envrc needle"),
52+
);
53+
expect(verdict.allowed).toBe(false);
54+
expect(asked).toBe(1);
55+
});
56+
4157
test("skipPermissions allows shell sensitive ref at gate", async () => {
4258
const gate = createPermissionGate({
4359
approvals: [],

0 commit comments

Comments
 (0)