Realpath secret-guard denylist so yolo symlinks cannot bypass (CL-6971) (#669)

Under skip-permissions, pathEscape absolutizes without resolving links, so
an innocuous name could defeat the secret denylist. Match lexical and
realpath forms (and Codex raw-read) before allowing the path.

Author: TheGreatAxios

src/agent/codex-read-raw-file.ts

@@ -14,8 +14,9 @@
  *
  * Reuses the existing containment and secret-file authorities rather than
  * reimplementing them: `resolveWorkspacePath` (symlink-aware realpath
- * containment, the same function pathEscapePlugin calls) and `isSensitivePath`
- * (the secretGuardPlugin denylist). Both are hard denials — the latter has no
+ * containment, the same function pathEscapePlugin calls) and
+ * `isSensitivePathResolved` (the secretGuardPlugin denylist, including the
+ * CL-6971 realpath floor). Both are hard denials — the latter has no
  * yolo/allowOutside bypass, matching secretGuardPlugin's own unconditional
  * behavior.
  */
@@ -25,7 +26,7 @@ import { resolve } from "node:path";
 import { hasCode } from "@intx/types";
 import { resolveWorkspacePath } from "../permission/path-restriction.js";
 import { createWorktreeRootsProvider } from "../permission/worktree-roots.js";
-import { isSensitivePath } from "../plugins/secret-guard-plugin.js";
+import { isSensitivePath, isSensitivePathResolved } from "../plugins/secret-guard-plugin.js";
 import type { PermissionGate } from "../permission/gate.js";
 import type { CodexReadRawFile } from "./codex-tool-proxies.js";
 
@@ -52,7 +53,9 @@ export function createCodexReadRawFile(
     if (resolved !== undefined) {
       absolutePath = resolved;
     } else if (allowOutside()) {
-      // Mirrors pathEscapePlugin's own allowOutside fallback (yolo mode).
+      // Mirrors pathEscapePlugin's own allowOutside fallback (yolo mode):
+      // lexical absolutize. The realpath floor below still catches symlinks
+      // whose innocuous names would otherwise defeat the denylist (CL-6971).
       absolutePath = resolve(cwd, path);
     } else {
       return {
@@ -61,9 +64,10 @@ export function createCodexReadRawFile(
       };
     }
 
-    // Re-check the resolved, symlink-realpath'd form too: a symlink can name
-    // something innocuous while pointing at a sensitive real path.
-    if (isSensitivePath(absolutePath)) {
+    // Re-check after resolve — and realpath when the allowOutside branch left
+    // a symlink unresolved — so a link can name something innocuous while
+    // pointing at a sensitive real path.
+    if (isSensitivePathResolved(absolutePath)) {
       return {
         content: `Access to sensitive file blocked by policy: ${path}`,
         isError: true,

src/plugins/secret-guard-plugin.ts

@@ -1,4 +1,6 @@
+import { isAbsolute } from "node:path";
 import type { ToolPlugin } from "@intx/tools-posix";
+import { realpathNearestOr, UNRESOLVABLE } from "../permission/path-restriction.js";
 import { looksLikePath } from "./path-escape-plugin.js";
 
 // Files that hold secrets and must never be read or written by path-keyed tools
@@ -81,6 +83,20 @@ export function isSensitivePath(value: string): boolean {
   return SENSITIVE_PATTERNS.some((pattern) => pattern.test(normalized));
 }
 
+// Secret-guard floor (CL-6971): match the lexical path AND its realpath. Under
+// yolo, pathEscape absolutizes outside paths without resolving symlinks, so an
+// innocuous name (config.txt → .env, or cache/ → ~/.aws) would otherwise pass
+// the denylist. realpathNearestOr also covers write targets that don't exist
+// yet when a parent component is a symlink into a sensitive directory.
+// Absolute-only for the realpath leg — pathEscape absolutizes in the live
+// stack; relative unit-test args still match on the lexical form.
+export function isSensitivePathResolved(value: string): boolean {
+  if (isSensitivePath(value)) return true;
+  if (!isAbsolute(value)) return false;
+  const real = realpathNearestOr(value);
+  return real !== UNRESOLVABLE && isSensitivePath(real);
+}
+
 // Break a shell command into the bare path-like tokens it references so each can
 // be matched against the secret-file denylist. Quote, backtick and backslash
 // characters are stripped first so split obfuscations (`.e''nv`, `'.env'`,
@@ -126,12 +142,13 @@ export function commandReferencesSensitivePath(command: string): string | undefi
 // permission gate (and auto-shell policy in auto mode).
 //
 // Path-arg hard deny runs before the permission plugin, so it holds even under
-// --dangerously-skip-permissions.
+// --dangerously-skip-permissions. Symlink resolution is part of that floor
+// (CL-6971): yolo must not let an innocuous link name defeat the denylist.
 export function secretGuardPlugin(): ToolPlugin {
   return {
     middleware: (next) => async (call, signal) => {
       for (const [key, value] of Object.entries(call.arguments)) {
-        if (typeof value === "string" && looksLikePath(key) && isSensitivePath(value)) {
+        if (typeof value === "string" && looksLikePath(key) && isSensitivePathResolved(value)) {
           return {
             callId: call.id,
             content: `Access to sensitive file blocked by policy: ${value}`,

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

@@ -0,0 +1,176 @@
+import { describe, expect, test } from "bun:test";
+import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { createPosixTools } from "@intx/tools-posix";
+import { createPermissionGate } from "../permission/gate.js";
+import { buildCorePosixToolPlugins } from "../agent/posix-tool-plugins.js";
+import { createCodexReadRawFile } from "../agent/codex-read-raw-file.js";
+
+/**
+ * CL-6971: under skip-permissions (yolo), pathEscape absolutizes outside paths
+ * without realpath'ing, so an innocuous symlink name defeats the secret-guard
+ * denylist. Secret guard is a floor — always realpath before isSensitivePath,
+ * yolo or not. Covers both shapes: file symlink → secret, and dir symlink →
+ * outside secret dir (where the lexical path no longer contains the sensitive
+ * segment).
+ */
+
+async function withFixture<T>(
+  run: (paths: {
+    cwd: string;
+    outsideEnv: string;
+    awsDir: string;
+    fileLink: string;
+    dirLink: string;
+  }) => Promise<T>,
+): Promise<T> {
+  const parent = await mkdtemp(join(tmpdir(), "cl6971-secret-symlink-"));
+  const cwd = join(parent, "ws");
+  const outside = join(parent, "outside");
+  const awsDir = join(parent, ".aws");
+  await mkdir(cwd);
+  await mkdir(outside);
+  await mkdir(awsDir);
+  const outsideEnv = join(outside, ".env");
+  await writeFile(outsideEnv, "SECRET=outside-env\n");
+  await writeFile(join(awsDir, "credentials"), "aws_secret_access_key=LEAKED\n");
+  // Shape 1: file symlink with an innocuous name → outside .env
+  const fileLink = join(cwd, "config.txt");
+  await symlink(outsideEnv, fileLink);
+  // Shape 2: dir symlink → ~/.aws-shaped dir; lexical path is cache/credentials
+  // (no ".aws/" segment) so basename-only matching is not enough.
+  const dirLink = join(cwd, "cache");
+  await symlink(awsDir, dirLink);
+  try {
+    return await run({ cwd, outsideEnv, awsDir, fileLink, dirLink });
+  } finally {
+    await rm(parent, { recursive: true, force: true });
+  }
+}
+
+function runner(cwd: string, skipPermissions: boolean) {
+  const gate = createPermissionGate({
+    approvals: [],
+    interactive: false,
+    skipPermissions,
+    auto: false,
+    cwd,
+  });
+  return {
+    gate,
+    tools: createPosixTools({
+      cwd,
+      plugins: buildCorePosixToolPlugins({ cwd, permissionGate: gate }),
+    }),
+  };
+}
+
+describe("CL-6971 secret-guard realpaths before denylist (symlink floor)", () => {
+  for (const skipPermissions of [true, false] as const) {
+    const mode = skipPermissions ? "yolo" : "normal";
+
+    test(`${mode}: file symlink → outside .env is blocked for read_file`, async () => {
+      await withFixture(async ({ cwd }) => {
+        const { tools } = runner(cwd, skipPermissions);
+        const result = await tools.run(
+          { id: "1", name: "read_file", arguments: { path: "config.txt" } },
+          new AbortController().signal,
+        );
+        expect(result.isError).toBe(true);
+        expect(String(result.content)).toMatch(/sensitive file|escapes working directory/i);
+        expect(String(result.content)).not.toContain("SECRET=outside-env");
+      });
+    });
+
+    test(`${mode}: file symlink → outside .env is blocked for write_file`, async () => {
+      await withFixture(async ({ cwd, outsideEnv }) => {
+        const before = await Bun.file(outsideEnv).text();
+        const { tools } = runner(cwd, skipPermissions);
+        const result = await tools.run(
+          {
+            id: "1",
+            name: "write_file",
+            arguments: { path: "config.txt", content: "pwned" },
+          },
+          new AbortController().signal,
+        );
+        expect(result.isError).toBe(true);
+        expect(String(result.content)).toMatch(/sensitive file|escapes working directory/i);
+        expect(await Bun.file(outsideEnv).text()).toBe(before);
+      });
+    });
+
+    test(`${mode}: dir symlink → outside .aws/credentials is blocked for read_file`, async () => {
+      await withFixture(async ({ cwd }) => {
+        const { tools } = runner(cwd, skipPermissions);
+        const result = await tools.run(
+          { id: "1", name: "read_file", arguments: { path: "cache/credentials" } },
+          new AbortController().signal,
+        );
+        expect(result.isError).toBe(true);
+        expect(String(result.content)).toMatch(/sensitive file|escapes working directory/i);
+        expect(String(result.content)).not.toContain("LEAKED");
+      });
+    });
+
+    test(`${mode}: dir symlink → outside .aws/credentials is blocked for write_file`, async () => {
+      await withFixture(async ({ cwd, awsDir }) => {
+        const target = join(awsDir, "credentials");
+        const before = await Bun.file(target).text();
+        const { tools } = runner(cwd, skipPermissions);
+        const result = await tools.run(
+          {
+            id: "1",
+            name: "write_file",
+            arguments: { path: "cache/credentials", content: "pwned" },
+          },
+          new AbortController().signal,
+        );
+        expect(result.isError).toBe(true);
+        expect(String(result.content)).toMatch(/sensitive file|escapes working directory/i);
+        expect(await Bun.file(target).text()).toBe(before);
+      });
+    });
+
+    test(`${mode}: file symlink → outside .env is blocked for apply_patch raw read`, async () => {
+      await withFixture(async ({ cwd }) => {
+        const { gate } = runner(cwd, skipPermissions);
+        const result = await createCodexReadRawFile(cwd, gate)("config.txt");
+        expect(result.isError).toBe(true);
+        expect(String(result.content)).toMatch(/sensitive file|escapes working directory/i);
+        expect(String(result.content)).not.toContain("SECRET=outside-env");
+      });
+    });
+
+    test(`${mode}: dir symlink → outside .aws/credentials is blocked for apply_patch raw read`, async () => {
+      await withFixture(async ({ cwd }) => {
+        const { gate } = runner(cwd, skipPermissions);
+        const result = await createCodexReadRawFile(cwd, gate)("cache/credentials");
+        expect(result.isError).toBe(true);
+        expect(String(result.content)).toMatch(/sensitive file|escapes working directory/i);
+        expect(String(result.content)).not.toContain("LEAKED");
+      });
+    });
+  }
+
+  // In-workspace file symlink → .env: pathEscape already realpaths in-bounds, but
+  // the floor must still hold under yolo without relying on that alone.
+  test("yolo: in-workspace file symlink → .env is blocked for read_file", async () => {
+    const cwd = await mkdtemp(join(tmpdir(), "cl6971-inws-"));
+    try {
+      await writeFile(join(cwd, ".env"), "SECRET=in-ws\n");
+      await symlink(join(cwd, ".env"), join(cwd, "looks-safe.txt"));
+      const { tools } = runner(cwd, true);
+      const result = await tools.run(
+        { id: "1", name: "read_file", arguments: { path: "looks-safe.txt" } },
+        new AbortController().signal,
+      );
+      expect(result.isError).toBe(true);
+      expect(String(result.content)).toMatch(/sensitive file/i);
+      expect(String(result.content)).not.toContain("SECRET=in-ws");
+    } finally {
+      await rm(cwd, { recursive: true, force: true });
+    }
+  });
+});