fix: allow worktree agents to read user skills (#3506)
## Summary - allow worktree sessions to read the standard user skill root at `~/.agents/skills` - keep sibling `~/.agents` files and all write/edit/Bash access outside the exception - canonicalize existing path components so symlinks cannot escape an allowed skill root - document the boundary and add a patch changeset This extends the same host-skill consistency fixed in #2384: Fusion should not tell an agent to load a skill and then block the skill body. ## Test plan - [x] 15 worktree-boundary tests - [x] `pnpm --filter @fusion/engine typecheck` - [x] scoped ESLint - [x] changeset and FNXC date checks - [x] `pnpm verify:fast` (20 steps, including build and boot smoke) - [x] CLI CI-shape test (72 tests) ## Local gate notes `pnpm test:gate` passed all static checks, 432 engine-core tests, and 184 core unit tests. Its PostgreSQL lane could not authenticate locally (`empty password returned by client`). The full `pi-create-fn-agent.test.ts` run also reaches an unrelated dashboard-chat principal assertion failure already present at the exact `origin/main` SHA; the 15 boundary tests pass. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Worktree agents can read and search skills installed in the standard `~/.agents/skills` directory. - **Bug Fixes** - Preserved worktree protections for writing, editing, and Bash operations. - Blocked access to unrelated files and prevented symlink-based boundary escapes across supported path operations. - Improved access validation for paths that do not yet exist. - **Documentation** - Updated worktree boundary documentation to describe skill access and its restrictions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Phil Larson
7
.changeset/read-user-agent-skills.md
Normal file
7
.changeset/read-user-agent-skills.md
Normal file
@@ -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.
|
||||
@@ -963,7 +963,7 @@ Task-detail Chat is included because it is a `task-planner:<taskId>` 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
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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.`,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user