feat(FN-1430): add worktree execution boundaries to prevent out-of-scope modifications

- Add worktree-aware path boundaries in agent factory to prevent cross-task contamination - Sync core agent prompts with worktree boundary guidance for consistent enforcement - Add boundary guidance to executor prompts so agents understand their scope - Fix TypeScript types for worktree boundary wrapping - Add comprehensive tests for boundary wrapping behavior - Create changeset for @gsxdsm/fusion (minor)
2026-04-09 23:59:45 -07:00
parent cba41e68d6
commit dfaf768601
7 changed files with 528 additions and 2 deletions
--- a/packages/engine/src/executor.test.ts
+++ b/packages/engine/src/executor.test.ts
@@ -9265,3 +9265,74 @@ describe("detectReviewHandoffIntent", () => {
    expect(detectReviewHandoffIntent("")).toBe(false);
  });
 });
+
+describe("buildExecutionPrompt", () => {
+  it("includes worktree boundary guidance in the execution prompt", () => {
+    const task: any = {
+      id: "FN-TEST",
+      title: "Test task",
+      dependencies: [],
+      prompt: "# Test task\n## Steps\n- Step 1",
+      steps: [],
+      currentStep: 0,
+      attachments: [],
+    };
+
+    const prompt = buildExecutionPrompt(task, "/project");
+
+    expect(prompt).toContain("## Worktree Boundaries");
+    expect(prompt).toContain("isolated git worktree");
+    expect(prompt).toContain("All code changes must be made inside the current worktree directory");
+  });
+
+  it("mentions project memory exception in worktree boundary guidance", () => {
+    const task: any = {
+      id: "FN-TEST",
+      title: "Test task",
+      dependencies: [],
+      prompt: "# Test task\n## Steps\n- Step 1",
+      steps: [],
+      currentStep: 0,
+      attachments: [],
+    };
+
+    const prompt = buildExecutionPrompt(task, "/project");
+
+    expect(prompt).toContain(".fusion/memory.md");
+    expect(prompt).toContain("memory");
+    expect(prompt).toContain("durable");
+  });
+
+  it("mentions task attachments exception in worktree boundary guidance", () => {
+    const task: any = {
+      id: "FN-TEST",
+      title: "Test task",
+      dependencies: [],
+      prompt: "# Test task\n## Steps\n- Step 1",
+      steps: [],
+      currentStep: 0,
+      attachments: [],
+    };
+
+    const prompt = buildExecutionPrompt(task, "/project");
+
+    expect(prompt).toContain("attachments");
+    expect(prompt).toContain("context");
+  });
+
+  it("includes worktree boundary guidance regardless of review level", () => {
+    const task: any = {
+      id: "FN-TEST",
+      title: "Test task",
+      dependencies: [],
+      prompt: "# Test task\n## Review Level: 0\n## Steps\n- Step 1",
+      steps: [],
+      currentStep: 0,
+      attachments: [],
+    };
+
+    const prompt = buildExecutionPrompt(task, "/project");
+
+    expect(prompt).toContain("## Worktree Boundaries");
+  });
+});
--- a/packages/engine/src/executor.ts
+++ b/packages/engine/src/executor.ts
@@ -180,6 +180,17 @@ Documents are versioned — each write creates a new revision. Use meaningful ke
 - Use conventional commit messages prefixed with the task ID
 - Do NOT commit broken or half-implemented code

+## Worktree Boundaries
+
+You are running in an **isolated git worktree**. This means:
+
+- **All code changes must be made inside the current worktree directory.** Do not modify files outside the worktree — the worktree is your isolated execution environment.
+- **Exception — Project memory:** You MAY read and write to .fusion/memory.md at the project root to save durable project learnings (architecture patterns, conventions, pitfalls).
+- **Exception — Task attachments:** You MAY read files under .fusion/tasks/{taskId}/attachments/ at the project root for context screenshots and documents attached to this task.
+- **Shell commands** run inside the worktree by default. Avoid using cd to navigate outside the worktree.
+
+If you attempt to write to a path outside the worktree, the file tools will reject the operation with an error explaining the boundary.
+
 ## Guardrails
 - Treat the File Scope in PROMPT.md as the expected starting scope, not a hard boundary when quality gates fail
 - Read "Context to Read First" files before starting
@@ -3486,6 +3497,15 @@ ${reviewLevel >= 2 ? `After implementing + committing each step, call:
 \`review_step(step=N, type="code", step_name="...", baseline="<SHA from before step>")\`` : ""}
 ${reviewLevel >= 3 ? `After tests, also call review_step with type="code" for test review.` : ""}

+## Worktree Boundaries
+
+You are running in an **isolated git worktree**. This means:
+
+- **All code changes must be made inside the current worktree directory.** Do not modify files outside the worktree.
+- **Exception — Project memory:** You MAY read and write to \`.fusion/memory.md\` at the project root to save durable project learnings.
+- **Exception — Task attachments:** You MAY read files under \`.fusion/tasks/{taskId}/attachments/\` at the project root for context.
+- **Shell commands** run inside the worktree by default. Avoid using \`cd\` to navigate outside the worktree.
+
 ## Begin

 ${hasProgress
--- a/packages/engine/src/pi-create-kb-agent.test.ts
+++ b/packages/engine/src/pi-create-kb-agent.test.ts
@@ -63,6 +63,219 @@ vi.mock("@mariozechner/pi-coding-agent", () => ({
  },
 }));

+describe("worktree path boundary helpers", () => {
+  // Test helper functions directly by importing them
+  // Note: These tests verify the boundary logic without needing a full agent session
+
+  describe("path boundary logic for worktree sessions", () => {
+    it("wraps file tools with boundary validation when cwd is a worktree", async () => {
+      const mockReadTool = {
+        name: "read",
+        label: "Read",
+        description: "Read a file",
+        parameters: {},
+        execute: vi.fn().mockResolvedValue({ ok: true, content: [{ type: "text", text: "file content" }] }),
+      };
+
+      // Import the wrapping function
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const tools = [mockReadTool as any];
+
+      // Simulate wrapping (normally done inside createKbAgent)
+      const { wrapToolsWithBoundary } = await import("./pi.js");
+      const wrapped = wrapToolsWithBoundary(
+        tools,
+        "/project/.worktrees/fn-001", // worktree path
+        "/project", // project root
+      );
+
+      // Read inside worktree should work
+      const insideResult = await (wrapped[0] as any).execute("call-1", { path: "/project/.worktrees/fn-001/src/file.ts" });
+      expect(insideResult).toEqual({ ok: true, content: [{ type: "text", text: "file content" }] });
+      expect(mockReadTool.execute).toHaveBeenCalled();
+
+      // Reset mock
+      mockReadTool.execute.mockClear();
+
+      // Read outside worktree should be rejected
+      const outsideResult = await (wrapped[0] as any).execute("call-2", { path: "/other/project/file.ts" });
+      expect(outsideResult).toEqual({
+        ok: false,
+        error: expect.stringContaining("outside the worktree boundary"),
+      });
+      expect(mockReadTool.execute).not.toHaveBeenCalled();
+    });
+
+    it("allows project root .fusion/memory.md from worktree session", async () => {
+      const mockReadTool = {
+        name: "read",
+        label: "Read",
+        description: "Read a file",
+        parameters: {},
+        execute: vi.fn().mockResolvedValue({ ok: true, content: [{ type: "text", text: "memory content" }] }),
+      };
+
+      const { wrapToolsWithBoundary } = await import("./pi.js");
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const wrapped = wrapToolsWithBoundary(
+        [mockReadTool as any],
+        "/project/.worktrees/fn-001",
+        "/project",
+      );
+
+      // Reading project root .fusion/memory.md should be allowed
+      const result = await (wrapped[0] as any).execute("call-1", { path: "/project/.fusion/memory.md" });
+      expect(mockReadTool.execute).toHaveBeenCalled();
+      expect(result).toEqual({ ok: true, content: [{ type: "text", text: "memory content" }] });
+    });
+
+    it("allows task attachments from worktree session", async () => {
+      const mockReadTool = {
+        name: "read",
+        label: "Read",
+        description: "Read a file",
+        parameters: {},
+        execute: vi.fn().mockResolvedValue({ ok: true, content: [{ type: "text", text: "attachment content" }] }),
+      };
+
+      const { wrapToolsWithBoundary } = await import("./pi.js");
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const wrapped = wrapToolsWithBoundary(
+        [mockReadTool as any],
+        "/project/.worktrees/fn-001",
+        "/project",
+      );
+
+      // Reading task attachment should be allowed
+      const result = await (wrapped[0] as any).execute("call-1", { path: "/project/.fusion/tasks/FN-001/attachments/screenshot.png" });
+      expect(mockReadTool.execute).toHaveBeenCalled();
+      expect(result).toEqual({ ok: true, content: [{ type: "text", text: "attachment content" }] });
+    });
+
+    it("does not wrap tools when cwd is not a worktree", async () => {
+      const mockTool = {
+        name: "read",
+        label: "Read",
+        description: "Read a file",
+        parameters: {},
+        execute: vi.fn().mockResolvedValue({ ok: true, content: [] }),
+      };
+
+      const { wrapToolsWithBoundary } = await import("./pi.js");
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const wrapped = wrapToolsWithBoundary([mockTool as any], null, null);
+
+      // Should be the same tool, not wrapped
+      expect(wrapped[0]).toBe(mockTool);
+
+      // Any path should work
+      await (wrapped[0] as any).execute("call-1", { path: "/any/path/file.ts" });
+      expect(mockTool.execute).toHaveBeenCalled();
+    });
+
+    it("wraps only file tools, not other tools", async () => {
+      const mockTaskTool = {
+        name: "task_create",
+        label: "Create Task",
+        description: "Create a task",
+        parameters: {},
+        execute: vi.fn().mockResolvedValue({ ok: true, content: [] }),
+      };
+
+      const { wrapToolsWithBoundary } = await import("./pi.js");
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const wrapped = wrapToolsWithBoundary(
+        [mockTaskTool as any],
+        "/project/.worktrees/fn-001",
+        "/project",
+      );
+
+      // task_create should be unchanged (not wrapped)
+      expect(wrapped[0]).toBe(mockTaskTool);
+    });
+
+    it("rejects write to paths outside worktree", async () => {
+      const mockWriteTool = {
+        name: "write",
+        label: "Write",
+        description: "Write a file",
+        parameters: {},
+        execute: vi.fn().mockResolvedValue({ ok: true, content: [] }),
+      };
+
+      const { wrapToolsWithBoundary } = await import("./pi.js");
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const wrapped = wrapToolsWithBoundary(
+        [mockWriteTool as any],
+        "/project/.worktrees/fn-001",
+        "/project",
+      );
+
+      // Writing outside worktree should be rejected
+      const result = await (wrapped[0] as any).execute("call-1", { path: "/another/project/file.ts" });
+      expect(result).toEqual({
+        ok: false,
+        error: expect.stringContaining("outside the worktree boundary"),
+      });
+      expect(mockWriteTool.execute).not.toHaveBeenCalled();
+    });
+
+    it("rejects bash commands with cwd outside worktree", async () => {
+      const mockBashTool = {
+        name: "bash",
+        label: "Bash",
+        description: "Run a command",
+        parameters: {},
+        execute: vi.fn().mockResolvedValue({ ok: true, content: [] }),
+      };
+
+      const { wrapToolsWithBoundary } = await import("./pi.js");
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const wrapped = wrapToolsWithBoundary(
+        [mockBashTool as any],
+        "/project/.worktrees/fn-001",
+        "/project",
+      );
+
+      // Bash with cwd outside worktree should be rejected
+      const result = await (wrapped[0] as any).execute("call-1", { command: "ls -la", cwd: "/another/project" });
+      expect(result).toEqual({
+        ok: false,
+        error: expect.stringContaining("outside the worktree boundary"),
+      });
+      expect(mockBashTool.execute).not.toHaveBeenCalled();
+    });
+
+    it("allows bash commands without cwd or with cwd inside worktree", async () => {
+      const mockBashTool = {
+        name: "bash",
+        label: "Bash",
+        description: "Run a command",
+        parameters: {},
+        execute: vi.fn().mockResolvedValue({ ok: true, content: [{ type: "text", text: "ls result" }] }),
+      };
+
+      const { wrapToolsWithBoundary } = await import("./pi.js");
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const wrapped = wrapToolsWithBoundary(
+        [mockBashTool as any],
+        "/project/.worktrees/fn-001",
+        "/project",
+      );
+
+      // Bash without cwd should work
+      let result = await (wrapped[0] as any).execute("call-1", { command: "ls -la" });
+      expect(mockBashTool.execute).toHaveBeenCalled();
+
+      mockBashTool.execute.mockClear();
+
+      // Bash with cwd inside worktree should work
+      result = await (wrapped[0] as any).execute("call-2", { command: "ls -la", cwd: "/project/.worktrees/fn-001" });
+      expect(mockBashTool.execute).toHaveBeenCalled();
+    });
+  });
+});
+
 describe("createKbAgent", () => {
  beforeEach(() => {
    vi.clearAllMocks();
--- a/packages/engine/src/pi.ts
+++ b/packages/engine/src/pi.ts
@@ -6,7 +6,7 @@
 */

 import { existsSync, readFileSync } from "node:fs";
-import { join } from "node:path";
+import { join, relative, isAbsolute, resolve } from "node:path";
 import {
  AuthStorage,
  createAgentSession,
@@ -253,6 +253,133 @@ async function registerExtensionProviders(cwd: string, modelRegistry: ModelRegis
  }
 }

+// ── Worktree Path Boundary Helpers ──────────────────────────────────────────
+
+/**
+ * Detect if a path is a task worktree under `.worktrees/`.
+ * Returns the project root if the path is a worktree, otherwise null.
+ *
+ * Examples:
+ *   `/project/.worktrees/fn-001` → `/project`
+ *   `/project/.worktrees/fn-001/src/file.ts` → `/project`
+ *   `/project` → null (not a worktree)
+ */
+function getProjectRootFromWorktree(cwd: string): string | null {
+  // Match paths like /project/.worktrees/task-id or /project/.worktrees/task-id/...
+  const match = cwd.match(/^(.+?)\/\.worktrees\/[^/]+/);
+  if (match) {
+    return match[1]!;
+  }
+  return null;
+}
+
+/**
+ * Check if a path is allowed to be accessed from a worktree session.
+ * Rules:
+ * - Paths inside the worktree are always allowed
+ * - Project root .fusion/memory.md is allowed (for durable project learnings)
+ * - Task attachments under .fusion/tasks/N/attachments/ are allowed (for reading context files)
+ * - All other paths outside the worktree are rejected
+ *
+ * @param worktreePath - Absolute path to the worktree directory
+ * @param projectRoot - Absolute path to the project root (derived from worktree)
+ * @param requestedPath - The path being accessed
+ * @returns true if allowed, false if rejected
+ */
+function isWorktreeAllowedPath(worktreePath: string, projectRoot: string, requestedPath: string): boolean {
+  // Normalize paths
+  const worktreeResolved = resolve(worktreePath);
+  const projectRootResolved = resolve(projectRoot);
+  const requestedResolved = isAbsolute(requestedPath) ? resolve(requestedPath) : resolve(worktreeResolved, requestedPath);
+
+  // Check if path is inside the worktree
+  const relToWorktree = relative(worktreeResolved, requestedResolved);
+  if (!relToWorktree.startsWith("..") && !isAbsolute(relToWorktree)) {
+    return true; // Path is inside the worktree
+  }
+
+  // Exception: project root `.fusion/memory.md` for durable project learnings
+  const relToProjectRoot = relative(projectRootResolved, requestedResolved);
+  if (relToProjectRoot === ".fusion/memory.md") {
+    return true;
+  }
+
+  // Exception: task attachments under `.fusion/tasks/*/attachments/*`
+  if (relToProjectRoot.match(/^\.fusion\/tasks\/[^/]+\/attachments\//)) {
+    return true;
+  }
+
+  // All other paths outside the worktree are rejected
+  return false;
+}
+
+/**
+ * Wrap tools with worktree boundary validation.
+ * When cwd is a worktree path, file operations are validated against worktree boundaries.
+ *
+ * @param tools - Array of tool definitions to wrap
+ * @param worktreePath - Absolute path to the worktree directory (if applicable)
+ * @param projectRoot - Absolute path to the project root (if applicable)
+ * @returns Wrapped tools with boundary validation
+ */
+export function wrapToolsWithBoundary(
+  tools: ToolDefinition[],
+  worktreePath: string | null,
+  projectRoot: string | null,
+): ToolDefinition[] {
+  if (!worktreePath || !projectRoot) {
+    return tools; // Not a worktree session, no wrapping needed
+  }
+
+  return tools.map((tool) => {
+    // Only wrap tools that access the filesystem
+    const fileToolNames = new Set(["read", "write", "edit", "glob", "grep", "bash"]);
+    if (!fileToolNames.has(tool.name)) {
+      return tool;
+    }
+
+    // Store the original execute function
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    const originalExecute = tool.execute as any;
+
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    return {
+      ...tool,
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      execute: async (...args: any[]) => {
+        const toolCallId = args[0] as string;
+        const params = args[1] as Record<string, unknown>;
+        const signal = args[2] as AbortSignal | undefined;
+
+        // Check path argument for file operations
+        const pathArg = params.path as string | undefined;
+        if (pathArg && !isWorktreeAllowedPath(worktreePath, projectRoot, pathArg)) {
+          const relToProject = relative(projectRoot, pathArg);
+          return {
+            ok: false,
+            error: `Path "${relToProject}" is outside the worktree boundary. ` +
+              `Coding agents can only modify files inside the current worktree. ` +
+              `Exception: .fusion/memory.md (project root) and .fusion/tasks/*/attachments/* are permitted for reading.`,
+          };
+        }
+
+        // For bash, also check the working directory if specified
+        const cwdArg = params.cwd as string | undefined;
+        if (tool.name === "bash" && cwdArg && !isWorktreeAllowedPath(worktreePath, projectRoot, cwdArg)) {
+          return {
+            ok: false,
+            error: `Working directory is outside the worktree boundary. ` +
+              `Commands must run inside the worktree.`,
+          };
+        }
+
+        // Call the original tool implementation with all arguments passed through
+        return originalExecute(...args);
+      },
+    };
+  });
+}
+
 /**
 * Create a pi agent session configured for fn.
 * Reuses the user's existing pi auth and model configuration.
@@ -268,6 +395,11 @@ export async function createKbAgent(options: AgentOptions): Promise<AgentResult>
      ? createReadOnlyTools(options.cwd)
      : createCodingTools(options.cwd);

+  // Detect if this is a worktree session and apply path boundaries
+  const worktreePath = options.cwd;
+  const projectRoot = getProjectRootFromWorktree(worktreePath);
+  const wrappedTools = wrapToolsWithBoundary(tools, worktreePath, projectRoot);
+
  // Compaction is explicitly enabled to prevent context-window overflow during
  // long-running agent conversations (triage, execution, review, merge).
  // When the context fills up, pi auto-compacts the conversation history to
@@ -308,7 +440,8 @@ export async function createKbAgent(options: AgentOptions): Promise<AgentResult>
      authStorage,
      modelRegistry,
      resourceLoader,
-      tools,
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      tools: wrappedTools as any,
      customTools: options.customTools,
      sessionManager,
      settingsManager,