Skip to content

Commit 0822e0e

Browse files
committed
fix(permission): follow dangling symlinks when judging sensitive paths
A write through a dangling link to a secret file, such as link -> .env, passed the secret guard because the resolver reported the path as unresolvable. The guard now follows link targets by hand, so the path a write would land on is judged like any other route to that file.
1 parent 0aee266 commit 0822e0e

3 files changed

Lines changed: 80 additions & 4 deletions

File tree

‎src/permission/path-restriction.ts‎

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { lstatSync, realpathSync } from "node:fs";
1+
import { lstatSync, readlinkSync, realpathSync } from "node:fs";
22
import { dirname, join, resolve, sep } from "node:path";
33
import { homedir } from "node:os";
44
import type { RootsProvider } from "./worktree-roots.js";
@@ -82,6 +82,52 @@ export function realpathNearestOr(path: string): string {
8282
}
8383
}
8484

85+
const MAX_SYMLINK_HOPS = 40;
86+
87+
// The deepest path-or-ancestor that is a symlink whose target does not exist.
88+
function danglingLinkOf(path: string): string | undefined {
89+
let current = path;
90+
for (;;) {
91+
try {
92+
if (lstatSync(current).isSymbolicLink()) {
93+
try {
94+
realpathSync(current);
95+
} catch {
96+
return current;
97+
}
98+
}
99+
} catch {
100+
// Component absent: keep walking up.
101+
}
102+
const parent = dirname(current);
103+
if (parent === current) return undefined;
104+
current = parent;
105+
}
106+
}
107+
108+
// realpathNearestOr reports a dangling symlink as UNRESOLVABLE because the
109+
// link's own name says nothing about where a write would land. A write
110+
// through `link -> .env` creates `.env`, so security judgments that need the
111+
// landing path follow the link targets by hand. Still UNRESOLVABLE for a
112+
// symlink loop or an unreadable link.
113+
export function realpathFollowingDangling(path: string): string {
114+
let current = path;
115+
for (let hop = 0; hop < MAX_SYMLINK_HOPS; hop++) {
116+
const real = realpathNearestOr(current);
117+
if (real !== UNRESOLVABLE) return real;
118+
const link = danglingLinkOf(current);
119+
if (link === undefined) return UNRESOLVABLE;
120+
let target: string;
121+
try {
122+
target = resolve(dirname(link), readlinkSync(link));
123+
} catch {
124+
return UNRESOLVABLE;
125+
}
126+
current = join(target, current.slice(link.length));
127+
}
128+
return UNRESOLVABLE;
129+
}
130+
85131
// An empty root must never reach the prefix compare: `"" + sep` is just
86132
// `sep` (e.g. "/" on Unix), which every absolute path starts with, turning
87133
// containment into allow-all. A root of exactly `sep` itself is not this bug

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

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, test, expect } from "bun:test";
2-
import { mkdir, mkdtemp, rm } from "node:fs/promises";
2+
import { mkdir, mkdtemp, rm, symlink } from "node:fs/promises";
33
import { tmpdir } from "node:os";
44
import { join } from "node:path";
55
import { createPosixTools } from "@intx/tools-posix";
@@ -468,3 +468,32 @@ describe("auto-mode project grant store", () => {
468468
});
469469
});
470470
});
471+
472+
describe("dangling symlinks", () => {
473+
const write = (path: string): ToolCall => ({
474+
id: "c",
475+
name: "write_file",
476+
arguments: { path, content: "x" },
477+
});
478+
479+
test("a write through a dangling link to a secret file is denied", async () => {
480+
const dir = await mkdtemp(join(tmpdir(), "secret-guard-dangling-"));
481+
await symlink(join(dir, ".env"), join(dir, "config.txt"));
482+
const result = await handler()(
483+
write(join(dir, "config.txt")),
484+
new AbortController().signal,
485+
);
486+
expect(result.isError).toBe(true);
487+
expect(result.content).toMatch(/sensitive file blocked/);
488+
});
489+
490+
test("a dangling link to an ordinary file is left to the permission gate", async () => {
491+
const dir = await mkdtemp(join(tmpdir(), "secret-guard-dangling-"));
492+
await symlink(join(dir, "notes.txt"), join(dir, "alias.txt"));
493+
const result = await handler()(
494+
write(join(dir, "alias.txt")),
495+
new AbortController().signal,
496+
);
497+
expect(result.isError).toBeUndefined();
498+
});
499+
});

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
} from "node:path";
88
import type { ToolPlugin } from "@intx/tools-posix";
99
import {
10+
realpathFollowingDangling,
1011
realpathNearestOr,
1112
UNRESOLVABLE,
1213
} from "../permission/path-restriction.js";
@@ -144,7 +145,7 @@ export function isSensitivePathResolved(
144145
): boolean {
145146
if (isSensitivePath(value, dialect)) return true;
146147
if (!isAbsolute(value)) return false;
147-
const real = realpathNearestOr(value);
148+
const real = realpathFollowingDangling(value);
148149
return real !== UNRESOLVABLE && isSensitivePath(real, dialect);
149150
}
150151

@@ -185,7 +186,7 @@ export function createExtraDeniedPathMatcher(
185186
return (value: string) => {
186187
if (lexical.has(value.replace(/\\/g, "/"))) return true;
187188
if (!isAbsolute(value)) return false;
188-
const real = realpathNearestOr(value);
189+
const real = realpathFollowingDangling(value);
189190
return real !== UNRESOLVABLE && resolved.has(real.replace(/\\/g, "/"));
190191
};
191192
}

0 commit comments

Comments
 (0)