Merge pull request #1009 from corbitsdev/cl-6729-share-workspace-roots-with-list_dir-and-delete_file-bounds

Share workspace roots with list_dir and delete_file bounds

Author: TheGreatAxios

src/agent/posix-tool-plugins.ts

@@ -99,6 +99,9 @@ export function buildCorePosixToolPlugins(
   // rebuilding the plugin stack. Secret-guard and authz still hard-deny
   // regardless.
   const allowOutside = (): boolean => permissionGate.getSkipPermissions();
+  // One shared workspace-roots provider for every bound in this stack, so
+  // pathEscape and delete_file admit the same registered sibling worktrees.
+  const rootsProvider = createWorktreeRootsProvider(cwd);
   const truncationOptions =
     getBlobWriter !== undefined ||
     getContextDir !== undefined ||
@@ -112,9 +115,9 @@ export function buildCorePosixToolPlugins(
   return [
     resultTruncationPlugin(truncationOptions),
     toolResultSecretScrubPlugin(),
-    pathEscapePlugin(cwd, createWorktreeRootsProvider(cwd), { allowOutside }),
+    pathEscapePlugin(cwd, rootsProvider, { allowOutside }),
     evidenceArchivePathGuardPlugin(),
-    deleteFilePlugin(cwd, { allowOutside }),
+    deleteFilePlugin(cwd, { allowOutside, rootsProvider }),
     toolOutputUriPlugin(),
     secretGuardPlugin(),
     authzPlugin(),

src/agent/tools.ts

@@ -19,6 +19,7 @@ import { advertiseEditFileLineRange } from "../plugins/edit-file-line-range.js";
 import { advertiseArchiveSurface } from "../plugins/evidence-archive-search-plugin.js";
 import type { Telemetry } from "../telemetry/index.js";
 import type { PermissionGate } from "../permission/gate.js";
+import { createWorktreeRootsProvider } from "../permission/worktree-roots.js";
 import { buildCorePosixToolPlugins } from "./posix-tool-plugins.js";
 import { createLazyBlobReader } from "./lazy-blob-reader.js";
 import type { BlobReader } from "@intx/types/runtime";
@@ -591,6 +592,7 @@ export async function createAgentToolset(
     }),
     createListDirTool(cwd, {
       allowOutside: () => permissionGate.getSkipPermissions(),
+      rootsProvider: createWorktreeRootsProvider(cwd),
     }),
     createUseSkillTool(cwd, skillDirs, args.telemetry),
     createSkillSearchTool({ skills }),

src/list-dir.test.ts

@@ -1,7 +1,7 @@
 import { test, expect, describe } from "bun:test";
-import { mkdtemp, mkdir, writeFile, symlink } from "node:fs/promises";
+import { mkdtemp, mkdir, realpath, symlink, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
-import { join } from "node:path";
+import { basename, join } from "node:path";
 import { listDirectory } from "./util/list-dir.js";
 
 async function fixture(): Promise<string> {
@@ -81,4 +81,40 @@ describe("listDirectory", () => {
     expect(out.split("\n")).toContain("other.txt");
     expect(out).not.toContain("outside the workspace");
   });
+
+  test("lists a registered sibling worktree root (CL-6729)", async () => {
+    const dir = await fixture();
+    const sibling = await mkdtemp(join(tmpdir(), "list-dir-sibling-"));
+    await writeFile(join(sibling, "sibling-file.txt"), "");
+    const roots = [await realpath(sibling)];
+
+    const out = await listDirectory(dir, sibling, {
+      rootsProvider: () => roots,
+    });
+    expect(out.split("\n")).toContain("sibling-file.txt");
+    expect(out).not.toContain("outside the workspace");
+  });
+
+  test("lists a sibling worktree via relative traversal (CL-6729)", async () => {
+    const dir = await fixture();
+    const sibling = await mkdtemp(join(tmpdir(), "list-dir-sibling-rel-"));
+    await writeFile(join(sibling, "sibling-file.txt"), "");
+    const roots = [await realpath(sibling)];
+
+    const out = await listDirectory(dir, join("..", basename(sibling)), {
+      rootsProvider: () => roots,
+    });
+    expect(out.split("\n")).toContain("sibling-file.txt");
+    expect(out).not.toContain("outside the workspace");
+  });
+
+  test("lists through an aliased session root (CL-6729)", async () => {
+    const dir = await fixture();
+    const realDir = await realpath(dir);
+    const alias = `${realDir}-alias`;
+    await symlink(realDir, alias);
+
+    const out = await listDirectory(alias, realDir);
+    expect(out.split("\n")).toEqual(["a.ts", "b.ts", "sub/"]);
+  });
 });

src/plugins/delete-file-plugin.test.ts

@@ -1,8 +1,11 @@
 import { afterEach, beforeEach, describe, expect, test } from "bun:test";
 import {
   chmod,
+  lstat,
   mkdtemp,
   mkdir,
+  readFile,
+  realpath,
   rm,
   stat,
   symlink,
@@ -30,6 +33,15 @@ async function exists(path: string): Promise<boolean> {
   }
 }
 
+async function linkExists(path: string): Promise<boolean> {
+  try {
+    await lstat(path);
+    return true;
+  } catch {
+    return false;
+  }
+}
+
 describe("deleteFilePlugin", () => {
   let cwd: string;
 
@@ -129,6 +141,43 @@ describe("deleteFilePlugin", () => {
     await rm(outside, { recursive: true, force: true });
   });
 
+  test("deletes a dangling symlink inside cwd (CL-6729)", async () => {
+    const link = join(cwd, "broken-link");
+    await symlink(join(cwd, "does-not-exist.txt"), link);
+    expect(await linkExists(link)).toBe(true);
+
+    const result = await handler()(
+      call("broken-link"),
+      new AbortController().signal,
+    );
+
+    expect(result.isError ?? false).toBe(false);
+    expect(String(result.content)).toContain("Deleted file: broken-link");
+    expect(await linkExists(link)).toBe(false);
+  });
+
+  test("deletes a link with an outside referent without touching the referent (CL-6729)", async () => {
+    const outside = await mkdtemp(
+      join(tmpdir(), "corbits-delete-link-referent-"),
+    );
+    const referent = join(outside, "keep.txt");
+    await writeFile(referent, "keep");
+    const link = join(cwd, "outside-link");
+    await symlink(referent, link);
+    expect(await linkExists(link)).toBe(true);
+
+    const result = await handler()(
+      call("outside-link"),
+      new AbortController().signal,
+    );
+
+    expect(result.isError ?? false).toBe(false);
+    expect(String(result.content)).toContain("Deleted file: outside-link");
+    expect(await linkExists(link)).toBe(false);
+    expect(await readFile(referent, "utf8")).toBe("keep");
+    await rm(outside, { recursive: true, force: true });
+  });
+
   test("allowOutside deletes a file outside the working directory", async () => {
     const outside = await mkdtemp(join(tmpdir(), "corbits-delete-yolo-"));
     const path = join(outside, "gone.txt");
@@ -201,6 +250,46 @@ describe("deleteFilePlugin", () => {
     expect(await exists(path)).toBe(true);
   });
 
+  test("deletes a file in a registered sibling worktree (CL-6729)", async () => {
+    const sibling = await mkdtemp(join(tmpdir(), "corbits-delete-sibling-"));
+    const path = join(sibling, "old.txt");
+    await writeFile(path, "old");
+    const roots = [await realpath(sibling)];
+    const tool = deleteFilePlugin(cwd, { rootsProvider: () => roots })
+      .tools?.[0];
+    if (tool === undefined)
+      throw new Error("delete_file tool was not registered");
+
+    const result = await tool.handler(call(path), new AbortController().signal);
+
+    expect(result.isError ?? false).toBe(false);
+    expect(String(result.content)).toContain("Deleted file");
+    expect(await exists(path)).toBe(false);
+    await rm(sibling, { recursive: true, force: true });
+  });
+
+  test("still refuses a genuinely outside file when roots are registered (CL-6729)", async () => {
+    const sibling = await mkdtemp(
+      join(tmpdir(), "corbits-delete-sibling-keep-"),
+    );
+    const outside = await mkdtemp(join(tmpdir(), "corbits-delete-outside-"));
+    const path = join(outside, "keep.txt");
+    await writeFile(path, "keep");
+    const roots = [await realpath(sibling)];
+    const tool = deleteFilePlugin(cwd, { rootsProvider: () => roots })
+      .tools?.[0];
+    if (tool === undefined)
+      throw new Error("delete_file tool was not registered");
+
+    const result = await tool.handler(call(path), new AbortController().signal);
+
+    expect(result.isError).toBe(true);
+    expect(String(result.content)).toContain("outside the working directory");
+    expect(await exists(path)).toBe(true);
+    await rm(sibling, { recursive: true, force: true });
+    await rm(outside, { recursive: true, force: true });
+  });
+
   test("preserves filesystem failure details", async () => {
     const path = join(cwd, "locked.txt");
     await writeFile(path, "keep");

src/plugins/delete-file-plugin.ts

@@ -1,8 +1,10 @@
-import { lstat, readFile, realpath, unlink } from "node:fs/promises";
-import { dirname, isAbsolute, relative, resolve, sep } from "node:path";
+import { lstat, readFile, unlink } from "node:fs/promises";
+import { dirname, resolve } from "node:path";
 import { type } from "arktype";
 import type { ExtraTool, ToolPlugin } from "@intx/tools-posix";
 import type { ToolCall, ToolResult } from "@intx/types/runtime";
+import { resolveWorkspacePath } from "../permission/path-restriction.js";
+import type { RootsProvider } from "../permission/worktree-roots.js";
 import { formatChangeDiff } from "./change-diff.js";
 
 const DeleteFileArgs = type({ path: "string>0" });
@@ -43,19 +45,14 @@ function failureDetail(error: unknown): string {
   return code === undefined ? error.message : `${code}: ${error.message}`;
 }
 
-function isWithin(root: string, path: string): boolean {
-  const rel = relative(root, path);
-  return (
-    rel === "" ||
-    (rel !== ".." && !rel.startsWith(`..${sep}`) && !isAbsolute(rel))
-  );
-}
-
 export interface DeleteFilePluginOptions {
   // When true (yolo / --dangerously-skip-permissions), delete outside the
   // working directory. A getter is resolved per call so `/yolo` mid-session
   // takes effect without rebuilding the plugin stack.
   allowOutside?: boolean | (() => boolean);
+  // Workspace roots beyond cwd (the session's registered git worktrees).
+  // Defaults to cwd alone.
+  rootsProvider?: RootsProvider;
 }
 
 function resolveAllowOutside(
@@ -84,18 +81,28 @@ export function deleteFilePlugin(
       }
 
       const allowOutside = resolveAllowOutside(options.allowOutside);
+      // Containment is keyed off the parent directory, not the full target:
+      // lstat/unlink never follow the final component, so unlinking a link
+      // itself cannot escape even when the link dangles or points outside.
+      // Resolving the full target would refuse both (UNRESOLVABLE / outside
+      // referent). The shared workspace resolver still realpaths the session
+      // root before comparing and admits registered sibling worktree roots —
+      // the same boundary pathEscapePlugin enforces.
       const target = resolve(cwd, args.path);
+      if (
+        !allowOutside &&
+        resolveWorkspacePath(
+          cwd,
+          dirname(target),
+          options.rootsProvider ?? (() => []),
+        ) === undefined
+      ) {
+        return errorResult(
+          call.id,
+          `${args.path} resolves outside the working directory`,
+        );
+      }
       try {
-        const [physicalRoot, physicalParent] = await Promise.all([
-          realpath(cwd),
-          realpath(dirname(target)),
-        ]);
-        if (!allowOutside && !isWithin(physicalRoot, physicalParent)) {
-          return errorResult(
-            call.id,
-            `${args.path} resolves outside the working directory`,
-          );
-        }
         const info = await lstat(target);
         if (info.isDirectory()) {
           return errorResult(

src/util/list-dir.ts

@@ -1,9 +1,11 @@
 import { type } from "arktype";
 import { readdir, realpath } from "node:fs/promises";
-import { resolve, sep } from "node:path";
+import { resolve } from "node:path";
 import { stringTool } from "@intx/agent";
 import type { AgentTool } from "@intx/agent";
 import type { ToolDefinition } from "@intx/types/runtime";
+import { resolveWorkspacePath } from "../permission/path-restriction.js";
+import type { RootsProvider } from "../permission/worktree-roots.js";
 
 const ListDirArgs = type({ "path?": "string" });
 
@@ -31,6 +33,9 @@ export interface ListDirectoryOptions {
   // workspace. A getter is resolved per call so `/yolo` mid-session takes
   // effect without rebuilding the tool.
   allowOutside?: boolean | (() => boolean);
+  // Workspace roots beyond cwd (the session's registered git worktrees).
+  // Defaults to cwd alone.
+  rootsProvider?: RootsProvider;
 }
 
 function resolveAllowOutside(
@@ -47,29 +52,26 @@ export async function listDirectory(
 ): Promise<string> {
   const allowOutside = resolveAllowOutside(options.allowOutside);
   const rel = path.length > 0 ? path : ".";
-  const abs = resolve(cwd, rel);
-  if (!allowOutside && abs !== cwd && !abs.startsWith(cwd + sep)) {
-    return `Error: ${rel} is outside the workspace.`;
-  }
+  const rootsProvider = options.rootsProvider ?? (() => []);
 
-  // A symlink inside the workspace can resolve to a target outside it; the
-  // string prefix check above only sees the lexical path. Resolve the real path
-  // of both the target and the root before comparing so symlink escapes are
-  // refused (unless allowOutside, which is the yolo-mode escape hatch).
+  // Containment is delegated to the shared workspace resolver: it realpaths
+  // the session root before comparing (so an aliased cwd such as macOS
+  // /tmp -> /private/tmp never false-denies) and admits registered sibling
+  // worktree roots. The canonical path feeds readdir directly, so a symlink
+  // retargeted after the check cannot redirect the read.
   let realAbs: string;
-  let realCwd: string;
-  try {
-    realAbs = await realpath(abs);
-    realCwd = await realpath(cwd);
-  } catch (err) {
-    return `Error: cannot list ${rel}: ${err instanceof Error ? err.message : String(err)}`;
-  }
-  if (
-    !allowOutside &&
-    realAbs !== realCwd &&
-    !realAbs.startsWith(realCwd + sep)
-  ) {
-    return `Error: ${rel} is outside the workspace.`;
+  if (allowOutside) {
+    try {
+      realAbs = await realpath(resolve(cwd, rel));
+    } catch (err) {
+      return `Error: cannot list ${rel}: ${err instanceof Error ? err.message : String(err)}`;
+    }
+  } else {
+    const resolved = resolveWorkspacePath(cwd, rel, rootsProvider);
+    if (resolved === undefined) {
+      return `Error: ${rel} is outside the workspace.`;
+    }
+    realAbs = resolved;
   }
 
   let entries;