Skip to content

Commit 1271571

Browse files
Resolve symlinks before the shell secret denylist (#950)
* 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. * Close the extensionless symlink gap in shell secret matching Bare tokens with no dot or slash skipped the resolve leg, so a link named notes into .env auto-allowed where notes.txt asked. Bare non-flag tokens now pay one lstat probe; a hit resolves the target and a miss stays syscall-free past that probe. Home expansion puts the secret reason on cat ~/notes instead of the coincidental outside-workspace ask, and the path-like comment no longer claims globs cannot reach secrets.
1 parent 4c2787a commit 1271571

6 files changed

Lines changed: 335 additions & 18 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: 124 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import { isAbsolute } from "node:path";
1+
import { lstatSync } from "node:fs";
2+
import { homedir } from "node:os";
3+
import {
4+
isAbsolute,
5+
join as joinPath,
6+
resolve as resolvePath,
7+
} from "node:path";
28
import type { ToolPlugin } from "@intx/tools-posix";
39
import {
410
realpathNearestOr,
@@ -132,14 +138,126 @@ function shellPathTokens(command: string): string[] {
132138
// dynamic construction of a path the matcher never sees as one token — e.g.
133139
// indirection through an unrelated variable (`F=.en; cat ${F}v`), character-by-
134140
// character assembly (`printf`), or reading via an interpreter that builds the
135-
// name at runtime. Perfect shell sandboxing is out of scope; the goal is to
136-
// force a prompt for the trivial, single-token references that make exfiltration
137-
// easy. Tool-result secret scrub still redacts credential-shaped output.
141+
// name at runtime. Unexpanded globs are the same class: `cat *` can open a
142+
// symlink the matcher only ever saw as `*`. Perfect shell sandboxing is out
143+
// of scope; the goal is to force a prompt for the trivial, single-token
144+
// references that make exfiltration easy. Tool-result secret scrub still redacts credential-shaped output.
145+
// Programs that only print directory names / metadata — listing a name never
146+
// dumps file contents. Single owner for this set: the resolve-leg skip below
147+
// and classify.ts's pure-listing exemption both read it, so a new names-only
148+
// program cannot drift into one list without the other.
149+
export const PURE_DIRECTORY_LISTING_PROGRAMS = new Set(["ls", "tree"]);
150+
151+
// Worth spending a realpath on: shaped like a path the shell could open
152+
// (a slash, an extension dot, or absolute), not a flag, variable, or fd
153+
// number — those can never name a file the shell opens, so they skip the
154+
// stat and the hot auto-allow path stays syscall-free for them. Globs are
155+
// skipped here for a different reason: the matcher only sees the unexpanded
156+
// pattern, so `cat *.txt` cannot resolve without running the shell — but a
157+
// glob CAN expand into a symlink at runtime, which stays a stated residual
158+
// (see the threat model below), not something this filter disproves.
159+
function isPathLikeShellToken(token: string): boolean {
160+
if (
161+
token.startsWith("-") ||
162+
token.includes("$") ||
163+
token.includes("*") ||
164+
token.includes("`")
165+
)
166+
return false;
167+
return (
168+
isAbsolute(token) ||
169+
token.includes("/") ||
170+
token.includes("\\") ||
171+
token.includes(".")
172+
);
173+
}
174+
175+
// `~` / `~/…` mean the operator's home to the shell, not a literal
176+
// cwd-relative name — expand before both matcher legs so `cat ~/notes`
177+
// resolves the home symlink instead of a (usually missing) cwd child.
178+
// classify.ts's outside-workspace rule would ask anyway; the expansion fixes
179+
// the *reason* (sensitive-path) rather than relying on that coincidence.
180+
function expandHome(token: string): string {
181+
if (token === "~") return homedir();
182+
if (token.startsWith("~/")) return joinPath(homedir(), token.slice(2));
183+
return token;
184+
}
185+
186+
// A bare token the shell could open as a cwd-relative file: not a flag,
187+
// variable, glob, or command substitution — same exclusions as the path-like
188+
// filter, minus the dot/slash shape requirement, so extensionless names
189+
// (`notes`, or `notes` split out of `--file=notes` / `cat -n notes`) still
190+
// get an existence probe below.
191+
function isBareProbeCandidate(token: string): boolean {
192+
return (
193+
token.length > 0 &&
194+
!token.startsWith("-") &&
195+
!token.includes("$") &&
196+
!token.includes("*") &&
197+
!token.includes("`")
198+
);
199+
}
200+
201+
// CL-7790: the ONE shell-token matcher both secret-guard call sites share —
202+
// commandReferencesSensitivePath below and classify.ts's per-arg sensitive
203+
// check. The cheap lexical denylist runs first so the hot auto-allow path
204+
// never touches the filesystem; only survivors pay for filesystem access, in
205+
// two bounded tiers: path-like tokens pay for a realpath via the CL-6971
206+
// helper, which catches a benign-named symlink into a secret file (notes.txt
207+
// -> .env) exactly like the secret name itself, while bare extensionless
208+
// tokens first pay a single lstat existence probe against the cwd-resolved
209+
// path — a miss (the common `cat Makefile` case) costs exactly that one
210+
// lstat and skips the resolve, a hit (file or symlink, dangling included)
211+
// pays the realpath and matches on the target. Flags, variables, globs, and
212+
// backticks never probe, so the worst case per command is one lstat per bare
213+
// token plus one realpath per existing entry. Relative tokens resolve
214+
// against cwd first because the helper takes absolute paths; `~` expands to
215+
// the home directory before resolving for the same reason. That cwd is the
216+
// session/process cwd, not a `cd` prefix inside the command —
217+
// `cd sub && cat notes.txt` resolves `notes.txt` against the session cwd
218+
// (absent) rather than cwd/sub (present). The chain still fails closed
219+
// because `cd` is not a safe program, but no secret reason fires;
220+
// per-segment `cd` modeling is deliberately out of scope.
221+
// Pass resolveSymlinks=false for pure name-listings: listing a name is not
222+
// dumping its contents (CL-5420), so `ls notes.txt` still lists freely while
223+
// `cat notes.txt` asks.
224+
export function isSensitiveShellToken(
225+
token: string,
226+
cwd: string = process.cwd(),
227+
resolveSymlinks = true,
228+
): boolean {
229+
const expanded = expandHome(token);
230+
if (isSensitivePath(expanded)) return true;
231+
if (!resolveSymlinks) return false;
232+
if (isPathLikeShellToken(expanded)) {
233+
if (isAbsolute(expanded)) return isSensitivePathResolved(expanded);
234+
return isSensitivePathResolved(resolvePath(cwd, expanded));
235+
}
236+
if (!isBareProbeCandidate(expanded)) return false;
237+
const abs = isAbsolute(expanded) ? expanded : resolvePath(cwd, expanded);
238+
try {
239+
lstatSync(abs);
240+
} catch {
241+
return false;
242+
}
243+
return isSensitivePathResolved(abs);
244+
}
245+
138246
export function commandReferencesSensitivePath(
139247
command: string,
248+
cwd: string = process.cwd(),
140249
): string | undefined {
141-
for (const token of shellPathTokens(command)) {
142-
if (isSensitivePath(token)) return token;
250+
const tokens = shellPathTokens(command);
251+
// Dump vs list: a lone name-listing never dumps file contents, so only the
252+
// cheap lexical leg applies and `ls notes.txt` still lists freely. Anything
253+
// composed (pipes, chains, redirects, subshells) takes the resolve leg —
254+
// `ls && cat notes.txt` must not ride the listing exemption.
255+
const program = tokens[0] ?? "";
256+
const listingOnly =
257+
PURE_DIRECTORY_LISTING_PROGRAMS.has(program) &&
258+
!/[;&|()<>\n]/.test(command);
259+
for (const token of tokens) {
260+
if (isSensitiveShellToken(token, cwd, !listingOnly)) return token;
143261
}
144262
return undefined;
145263
}

0 commit comments

Comments
 (0)