Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions src/permission/auto-shell-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import {
commandHasRecursiveRm,
expandShellSubjects,
} from "../shell/run-shell-authz.js";
import { commandReferencesSensitivePath } from "../plugins/secret-guard-plugin.js";
import {
inspectShellSecretReference,
shellSecretInspectionRequiresApproval,
} from "../plugins/secret-guard-plugin.js";
import {
commandHasUnboundedDirectoryListing,
commandTargetsRestricted,
Expand Down Expand Up @@ -510,7 +513,7 @@ export function autoShellRuleForCall(
const command = call.arguments.command;
if (typeof command !== "string") return undefined;

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

for (const subject of subjects) {
if (
commandReferencesSensitivePath(subject, cwd, isExtraDenied) !== undefined
)
return SENSITIVE_PATH_ASK_RULE;
const secretInspection = inspectShellSecretReference(
command,
cwd,
isExtraDenied,
);
if (
shellSecretInspectionRequiresApproval(secretInspection) &&
(secretInspection.reference !== undefined || !opaque)
) {
return SENSITIVE_PATH_ASK_RULE;
}

// Even inside the workspace: unbounded listing must ask so auto mode cannot OOM.
Expand Down
101 changes: 96 additions & 5 deletions src/permission/classify-security.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,92 @@ describe("isAutoAllowedShellCall — sensitive-path arguments", () => {
expect(isAutoAllowedShellCall(shellCall("cat .git-credentials"))).toBe(
false,
);
expect(
isAutoAllowedShellCall(
shellCall("env FILE=.envrc sh -c 'cat \"$FILE\"'"),
),
).toBe(false);
expect(isAutoAllowedShellCall(shellCall("sed -Enf.flaskenv input"))).toBe(
false,
);
expect(
isAutoAllowedShellCall(shellCall("sed --fil=.envrc input.txt")),
).toBe(false);
});

test("aliases, clusters, control prefixes, and ambiguity cannot auto-allow", () => {
for (const command of [
"egrep -Jf.envrc needle",
"grep -2f.flaskenv needle",
"sed -anf.envrc input.txt",
"{ awk -f.flaskenv input.txt; }",
"! grep -Tf.envrc needle",
"grep -Xf.envrc needle",
"grep -uf.envrc needle",
"cat $'.envrc'",
"bash -c \"cat \\$'.envrc'\"",
"bash -lc \"cat \\$'.envrc'\"",
"bash -lc \"cat \\$'.flaskenv'\"",
`bash -c "cat "'.envrc'`,
`sh -cc "cat "'.flaskenv'`,
"cat $'notes\\cQ'",
]) {
expect(isAutoAllowedShellCall(shellCall(command))).toBe(false);
}
});

test("still auto-allows reads of ordinary files", () => {
expect(isAutoAllowedShellCall(shellCall("cat src/index.ts"))).toBe(true);
expect(isAutoAllowedShellCall(shellCall("cat .env.example"))).toBe(true);
expect(isAutoAllowedShellCall(shellCall("echo grep --file=.envrc"))).toBe(
true,
);
expect(isAutoAllowedShellCall(shellCall("echo dd if=.flaskenv"))).toBe(
true,
);
});
});

describe("nested interpreter secret reads", () => {
const nestedSecrets = [
'fish -c "cat .envrc"',
'fish -c "cat .env"',
'busybox sh -c "cat .envrc"',
'csh -c "cat .envrc"',
'tcsh -c "cat .envrc"',
'pwsh -c "cat .envrc"',
];

test("does not auto-allow secret reads behind nested interpreters", () => {
for (const command of nestedSecrets) {
expect(isAutoAllowedShellCall(shellCall(command))).toBe(false);
expect(autoShellRuleForCall(shellCall(command))).toMatchObject({
name: "sensitive-path",
effect: "ask",
});
}
});

test("auto mode does not allow nested-interpreter secret reads", async () => {
for (const command of nestedSecrets) {
const gate = createPermissionGate({
approvals: [],
interactive: false,
skipPermissions: false,
reactorGated: false,
auto: true,
});
expect((await gate.evaluate(shellCall(command))).allowed).toBe(false);
}
});

test("templates behind nested interpreters stay unsensitive", () => {
expect(
autoShellRuleForCall(shellCall('fish -c "cat .env.example"'))?.name,
).not.toBe("sensitive-path");
expect(
autoShellRuleForCall(shellCall('busybox sh -c "cat .env.sample"'))?.name,
).not.toBe("sensitive-path");
});
});

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

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

test("auto mode asks for clustered bash secret reads", () => {
for (const command of [
"bash -lc \"cat \\$'.envrc'\"",
"bash -lc \"cat \\$'.flaskenv'\"",
]) {
expect(autoShellRuleForCall(shellCall(command))).toMatchObject({
name: "sensitive-path",
effect: "ask",
});
}
});

test("operator approval lets a sensitive-path shell command through the gate", async () => {
let asked = 0;
const gate = createPermissionGate({
Expand Down Expand Up @@ -484,7 +575,7 @@ describe("sensitive-path shell commands require approval, not a hard deny", () =
skipPermissions: false,
reactorGated: false,
});
const verdict = await gate.evaluate(shellCall("cat .env"));
const verdict = await gate.evaluate(shellCall("cat .flaskenv"));
expect(verdict.allowed).toBe(true);
expect(asked).toBe(1);
});
Expand All @@ -501,7 +592,7 @@ describe("sensitive-path shell commands require approval, not a hard deny", () =
skipPermissions: false,
reactorGated: false,
});
const verdict = await gate.evaluate(shellCall("cat README.md"));
const verdict = await gate.evaluate(shellCall("cat ordinary=.envrc"));
expect(verdict.allowed).toBe(true);
expect(asked).toBe(0);
});
Expand Down
17 changes: 14 additions & 3 deletions src/permission/classify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ import {
} from "../mcp/tool-name.js";
import type { McpToolPermissionRegistry } from "../mcp/tool-permissions.js";
import {
commandReferencesSensitivePath,
inspectShellSecretReference,
isSensitiveShellToken,
shellSecretInspectionRequiresApproval,
PURE_DIRECTORY_LISTING_PROGRAMS,
} from "../plugins/secret-guard-plugin.js";
import {
Expand Down Expand Up @@ -440,7 +441,12 @@ function isAutoAllowedSegment(
const trimmed = segment.trim();
if (trimmed.length === 0) return false;
if (isShellCommentOnly(trimmed) || isShellNoOp(trimmed)) return true;
if (commandReferencesSensitivePath(trimmed, cwd, isExtraDenied)) return false;
if (
shellSecretInspectionRequiresApproval(
inspectShellSecretReference(trimmed, cwd, isExtraDenied),
)
)
return false;
// Same metacharacter gate as isAutoAllowedShellCommand: this classifier also
// runs standalone per pipeline/chain segment (see isAutoAllowedShellSegment),
// so a segment carrying its own command substitution or redirect must not
Expand Down Expand Up @@ -502,7 +508,12 @@ export function isAutoAllowedShellCommand(
(isShellCommentOnly(trimmed) || isShellNoOp(trimmed))
)
return true;
if (commandReferencesSensitivePath(trimmed, cwd, isExtraDenied)) return false;
if (
shellSecretInspectionRequiresApproval(
inspectShellSecretReference(trimmed, cwd, isExtraDenied),
)
)
return false;
// Never auto-allow a command the authz layer would hard-deny at execution.
if (runShellAuthzBlockReason(trimmed) !== undefined) return false;
// Reject anything with metacharacters that compose or redirect (& ; < > ` $ etc).
Expand Down
26 changes: 21 additions & 5 deletions src/permission/critique-grep-file-env.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { describe, test, expect } from "bun:test";
import { isAutoAllowedShellCall } from "./classify.js";
import { commandReferencesSensitivePath } from "../plugins/secret-guard-plugin.js";
import { createPermissionGate } from "./gate.js";

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

test("commandReferencesSensitivePath still sees glued flag .env", () => {
expect(commandReferencesSensitivePath("grep --file=.env foo")).toBe(".env");
});

test("interactive gate must prompt for grep --file=.env, not auto-allow", async () => {
let asked = 0;
const gate = createPermissionGate({
Expand All @@ -38,6 +33,27 @@ describe("critique permission lane", () => {
expect(asked).toBe(1);
});

test("a stored grant cannot bypass an expanded sensitive operand", async () => {
let asked = 0;
const gate = createPermissionGate({
approvals: [{ tool: "run_shell", pattern: "*" }],
requestApproval: async () => {
asked++;
return { allow: false };
},
interactive: true,
skipPermissions: false,
reactorGated: false,
auto: false,
});

const verdict = await gate.evaluate(
shellCall("xargs grep --file=.envrc needle"),
);
expect(verdict.allowed).toBe(false);
expect(asked).toBe(1);
});

test("skipPermissions allows shell sensitive ref at gate", async () => {
const gate = createPermissionGate({
approvals: [],
Expand Down
Loading
Loading