From 56daf4664d06f3a7720e61fb3dd0342c542397eb Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 8 Sep 2026 15:05:28 -0500 Subject: [PATCH] fix(runner): expose Pi semantic tools and preserve projected Git credentials --- doc/sandbox-work-folders.md | 9 +- .../src/drivers/acpx/environment.test.ts | 37 ++++- .../src/drivers/acpx/environment.ts | 5 + .../drivers/acpx/pi-tool-extension.test.ts | 126 ++++++++++++++++++ .../src/drivers/acpx/pi-tool-extension.ts | 92 +++++++++++++ .../src/drivers/acpx/runtime-host.test.ts | 17 ++- .../src/drivers/acpx/runtime-host.ts | 8 +- .../src/drivers/acpx/runtime-sandbox.test.ts | 13 ++ .../src/drivers/acpx/runtime-sandbox.ts | 9 ++ 9 files changed, 309 insertions(+), 7 deletions(-) create mode 100644 packages/paperclip-runner/src/drivers/acpx/pi-tool-extension.test.ts create mode 100644 packages/paperclip-runner/src/drivers/acpx/pi-tool-extension.ts diff --git a/doc/sandbox-work-folders.md b/doc/sandbox-work-folders.md index 4e39225aeb..91638f93c9 100644 --- a/doc/sandbox-work-folders.md +++ b/doc/sandbox-work-folders.md @@ -37,6 +37,11 @@ launcher. It does not make a model request. Live staging must still verify the qualified OpenRouter model, work folders, saves, and recovery. The same image exposes this Pi executable to the legacy adapter. Provider-pack shims resolve links before locating their runtime so task-local launch paths remain valid. +Because this pinned Pi ACP adapter does not forward MCP tools, the native runner +stages a private Pi extension that reads the authenticated run-owned tool catalog. +It preserves tool schemas, idempotent call IDs, cancellation, and the bridge's +private-tool boundary. The bridge credential is injected at launch and excluded +from persisted environment records; project extensions remain untrusted. Sandbox runs use the operating-system user's home directory. Both legacy adapters and the native runner enter the same host-owned lifecycle before @@ -46,7 +51,9 @@ in the sandbox home. For API-key Codex ACP runs, the adapter writes the explicit key to an owner-only login file in the staging copy; host credentials stay unchanged. Per-run GitHub launchers declare their own CommonJS package scope so warm runs inside ES-module repositories can still execute Git and GitHub CLI commands. Native runner launches carry the validated scoped paths -through runnerd to ACPX; CLI configuration remains in its private runtime directories. +through runnerd to ACPX, including the explicitly controller-projected GitHub +broker environment. ACPX does not inherit ambient host GitHub credentials or +shell startup hooks. CLI configuration remains in its private runtime directories. Warm sandbox task bindings persist independently of the experimental isolated workspace setting. Only the active host run can establish that binding; the setting still controls user-configurable worktree operations. diff --git a/packages/paperclip-runner/src/drivers/acpx/environment.test.ts b/packages/paperclip-runner/src/drivers/acpx/environment.test.ts index 03e7d00aae..5d49a99970 100644 --- a/packages/paperclip-runner/src/drivers/acpx/environment.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/environment.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { createSanitizedAcpxSpawnInput } from "./environment.js"; @@ -45,6 +45,41 @@ describe("ACPX launch environment", () => { expect(Object.isFrozen(codex.env)).toBe(true); }); + it.each(["codex", "claude", "pi"] as const)( + "preserves controller-projected GitHub credentials for %s", + (agent) => { + const projected = { + PAPERCLIP_GITHUB_BROKER_URL: "http://127.0.0.1:1234", + PAPERCLIP_GITHUB_BROKER_TOKEN: "run-scoped-secret", + PAPERCLIP_GITHUB_LAUNCHER_DIR: "/private/github", + BASH_ENV: "/private/github/bashrc", + GIT_CONFIG_COUNT: "1", + GIT_CONFIG_KEY_0: "credential.helper", + GIT_CONFIG_VALUE_0: "/private/helper", + }; + const input = createSanitizedAcpxSpawnInput({ + ...projected, + GIT_CONFIG_KEY_1: "not.projected", + GIT_CONFIG_VALUE_1: "not-visible", + }, agent); + expect(input.env).toEqual(projected); + }, + ); + + it("does not inherit ambient host GitHub credentials or shell hooks", () => { + vi.stubEnv("PAPERCLIP_GITHUB_BROKER_TOKEN", "host-secret"); + vi.stubEnv("GITHUB_TOKEN", "host-secret"); + vi.stubEnv("BASH_ENV", "/host/startup"); + try { + const { env } = createSanitizedAcpxSpawnInput(undefined, "pi"); + expect(env).not.toHaveProperty("PAPERCLIP_GITHUB_BROKER_TOKEN"); + expect(env).not.toHaveProperty("GITHUB_TOKEN"); + expect(env).not.toHaveProperty("BASH_ENV"); + } finally { + vi.unstubAllEnvs(); + } + }); + it("rejects unsafe or unbounded retained values", () => { expect(() => createSanitizedAcpxSpawnInput({ PATH: "bad\0path" }, "codex"), diff --git a/packages/paperclip-runner/src/drivers/acpx/environment.ts b/packages/paperclip-runner/src/drivers/acpx/environment.ts index 4c2f7e285a..f9590cc8a1 100644 --- a/packages/paperclip-runner/src/drivers/acpx/environment.ts +++ b/packages/paperclip-runner/src/drivers/acpx/environment.ts @@ -1,3 +1,4 @@ +import { githubCredentialEnvironment } from "../../github-credential-environment.js"; import type { QualifiedAcpxAgent } from "./qualified-profiles.js"; import { externalWorkFolderEnvironment } from "../../work-folder-environment.js"; @@ -25,6 +26,9 @@ export function createSanitizedAcpxSpawnInput( ): SanitizedAcpxSpawnInput { const source = environment ?? process.env; const scoped = externalWorkFolderEnvironment(source); + // Only explicit controller input may carry repository credentials. Never + // discover GitHub credentials or shell startup hooks in the host environment. + const github = environment ? githubCredentialEnvironment(environment) : {}; const result: NodeJS.ProcessEnv = {}; const credentialNames = agent === "pi" @@ -59,6 +63,7 @@ export function createSanitizedAcpxSpawnInput( "PAPERCLIP_NATIVE_MCP_URL", ...credentialNames, ...Object.keys(scoped), + ...Object.keys(github), ]); let retainedBytes = 0; for (const [key, value] of Object.entries({ ...source, ...scoped })) { diff --git a/packages/paperclip-runner/src/drivers/acpx/pi-tool-extension.test.ts b/packages/paperclip-runner/src/drivers/acpx/pi-tool-extension.test.ts new file mode 100644 index 0000000000..576e3fca4b --- /dev/null +++ b/packages/paperclip-runner/src/drivers/acpx/pi-tool-extension.test.ts @@ -0,0 +1,126 @@ +import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { DefaultResourceLoader } from "@earendil-works/pi-coding-agent"; +import { startRunnerToolBridge, type RunnerToolBridge } from "../runner-tool-bridge.js"; +import { PRP_COMPLETION_TOOL_NAME } from "../../contracts/completion-result.js"; +import { PI_RUNNER_TOOL_EXTENSION } from "./pi-tool-extension.js"; + +const directories: string[] = []; +const bridges: RunnerToolBridge[] = []; +afterEach(async () => { + vi.unstubAllEnvs(); + await Promise.all(bridges.splice(0).map(bridge => bridge.close())); + await Promise.all(directories.splice(0).map(dir => rm(dir, { recursive: true, force: true }))); +}); + +async function loadExtension(bridge?: RunnerToolBridge) { + const root = await mkdtemp(join(tmpdir(), "paperclip-pi-tools-")); + directories.push(root); + const agentDir = join(root, "private-pi"); + const cwd = join(root, "workspace"); + await mkdir(join(agentDir, "extensions"), { recursive: true }); + await mkdir(join(cwd, ".pi", "extensions"), { recursive: true }); + await writeFile(join(agentDir, "extensions", "paperclip-runner-tools.js"), PI_RUNNER_TOOL_EXTENSION); + await writeFile(join(agentDir, "settings.json"), JSON.stringify({ defaultProjectTrust: "never" })); + await writeFile(join(cwd, ".pi", "extensions", "untrusted.js"), 'export default function() { throw new Error("PROJECT_EXTENSION_LOADED"); }'); + vi.stubEnv("PAPERCLIP_PI_TOOL_BRIDGE_URL", bridge?.url); + vi.stubEnv("PAPERCLIP_PI_TOOL_BRIDGE_TOKEN", bridge?.secret); + const loader = new DefaultResourceLoader({ cwd, agentDir, noSkills: true, noPromptTemplates: true, noThemes: true, noContextFiles: true }); + await loader.reload({ resolveProjectTrust: async () => false }); + return loader.getExtensions(); +} + +const definition = { + name: "read_cached_file", + description: "Read a cached file", + inputSchema: { + type: "object", properties: { path: { type: "string" } }, + required: ["path"], additionalProperties: false, + }, +}; + +describe("Pi runner tool extension", () => { + it("loads through the pinned Pi runtime, preserves schemas and uses the authenticated bridge", async () => { + const handler = vi.fn(async () => ({ bytes: "saved-content" })); + const bridge = await startRunnerToolBridge({ + tools: [definition], + privateTools: [{ ...definition, name: "private_control" }], + handler, + }); + bridges.push(bridge); + const loaded = await loadExtension(bridge); + expect(loaded.errors).toEqual([]); + expect(loaded.extensions).toHaveLength(1); + const tools = loaded.extensions[0]!.tools; + expect(tools.has("private_control")).toBe(false); + expect([...tools.keys()]).toContain(PRP_COMPLETION_TOOL_NAME); + const tool = tools.get(definition.name)!.definition; + expect(tool.parameters).toEqual(definition.inputSchema); + const execute = (id: string, args: unknown) => tool.execute(id, args, undefined, undefined, {} as never); + expect(await execute("same-call", { path: "file.txt" })).toMatchObject({ + content: [{ type: "text", text: '{"bytes":"saved-content"}' }], + }); + await execute("same-call", { path: "file.txt" }); + expect(handler).toHaveBeenCalledTimes(1); + await expect(execute("same-call", { path: "different.txt" })).rejects.toThrow("Duplicate call identity conflict"); + await expect(execute("invalid-input", { path: 123 })).rejects.toThrow("Invalid tool input"); + expect(handler).toHaveBeenCalledTimes(1); + await bridge.close(); + await expect(execute("closed-bridge", { path: "file.txt" })).rejects.toThrow(); + }); + + it("cancels the admitted server operation when Pi aborts the tool", async () => { + let started!: () => void; + const admitted = new Promise(resolve => { started = resolve; }); + let serverSignal: AbortSignal | undefined; + const bridge = await startRunnerToolBridge({ tools: [definition], handler: async ({ signal }) => { + serverSignal = signal; + started(); + return new Promise((_resolve, reject) => signal.addEventListener("abort", () => reject(new Error("cancelled")), { once: true })); + } }); + bridges.push(bridge); + const loaded = await loadExtension(bridge); + expect(loaded.errors).toEqual([]); + const tool = loaded.extensions[0]!.tools.get(definition.name)!.definition; + const controller = new AbortController(); + const call = tool.execute("cancel-me", { path: "file.txt" }, controller.signal, undefined, {} as never); + const rejected = expect(call).rejects.toThrow(); + await admitted; + controller.abort(); + await rejected; + await vi.waitFor(() => expect(serverSignal?.aborted).toBe(true)); + }); + + it("rejects oversized requests and responses with bounded reads", async () => { + const handler = vi.fn(async () => ({ data: "x".repeat(4 * 1024 * 1024) })); + const bridge = await startRunnerToolBridge({ tools: [definition], handler }); + bridges.push(bridge); + const loaded = await loadExtension(bridge); + expect(loaded.errors).toEqual([]); + const tool = loaded.extensions[0]!.tools.get(definition.name)!.definition; + await expect(tool.execute("large-request", { path: "x".repeat(1024 * 1024) }, undefined, undefined, {} as never)) + .rejects.toThrow("request is too large"); + expect(handler).not.toHaveBeenCalled(); + await expect(tool.execute("large-response", { path: "file.txt" }, undefined, undefined, {} as never)) + .rejects.toThrow("response is too large"); + expect(handler).toHaveBeenCalledOnce(); + }); + + it("fails loading with invalid credentials without exposing the credential", async () => { + const bridge = await startRunnerToolBridge({ tools: [definition], handler: async () => ({}) }); + bridges.push(bridge); + const loaded = await loadExtension({ ...bridge, secret: "invalid-secret" }); + expect(loaded.extensions).toHaveLength(0); + expect(loaded.errors).toHaveLength(1); + expect(loaded.errors[0]!.error).toContain("HTTP 401"); + expect(loaded.errors[0]!.error).not.toContain("invalid-secret"); + }); + + it("registers nothing without a run-owned bridge", async () => { + const loaded = await loadExtension(); + expect(loaded.errors).toEqual([]); + expect(loaded.extensions[0]!.tools.size).toBe(0); + }); +}); diff --git a/packages/paperclip-runner/src/drivers/acpx/pi-tool-extension.ts b/packages/paperclip-runner/src/drivers/acpx/pi-tool-extension.ts new file mode 100644 index 0000000000..da6c3fc8a1 --- /dev/null +++ b/packages/paperclip-runner/src/drivers/acpx/pi-tool-extension.ts @@ -0,0 +1,92 @@ +/** + * pi-acp 0.0.33 does not forward MCP servers to Pi. Install this runner-owned + * extension in Pi's private global extension directory instead. The source is + * self-contained so the verified provider needs no additional package imports. + * Credentials are supplied only in the admitted child environment, never here. + */ +export const PI_RUNNER_TOOL_EXTENSION = String.raw` +export default async function paperclipRunnerTools(pi) { + const endpoint = process.env.PAPERCLIP_PI_TOOL_BRIDGE_URL; + const token = process.env.PAPERCLIP_PI_TOOL_BRIDGE_TOKEN; + if (!endpoint && !token) return; + const url = new URL(endpoint); + if (url.protocol !== "http:" || url.hostname !== "127.0.0.1" || + url.pathname !== "/mcp" || url.search || url.hash || + url.username || url.password || !token) { + throw new Error("Invalid private Paperclip tool bridge"); + } + const headers = { "Authorization": "Bearer " + token, "Content-Type": "application/json" }; + const maxBytes = 4 * 1024 * 1024; + async function rpc(method, params, id, signal) { + const body = JSON.stringify({ jsonrpc: "2.0", id, method, params }); + if (Buffer.byteLength(body) > 1024 * 1024) throw new Error("Paperclip tool request is too large"); + const deadline = AbortSignal.timeout(120000); + const abort = signal ? AbortSignal.any([signal, deadline]) : deadline; + const cancel = () => { + if (method !== "tools/call") return; + // Cancel the server operation as well as the HTTP read. A fresh short + // deadline lets cancellation reach the bridge after the caller aborts. + void fetch(url, { + method: "POST", headers, redirect: "error", signal: AbortSignal.timeout(5000), + body: JSON.stringify({ jsonrpc: "2.0", method: "notifications/cancelled", params: { requestId: id } }), + }).then(response => response.body?.cancel()).catch(() => {}); + }; + abort.throwIfAborted(); + abort.addEventListener("abort", cancel, { once: true }); + let reader; + try { + const response = await fetch(url, { method: "POST", headers, body, signal: abort, redirect: "error" }); + if (!response.ok || !response.body) { + await response.body?.cancel(); + throw new Error("Paperclip tool bridge request failed (HTTP " + response.status + ")"); + } + reader = response.body.getReader(); + let size = 0; + const chunks = []; + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + size += value.byteLength; + if (size > maxBytes) throw new Error("Paperclip tool response is too large"); + chunks.push(value); + } + const message = JSON.parse(Buffer.concat(chunks).toString("utf8")); + if (message.jsonrpc !== "2.0" || message.id !== id || message.error || !message.result) { + throw new Error("Paperclip tool bridge returned an invalid response"); + } + return message.result; + } finally { + abort.removeEventListener("abort", cancel); + await reader?.cancel().catch(() => {}); + } + } + const catalog = await rpc("tools/list", {}, "pi-catalog"); + if (!Array.isArray(catalog.tools) || catalog.tools.length > 256) { + throw new Error("Invalid Paperclip tool catalog"); + } + const names = new Set(); + for (const tool of catalog.tools) { + if (typeof tool.name !== "string" || !tool.name || names.has(tool.name) || + !tool.inputSchema || tool.inputSchema.type !== "object") { + throw new Error("Invalid Paperclip tool definition"); + } + names.add(tool.name); + } + for (const tool of catalog.tools) { + pi.registerTool({ + name: tool.name, + label: tool.name, + description: tool.description || tool.name, + parameters: tool.inputSchema, + async execute(callId, args, signal) { + const result = await rpc("tools/call", { name: tool.name, arguments: args }, callId, signal); + if (!Array.isArray(result.content)) throw new Error("Invalid Paperclip tool result"); + if (result.isError) { + throw new Error(result.content.filter(item => item.type === "text").map(item => item.text).join("\n")); + } + return { content: result.content, details: {} }; + }, + }); + } +} +`; diff --git a/packages/paperclip-runner/src/drivers/acpx/runtime-host.test.ts b/packages/paperclip-runner/src/drivers/acpx/runtime-host.test.ts index b168c9ce80..ef7e956711 100644 --- a/packages/paperclip-runner/src/drivers/acpx/runtime-host.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/runtime-host.test.ts @@ -452,7 +452,7 @@ describe("ACPX runtime host", () => { expect(fixture.commandClose).toHaveBeenCalledOnce(); }); - it("owns an authenticated semantic bridge without persisting its secret", async () => { + it.each(["codex", "pi"] as const)("owns a %s semantic bridge without persisting its secret", async (agent) => { const fixture = await hostFixture(); const handler = vi.fn(async ({ tool }) => ({ tool, ok: true })); let bridge: @@ -461,8 +461,8 @@ describe("ACPX runtime host", () => { const host = await AcpxRuntimeHost.open( { ...fixture.options, - agent: "codex", - model: "gpt-5.6-sol", + agent, + model: resolveQualifiedAcpxProfile(agent, agent === "pi" ? "openrouter/deepseek/deepseek-v4-flash-0731" : "gpt-5.6-sol").qualificationModel, permissionMode: "deny-all", environment: { PAPERCLIP_ACPX_CODEX_AUTH_JSON_SECRET: "{}" }, semanticTools: { @@ -484,7 +484,16 @@ describe("ACPX runtime host", () => { fixture.dependencies({ openRuntime: async (options) => { bridge = options.mcpServers[0]; - return runtimePort(); + if (agent === "pi") { + expect(options.launchEnvironment.PAPERCLIP_PI_TOOL_BRIDGE_URL).toBe(bridge!.url); + expect(options.launchEnvironment.PAPERCLIP_PI_TOOL_BRIDGE_TOKEN).toBe(bridge!.bearerToken); + } else { + expect(options.launchEnvironment.PAPERCLIP_PI_TOOL_BRIDGE_TOKEN).toBeUndefined(); + } + return runtimePort({ getStatus: async () => ({ models: { + currentModelId: options.profile.reportedModelId, + availableModelIds: [options.profile.reportedModelId], + } }) }); }, }), ); diff --git a/packages/paperclip-runner/src/drivers/acpx/runtime-host.ts b/packages/paperclip-runner/src/drivers/acpx/runtime-host.ts index 11b61ad4c0..f6ffe0ead5 100644 --- a/packages/paperclip-runner/src/drivers/acpx/runtime-host.ts +++ b/packages/paperclip-runner/src/drivers/acpx/runtime-host.ts @@ -398,7 +398,13 @@ export class AcpxRuntimeHost { permissionPolicy: acpxRuntimePermissionPolicy( binding.permissionMode, ), - launchEnvironment: sandbox.launchEnvironment, + launchEnvironment: profile.agent === "pi" && toolBridge + ? Object.freeze({ + ...sandbox.launchEnvironment, + PAPERCLIP_PI_TOOL_BRIDGE_URL: toolBridge.url, + PAPERCLIP_PI_TOOL_BRIDGE_TOKEN: toolBridge.secret, + }) + : sandbox.launchEnvironment, credentialFenceFds: admittedLifetime.lifetimeFenceFds, activateCredentialFenceOwner: admittedLifetime.activateLifetimeOwner.bind(admittedLifetime), diff --git a/packages/paperclip-runner/src/drivers/acpx/runtime-sandbox.test.ts b/packages/paperclip-runner/src/drivers/acpx/runtime-sandbox.test.ts index cd3eb07e8d..4c9845e6a1 100644 --- a/packages/paperclip-runner/src/drivers/acpx/runtime-sandbox.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/runtime-sandbox.test.ts @@ -53,6 +53,8 @@ describe("ACPX runtime sandbox", () => { PAPERCLIP_NATIVE_MCP_URL: "https://mcp.example.test/connect?ticket=secret", PAPERCLIP_NATIVE_MCP_TOKEN: "native-secret", + PAPERCLIP_GITHUB_BROKER_TOKEN: "github-secret", + PAPERCLIP_PI_TOOL_BRIDGE_TOKEN: "untrusted-bridge-secret", }, }); @@ -75,6 +77,9 @@ describe("ACPX runtime sandbox", () => { ); expect(Object.isFrozen(sandbox.launchEnvironment)).toBe(true); expect(sandbox.persistedEnvironment[credentialName]).toBeUndefined(); + expect(sandbox.launchEnvironment.PAPERCLIP_GITHUB_BROKER_TOKEN).toBe("github-secret"); + expect(sandbox.persistedEnvironment.PAPERCLIP_GITHUB_BROKER_TOKEN).toBeUndefined(); + expect(sandbox.launchEnvironment.PAPERCLIP_PI_TOOL_BRIDGE_TOKEN).toBeUndefined(); expect(sandbox.persistedEnvironment.HTTPS_PROXY).toBeUndefined(); expect( sandbox.persistedEnvironment.PAPERCLIP_NATIVE_MCP_URL, @@ -109,6 +114,14 @@ describe("ACPX runtime sandbox", () => { ); } if (agent === "pi") { + const extensionPath = join(sandbox.agentHomeDirectory, "extensions", "paperclip-runner-tools.js"); + const extension = await readFile(extensionPath, "utf8"); + expect(extension).toContain("pi.registerTool"); + expect(extension).not.toContain("github-secret"); + expect(extension).not.toContain("untrusted-bridge-secret"); + if (process.platform !== "win32") { + expect((await stat(extensionPath)).mode & 0o777).toBe(0o600); + } await expect( readFile(join(sandbox.agentHomeDirectory, "settings.json"), "utf8"), ).resolves.toContain('"defaultProjectTrust":"never"'); diff --git a/packages/paperclip-runner/src/drivers/acpx/runtime-sandbox.ts b/packages/paperclip-runner/src/drivers/acpx/runtime-sandbox.ts index 78d760ae8f..8a0a7d341d 100644 --- a/packages/paperclip-runner/src/drivers/acpx/runtime-sandbox.ts +++ b/packages/paperclip-runner/src/drivers/acpx/runtime-sandbox.ts @@ -28,6 +28,7 @@ import { resolve, } from "node:path"; +import { PI_RUNNER_TOOL_EXTENSION } from "./pi-tool-extension.js"; import { createSanitizedAcpxSpawnInput } from "./environment.js"; import type { QualifiedAcpxAgent } from "./qualified-profiles.js"; import { @@ -368,6 +369,14 @@ export async function prepareAcpxRuntimeSandbox(input: { `${input.binding.workspacePath}\n`, ); if (input.agent === "pi") { + const extensionsDirectory = await ensurePrivateDirectory( + join(agentHomeDirectory, "extensions"), + agentHomeDirectory, + ); + await writePrivateFile( + join(extensionsDirectory, "paperclip-runner-tools.js"), + PI_RUNNER_TOOL_EXTENSION, + ); await writePrivateFile( join(agentHomeDirectory, "settings.json"), `${JSON.stringify({