Merge pull request #1262 from corbitsdev/cl-9632-secret-guard-follows-dangling-symlinks-to-sensitive-targets

fix(permission): follow dangling symlinks when judging sensitive paths

Author: TheGreatAxios

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

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();
+  });
+});

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, "/"));
   };
 }