From ff77f9d864f4c63915d53f2a97a61664c68f06cd Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Wed, 30 Sep 2026 10:49:06 -0700 Subject: [PATCH] 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. --- src/permission/path-restriction.ts | 48 ++++++++++++++++++++++++- src/plugins/secret-guard-plugin.test.ts | 29 +++++++++++++++ src/plugins/secret-guard-plugin.ts | 5 +-- 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/src/permission/path-restriction.ts b/src/permission/path-restriction.ts index 70e212c95..7e4eed804 100644 --- a/src/permission/path-restriction.ts +++ b/src/permission/path-restriction.ts @@ -1,4 +1,4 @@ -import { lstatSync, realpathSync } from "node:fs"; +import { lstatSync, readlinkSync, realpathSync } from "node:fs"; import { dirname, join, resolve, sep } from "node:path"; import { homedir } from "node:os"; import type { RootsProvider } from "./worktree-roots.js"; @@ -82,6 +82,52 @@ export function realpathNearestOr(path: string): string { } } +const MAX_SYMLINK_HOPS = 40; + +// The deepest path-or-ancestor that is a symlink whose target does not exist. +function danglingLinkOf(path: string): string | undefined { + let current = path; + for (;;) { + try { + if (lstatSync(current).isSymbolicLink()) { + try { + realpathSync(current); + } catch { + return current; + } + } + } catch { + // Component absent: keep walking up. + } + const parent = dirname(current); + if (parent === current) return undefined; + current = parent; + } +} + +// realpathNearestOr reports a dangling symlink as UNRESOLVABLE because the +// link's own name says nothing about where a write would land. A write +// through `link -> .env` creates `.env`, so security judgments that need the +// landing path follow the link targets by hand. Still UNRESOLVABLE for a +// symlink loop or an unreadable link. +export function realpathFollowingDangling(path: string): string { + let current = path; + for (let hop = 0; hop < MAX_SYMLINK_HOPS; hop++) { + const real = realpathNearestOr(current); + if (real !== UNRESOLVABLE) return real; + const link = danglingLinkOf(current); + if (link === undefined) return UNRESOLVABLE; + let target: string; + try { + target = resolve(dirname(link), readlinkSync(link)); + } catch { + return UNRESOLVABLE; + } + current = join(target, current.slice(link.length)); + } + return UNRESOLVABLE; +} + // An empty root must never reach the prefix compare: `"" + sep` is just // `sep` (e.g. "/" on Unix), which every absolute path starts with, turning // containment into allow-all. A root of exactly `sep` itself is not this bug diff --git a/src/plugins/secret-guard-plugin.test.ts b/src/plugins/secret-guard-plugin.test.ts index 9bb283875..5456de495 100644 --- a/src/plugins/secret-guard-plugin.test.ts +++ b/src/plugins/secret-guard-plugin.test.ts @@ -514,3 +514,32 @@ describe("auto-mode project grant store", () => { }); }); }); + +describe("dangling symlinks", () => { + const write = (path: string): ToolCall => ({ + id: "c", + name: "write_file", + arguments: { path, content: "x" }, + }); + + test("a write through a dangling link to a secret file is denied", async () => { + const dir = await mkdtemp(join(tmpdir(), "secret-guard-dangling-")); + await symlink(join(dir, ".env"), join(dir, "config.txt")); + const result = await handler()( + write(join(dir, "config.txt")), + new AbortController().signal, + ); + expect(result.isError).toBe(true); + expect(result.content).toMatch(/sensitive file blocked/); + }); + + test("a dangling link to an ordinary file is left to the permission gate", async () => { + const dir = await mkdtemp(join(tmpdir(), "secret-guard-dangling-")); + await symlink(join(dir, "notes.txt"), join(dir, "alias.txt")); + const result = await handler()( + write(join(dir, "alias.txt")), + new AbortController().signal, + ); + expect(result.isError).toBeUndefined(); + }); +}); diff --git a/src/plugins/secret-guard-plugin.ts b/src/plugins/secret-guard-plugin.ts index c9b8385ea..708c5a26e 100644 --- a/src/plugins/secret-guard-plugin.ts +++ b/src/plugins/secret-guard-plugin.ts @@ -7,6 +7,7 @@ import { } from "node:path"; import type { ToolPlugin } from "@intx/tools-posix"; import { + realpathFollowingDangling, realpathNearestOr, UNRESOLVABLE, } from "../permission/path-restriction.js"; @@ -144,7 +145,7 @@ export function isSensitivePathResolved( ): boolean { if (isSensitivePath(value, dialect)) return true; if (!isAbsolute(value)) return false; - const real = realpathNearestOr(value); + const real = realpathFollowingDangling(value); return real !== UNRESOLVABLE && isSensitivePath(real, dialect); } @@ -185,7 +186,7 @@ export function createExtraDeniedPathMatcher( return (value: string) => { if (lexical.has(value.replace(/\\/g, "/"))) return true; if (!isAbsolute(value)) return false; - const real = realpathNearestOr(value); + const real = realpathFollowingDangling(value); return real !== UNRESOLVABLE && resolved.has(real.replace(/\\/g, "/")); }; }