Skip to content

Commit 79f5de6

Browse files
committed
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 547e851 commit 79f5de6

2 files changed

Lines changed: 167 additions & 16 deletions

File tree

src/plugins/secret-guard-plugin.ts

Lines changed: 76 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import { isAbsolute, resolve as resolvePath } 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,19 +138,24 @@ 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.
138145
// Programs that only print directory names / metadata — listing a name never
139146
// dumps file contents. Single owner for this set: the resolve-leg skip below
140147
// and classify.ts's pure-listing exemption both read it, so a new names-only
141148
// program cannot drift into one list without the other.
142149
export const PURE_DIRECTORY_LISTING_PROGRAMS = new Set(["ls", "tree"]);
143150

144151
// 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.
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.
148159
function isPathLikeShellToken(token: string): boolean {
149160
if (
150161
token.startsWith("-") ||
@@ -161,13 +172,52 @@ function isPathLikeShellToken(token: string): boolean {
161172
);
162173
}
163174

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+
164201
// CL-7790: the ONE shell-token matcher both secret-guard call sites share —
165202
// commandReferencesSensitivePath below and classify.ts's per-arg sensitive
166203
// 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.
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.
171221
// Pass resolveSymlinks=false for pure name-listings: listing a name is not
172222
// dumping its contents (CL-5420), so `ls notes.txt` still lists freely while
173223
// `cat notes.txt` asks.
@@ -176,10 +226,21 @@ export function isSensitiveShellToken(
176226
cwd: string = process.cwd(),
177227
resolveSymlinks = true,
178228
): 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));
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);
183244
}
184245

185246
export function commandReferencesSensitivePath(

src/plugins/secret-guard-shell-symlink.test.ts

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, test } from "bun:test";
2-
import { mkdtemp, rm, symlink, writeFile } from "node:fs/promises";
2+
import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises";
33
import { tmpdir } from "node:os";
44
import { join } from "node:path";
55
import {
@@ -25,12 +25,39 @@ async function withFixture<T>(
2525
await writeFile(join(cwd, ".env"), "SECRET=fixture-env\n");
2626
await writeFile(join(cwd, "README.md"), "# fixture\n");
2727
await symlink(join(cwd, ".env"), join(cwd, "notes.txt"));
28+
// Extensionless twin of notes.txt: no dot, no slash.
29+
await symlink(join(cwd, ".env"), join(cwd, "notes"));
30+
// A link visible only from a subdirectory, for the cd-prefix case.
31+
await mkdir(join(cwd, "sub"), { recursive: true });
32+
await symlink(join(cwd, ".env"), join(cwd, "sub", "secret-link.txt"));
2833
return await run({ cwd });
2934
} finally {
3035
await rm(cwd, { recursive: true, force: true });
3136
}
3237
}
3338

39+
// A `~`-reachable secret and a benign-named link into it. Bun's homedir()
40+
// does not follow a runtime-overridden $HOME, so the fixture lives in the
41+
// real home directory under unique per-run names (never the real ~/.env)
42+
// and is removed in `finally`.
43+
async function withHomeFixture<T>(
44+
run: (paths: { linkName: string }) => Promise<T>,
45+
): Promise<T> {
46+
const { homedir } = await import("node:os");
47+
const home = homedir();
48+
const tag = `cl7790-probe-${process.pid}-${Math.floor(Math.random() * 1e9)}`;
49+
const secretName = `${tag}.pem`;
50+
const linkName = `${tag}-notes`;
51+
try {
52+
await writeFile(join(home, secretName), "SECRET=fixture-home-pem\n");
53+
await symlink(join(home, secretName), join(home, linkName));
54+
return await run({ linkName });
55+
} finally {
56+
await rm(join(home, linkName), { force: true });
57+
await rm(join(home, secretName), { force: true });
58+
}
59+
}
60+
3461
const shellCall = (command: string) => ({
3562
id: "c",
3663
name: "run_shell",
@@ -94,4 +121,67 @@ describe("CL-7790 shell tokens resolve symlinks before the secret denylist", ()
94121
).toBeUndefined();
95122
});
96123
});
124+
125+
test("cat through an extensionless symlink does not auto-allow", async () => {
126+
await withFixture(async ({ cwd }) => {
127+
expect(isAutoAllowedShellCommand("cat notes", cwd)).toBe(false);
128+
expect(commandReferencesSensitivePath("cat notes", cwd)).toBe("notes");
129+
expect(isSensitiveShellToken("notes", cwd)).toBe(true);
130+
});
131+
});
132+
133+
test("flag-adjacent bare names do not auto-allow", async () => {
134+
await withFixture(async ({ cwd }) => {
135+
// `=` splits `--file=notes` into a bare `notes` token; `-n` is a flag.
136+
expect(isAutoAllowedShellCommand("cat -n notes", cwd)).toBe(false);
137+
expect(commandReferencesSensitivePath("cat -n notes", cwd)).toBe("notes");
138+
expect(isAutoAllowedShellCommand("grep --file=notes foo", cwd)).toBe(
139+
false,
140+
);
141+
});
142+
});
143+
144+
test("missing bare names still auto-allow (no false positive on a miss)", async () => {
145+
await withFixture(async ({ cwd }) => {
146+
// Nothing named Makefile exists in the fixture: the existence probe
147+
// misses and the command stays auto-allowed.
148+
expect(isAutoAllowedShellCommand("cat Makefile", cwd)).toBe(true);
149+
expect(
150+
commandReferencesSensitivePath("cat Makefile", cwd),
151+
).toBeUndefined();
152+
});
153+
});
154+
155+
test("home-relative link asks for the secret reason, not just outside-workspace", async () => {
156+
await withFixture(async ({ cwd }) => {
157+
await withHomeFixture(async ({ linkName }) => {
158+
const command = `cat ~/${linkName}`;
159+
// classify.ts would already ask here via the `~` outside-workspace
160+
// rule; the point of `~` expansion is that the *secret* reason fires.
161+
expect(commandReferencesSensitivePath(command, cwd)).toBe(
162+
`~/${linkName}`,
163+
);
164+
expect(isAutoAllowedShellCommand(command, cwd)).toBe(false);
165+
expect(
166+
autoShellRuleForCall(shellCall(command), () => false, cwd)?.name,
167+
).toBe("sensitive-path");
168+
});
169+
});
170+
});
171+
172+
test("cd-prefixed dump fails closed; per-segment cd tracking is out of scope", async () => {
173+
await withFixture(async ({ cwd }) => {
174+
// Relative tokens resolve against the session cwd, not a `cd` prefix
175+
// inside the command — the shell would open cwd/sub/secret-link.txt but
176+
// the secret leg only sees cwd/secret-link.txt (absent), so no secret
177+
// reason fires here. The chain still asks because `cd` is not a safe
178+
// program; per-segment `cd` modeling is deliberately not attempted.
179+
expect(
180+
isAutoAllowedShellCommand("cd sub && cat secret-link.txt", cwd),
181+
).toBe(false);
182+
expect(
183+
commandReferencesSensitivePath("cd sub && cat secret-link.txt", cwd),
184+
).toBeUndefined();
185+
});
186+
});
97187
});

0 commit comments

Comments
 (0)