mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
fix: preserve control plane access in Codex sandbox (#10152)
## 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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
762ce5b4ef
commit
e4662b1f9d
5 files changed
+167
-12
No files matched your search
@@ -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<string>((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<void>((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<void>((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<void>((resolve) => server.close(() => resolve()));
|
||||
|
||||
@@ -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<NetworkAllowlistProxy> {
|
||||
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<NetworkAllowlistProxy> {
|
||||
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;
|
||||
});
|
||||
|
||||
@@ -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",
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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;
|
||||
|
||||
@@ -809,6 +809,10 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
||||
homeDir: filesystemScope ? effectiveCodexHome : null,
|
||||
networkScope,
|
||||
networkAllowlist: parseLocalProcessNetworkAllowlist(config.networkAllowlist),
|
||||
networkTrustedUrls: [
|
||||
paperclipBaseEnv.PAPERCLIP_API_URL,
|
||||
...runtimeMcpGateways.map((gateway) => gateway.endpointPath),
|
||||
],
|
||||
command: asString(config.filesystemSandboxCommand, "bwrap"),
|
||||
}
|
||||
: null;
|
||||
|
||||
Reference in new issue
Block a user