From e4662b1f9dbcb6c65ed6a417ab37c69f619f853b Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 24 Jul 2026 15:25:59 -0500 Subject: [PATCH] fix: preserve control plane access in Codex sandbox (#10152) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source control plane people use to manage AI agents for work > - Local coding adapters can be confined with filesystem and network sandbox policies > - Codex confinement proxied only user-allowlisted hosts, so it denied Paperclip's own run API and managed MCP endpoints > - The same proxy returned untyped plaintext denials, which MCP clients could treat as a fatal unexpected content type > - Host-only `PAPERCLIPAI_CMD` values could also leak into agents even when the referenced checkout module did not exist > - This pull request gives the sandbox an explicit trusted-URL channel for Paperclip endpoints, returns structured JSON errors, and removes the inherited host CLI pointer > - The benefit is confined Codex agents retain control-plane access without broadening the operator's external network allowlist or crashing on policy denials ## Linked Issues or Issue Description Refs #3802 Related foundation: #9504 ### What happened? A `codex_local` agent configured with `networkScope: "allowlist"` and an external-only allowlist could not reach its own Paperclip API or Paperclip-managed MCP endpoints. The sandbox proxy returned a `403` plaintext response without `Content-Type`, and an inherited `PAPERCLIPAI_CMD` could point at a missing checkout-local CLI module. ### Expected behavior Paperclip's run-scoped API and managed MCP endpoints remain reachable regardless of the user external allowlist. Policy denials are valid structured JSON responses with an explicit media type, and host-only CLI pointers are not inherited by agent processes. ### Steps to reproduce 1. Configure a `codex_local` agent with `networkScope: "allowlist"` and `networkAllowlist: ["api.openai.com"]`. 2. Run the agent and request its Paperclip issue API or a Paperclip-managed MCP endpoint. 3. Observe the sandbox proxy deny the request with an untyped plaintext `403` response. ### Environment - Paperclip commit: `f49a3f99` originally exhibited the defect; fix is based on current `master`. - Deployment: local source build on Linux. - Adapter: Codex. - Database: not database-related. - Access context: agent bearer/run-scoped credentials. ## What Changed - Added internal trusted URL rules to the local network allowlist proxy and supplied the Codex run API plus managed MCP endpoints. - Returned JSON error envelopes with `Content-Type` and `Content-Length` for HTTP and CONNECT policy denials. - Removed inherited `PAPERCLIPAI_CMD` from child process environments while preserving explicitly constructed runtime variables. - Added focused proxy and environment sanitizer regression tests. ## Verification - `pnpm exec vitest run packages/adapter-utils/src/local-process-sandbox.test.ts packages/adapter-utils/src/server-utils-env.test.ts --reporter=verbose` - 2 test files passed; 8 tests passed; 4 platform-dependent tests skipped. - `git diff --check` - Package typecheck was attempted; it reaches unrelated current-`master` type drift in untouched files (`spawnCwd` in `adapter-utils`, and staged-runtime ACP types in `codex-local`). ## Risks - Low risk: trusted access is restricted to exact HTTP(S) hostname and port pairs derived from Paperclip-provided URLs. - Invalid or non-HTTP trusted URL values are ignored rather than broadening access. - Denial response bodies change from plaintext to structured JSON; status codes remain unchanged. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex coding agent (exact model ID and context window are not exposed to this runtime), reasoning and terminal/tool execution enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- .../src/local-process-sandbox.test.ts | 87 ++++++++++++++++++- .../src/local-process-sandbox.ts | 72 +++++++++++++-- .../src/server-utils-env.test.ts | 15 ++++ packages/adapter-utils/src/server-utils.ts | 1 + .../codex-local/src/server/execute.ts | 4 + 5 files changed, 167 insertions(+), 12 deletions(-) create mode 100644 packages/adapter-utils/src/server-utils-env.test.ts diff --git a/packages/adapter-utils/src/local-process-sandbox.test.ts b/packages/adapter-utils/src/local-process-sandbox.test.ts index 0347529165..17d6055b86 100644 --- a/packages/adapter-utils/src/local-process-sandbox.test.ts +++ b/packages/adapter-utils/src/local-process-sandbox.test.ts @@ -1,5 +1,6 @@ import fs from "node:fs/promises"; import http from "node:http"; +import net from "node:net"; import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; @@ -40,6 +41,23 @@ describe("local process sandbox", () => { expect(() => parseLocalProcessNetworkScope("public")).toThrow('"deny" or "allowlist"'); }); + it("describes every valid allowlist input when no proxy rules remain", async () => { + const workspace = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-network-rules-")); + cleanup.push(workspace); + + await expect(buildLocalProcessSandboxSpawnTarget({ + executable: process.execPath, + args: ["-e", "process.exit(0)"], + cwd: workspace, + options: { + workspaceDir: workspace, + networkScope: "allowlist", + networkAllowlist: [], + networkTrustedUrls: ["file:///not-a-network-target"], + }, + })).rejects.toThrow("valid networkAllowlist hostname or HTTP(S) networkTrustedUrl"); + }); + it("builds a fresh-root bubblewrap command with workspace access", async () => { const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-fs-sandbox-")); cleanup.push(root); @@ -102,13 +120,17 @@ describe("local process sandbox", () => { }); const delimiterIndex = target.args.indexOf("--"); const socketPath = target.args[delimiterIndex + 3]; - const request = (url: string) => new Promise<{ status: number; body: string }>((resolve, reject) => { + const request = (url: string) => new Promise<{ status: number; contentType: string | null; body: string }>((resolve, reject) => { const outgoing = http.request({ socketPath, path: url, headers: { host: new URL(url).host } }, (response) => { let body = ""; response.on("data", (chunk) => { body += chunk; }); - response.on("end", () => resolve({ status: response.statusCode ?? 0, body })); + response.on("end", () => resolve({ + status: response.statusCode ?? 0, + contentType: typeof response.headers["content-type"] === "string" ? response.headers["content-type"] : null, + body, + })); }); outgoing.on("error", reject); outgoing.end(); @@ -117,12 +139,71 @@ describe("local process sandbox", () => { try { await expect(request(`http://127.0.0.1:${address.port}/canary`)).resolves.toEqual({ status: 200, + contentType: null, body: "allowed-response", }); await expect(request("http://example.com/")).resolves.toEqual({ status: 403, - body: "Network target denied by Paperclip sandbox policy.\n", + contentType: "application/json; charset=utf-8", + body: '{"error":{"code":"network_target_denied","message":"Network target denied by Paperclip sandbox policy."}}\n', }); + const connectResponse = await new Promise((resolve, reject) => { + const socket = net.createConnection(socketPath, () => { + socket.end("CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\n\r\n"); + }); + let response = ""; + socket.setEncoding("utf8"); + socket.on("data", (chunk) => { response += chunk; }); + socket.on("end", () => resolve(response)); + socket.on("error", reject); + }); + expect(connectResponse).toContain("HTTP/1.1 403 Forbidden\r\n"); + expect(connectResponse).toContain("Content-Type: application/json; charset=utf-8\r\n"); + expect(connectResponse).toContain( + '{"error":{"code":"network_target_denied","message":"Network target denied by Paperclip sandbox policy."}}\n', + ); + } finally { + await target.cleanup?.(); + await new Promise((resolve) => server.close(() => resolve())); + } + }); + + it("always permits trusted Paperclip control-plane URLs", async () => { + const workspace = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-network-trusted-")); + cleanup.push(workspace); + const server = http.createServer((_request, response) => response.end("control-plane-response")); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + if (!address || typeof address === "string") throw new Error("Expected TCP test server address."); + const target = await buildLocalProcessSandboxSpawnTarget({ + executable: process.execPath, + args: ["-e", "process.exit(0)"], + cwd: workspace, + options: { + workspaceDir: workspace, + networkScope: "allowlist", + networkAllowlist: ["api.openai.com"], + networkTrustedUrls: [`http://127.0.0.1:${address.port}/api/issues/issue-1`], + }, + }); + const delimiterIndex = target.args.indexOf("--"); + const socketPath = target.args[delimiterIndex + 3]; + + try { + const response = await new Promise<{ status: number; body: string }>((resolve, reject) => { + const outgoing = http.request({ + socketPath, + path: `http://127.0.0.1:${address.port}/api/issues/issue-1`, + headers: { host: `127.0.0.1:${address.port}` }, + }, (incoming) => { + let body = ""; + incoming.on("data", (chunk) => { body += chunk; }); + incoming.on("end", () => resolve({ status: incoming.statusCode ?? 0, body })); + }); + outgoing.on("error", reject); + outgoing.end(); + }); + expect(response).toEqual({ status: 200, body: "control-plane-response" }); } finally { await target.cleanup?.(); await new Promise((resolve) => server.close(() => resolve())); diff --git a/packages/adapter-utils/src/local-process-sandbox.ts b/packages/adapter-utils/src/local-process-sandbox.ts index d3ad7a9ac9..971aec31d2 100644 --- a/packages/adapter-utils/src/local-process-sandbox.ts +++ b/packages/adapter-utils/src/local-process-sandbox.ts @@ -20,6 +20,7 @@ export interface LocalProcessSandboxOptions { homeDir?: string | null; networkScope?: LocalProcessNetworkScope | null; networkAllowlist?: string[]; + networkTrustedUrls?: string[]; command?: string; } @@ -155,26 +156,68 @@ function isNetworkTargetAllowed(hostname: string, port: string, rules: NetworkAl return rules.some((rule) => rule.hostname === normalizedHostname && (rule.port === null || rule.port === port)); } -async function startNetworkAllowlistProxy(allowlist: string[], socketPath: string): Promise { - const rules = allowlist.map(parseNetworkAllowlistEntry); +function parseTrustedNetworkUrl(value: string): NetworkAllowlistRule | null { + try { + const parsed = new URL(value); + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") return null; + return { + hostname: parsed.hostname.toLowerCase(), + port: parsed.port || (parsed.protocol === "https:" ? "443" : "80"), + }; + } catch { + return null; + } +} + +function writeProxyError(response: http.ServerResponse, status: number, code: string, message: string): void { + const body = `${JSON.stringify({ error: { code, message } })}\n`; + response.writeHead(status, { + "Content-Type": "application/json; charset=utf-8", + "Content-Length": Buffer.byteLength(body), + }).end(body); +} + +function connectProxyError(code: string, message: string): string { + const body = `${JSON.stringify({ error: { code, message } })}\n`; + return [ + "HTTP/1.1 403 Forbidden", + "Connection: close", + "Content-Type: application/json; charset=utf-8", + `Content-Length: ${Buffer.byteLength(body)}`, + "", + body, + ].join("\r\n"); +} + +async function startNetworkAllowlistProxy( + allowlist: string[], + trustedUrls: string[], + socketPath: string, +): Promise { + const rules = [ + ...allowlist.map(parseNetworkAllowlistEntry), + ...trustedUrls.map(parseTrustedNetworkUrl).filter((rule): rule is NetworkAllowlistRule => rule !== null), + ]; if (rules.length === 0) { - throw new Error('networkScope="allowlist" requires at least one networkAllowlist hostname.'); + throw new Error( + 'networkScope="allowlist" requires at least one valid networkAllowlist hostname or HTTP(S) networkTrustedUrl.', + ); } const server = http.createServer((request, response) => { let target: URL; try { target = new URL(request.url ?? ""); } catch { - response.writeHead(400).end("Paperclip sandbox proxy requires an absolute request URL.\n"); + writeProxyError(response, 400, "invalid_request_url", "Paperclip sandbox proxy requires an absolute request URL."); return; } const port = target.port || (target.protocol === "https:" ? "443" : "80"); if (target.protocol !== "http:") { - response.writeHead(400).end("HTTPS targets must use CONNECT through the Paperclip sandbox proxy.\n"); + writeProxyError(response, 400, "https_requires_connect", "HTTPS targets must use CONNECT through the Paperclip sandbox proxy."); return; } if (!isNetworkTargetAllowed(target.hostname, port, rules)) { - response.writeHead(403).end("Network target denied by Paperclip sandbox policy.\n"); + writeProxyError(response, 403, "network_target_denied", "Network target denied by Paperclip sandbox policy."); return; } const upstream = http.request(target, { @@ -192,7 +235,10 @@ async function startNetworkAllowlistProxy(allowlist: string[], socketPath: strin const hostname = separator > 0 ? request.url!.slice(0, separator).replace(/^\[|\]$/g, "") : ""; const port = separator > 0 ? request.url!.slice(separator + 1) : "443"; if (!hostname || !/^\d+$/.test(port) || !isNetworkTargetAllowed(hostname, port, rules)) { - clientSocket.end("HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n"); + clientSocket.end(connectProxyError( + "network_target_denied", + "Network target denied by Paperclip sandbox policy.", + )); return; } const upstream = net.connect(Number(port), hostname, () => { @@ -314,7 +360,11 @@ export async function buildLocalProcessSandboxSpawnTarget(input: { const socketPath = path.join(tempDir, "proxy.sock"); const bridgePath = path.join(tempDir, "bridge.cjs"); await fs.writeFile(bridgePath, await createNetworkProxyBridge(), { mode: 0o500 }); - const proxy = await startNetworkAllowlistProxy(input.options.networkAllowlist ?? [], socketPath).catch(async (error) => { + const proxy = await startNetworkAllowlistProxy( + input.options.networkAllowlist ?? [], + input.options.networkTrustedUrls ?? [], + socketPath, + ).catch(async (error) => { await fs.rm(tempDir, { recursive: true, force: true }); throw error; }); @@ -333,7 +383,11 @@ export async function buildLocalProcessSandboxSpawnTarget(input: { const socketPath = path.join(tempDir, "proxy.sock"); const bridgePath = path.join(tempDir, "bridge.cjs"); await fs.writeFile(bridgePath, await createNetworkProxyBridge(), { mode: 0o500 }); - const proxy = await startNetworkAllowlistProxy(input.options.networkAllowlist ?? [], socketPath).catch(async (error) => { + const proxy = await startNetworkAllowlistProxy( + input.options.networkAllowlist ?? [], + input.options.networkTrustedUrls ?? [], + socketPath, + ).catch(async (error) => { await fs.rm(tempDir, { recursive: true, force: true }); throw error; }); diff --git a/packages/adapter-utils/src/server-utils-env.test.ts b/packages/adapter-utils/src/server-utils-env.test.ts new file mode 100644 index 0000000000..aa9f20bdbb --- /dev/null +++ b/packages/adapter-utils/src/server-utils-env.test.ts @@ -0,0 +1,15 @@ +import { describe, expect, it } from "vitest"; +import { sanitizeInheritedPaperclipEnv } from "./server-utils.js"; + +describe("sanitizeInheritedPaperclipEnv", () => { + it("drops the host-only Paperclip CLI command pointer", () => { + expect(sanitizeInheritedPaperclipEnv({ + PAPERCLIPAI_CMD: "node /missing/paperclipai/dist/index.js", + PAPERCLIP_RUNTIME_API_URL: "http://127.0.0.1:3100", + PATH: "/usr/bin", + })).toEqual({ + PAPERCLIP_RUNTIME_API_URL: "http://127.0.0.1:3100", + PATH: "/usr/bin", + }); + }); +}); diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index 2459168a49..3d4192901b 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -2115,6 +2115,7 @@ export function refreshPaperclipWorkspaceEnvForExecution(input: { export function sanitizeInheritedPaperclipEnv(baseEnv: NodeJS.ProcessEnv): NodeJS.ProcessEnv { const env: NodeJS.ProcessEnv = { ...baseEnv }; + delete env.PAPERCLIPAI_CMD; for (const key of Object.keys(env)) { if (!key.startsWith("PAPERCLIP_")) continue; if (key === "PAPERCLIP_RUNTIME_API_URL") continue; diff --git a/packages/adapters/codex-local/src/server/execute.ts b/packages/adapters/codex-local/src/server/execute.ts index fec5e6bfb9..2e71c0a2b5 100644 --- a/packages/adapters/codex-local/src/server/execute.ts +++ b/packages/adapters/codex-local/src/server/execute.ts @@ -809,6 +809,10 @@ export async function execute(ctx: AdapterExecutionContext): Promise gateway.endpointPath), + ], command: asString(config.filesystemSandboxCommand, "bwrap"), } : null;