diff --git a/doc/architecture/runner-pi-capabilities.md b/doc/architecture/runner-pi-capabilities.md index f27b4897495..35d088f830c 100644 --- a/doc/architecture/runner-pi-capabilities.md +++ b/doc/architecture/runner-pi-capabilities.md @@ -21,7 +21,7 @@ boundaries explicit. | Sessions | Native Pi JSONL create/load; resume is confined to the execution's private session home and original workspace. Cold `prompt` cannot implicitly load an arbitrary session. | | Text and reasoning | Upstream streams text and thought chunks. The common runner's private reasoning policy still applies; provider support does not authorize retention or display. | | Native tools | `read`, `grep`, `find`, `ls`, `write`, `edit`, and `bash` pass the immutable extension gate before execution. File roots and read-only mode are checked before and after a permission wait. The host sandbox remains authoritative for shell commands and races. | -| Permissions | Native tool approval uses ACP `session/request_permission`, with allow once, allow for the session, and deny. Only offered options are accepted. Questions never become approvals. | +| Permissions | Native tool approval uses ACP `session/request_permission`, with allow once, allow for the session, and deny. Only offered options are accepted. Session grants cover an identical operation and still revalidate paths. Questions never become approvals. | | Questions | Pi `select`, `confirm`, `input`, and `editor` map to ACP form elicitation with typed schemas. Decline, cancellation, timeout, malformed replies, and duplicate/late replies cannot become accepted answers. | | Semantic tools | Runner-owned loopback HTTP MCP catalogs register under exact `mcp____` names. Calls retain the native tool call ID and cancellation signal. Authenticated PRP tool handling owns semantic authorization and durable interactions. Ambient MCP and external MCP servers are not admitted. | | Plans and artifacts | Pi has no native structured plan or artifact channel. Paperclip plan and artifact semantic tools remain available through the MCP bridge; native file edits retain bounded, workspace-confined ACP diff projection. Tool text/image results are preserved, and resource blocks are recorded without following URLs. | @@ -62,7 +62,10 @@ without a shell or PATH lookup. The common command lease must supply: - `PI_CODING_AGENT_DIR`: isolated persisted session/auth/settings home. - `PAPERCLIP_PI_READ_ONLY`: exactly `0` or `1`. - `PAPERCLIP_PI_READ_ROOTS` and `PAPERCLIP_PI_PROTECTED_ROOTS`: JSON arrays of - absolute allowed skill/read roots and protected runtime/config roots. + absolute allowed skill/read roots and protected runtime/config roots. Assigned + skills must use a separate immutable lease outside the protected runtime and + executable roots; overlapping roots fail admission. Skill roots are always + immutable, even when nested inside a writable workspace. - `PAPERCLIP_PI_SYSTEM_INSTRUCTIONS`: admitted instructions, bounded to 32 KiB. The wrapper synthesizes `PAPERCLIP_PI_RUNTIME_CONFIGURATION` for its child, diff --git a/packages/paperclip-runner/src/drivers/acpx/pi-runtime-extension.test.ts b/packages/paperclip-runner/src/drivers/acpx/pi-runtime-extension.test.ts index 3f3d86cf8a5..6497c9e6660 100644 --- a/packages/paperclip-runner/src/drivers/acpx/pi-runtime-extension.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/pi-runtime-extension.test.ts @@ -70,7 +70,36 @@ describe("owned Pi runtime extension", () => { await writeFile(join(config.readRoots[0]!, "SKILL.md"), "assigned"); const context = { cwd: config.workspace, ui: { select: vi.fn() } }; expect(await checkPiNativeTool({ toolName: "read", toolCallId: "r", input: { path: join(config.readRoots[0]!, "SKILL.md") } }, context, config)).toBeNull(); - expect(await checkPiNativeTool({ toolName: "write", toolCallId: "w", input: { path: join(config.readRoots[0]!, "SKILL.md") } }, context, config)).toMatch("outside"); + expect(await checkPiNativeTool({ toolName: "write", toolCallId: "w", input: { path: join(config.readRoots[0]!, "SKILL.md") } }, context, config)).toMatch("read-only"); + }); + + it("keeps workspace-nested skills immutable and refuses protected-root overlap", async () => { + const { config } = await workspace(); + const assigned = join(config.workspace, "assigned"); await mkdir(assigned); + config.readRoots = [assigned]; + const context = { cwd: config.workspace, ui: { select: vi.fn() } }; + expect(await checkPiNativeTool({ toolName: "write", toolCallId: "w", input: { path: join(assigned, "SKILL.md") } }, context, config)).toMatch("read-only"); + config.readRoots = [config.protectedRoots[0]!]; + const h = harness(); + await expect(installPiRuntimeExtension(h.api, config)).rejects.toThrow("overlap"); + expect(h.api.registerCommand).not.toHaveBeenCalled(); + }); + + it("limits session grants to identical operations and rechecks their paths", async () => { + const { config, root } = await workspace(); const h = harness(); + await installPiRuntimeExtension(h.api, config); + const select = vi.fn().mockResolvedValue("Allow for this session"); + const context = { cwd: config.workspace, ui: { select } }; + const tool = h.handlers.get("tool_call")!; + const input = { path: "target/new" }; + expect(await tool({ toolName: "write", toolCallId: "a", input }, context)).toBeUndefined(); + expect(await tool({ toolName: "write", toolCallId: "b", input }, context)).toBeUndefined(); + expect(select).toHaveBeenCalledTimes(1); + expect(await tool({ toolName: "write", toolCallId: "c", input: { path: "different" } }, context)).toBeUndefined(); + expect(select).toHaveBeenCalledTimes(2); + await symlink(join(root, "private"), join(config.workspace, "target")); + expect(await tool({ toolName: "write", toolCallId: "d", input }, context)).toMatchObject({ block: true }); + expect(select).toHaveBeenCalledTimes(2); }); it("registers exact authenticated MCP tools with stable idempotency and abort signal", async () => { diff --git a/packages/paperclip-runner/src/drivers/acpx/pi-runtime-extension.ts b/packages/paperclip-runner/src/drivers/acpx/pi-runtime-extension.ts index 4965e1b2d42..2a1e0754f54 100644 --- a/packages/paperclip-runner/src/drivers/acpx/pi-runtime-extension.ts +++ b/packages/paperclip-runner/src/drivers/acpx/pi-runtime-extension.ts @@ -142,6 +142,11 @@ export async function checkPiNativeTool( if (typeof pathValue !== "string" || /[\0\r\n]/.test(pathValue)) return "Pi tool path is invalid"; const logical = resolve(config.workspace, pathValue); const target = await physicalPath(logical); + if (!readTools.has(event.toolName)) { + for (const root of config.readRoots) { + if (inside(root, logical) || inside(await realpath(root), target)) return "Assigned Pi skills are read-only"; + } + } for (const root of config.protectedRoots) { if (inside(root, logical) || inside(await physicalPath(root), target)) return "Pi tool targets protected runtime state"; } @@ -196,6 +201,18 @@ export async function installPiRuntimeExtension( // Registered names are held in this closure; provider-originated metadata // cannot promote a tool to the authenticated Paperclip bridge. const bridgeTools = new Set(); + const permissionGrants = new Set(); + // Assigned skills are a separate immutable lease. A config or executable + // directory must never gain readability by being presented as a skill root. + for (const root of config.readRoots) { + const physicalRoot = await realpath(root); + for (const protectedRoot of config.protectedRoots) { + const physicalProtected = await physicalPath(protectedRoot); + if (inside(protectedRoot, root) || inside(root, protectedRoot) || inside(physicalProtected, physicalRoot) || inside(physicalRoot, physicalProtected)) { + throw new Error("Pi assigned skills overlap protected runtime state"); + } + } + } pi.on("project_trust", () => ({ trusted: "no", remember: false })); pi.on("user_bash", () => ({ result: { output: "Interactive shell commands are disabled in Paperclip Runner", exitCode: 1, cancelled: false, truncated: false } })); pi.on("before_agent_start", (event) => config.instructions @@ -207,10 +224,15 @@ export async function installPiRuntimeExtension( if (denial) return { block: true, reason: denial }; const detail = JSON.stringify({ toolCallId: event.toolCallId, toolName: event.toolName, input: event.input }); if (Buffer.byteLength(detail) > 48 * 1024) return { block: true, reason: "Pi permission request is oversized" }; - const selection = await context.ui.select(`${PI_PERMISSION_TITLE_PREFIX}${detail}`, [...PI_PERMISSION_OPTIONS]); + // A session grant covers only the identical operation, never a whole + // tool class or a subsequent path. Paths are still revalidated each time. + const grantKey = JSON.stringify([event.toolName, event.input]); + const selection = permissionGrants.has(grantKey) ? "Allow for this session" + : await context.ui.select(`${PI_PERMISSION_TITLE_PREFIX}${detail}`, [...PI_PERMISSION_OPTIONS]); if (selection !== "Allow once" && selection !== "Allow for this session") return { block: true, reason: "Pi operation was denied or cancelled" }; // Re-check file bindings after a human wait; approval never freezes paths. const changed = await checkPiNativeTool(event, context, config); + if (!changed && selection === "Allow for this session" && permissionGrants.size < 4096) permissionGrants.add(grantKey); return changed ? { block: true, reason: changed } : undefined; } catch { return { block: true, reason: "Pi permission boundary is unavailable" }; } });