Skip to content

Commit 547e851

Browse files
committed
Resolve symlinks before the shell secret denylist
A benign-named symlink into a secret file asks exactly like the secret name itself, while pure name-listings still list freely.
1 parent fcc150e commit 547e851

6 files changed

Lines changed: 181 additions & 15 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ export function autoShellRuleForCall(
523523
}
524524

525525
for (const subject of subjects) {
526-
if (commandReferencesSensitivePath(subject) !== undefined)
526+
if (commandReferencesSensitivePath(subject, cwd) !== undefined)
527527
return SENSITIVE_PATH_ASK_RULE;
528528
}
529529

‎src/permission/classify.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ import {
1515
import type { McpToolPermissionRegistry } from "../mcp/tool-permissions.js";
1616
import {
1717
commandReferencesSensitivePath,
18-
isSensitivePath,
18+
isSensitiveShellToken,
19+
PURE_DIRECTORY_LISTING_PROGRAMS,
1920
} from "../plugins/secret-guard-plugin.js";
2021
import {
2122
runShellAuthzBlockReason,
@@ -113,10 +114,10 @@ export function restrictedPathArg(
113114
return isRestricted(path, isWriteTool(call.name)) ? path : undefined;
114115
}
115116

116-
// Programs that only print directory names / metadata. Outside-workspace path
117-
// arguments are fine for these — listing is not a content read. Content readers
118-
// (cat, head, xxd, …) still fail the restricted-path check below.
119-
const PURE_DIRECTORY_LISTING_PROGRAMS = new Set(["ls", "tree"]);
117+
// Outside-workspace path arguments are fine for pure-listing programs — listing
118+
// is not a content read. Content readers (cat, head, xxd, …) still fail the
119+
// restricted-path check below. The program set itself is owned by
120+
// secret-guard-plugin.ts (shared with the CL-7790 resolve-leg skip).
120121

121122
// Cap accepted tree depth so `tree -L 999999 /` cannot auto-allow an OOM walk.
122123
const MAX_PURE_TREE_DEPTH = 10;
@@ -405,7 +406,7 @@ function isAutoAllowedSegment(
405406
const trimmed = segment.trim();
406407
if (trimmed.length === 0) return false;
407408
if (isShellCommentOnly(trimmed) || isShellNoOp(trimmed)) return true;
408-
if (commandReferencesSensitivePath(trimmed)) return false;
409+
if (commandReferencesSensitivePath(trimmed, cwd)) return false;
409410
// Same metacharacter gate as isAutoAllowedShellCommand: this classifier also
410411
// runs standalone per pipeline/chain segment (see isAutoAllowedShellSegment),
411412
// so a segment carrying its own command substitution or redirect must not
@@ -429,7 +430,12 @@ function isAutoAllowedSegment(
429430
if (args.some((token) => WRITE_FLAG.test(token))) return false;
430431
if (args.some((token) => EXEC_FLAG.test(token))) return false;
431432
}
432-
if (args.some((token) => isSensitivePath(token))) return false;
433+
// CL-7790: resolve symlinks before the secret denylist — a benign-named
434+
// symlink into a secret file (notes.txt -> .env) asks exactly like the
435+
// secret name itself. Pure name-listings skip the resolve leg: `ls
436+
// notes.txt` lists freely (CL-5420), and an impure listing fails above.
437+
if (args.some((token) => isSensitiveShellToken(token, cwd, !pureListing)))
438+
return false;
433439
// Pure directory listing may target outside-workspace paths (names only).
434440
// Content readers must stay inside the workspace.
435441
if (
@@ -457,7 +463,7 @@ export function isAutoAllowedShellCommand(
457463
(isShellCommentOnly(trimmed) || isShellNoOp(trimmed))
458464
)
459465
return true;
460-
if (commandReferencesSensitivePath(trimmed)) return false;
466+
if (commandReferencesSensitivePath(trimmed, cwd)) return false;
461467
// Never auto-allow a command the authz layer would hard-deny at execution.
462468
if (runShellAuthzBlockReason(trimmed) !== undefined) return false;
463469
// Reject anything with metacharacters that compose or redirect (& ; < > ` $ etc).

‎src/permission/gate.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ function segmentGuard(
131131
cwd?: string,
132132
rootsProvider?: RootsProvider,
133133
): SegmentGuard | undefined {
134-
if (commandReferencesSensitivePath(segment) !== undefined)
134+
if (commandReferencesSensitivePath(segment, cwd) !== undefined)
135135
return { kind: "secret" };
136136
if (
137137
cwd !== undefined &&
@@ -654,7 +654,7 @@ export function createPermissionGate(
654654
// segment mentions a secret path.
655655
const shellReferencesSecret =
656656
shellCmd !== undefined &&
657-
commandReferencesSensitivePath(shellCmd) !== undefined;
657+
commandReferencesSensitivePath(shellCmd, effectiveCwd) !== undefined;
658658
if (!restricted && classifyTool(call.name, mcpTiers) === "allow") {
659659
return { kind: "allow" };
660660
}
@@ -949,7 +949,8 @@ export function createPermissionGate(
949949
) => {
950950
const anySecret =
951951
request.tool === "run_shell" &&
952-
commandReferencesSensitivePath(request.subject) !== undefined;
952+
commandReferencesSensitivePath(request.subject, request.cwd) !==
953+
undefined;
953954
return resolveInteractiveAsk(
954955
{
955956
kind: "ask",

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

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { isAbsolute } from "node:path";
1+
import { isAbsolute, resolve as resolvePath } from "node:path";
22
import type { ToolPlugin } from "@intx/tools-posix";
33
import {
44
realpathNearestOr,
@@ -135,11 +135,68 @@ function shellPathTokens(command: string): string[] {
135135
// name at runtime. Perfect shell sandboxing is out of scope; the goal is to
136136
// force a prompt for the trivial, single-token references that make exfiltration
137137
// easy. Tool-result secret scrub still redacts credential-shaped output.
138+
// Programs that only print directory names / metadata — listing a name never
139+
// dumps file contents. Single owner for this set: the resolve-leg skip below
140+
// and classify.ts's pure-listing exemption both read it, so a new names-only
141+
// program cannot drift into one list without the other.
142+
export const PURE_DIRECTORY_LISTING_PROGRAMS = new Set(["ls", "tree"]);
143+
144+
// Worth spending a realpath on: shaped like a path the shell could open
145+
// (a slash, an extension dot, or absolute), not a flag, variable, glob, or
146+
// fd number — those can never resolve into a secret file, so they skip the
147+
// stat and the hot auto-allow path stays syscall-free for them.
148+
function isPathLikeShellToken(token: string): boolean {
149+
if (
150+
token.startsWith("-") ||
151+
token.includes("$") ||
152+
token.includes("*") ||
153+
token.includes("`")
154+
)
155+
return false;
156+
return (
157+
isAbsolute(token) ||
158+
token.includes("/") ||
159+
token.includes("\\") ||
160+
token.includes(".")
161+
);
162+
}
163+
164+
// CL-7790: the ONE shell-token matcher both secret-guard call sites share —
165+
// commandReferencesSensitivePath below and classify.ts's per-arg sensitive
166+
// check. The cheap lexical denylist runs first so the hot auto-allow path
167+
// never touches the filesystem; only path-like survivors pay for a realpath
168+
// via the CL-6971 helper, which catches a benign-named symlink into a secret
169+
// file (notes.txt -> .env) exactly like the secret name itself. Relative
170+
// tokens resolve against cwd first because the helper takes absolute paths.
171+
// Pass resolveSymlinks=false for pure name-listings: listing a name is not
172+
// dumping its contents (CL-5420), so `ls notes.txt` still lists freely while
173+
// `cat notes.txt` asks.
174+
export function isSensitiveShellToken(
175+
token: string,
176+
cwd: string = process.cwd(),
177+
resolveSymlinks = true,
178+
): boolean {
179+
if (isSensitivePath(token)) return true;
180+
if (!resolveSymlinks || !isPathLikeShellToken(token)) return false;
181+
if (isAbsolute(token)) return isSensitivePathResolved(token);
182+
return isSensitivePathResolved(resolvePath(cwd, token));
183+
}
184+
138185
export function commandReferencesSensitivePath(
139186
command: string,
187+
cwd: string = process.cwd(),
140188
): string | undefined {
141-
for (const token of shellPathTokens(command)) {
142-
if (isSensitivePath(token)) return token;
189+
const tokens = shellPathTokens(command);
190+
// Dump vs list: a lone name-listing never dumps file contents, so only the
191+
// cheap lexical leg applies and `ls notes.txt` still lists freely. Anything
192+
// composed (pipes, chains, redirects, subshells) takes the resolve leg —
193+
// `ls && cat notes.txt` must not ride the listing exemption.
194+
const program = tokens[0] ?? "";
195+
const listingOnly =
196+
PURE_DIRECTORY_LISTING_PROGRAMS.has(program) &&
197+
!/[;&|()<>\n]/.test(command);
198+
for (const token of tokens) {
199+
if (isSensitiveShellToken(token, cwd, !listingOnly)) return token;
143200
}
144201
return undefined;
145202
}
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { mkdtemp, rm, symlink, writeFile } from "node:fs/promises";
3+
import { tmpdir } from "node:os";
4+
import { join } from "node:path";
5+
import {
6+
commandReferencesSensitivePath,
7+
isSensitiveShellToken,
8+
} from "./secret-guard-plugin.js";
9+
import { isAutoAllowedShellCommand } from "../permission/classify.js";
10+
import { autoShellRuleForCall } from "../permission/auto-shell-policy.js";
11+
12+
/**
13+
* CL-7790: shell token matching ignores symlinks. A benign-named symlink
14+
* into a secret file (notes.txt -> .env) must not auto-allow a content dump
15+
* (`cat notes.txt`), identically to the direct name (`cat .env`). Pure
16+
* name-listing (`ls notes.txt`) still lists freely — dumping contents is the
17+
* threat, listing a name is not (dump-vs-list distinction, CL-5420).
18+
*/
19+
20+
async function withFixture<T>(
21+
run: (paths: { cwd: string }) => Promise<T>,
22+
): Promise<T> {
23+
const cwd = await mkdtemp(join(tmpdir(), "cl7790-shell-symlink-"));
24+
try {
25+
await writeFile(join(cwd, ".env"), "SECRET=fixture-env\n");
26+
await writeFile(join(cwd, "README.md"), "# fixture\n");
27+
await symlink(join(cwd, ".env"), join(cwd, "notes.txt"));
28+
return await run({ cwd });
29+
} finally {
30+
await rm(cwd, { recursive: true, force: true });
31+
}
32+
}
33+
34+
const shellCall = (command: string) => ({
35+
id: "c",
36+
name: "run_shell",
37+
arguments: { command },
38+
});
39+
40+
describe("CL-7790 shell tokens resolve symlinks before the secret denylist", () => {
41+
test("cat through a benign-named symlink does not auto-allow", async () => {
42+
await withFixture(async ({ cwd }) => {
43+
expect(isAutoAllowedShellCommand("cat notes.txt", cwd)).toBe(false);
44+
});
45+
});
46+
47+
test("cat of the direct secret name still does not auto-allow", async () => {
48+
await withFixture(async ({ cwd }) => {
49+
expect(isAutoAllowedShellCommand("cat .env", cwd)).toBe(false);
50+
});
51+
});
52+
53+
test("pure listing of the symlink still lists freely", async () => {
54+
await withFixture(async ({ cwd }) => {
55+
expect(isAutoAllowedShellCommand("ls notes.txt", cwd)).toBe(true);
56+
expect(isAutoAllowedShellCommand("ls -la", cwd)).toBe(true);
57+
});
58+
});
59+
60+
test("pure listing of the direct secret name still asks (CL-5420)", async () => {
61+
await withFixture(async ({ cwd }) => {
62+
expect(isAutoAllowedShellCommand("ls .env", cwd)).toBe(false);
63+
});
64+
});
65+
66+
test("plugin flags the symlinked dump but not the listing", async () => {
67+
await withFixture(async ({ cwd }) => {
68+
expect(commandReferencesSensitivePath("cat notes.txt", cwd)).toBe(
69+
"notes.txt",
70+
);
71+
expect(
72+
commandReferencesSensitivePath("ls notes.txt", cwd),
73+
).toBeUndefined();
74+
});
75+
});
76+
77+
test("shared helper matches the resolved target, not just the lexical name", async () => {
78+
await withFixture(async ({ cwd }) => {
79+
expect(isSensitiveShellToken("notes.txt", cwd)).toBe(true);
80+
// The listing leg never resolves: names are not contents.
81+
expect(isSensitiveShellToken("notes.txt", cwd, false)).toBe(false);
82+
expect(isSensitiveShellToken("README.md", cwd)).toBe(false);
83+
});
84+
});
85+
86+
test("auto mode asks for the symlinked dump, not the listing", async () => {
87+
await withFixture(async ({ cwd }) => {
88+
expect(
89+
autoShellRuleForCall(shellCall("cat notes.txt"), () => false, cwd)
90+
?.name,
91+
).toBe("sensitive-path");
92+
expect(
93+
autoShellRuleForCall(shellCall("ls notes.txt"), () => false, cwd),
94+
).toBeUndefined();
95+
});
96+
});
97+
});

‎src/plugins/tool-result-secret-scrub.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ const JSON_CREDENTIAL_FIELD =
2727
// Grep/shell lines often look like path:line:KEY=value
2828
const ENV_ASSIGNMENT = /(?:^|:)([A-Z][A-Z0-9_]+)=([^\n]+)/gm;
2929

30+
// CL-7790 decision: connection-string keys (DATABASE_URL and friends) are
31+
// deliberately NOT matched here. Widening this shape-classifier would redact
32+
// every benign connection string in tool output — a false-positive blast
33+
// radius on a scrub path, not a prompt path. That needs its own measured
34+
// ticket; the gap stays documented, not silently fixed.
3035
function isSecretEnvKey(key: string): boolean {
3136
return (
3237
key === "API_KEY" ||

0 commit comments

Comments
 (0)