fix(sandbox): harden the duplex transport before the bridge merges

Bound the pre-READY readiness buffer, apply the header allowlist on the
host duplex forward path, derive the nested broker budgets from the
forward budget, and remove a dead cleanup branch.

- The readiness gate caps the pre-READY buffer at the codec frame-size
  bound plus one line of margin. Past the cap with no newline, the gate
  finishes with protocol contamination. The gate drops later bytes after
  it settles, so a failed handshake never grows the buffer.
- The broker forward callback applies the host header allowlist next to
  the route check, so a frame header outside the allowlist never reaches
  the authenticated fetch. The sandbox-side filter stays a convenience.
- The broker budgets derive the response budget and the gateway wait
  budget from the forward budget, so any forward budget keeps the nested
  order. The host constructs the broker inside the guarded region, so a
  construction throw closes the channel and selects the file bridge.
- The channel-open catch block drops the dead `if (channel)` branch,
  because the open call is the last statement of the try.

Co-authored-by: Paperclip <noreply@paperclip.ing>

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<string, string>;
+    }>;
     close: () => Promise<void>;
   }> {
-    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<string, string>;
+    }> = [];
     const server = createServer((req, res) => {
+      const headers: Record<string, string> = {};
+      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

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<DuplexBrokerForwardResult> => {
-            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<DuplexBrokerForwardResult> => {
+              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();
+            },
+          };
+        }
       }
     }
   }