diff --git a/packages/adapter-utils/src/execution-target-sandbox.test.ts b/packages/adapter-utils/src/execution-target-sandbox.test.ts index 1a80090b747..619ed4656e8 100644 --- a/packages/adapter-utils/src/execution-target-sandbox.test.ts +++ b/packages/adapter-utils/src/execution-target-sandbox.test.ts @@ -45,6 +45,7 @@ import { runChildProcess } from "./server-utils.js"; import { shellQuote } from "./ssh.js"; import type { CommandManagedDuplexChannel } from "./command-managed-runtime.js"; import { + DEFAULT_MAX_DUPLEX_FRAME_BYTES, DUPLEX_FRAME_VERSION, decodeDuplexLine, encodeDuplexFrame, @@ -2916,16 +2917,33 @@ describe("sandbox adapter execution targets", () => { // request never forwards. async function startRecordingApiServer(): Promise<{ origin: string; - requests: Array<{ method: string; url: string; auth: string | null; runId: string | null }>; + requests: Array<{ + method: string; + url: string; + auth: string | null; + runId: string | null; + headers: Record; + }>; close: () => Promise; }> { - const requests: Array<{ method: string; url: string; auth: string | null; runId: string | null }> = []; + const requests: Array<{ + method: string; + url: string; + auth: string | null; + runId: string | null; + headers: Record; + }> = []; const server = createServer((req, res) => { + const headers: Record = {}; + for (const [key, value] of Object.entries(req.headers)) { + if (typeof value === "string") headers[key] = value; + } requests.push({ method: req.method ?? "GET", url: req.url ?? "/", auth: req.headers.authorization ?? null, runId: typeof req.headers["x-paperclip-run-id"] === "string" ? req.headers["x-paperclip-run-id"] : null, + headers, }); res.writeHead(200, { "content-type": "application/json" }); res.end(JSON.stringify({ ok: true })); @@ -3755,6 +3773,225 @@ describe("sandbox adapter execution targets", () => { await api.close(); } }, 20000); + + it("caps the pre-READY readiness buffer and falls back with a contaminated reason", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-duplex-cap-")); + cleanupDirs.push(rootDir); + const remoteCwd = path.join(rootDir, "workspace"); + await mkdir(remoteCwd, { recursive: true }); + const api = await startRecordingApiServer(); + // The fake gateway sends a large pre-READY blob with no newline, then one + // more blob after the gate settles. The gate must cap the buffer, finish with + // protocol contamination, and drop the later blob. The blob is larger than + // the codec frame-size bound, so it passes the readiness buffer cap. + const oversizedBlob = "x".repeat(DEFAULT_MAX_DUPLEX_FRAME_BYTES * 2); + const { runner, control } = makeDuplexSelectionRunner((ctx) => { + ctx.emitRaw(oversizedBlob); + ctx.emitRaw(oversizedBlob); + }); + const { recorder, counters } = createRecordingDuplexRecorder(); + const target: AdapterSandboxExecutionTarget = { + kind: "remote", + transport: "sandbox", + providerKey: "daytona", + remoteCwd, + timeoutMs: 30_000, + runner, + effectiveCapabilities: duplexCapabilities(true), + }; + + const bridge = await startAdapterExecutionTargetPaperclipBridge({ + runId: "run-cap", + target, + runtimeRootDir: path.join(remoteCwd, ".paperclip-runtime", "codex"), + adapterKey: "codex", + hostApiToken: "real-run-jwt", + hostApiUrl: api.origin, + enableSandboxDuplexBridge: true, + // A long readiness timeout, so the buffer cap, not the timeout, drives the + // failure. + duplexReadinessTimeoutMs: 5_000, + duplexTelemetryRecorder: recorder, + }); + try { + expect(bridge).not.toBeNull(); + expect(control.openCount).toBe(1); + // The cap drove the failure, so the file bridge serves after the bounded cleanup. + expect(bridge?.env.PAPERCLIP_API_BRIDGE_MODE).toBe("queue_v1"); + const fallback = counters.find((c) => c.metric === DUPLEX_COUNTER_FALLBACK_TOTAL); + expect(fallback?.dimensions.fallback_reason).toBe("contaminated"); + // The bounded cleanup left no live provider session. + expect(control.closeCount + control.stopCount).toBeGreaterThanOrEqual(1); + } finally { + await bridge?.stop(); + await api.close(); + } + }, 20000); + + it("drops a frame header outside the allowlist on the host duplex forward path", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-duplex-hdr-")); + cleanupDirs.push(rootDir); + const remoteCwd = path.join(rootDir, "workspace"); + await mkdir(remoteCwd, { recursive: true }); + const api = await startRecordingApiServer(); + const { runner, control } = makeDuplexSelectionRunner(); + const target: AdapterSandboxExecutionTarget = { + kind: "remote", + transport: "sandbox", + providerKey: "e2b", + remoteCwd, + timeoutMs: 30_000, + runner, + effectiveCapabilities: duplexCapabilities(true), + }; + + const bridge = await startAdapterExecutionTargetPaperclipBridge({ + runId: "run-hdr", + target, + runtimeRootDir: path.join(remoteCwd, ".paperclip-runtime", "codex"), + adapterKey: "codex", + hostApiToken: "real-run-jwt", + hostApiUrl: api.origin, + enableSandboxDuplexBridge: true, + }); + try { + expect(bridge?.env.PAPERCLIP_API_BRIDGE_MODE).toBe("duplex_v1"); + control.emitData( + encodeDuplexFrame({ + version: DUPLEX_FRAME_VERSION, + type: "request", + id: "req-hdr", + method: "GET", + path: "/api/agents/me", + query: "", + headers: { + // An allowlisted header the host must keep. + accept: "application/json", + // A header outside the allowlist the host must drop. + "x-injected-header": "attacker", + // A sandbox-supplied auth header the host must replace with the real token. + authorization: "Bearer bridge-token", + }, + body: "", + }), + ); + await waitForCondition( + () => api.requests.length >= 1, + "the broker to forward the duplex request", + 4000, + ); + const forwarded = api.requests[0]; + // The allowlisted header reaches the host. + expect(forwarded.headers.accept).toBe("application/json"); + // The header outside the allowlist never reaches the authenticated fetch. + expect(forwarded.headers["x-injected-header"]).toBeUndefined(); + // The host applied the real token and the run id in place of the frame values. + expect(forwarded.auth).toBe("Bearer real-run-jwt"); + expect(forwarded.runId).toBe("run-hdr"); + } finally { + await bridge?.stop(); + await api.close(); + } + }, 20000); + + it("selects the duplex transport for a large forward budget and starts the broker", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-duplex-budget-")); + cleanupDirs.push(rootDir); + const remoteCwd = path.join(rootDir, "workspace"); + await mkdir(remoteCwd, { recursive: true }); + const api = await startRecordingApiServer(); + const { runner, control } = makeDuplexSelectionRunner(); + const target: AdapterSandboxExecutionTarget = { + kind: "remote", + transport: "sandbox", + providerKey: "e2b", + remoteCwd, + timeoutMs: 30_000, + runner, + effectiveCapabilities: duplexCapabilities(true), + }; + + // A forward budget past the default response budget (32 s). Before the budget + // derivation, this made the nested budget assertion throw and leak the channel. + const bridge = await startAdapterExecutionTargetPaperclipBridge({ + runId: "run-budget", + target, + runtimeRootDir: path.join(remoteCwd, ".paperclip-runtime", "codex"), + adapterKey: "codex", + hostApiToken: "real-run-jwt", + hostApiUrl: api.origin, + enableSandboxDuplexBridge: true, + forwardTimeoutMs: 60_000, + }); + try { + // The broker started with derived nested budgets, so the duplex transport serves. + expect(bridge?.env.PAPERCLIP_API_BRIDGE_MODE).toBe("duplex_v1"); + control.emitData( + encodeDuplexFrame({ + version: DUPLEX_FRAME_VERSION, + type: "request", + id: "req-budget", + method: "GET", + path: "/api/agents/me", + query: "", + headers: { authorization: "Bearer bridge-token" }, + body: "", + }), + ); + await waitForCondition( + () => control.written.length >= 1, + "the broker to write a duplex response frame", + 4000, + ); + expect(control.written[0]).toMatchObject({ type: "response", id: "req-budget", status: 200 }); + } finally { + await bridge?.stop(); + await api.close(); + } + }, 20000); + + it("falls back to the file bridge when the broker construction throws", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-duplex-ctor-")); + cleanupDirs.push(rootDir); + const remoteCwd = path.join(rootDir, "workspace"); + await mkdir(remoteCwd, { recursive: true }); + const api = await startRecordingApiServer(); + const { runner, control } = makeDuplexSelectionRunner(); + const target: AdapterSandboxExecutionTarget = { + kind: "remote", + transport: "sandbox", + providerKey: "e2b", + remoteCwd, + timeoutMs: 30_000, + runner, + effectiveCapabilities: duplexCapabilities(true), + }; + + // A non-finite forward budget makes the nested budget assertion throw at + // broker construction. Readiness passes first, so the guarded region must + // close the channel and select the file bridge instead of leaking it. + const bridge = await startAdapterExecutionTargetPaperclipBridge({ + runId: "run-ctor", + target, + runtimeRootDir: path.join(remoteCwd, ".paperclip-runtime", "codex"), + adapterKey: "codex", + hostApiToken: "real-run-jwt", + hostApiUrl: api.origin, + enableSandboxDuplexBridge: true, + forwardTimeoutMs: Number.NaN, + }); + try { + expect(bridge).not.toBeNull(); + expect(control.openCount).toBe(1); + // Readiness passed, then the broker construction threw; the file bridge serves. + expect(bridge?.env.PAPERCLIP_API_BRIDGE_MODE).toBe("queue_v1"); + // The bounded cleanup left no live provider session. + expect(control.closeCount + control.stopCount).toBeGreaterThanOrEqual(1); + } finally { + await bridge?.stop(); + await api.close(); + } + }, 20000); }); // One decoded stdout frame from the generated duplex gateway. The gateway writes diff --git a/packages/adapter-utils/src/execution-target.ts b/packages/adapter-utils/src/execution-target.ts index 380bf5a8406..4d93f11db01 100644 --- a/packages/adapter-utils/src/execution-target.ts +++ b/packages/adapter-utils/src/execution-target.ts @@ -32,6 +32,7 @@ import { SANDBOX_CALLBACK_BRIDGE_DUPLEX_MODE, SANDBOX_CALLBACK_BRIDGE_ENTRYPOINT, sandboxCallbackBridgeDirectories, + sanitizeSandboxCallbackBridgeHeaders, startSandboxCallbackBridgeServer, startSandboxCallbackBridgeWorker, syncRemoteTextFileWithHashSkip, @@ -45,10 +46,12 @@ import { createDuplexBridgeBroker, DEFAULT_DUPLEX_BROKER_BUDGETS, type DuplexBridgeBroker, + type DuplexBrokerBudgets, type DuplexBrokerForwardResult, } from "./duplex-bridge-broker.js"; import { decodeDuplexLine, + DEFAULT_MAX_DUPLEX_FRAME_BYTES, type DuplexRequestFrame, } from "./duplex-frame-codec.js"; import { @@ -2311,6 +2314,32 @@ function duplexReadinessFallbackReason(reason: DuplexReadinessFailure): DuplexFa } } +/** + * The cap on the pre-READY readiness buffer, in bytes. The gate reads untrusted + * bytes before the READY line arrives, so it bounds the buffer. The cap is the + * codec frame-size bound plus one line of margin, so a legitimate maximum-size + * READY frame still fits. Past the cap with no newline, the stream cannot be a + * valid READY frame, so the gate finishes with protocol contamination. + */ +const DUPLEX_READINESS_BUFFER_CAP_BYTES = DEFAULT_MAX_DUPLEX_FRAME_BYTES + 4_096; + +/** + * Derive the nested broker budgets from one forward budget. The response budget + * and the gateway wait budget each add a fixed margin, so the nested budget + * order holds for any finite forward budget. The default forward budget (30 s) + * yields the historical 32 s response budget and 35 s gateway wait budget, so a + * caller that sets no forward budget sees no change. + */ +function deriveNestedDuplexBrokerBudgets(forwardTimeoutMs: number): DuplexBrokerBudgets { + const responseMargin = + DEFAULT_DUPLEX_BROKER_BUDGETS.responseBudgetMs - DEFAULT_DUPLEX_BROKER_BUDGETS.forwardTimeoutMs; + const gatewayMargin = + DEFAULT_DUPLEX_BROKER_BUDGETS.gatewayWaitMs - DEFAULT_DUPLEX_BROKER_BUDGETS.responseBudgetMs; + const responseBudgetMs = forwardTimeoutMs + responseMargin; + const gatewayWaitMs = responseBudgetMs + gatewayMargin; + return { forwardTimeoutMs, responseBudgetMs, gatewayWaitMs }; +} + /** * The duplex readiness gate. The gate owns the single data listener and the * single exit listener of the channel while the host waits for a valid READY @@ -2374,10 +2403,24 @@ function createDuplexReadinessGate( pending += chunk; return; } + if (settled) { + // The handshake already failed. The channel is untrusted and can keep + // sending bytes until the host closes it. Drop them, so a failed handshake + // never grows the buffer after the gate settles. + return; + } buffer += chunk; for (;;) { const newlineIndex = buffer.indexOf("\n"); - if (newlineIndex === -1) return; + if (newlineIndex === -1) { + // No READY line yet. The gate reads untrusted bytes, so bound the + // pre-READY buffer. Past the cap with no newline, the stream cannot be a + // valid READY frame, so finish with protocol contamination. + if (Buffer.byteLength(buffer, "utf8") > DUPLEX_READINESS_BUFFER_CAP_BYTES) { + finish({ ok: false, reason: "protocol_contamination" }); + } + return; + } const line = buffer.slice(0, newlineIndex); const rest = buffer.slice(newlineIndex + 1); if (line.length === 0) { @@ -2742,9 +2785,10 @@ export async function startAdapterExecutionTargetPaperclipBridge(input: { channel = await openDuplexChannel({ command }); } catch { // The channel never opened, so no request could carry the bridge token. - // Fall through to the file bridge below. The log line names no raw provider - // error, so no provider error rides a log line on the duplex path. - if (channel) await closeDuplexChannelWithinBudget(channel, DEFAULT_DUPLEX_CLEANUP_BUDGET_MS); + // The open call is the last statement of the try, so `channel` is still + // null here; there is nothing to close. Fall through to the file bridge + // below. The log line names no raw provider error, so no provider error + // rides a log line on the duplex path. duplexChannelOpen.fallback("open_failed"); await onLog( "stderr", @@ -2769,61 +2813,92 @@ export async function startAdapterExecutionTargetPaperclipBridge(input: { `[paperclip] Sandbox duplex readiness failed (${readiness.reason}). Using the file bridge.\n`, ); } else { - // Readiness passed. Start the broker on the handed-off channel. The - // broker enforces the same route allowlist as the file bridge, then - // forwards each request with the real token and the run id. The agent - // environment below receives the bridge URL and token only now, after - // readiness passed. - const broker: DuplexBridgeBroker = createDuplexBridgeBroker({ - channel: gate.brokerChannel, - budgets: { forwardTimeoutMs }, - forwardRequest: async ( - request: DuplexRequestFrame, - options: { signal: AbortSignal }, - ): Promise => { - const denialReason = authorizeSandboxCallbackBridgeRequestWithRoutes(request); - if (denialReason) { - return { - status: 403, - headers: { "content-type": "application/json" }, - body: JSON.stringify({ error: denialReason }), + // Readiness passed. Construct the broker inside the guarded region, so a + // construction throw closes the channel within the cleanup budget and + // selects the file bridge. The broker enforces the same route allowlist + // as the file bridge, then forwards each request with the real token and + // the run id. The agent environment below receives the bridge URL and + // token only now, after readiness passed. + let broker: DuplexBridgeBroker | null = null; + try { + broker = createDuplexBridgeBroker({ + channel: gate.brokerChannel, + // Derive the nested budgets from the forward budget, so any forward + // budget keeps the nested order the broker asserts at construction. + budgets: deriveNestedDuplexBrokerBudgets(forwardTimeoutMs), + forwardRequest: async ( + request: DuplexRequestFrame, + options: { signal: AbortSignal }, + ): Promise => { + const denialReason = authorizeSandboxCallbackBridgeRequestWithRoutes(request); + if (denialReason) { + return { + status: 403, + headers: { "content-type": "application/json" }, + body: JSON.stringify({ error: denialReason }), + }; + } + // Apply the header allowlist on the host, next to the route check. + // The host is the trust boundary: the sandbox controls the frame + // headers, so the host drops every header outside the allowlist + // before the authenticated forward. The forward then applies the + // real token and the run id. + const sanitizedRequest = { + ...request, + headers: sanitizeSandboxCallbackBridgeHeaders(request.headers), }; - } - // Suppress the per-request debug log on the duplex path, so no route - // or query rides a log line here. - return forwardBridgeRequest(request, options.signal, { suppressDebugLog: true }); - }, - // The duplex path emits only the fixed transport telemetry. It passes no - // free-form logger, so no raw provider error rides a log line here. - telemetry: duplexTelemetry, - }); - broker.start(); - duplexChannelOpen.ready(); - await onLog( - "stdout", - "[paperclip] Sandbox duplex transport ready; serving the host-assigned origin.\n", - ); - return { - env: { - PAPERCLIP_API_URL: sandboxOrigin, - PAPERCLIP_API_KEY: bridgeToken, - PAPERCLIP_API_BRIDGE_MODE: SANDBOX_CALLBACK_BRIDGE_DUPLEX_MODE, - }, - runLogTail: null, - stop: async () => { - // Close the channel before lease release. The broker sends an orderly - // close and releases the route, then stops the child, so no live - // provider session remains when the caller releases the lease. - await broker.close(); - broker.stop(); - // A channel that did not reach the `closed` state may leave a live - // provider session, so record one session leak. - if (broker.state !== "closed") { - duplexTelemetry.recordSessionLeak(); - } - await bridgeAsset.cleanup(); - }, - }; + // Suppress the per-request debug log on the duplex path, so no route + // or query rides a log line here. + return forwardBridgeRequest(sanitizedRequest, options.signal, { + suppressDebugLog: true, + }); + }, + // The duplex path emits only the fixed transport telemetry. It passes no + // free-form logger, so no raw provider error rides a log line here. + telemetry: duplexTelemetry, + }); + } catch { + // The broker construction failed, so no broker owns the channel. Close + // the channel within the cleanup budget, then select the file bridge. + // The log line names no raw error, so no raw error rides a log line on + // the duplex path. + await closeDuplexChannelWithinBudget(channel, DEFAULT_DUPLEX_CLEANUP_BUDGET_MS); + duplexChannelOpen.fallback("open_failed"); + await onLog( + "stderr", + "[paperclip] Could not start the sandbox duplex broker. Using the file bridge.\n", + ); + } + if (broker) { + const activeBroker = broker; + activeBroker.start(); + duplexChannelOpen.ready(); + await onLog( + "stdout", + "[paperclip] Sandbox duplex transport ready; serving the host-assigned origin.\n", + ); + return { + env: { + PAPERCLIP_API_URL: sandboxOrigin, + PAPERCLIP_API_KEY: bridgeToken, + PAPERCLIP_API_BRIDGE_MODE: SANDBOX_CALLBACK_BRIDGE_DUPLEX_MODE, + }, + runLogTail: null, + stop: async () => { + // Close the channel before lease release. The broker sends an orderly + // close and releases the route, then stops the child, so no live + // provider session remains when the caller releases the lease. + await activeBroker.close(); + activeBroker.stop(); + // A channel that did not reach the `closed` state may leave a live + // provider session, so record one session leak. + if (activeBroker.state !== "closed") { + duplexTelemetry.recordSessionLeak(); + } + await bridgeAsset.cleanup(); + }, + }; + } } } }