Return canonical realpath from workspace containment allow

resolveWorkspacePath already computes each candidate's realpath to decide
containment but returned the lexical path, leaving a TOCTOU window: a
symlink in-bounds at allow time could be retargeted before the actual
write, escaping the workspace. Return the resolved real path instead so
pathEscapePlugin substitutes it into the tool call args, and writers act
on the already-resolved location rather than re-traversing the symlink.

Fixes CL-6712
https://linear.app/abklabs/issue/CL-6712

Author: TheGreatAxios

src/permission/path-restriction.ts

@@ -68,9 +68,18 @@ const inKnownRoots = (real: string, roots: readonly string[]): boolean =>
 // Resolves `path` (relative or absolute, possibly traversing `..`) against
 // `cwd` and checks it against the workspace boundary: `cwd` itself plus every
 // root `rootsProvider` reports (the session's registered git worktrees, or
-// any other allowlisted sibling). Returns the resolved absolute path when the
-// target is in bounds, `undefined` otherwise — callers that need a hard
-// allow/deny (rather than an allow/ask distinction) can key off that.
+// any other allowlisted sibling). Returns the CANONICAL real path (symlink
+// segments resolved; for a not-yet-created target, the nearest existing
+// ancestor's real path rejoined with the missing tail) when the target is in
+// bounds, `undefined` otherwise — callers that need a hard allow/deny (rather
+// than an allow/ask distinction) can key off that.
+//
+// Returning the canonical path rather than the lexical `abs` closes a
+// TOCTOU: a symlink segment that is in-bounds at check time can be
+// retargeted before a write actually happens. Callers (e.g. pathEscapePlugin)
+// substitute this return value into the tool call's path argument, so the
+// writer that ultimately opens the file never re-traverses the original
+// symlink — it uses the already-resolved location.
 //
 // A relative `../` is deliberately resolved and realpath-checked against the
 // allowlist rather than rejected outright: the raw path alone can't tell a
@@ -84,9 +93,9 @@ export function resolveWorkspacePath(
   const abs = resolve(cwd, path);
   const realCwd = realpathOr(resolve(cwd));
   const real = realpathNearestOr(abs);
-  if (real === realCwd || real.startsWith(realCwd + sep)) return abs;
-  if (inKnownRoots(real, rootsProvider())) return abs;
-  if (inKnownRoots(real, rootsProvider(true))) return abs;
+  if (real === realCwd || real.startsWith(realCwd + sep)) return real;
+  if (inKnownRoots(real, rootsProvider())) return real;
+  if (inKnownRoots(real, rootsProvider(true))) return real;
   return undefined;
 }
 

src/permission/permission.test.ts

@@ -2813,7 +2813,10 @@ describe("listWorktreeRoots", () => {
     const { repo, worktree } = createRepoWithWorktree();
     const roots = await listWorktreeRoots(repo);
     const relativeTarget = join("..", "secondary", "notes.md");
-    expect(resolveWorkspacePath(repo, relativeTarget, () => roots)).toBe(join(repo, "..", "secondary", "notes.md"));
+    // Canonical (realpath-resolved), not the lexical join — see CL-6712.
+    expect(resolveWorkspacePath(repo, relativeTarget, () => roots)).toBe(
+      join(realpathSync(join(repo, "..")), "secondary", "notes.md"),
+    );
   });
 
   test("resolveWorkspacePath still rejects a genuinely unrelated outside path", async () => {

src/permission/workspace-containment.test.ts

@@ -6,7 +6,7 @@ import { tmpdir } from "node:os";
 import type { ToolCall } from "@intx/types/runtime";
 
 import { isAutoAllowedShellCall } from "./classify.js";
-import { createPathRestriction } from "./path-restriction.js";
+import { createPathRestriction, resolveWorkspacePath } from "./path-restriction.js";
 
 let cwd = "";
 let worktree = "";
@@ -91,3 +91,34 @@ test("a symlink pointing outside the workspace is refused, even for a not-yet-ex
   expect(autoAllowed).toBe(false);
   expect(restricted).toBe(true);
 });
+
+test("resolveWorkspacePath returns the canonical target so a later symlink retarget cannot redirect a write (CL-6712 TOCTOU)", async () => {
+  // A symlink that is in-bounds at check time (target -> inside cwd) must
+  // resolve to the canonical real path, not the lexical path through the
+  // symlink. If the caller only remembered the lexical path and re-opened it
+  // after the symlink is retargeted, the write would follow the new target
+  // instead of the one that was actually approved.
+  const rootsProvider = () => [];
+  const realTarget = join(cwd, "real-target");
+  await mkdir(realTarget, { recursive: true });
+  const link = join(cwd, "link");
+  await symlink(realTarget, link);
+
+  const resolved = resolveWorkspacePath(cwd, join("link", "note.txt"), rootsProvider);
+  expect(resolved).toBe(join(realpathSync(realTarget), "note.txt"));
+
+  // Retarget the symlink to point outside the workspace, as an attacker
+  // would do between the allow check and the actual write.
+  await rm(link);
+  await symlink(outside, link);
+
+  // The canonical path captured before the retarget still points at the
+  // originally-approved location inside the workspace — it never traverses
+  // "link" again, so it is unaffected by the retarget.
+  expect(resolved).not.toContain(outside);
+
+  // A fresh check against the now-retargeted symlink correctly sees the
+  // escape and denies it.
+  const restriction = createPathRestriction(cwd, rootsProvider, home);
+  expect(restriction.isRestricted(join(link, "note.txt"), true)).toBe(true);
+});

src/plugins/path-escape-plugin.test.ts

@@ -1,4 +1,8 @@
-import { describe, test, expect } from "bun:test";
+import { describe, test, expect, beforeEach, afterEach } from "bun:test";
+import { mkdir, mkdtemp, rm, symlink } from "node:fs/promises";
+import { realpathSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
 
 import { pathEscapePlugin } from "./path-escape-plugin.js";
 import type { ToolCall, ToolResult } from "@intx/types/runtime";
@@ -175,4 +179,48 @@ describe("pathEscapePlugin", () => {
     const args = JSON.parse(String(allowed.content)) as { path: string };
     expect(args.path).toBe("/other-repo/README.md");
   });
+
+  describe("symlink TOCTOU (CL-6712)", () => {
+    let cwd = "";
+
+    beforeEach(async () => {
+      cwd = await mkdtemp(join(tmpdir(), "corbits-path-escape-"));
+    });
+
+    afterEach(async () => {
+      await rm(cwd, { recursive: true, force: true });
+    });
+
+    test("write_file receives the canonical path, unaffected by a later symlink retarget", async () => {
+      const realTarget = join(cwd, "real-target");
+      await mkdir(realTarget, { recursive: true });
+      const link = join(cwd, "link");
+      await symlink(realTarget, link);
+
+      const plugin = pathEscapePlugin(cwd);
+      const next = async (call: ToolCall): Promise<ToolResult> => ({
+        callId: call.id,
+        content: JSON.stringify(call.arguments),
+      });
+      const handler = plugin.middleware ? plugin.middleware(next) : next;
+
+      const result = await handler(
+        makeCall("write_file", { path: join("link", "note.txt"), content: "hi" }),
+        new AbortController().signal,
+      );
+      const args = JSON.parse(String(result.content)) as { path: string };
+      // The path handed to write_file is already the resolved real-target
+      // location, not the symlink-relative path.
+      expect(args.path).toBe(join(realpathSync(realTarget), "note.txt"));
+
+      // An attacker retargets the symlink after the allow check. A writer
+      // that (correctly) uses the path it was given above is unaffected —
+      // it never re-traverses "link".
+      const outside = await mkdtemp(join(tmpdir(), "corbits-path-escape-outside-"));
+      await rm(link);
+      await symlink(outside, link);
+      expect(args.path).not.toContain(outside);
+      await rm(outside, { recursive: true, force: true });
+    });
+  });
 });