diff --git a/.changeset/read-user-agent-skills.md b/.changeset/read-user-agent-skills.md new file mode 100644 index 0000000000..62eb8d09dc --- /dev/null +++ b/.changeset/read-user-agent-skills.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Let worktree agents read skills installed under ~/.agents/skills. +category: fix +dev: Keeps writes, edits, Bash, sibling ~/.agents files, and symlink escapes outside the worktree boundary. diff --git a/docs/agents.md b/docs/agents.md index 9ebe6e5fcc..2f63fcda86 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -963,7 +963,7 @@ Task-detail Chat is included because it is a `task-planner:` ChatManager ### Worktree session file boundary -Pi sessions started in an isolated task worktree reject filesystem paths outside that worktree. The established project-memory and task-attachment exceptions remain unchanged. Separately, when Fusion advertises skill bodies through `AgentOptions.additionalSkillPaths` (including enabled plugin skill roots), it allows only `read`, `glob`, and `grep` to access those exact normalized roots. `write`, `edit`, and Bash working directories remain worktree-bound for skill roots; this is not a general `~/.fusion/plugins` exception. +Pi sessions started in an isolated task worktree reject filesystem paths outside that worktree. The established project-memory and task-attachment exceptions remain unchanged. The standard user Agent Skills root at `~/.agents/skills` is readable from worktree sessions. Separately, when Fusion advertises skill bodies through `AgentOptions.additionalSkillPaths` (including enabled plugin skill roots), it allows only `read`, `glob`, and `grep` to access those exact normalized roots. `write`, `edit`, and Bash working directories remain worktree-bound for skill roots; these are not general `~/.agents` or `~/.fusion/plugins` exceptions. ```bash fn message inbox diff --git a/packages/engine/src/__tests__/pi-create-fn-agent.test.ts b/packages/engine/src/__tests__/pi-create-fn-agent.test.ts index a851aaf22d..9d5791105b 100644 --- a/packages/engine/src/__tests__/pi-create-fn-agent.test.ts +++ b/packages/engine/src/__tests__/pi-create-fn-agent.test.ts @@ -1,5 +1,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { PathLike } from "node:fs"; +import { homedir } from "node:os"; +import { join } from "node:path"; const createAgentSessionMock = vi.fn(); const createBashToolMock = vi.fn((cwd: string, options?: any) => ({ name: "bash", cwd, options })); @@ -418,6 +420,70 @@ describe("worktree path boundary helpers", () => { expect(mockBashTool.execute).toHaveBeenCalled(); }); + it("rejects symlink escapes through every allowed root across all path-taking tools", async () => { + const makeTool = (name: string) => ({ + name, + label: name, + description: `${name} a boundary path`, + parameters: {}, + execute: vi.fn().mockResolvedValue({ ok: true, content: [] }), + }); + /* + FNXC:WorktreeBoundary 2026-08-23-03:44: + Symlink-escape coverage must include every filesystem wrapper plus both Bash path surfaces. Read-only aliases and verification cwd checks are part of the same boundary invariant, not optional follow-up cases. + */ + const toolNames = ["read", "glob", "grep", "find", "ls", "write", "edit", "bash", "fn_run_verification"]; + const tools = toolNames.map(makeTool); + const worktreeRoot = "/project/.worktrees/fn-001"; + const projectRoot = "/project"; + const hostSkillRoot = "/host/skills"; + const userSkillRoot = join(homedir(), ".agents", "skills"); + const externalRoot = "/host/private"; + const escapeCases = [ + { symlink: `${worktreeRoot}/escape`, path: `${worktreeRoot}/escape/secret.txt` }, + { symlink: `${projectRoot}/.fusion/memory/escape`, path: `${projectRoot}/.fusion/memory/escape/secret.txt` }, + { symlink: `${projectRoot}/.fusion/tasks/FN-001/attachments/escape`, path: `${projectRoot}/.fusion/tasks/FN-001/attachments/escape/secret.txt` }, + { symlink: `${projectRoot}/.fusion/tasks/FN-002`, path: `${projectRoot}/.fusion/tasks/FN-002/PROMPT.md` }, + { symlink: `${userSkillRoot}/escape`, path: `${userSkillRoot}/escape/secret.txt` }, + { symlink: `${hostSkillRoot}/escape`, path: `${hostSkillRoot}/escape/secret.txt` }, + ]; + const escapedPaths = new Set(escapeCases.map(({ path }) => path)); + const symlinkTargets = new Map(escapeCases.map(({ symlink }) => [symlink, externalRoot])); + realpathSyncNativeMock.mockImplementation((path: PathLike) => { + const text = String(path); + if (escapedPaths.has(text)) throw new Error("ENOENT"); + return symlinkTargets.get(text) ?? text; + }); + + const { wrapToolsWithBoundary } = await import("../pi.js"); + const wrapped = wrapToolsWithBoundary(tools as any, worktreeRoot, projectRoot, [hostSkillRoot]); + + for (const { symlink, path } of escapeCases) { + for (const tool of wrapped as any[]) { + const params = tool.name === "bash" + ? { command: "pwd", cwd: symlink } + : tool.name === "fn_run_verification" + ? { command: "pnpm test", cwd: symlink } + : { path }; + const result = await tool.execute(`call-${tool.name}-${symlink}`, params); + expect(result).toMatchObject({ ok: false, error: expect.stringContaining("outside the worktree boundary") }); + } + + const wrappedBash = (wrapped as any[]).find((tool) => tool.name === "bash"); + const commandTargetResult = await wrappedBash.execute(`call-bash-command-${symlink}`, { + command: `cd ${symlink} && pwd`, + cwd: worktreeRoot, + }); + expect(commandTargetResult).toMatchObject({ + ok: false, + error: expect.stringContaining("outside the worktree boundary"), + }); + } + for (const tool of tools) { + expect(tool.execute).not.toHaveBeenCalled(); + } + }); + it("allows project root .fusion/memory/ files from worktree session", async () => { const mockReadTool = { name: "read", @@ -569,6 +635,82 @@ describe("worktree path boundary helpers", () => { expect(readTool.execute).toHaveBeenCalledOnce(); }); + it("allows read/glob/grep/find/ls under the standard user agent skill root without symlink escapes", async () => { + const makeTool = (name: string) => ({ + name, + label: name, + description: `${name} a user skill file`, + parameters: {}, + execute: vi.fn().mockResolvedValue({ ok: true, content: [] }), + }); + const userAgentRoot = join(homedir(), ".agents"); + const userSkillRoot = join(userAgentRoot, "skills"); + const skillPath = join(userSkillRoot, "code-review", "SKILL.md"); + /* + FNXC:SkillReadBoundary 2026-08-23-03:54: + Every read-only filesystem alias must share the user-skill allowance and symlink-escape denial. Cover `find` and `ls` alongside `read`, `glob`, and `grep` so aliases cannot drift into a broader or narrower host boundary. + */ + const [readTool, globTool, grepTool, findTool, lsTool, writeTool, editTool, bashTool] = [ + makeTool("read"), + makeTool("glob"), + makeTool("grep"), + makeTool("find"), + makeTool("ls"), + makeTool("write"), + makeTool("edit"), + makeTool("bash"), + ]; + const { wrapToolsWithBoundary } = await import("../pi.js"); + const wrapped = wrapToolsWithBoundary( + [readTool, globTool, grepTool, findTool, lsTool, writeTool, editTool, bashTool] as any, + "/project/.worktrees/fn-user-skills", + "/project", + ); + + for (const tool of wrapped.slice(0, 5) as any[]) { + await tool.execute(`call-${tool.name}`, { path: skillPath }); + } + expect(readTool.execute).toHaveBeenCalledOnce(); + expect(globTool.execute).toHaveBeenCalledOnce(); + expect(grepTool.execute).toHaveBeenCalledOnce(); + expect(findTool.execute).toHaveBeenCalledOnce(); + expect(lsTool.execute).toHaveBeenCalledOnce(); + + for (const tool of wrapped.slice(5, 7) as any[]) { + const result = await tool.execute(`call-${tool.name}`, { path: skillPath }); + expect(result).toMatchObject({ ok: false, error: expect.stringContaining("outside the worktree boundary") }); + } + expect(writeTool.execute).not.toHaveBeenCalled(); + expect(editTool.execute).not.toHaveBeenCalled(); + + const bashResult = await (wrapped[7] as any).execute("call-bash", { command: "pwd", cwd: userSkillRoot }); + expect(bashResult).toMatchObject({ ok: false, error: expect.stringContaining("outside the worktree boundary") }); + expect(bashTool.execute).not.toHaveBeenCalled(); + + const siblingConfigResult = await (wrapped[0] as any).execute("call-config", { + path: join(userAgentRoot, "config.json"), + }); + expect(siblingConfigResult).toMatchObject({ ok: false, error: expect.stringContaining("outside the worktree boundary") }); + expect(readTool.execute).toHaveBeenCalledOnce(); + + const symlinkDir = join(userSkillRoot, "linked-config"); + const symlinkEscapePath = join(symlinkDir, "config.json"); + realpathSyncNativeMock.mockImplementation((path: PathLike) => { + const text = String(path); + if (text === symlinkEscapePath) throw new Error("ENOENT"); + return text === symlinkDir ? userAgentRoot : text; + }); + for (const tool of wrapped.slice(0, 5) as any[]) { + const result = await tool.execute(`call-${tool.name}-symlink`, { path: symlinkEscapePath }); + expect(result).toMatchObject({ ok: false, error: expect.stringContaining("outside the worktree boundary") }); + } + expect(readTool.execute).toHaveBeenCalledOnce(); + expect(globTool.execute).toHaveBeenCalledOnce(); + expect(grepTool.execute).toHaveBeenCalledOnce(); + expect(findTool.execute).toHaveBeenCalledOnce(); + expect(lsTool.execute).toHaveBeenCalledOnce(); + }); + it("rejects host skill paths when no read-only extra roots are provided", async () => { const mockReadTool = { name: "read", diff --git a/packages/engine/src/pi.ts b/packages/engine/src/pi.ts index fef6cd988b..2c23923076 100644 --- a/packages/engine/src/pi.ts +++ b/packages/engine/src/pi.ts @@ -11,6 +11,7 @@ import { exec, execFile } from "node:child_process"; import { promisify } from "node:util"; import { createRequire } from "node:module"; import { randomUUID } from "node:crypto"; +import { homedir } from "node:os"; import { basename, dirname, join, relative, isAbsolute, resolve } from "node:path"; const execAsync = promisify(exec); @@ -1660,6 +1661,22 @@ function normalizeExistingPathForGitComparison(path: string): string { } } +function normalizePathThroughExistingAncestor(path: string): string { + const resolvedPath = resolve(path); + let existingAncestor = resolvedPath; + + while (true) { + try { + const canonicalAncestor = realpathSync.native(existingAncestor); + return resolve(canonicalAncestor, relative(existingAncestor, resolvedPath)); + } catch { + const parent = dirname(existingAncestor); + if (parent === existingAncestor) return resolvedPath; + existingAncestor = parent; + } + } +} + /** * FNXC:SkillReadBoundary 2026-07-21-12:00: * GitHub #2384 / FN-8466 requires the exact host-advertised additional skill @@ -1771,6 +1788,7 @@ export async function resolveSessionBoundaryRoot( * - Task attachments under .fusion/tasks/N/attachments/ are allowed (for reading context files) * - Sibling task specs (.fusion/tasks/N/PROMPT.md and task.json) are allowed for * read-only tools (read/glob/grep) so agents can consult dependency specs. + * - User skills under ~/.agents/skills are allowed for read-only tools only. * - Host-advertised additional skill roots are allowed for read-only tools only. * - All other paths outside the worktree are rejected * @@ -1794,32 +1812,29 @@ function isWorktreeAllowedPath( const requestedResolved = isAbsolute(requestedPath) ? resolve(requestedPath) : resolve(worktreeResolved, requestedPath); const worktreeCanonical = normalizeExistingPathForGitComparison(worktreeResolved); const projectRootCanonical = normalizeExistingPathForGitComparison(projectRootResolved); - const requestedCanonical = normalizeExistingPathForGitComparison(requestedResolved); + const requestedCanonical = normalizePathThroughExistingAncestor(requestedResolved); + /* + FNXC:WorktreeBoundary 2026-08-22-02:52: + Every worktree and project exception must use canonical containment only. A lexical path beneath an allowed root can cross a symlink to host files, including when the final glob/write target does not exist yet; normalize through the deepest existing ancestor before deciding. + */ // Check if path is inside the worktree - if ( - isSameOrInsidePath(worktreeResolved, requestedResolved) || - isSameOrInsidePath(worktreeCanonical, requestedCanonical) - ) { + if (isSameOrInsidePath(worktreeCanonical, requestedCanonical)) { return true; // Path is inside the worktree } // Exception: project root `.fusion/memory/` files for durable project learnings - const relToProjectRoot = relative(projectRootResolved, requestedResolved).replace(/\\/g, "/"); const relToCanonicalProjectRoot = relative(projectRootCanonical, requestedCanonical).replace(/\\/g, "/"); - const projectRelativePaths = [relToProjectRoot, relToCanonicalProjectRoot]; if ( - projectRelativePaths.some((relPath) => - relPath === ".fusion/memory" || - relPath === ".fusion/memory/" || - relPath.startsWith(".fusion/memory/") - ) + relToCanonicalProjectRoot === ".fusion/memory" || + relToCanonicalProjectRoot === ".fusion/memory/" || + relToCanonicalProjectRoot.startsWith(".fusion/memory/") ) { return true; } // Exception: task attachments under `.fusion/tasks/*/attachments/*` - if (projectRelativePaths.some((relPath) => relPath.match(/^\.fusion\/tasks\/[^/]+\/attachments\//))) { + if (relToCanonicalProjectRoot.match(/^\.fusion\/tasks\/[^/]+\/attachments\//)) { return true; } @@ -1829,7 +1844,7 @@ function isWorktreeAllowedPath( // the agent can discover them; writes and bash remain restricted. const readOnlyTools = new Set(["read", "glob", "grep", "find", "ls"]); if (toolName && readOnlyTools.has(toolName)) { - if (projectRelativePaths.some((relPath) => /^\.fusion\/tasks\/[^/]+\/(PROMPT\.md|task\.json)$/.test(relPath))) { + if (/^\.fusion\/tasks\/[^/]+\/(PROMPT\.md|task\.json)$/.test(relToCanonicalProjectRoot)) { return true; } @@ -1838,12 +1853,17 @@ function isWorktreeAllowedPath( GitHub #2384 / FN-8466 lets agents Read only the specific additional skill roots advertised by this session. Do not extend this exception to write, edit, or bash: plugin skill bodies remain host-owned read-only context. + + FNXC:SkillReadBoundary 2026-08-22-09:37: + Skill-root containment must compare canonical paths only. A lexical path + beneath an allowed root can traverse a symlink whose real target is outside + that root; canonicalizing the deepest existing ancestor also closes this + escape for glob paths and nonexistent descendants. */ if (readOnlyExtraRoots.some((root) => { const rootResolved = resolve(root); - const rootCanonical = normalizeExistingPathForGitComparison(rootResolved); - return isSameOrInsidePath(rootResolved, requestedResolved) - || isSameOrInsidePath(rootCanonical, requestedCanonical); + const rootCanonical = normalizePathThroughExistingAncestor(rootResolved); + return isSameOrInsidePath(rootCanonical, requestedCanonical); })) { return true; } @@ -1944,7 +1964,17 @@ export function wrapToolsWithBoundary( return tools; // Not a worktree session, no wrapping needed } - const normalizedReadOnlyExtraRoots = normalizeAdditionalSkillPaths(readOnlyExtraRoots); + /* + FNXC:SkillReadBoundary 2026-08-22-09:20: + Agent Skills installs reusable user skills under ~/.agents/skills. Worktree + sessions must be able to read those skill bodies and references, but the + exception must not expose sibling ~/.agents configuration or permit writes, + edits, or Bash outside the worktree. + */ + const normalizedReadOnlyExtraRoots = normalizeAdditionalSkillPaths([ + join(homedir(), ".agents", "skills"), + ...readOnlyExtraRoots, + ]); return tools.map((tool) => { // Only wrap tools that access the filesystem @@ -1975,9 +2005,9 @@ export function wrapToolsWithBoundary( const relToProject = relative(projectRoot, pathArg); return boundaryRejection( `Path "${relToProject}" is outside the worktree boundary. ` + - `Coding agents can only modify files inside the current worktree. ` + + `Coding agents can only access files inside the current worktree. ` + `Existing exceptions include .fusion/memory/ and task attachments; ` + - `read-only tools may also access sibling task specs and host-advertised skill roots.`, + `read-only tools may also access sibling task specs, ~/.agents/skills, and host-advertised skill roots.`, ); }