refactor(adapter-utils): carry the http2 bridge forward response body as bytes

The host held two copies of every forwarded body on the HTTP/2 bridge
path: readBridgeForwardResponseBody decoded the response to a UTF-8
string, then the caller encoded it back to bytes for the sandbox
stream. This change carries the body as one Buffer end to end.

- readBridgeForwardResponseBody now returns Promise<Buffer>.
- forwardBridgeRequest accepts a string or Uint8Array request body and
  returns a Buffer response body. Every error path returns a Buffer.
- The HTTP/2 forward handler passes and returns raw bytes with no
  toString("utf8") call.
- The file bridge call site converts the body to a string, so the file
  bridge response file format stays unchanged.
- postedIssueCommentLogMarker now accepts a Buffer or a string. It
  decodes at most 4,096 bytes, and it logs a comment id only when the
  parsed value is a canonical lowercase UUID.

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

packages/adapter-utils/src/execution-target-sandbox.test.ts

@@ -25,6 +25,7 @@ import {
   formatAdapterExecutionTimeoutStartLogLine,
   parseAdapterExecutionTarget,
   postedIssueCommentLogMarker,
+  POSTED_COMMENT_MARKER_DECODE_LIMIT_BYTES,
   resolveAdapterExecutionTargetTimeout,
   resolveAdapterExecutionTargetTimeoutSec,
   runAdapterExecutionTargetProcess,
@@ -119,10 +120,46 @@ describe("sandbox adapter execution targets", () => {
   const cleanupDirs: string[] = [];
 
   it("records successful issue comment ids for attribution recovery", () => {
-    expect(postedIssueCommentLogMarker("POST", "/api/issues/issue-1/comments", 201, '{"id":"comment-1"}'))
-      .toBe("comment id: comment-1\n");
-    expect(postedIssueCommentLogMarker("POST", "/api/issues/issue-1/comments", 401, '{"id":"comment-1"}'))
-      .toBeNull();
+    const uuid = "550e8400-e29b-41d4-a716-446655440000";
+    expect(postedIssueCommentLogMarker("POST", "/api/issues/issue-1/comments", 201, `{"id":"${uuid}"}`)).toBe(
+      `comment id: ${uuid}\n`,
+    );
+    expect(postedIssueCommentLogMarker("POST", "/api/issues/issue-1/comments", 401, `{"id":"${uuid}"}`)).toBeNull();
+    // The function accepts a `Buffer` body, matching the bytes the http2 path
+    // hands it after it drops the string round trip.
+    expect(
+      postedIssueCommentLogMarker(
+        "POST",
+        "/api/issues/issue-1/comments",
+        201,
+        Buffer.from(`{"id":"${uuid}"}`, "utf8"),
+      ),
+    ).toBe(`comment id: ${uuid}\n`);
+  });
+
+  it("the comment marker returns null for an identifier that is not a canonical UUID", () => {
+    expect(
+      postedIssueCommentLogMarker("POST", "/api/issues/issue-1/comments", 201, '{"id":"comment-1"}'),
+    ).toBeNull();
+    // Uppercase hex is a valid UUID, but not a canonical *lowercase* UUID.
+    expect(
+      postedIssueCommentLogMarker(
+        "POST",
+        "/api/issues/issue-1/comments",
+        201,
+        '{"id":"550E8400-E29B-41D4-A716-446655440000"}',
+      ),
+    ).toBeNull();
+  });
+
+  it("the comment marker returns null for a body past the decode limit", () => {
+    const uuid = "550e8400-e29b-41d4-a716-446655440000";
+    const oversized = Buffer.from(
+      JSON.stringify({ id: uuid, padding: "x".repeat(POSTED_COMMENT_MARKER_DECODE_LIMIT_BYTES) }),
+      "utf8",
+    );
+    expect(oversized.length).toBeGreaterThan(POSTED_COMMENT_MARKER_DECODE_LIMIT_BYTES);
+    expect(postedIssueCommentLogMarker("POST", "/api/issues/issue-1/comments", 201, oversized)).toBeNull();
   });
 
   afterEach(async () => {
@@ -3264,10 +3301,10 @@ describe("sandbox adapter execution targets", () => {
    */
   function http2TestRequest(
     session: http2.ClientHttp2Session,
-    request: { method: string; path: string; headers?: Record<string, string>; body?: string },
-  ): Promise<{ status: number; headers: Record<string, string>; body: string }> {
+    request: { method: string; path: string; headers?: Record<string, string>; body?: string | Buffer },
+  ): Promise<{ status: number; headers: Record<string, string>; body: string; bodyBytes: Buffer }> {
     return new Promise((resolve, reject) => {
-      const body = Buffer.from(request.body ?? "", "utf8");
+      const body = Buffer.isBuffer(request.body) ? request.body : Buffer.from(request.body ?? "", "utf8");
       const stream = session.request(
         { ":method": request.method, ":path": request.path, ...request.headers },
         { endStream: body.length === 0 },
@@ -3284,7 +3321,10 @@ describe("sandbox adapter execution targets", () => {
         }
       });
       stream.on("data", (chunk: Buffer) => chunks.push(chunk));
-      stream.once("end", () => resolve({ status, headers, body: Buffer.concat(chunks).toString("utf8") }));
+      stream.once("end", () => {
+        const bodyBytes = Buffer.concat(chunks);
+        resolve({ status, headers, body: bodyBytes.toString("utf8"), bodyBytes });
+      });
       stream.once("error", (error) => reject(error));
       if (body.length > 0) stream.end(body);
       else if (!stream.writableEnded) stream.end();
@@ -3294,7 +3334,10 @@ describe("sandbox adapter execution targets", () => {
   // Start a host API server that records each forwarded request, so a test can
   // assert the real token and the run id reach the host, or that a rejected
   // request never forwards.
-  async function startRecordingApiServer(): Promise<{
+  async function startRecordingApiServer(options?: {
+    /** Overrides the default `{ ok: true }` JSON reply with these raw bytes. */
+    responseBody?: Buffer;
+  }): Promise<{
     origin: string;
     requests: Array<{
       method: string;
@@ -3302,6 +3345,7 @@ describe("sandbox adapter execution targets", () => {
       auth: string | null;
       runId: string | null;
       headers: Record<string, string>;
+      body: Buffer;
     }>;
     close: () => Promise<void>;
   }> {
@@ -3311,21 +3355,28 @@ describe("sandbox adapter execution targets", () => {
       auth: string | null;
       runId: string | null;
       headers: Record<string, string>;
+      body: Buffer;
     }> = [];
+    const responseBody = options?.responseBody ?? Buffer.from(JSON.stringify({ ok: true }), "utf8");
     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,
+      const bodyChunks: Buffer[] = [];
+      req.on("data", (chunk: Buffer) => bodyChunks.push(chunk));
+      req.on("end", () => {
+        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,
+          body: Buffer.concat(bodyChunks),
+        });
+        res.writeHead(200, { "content-type": "application/json" });
+        res.end(responseBody);
       });
-      res.writeHead(200, { "content-type": "application/json" });
-      res.end(JSON.stringify({ ok: true }));
     });
     await new Promise<void>((resolve, reject) => {
       server.once("error", reject);
@@ -3460,6 +3511,109 @@ describe("sandbox adapter execution targets", () => {
     expect(control.stopCount).toBeGreaterThanOrEqual(1);
   }, 20000);
 
+  it("the HTTP/2 path forwards malformed UTF-8 request bytes to the host API unchanged", async () => {
+    // A lone continuation byte and a truncated multi-byte sequence: bytes no
+    // UTF-8 decode-then-encode round trip preserves.
+    const malformed = Buffer.from([0x80, 0xc3, 0x28, 0xff, 0xfe, 0x00]);
+    const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-http2-req-bytes-"));
+    cleanupDirs.push(rootDir);
+    const remoteCwd = path.join(rootDir, "workspace");
+    await mkdir(remoteCwd, { recursive: true });
+    const api = await startRecordingApiServer();
+    const sessionRef: { current: http2.ClientHttp2Session | null } = { current: null };
+    let bridgeToken = "";
+    const { runner } = makeHttp2SelectionRunner((ctx) => {
+      bridgeToken = ctx.bridgeToken;
+      ctx.emitReady();
+      sessionRef.current = ctx.connectHttp2();
+    });
+    const target: AdapterSandboxExecutionTarget = {
+      kind: "remote",
+      transport: "sandbox",
+      providerKey: "daytona",
+      remoteCwd,
+      timeoutMs: 30_000,
+      runner,
+      effectiveCapabilities: duplexCapabilities(true),
+    };
+
+    const bridge = await startAdapterExecutionTargetPaperclipBridge({
+      runId: "run-req-bytes",
+      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("http2_v1");
+      await waitForCondition(() => sessionRef.current !== null, "the http2 client session to open", 4000);
+      await http2TestRequest(sessionRef.current!, {
+        method: "POST",
+        path: "/api/issues/issue-1/comments",
+        headers: { authorization: `Bearer ${bridgeToken}`, "content-type": "application/octet-stream" },
+        body: malformed,
+      });
+      await waitForCondition(() => api.requests.length >= 1, "the host to forward the http2 request", 4000);
+      expect(api.requests[0].body.equals(malformed)).toBe(true);
+    } finally {
+      sessionRef.current?.close();
+      await bridge?.stop();
+      await api.close();
+    }
+  }, 20000);
+
+  it("the HTTP/2 path returns malformed UTF-8 host response bytes to the sandbox unchanged", async () => {
+    const malformed = Buffer.from([0xff, 0xfe, 0x80, 0x81, 0x00, 0x01]);
+    const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-http2-resp-bytes-"));
+    cleanupDirs.push(rootDir);
+    const remoteCwd = path.join(rootDir, "workspace");
+    await mkdir(remoteCwd, { recursive: true });
+    const api = await startRecordingApiServer({ responseBody: malformed });
+    const sessionRef: { current: http2.ClientHttp2Session | null } = { current: null };
+    let bridgeToken = "";
+    const { runner } = makeHttp2SelectionRunner((ctx) => {
+      bridgeToken = ctx.bridgeToken;
+      ctx.emitReady();
+      sessionRef.current = ctx.connectHttp2();
+    });
+    const target: AdapterSandboxExecutionTarget = {
+      kind: "remote",
+      transport: "sandbox",
+      providerKey: "daytona",
+      remoteCwd,
+      timeoutMs: 30_000,
+      runner,
+      effectiveCapabilities: duplexCapabilities(true),
+    };
+
+    const bridge = await startAdapterExecutionTargetPaperclipBridge({
+      runId: "run-resp-bytes",
+      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("http2_v1");
+      await waitForCondition(() => sessionRef.current !== null, "the http2 client session to open", 4000);
+      const response = await http2TestRequest(sessionRef.current!, {
+        method: "GET",
+        path: "/api/agents/me",
+        headers: { authorization: `Bearer ${bridgeToken}` },
+      });
+      expect(response.status).toBe(200);
+      expect(response.bodyBytes.equals(malformed)).toBe(true);
+    } finally {
+      sessionRef.current?.close();
+      await bridge?.stop();
+      await api.close();
+    }
+  }, 20000);
+
   it("falls back to the file bridge when a post-READY pre-bind flood exceeds the aggregate ceiling", async () => {
     // The gateway sends a valid READY, then floods the channel before the broker
     // binds. The pre-READY buffer cap does not bound the post-READY replay buffer,

packages/adapter-utils/src/execution-target.ts

@@ -88,13 +88,36 @@ import type { LocalProcessSandboxOptions } from "./local-process-sandbox.js";
 
 export type { RuntimeProgressSink } from "./runtime-progress.js";
 
-export function postedIssueCommentLogMarker(method: string, requestPath: string, status: number, body: string) {
+/**
+ * The decode limit for {@link postedIssueCommentLogMarker}. The response body
+ * is untrusted output, so the host never decodes more than this many bytes of
+ * it to build one log line, and it never allocates a second full-size string
+ * for a body over this limit.
+ */
+export const POSTED_COMMENT_MARKER_DECODE_LIMIT_BYTES = 4096;
+
+const CANONICAL_LOWERCASE_UUID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/;
+
+export function postedIssueCommentLogMarker(
+  method: string,
+  requestPath: string,
+  status: number,
+  body: Buffer | string,
+) {
   if (method !== "POST" || !/^\/api\/issues\/[^/]+\/comments$/.test(requestPath) || status < 200 || status >= 300) {
     return null;
   }
+  const bodyBytes = typeof body === "string" ? Buffer.byteLength(body, "utf8") : body.length;
+  if (bodyBytes > POSTED_COMMENT_MARKER_DECODE_LIMIT_BYTES) {
+    return null;
+  }
+  const decoded =
+    typeof body === "string" ? body : body.toString("utf8", 0, POSTED_COMMENT_MARKER_DECODE_LIMIT_BYTES);
   try {
-    const parsed = JSON.parse(body) as { id?: unknown };
-    return typeof parsed.id === "string" && parsed.id.length > 0 ? `comment id: ${parsed.id}\n` : null;
+    const parsed = JSON.parse(decoded) as { id?: unknown };
+    return typeof parsed.id === "string" && CANONICAL_LOWERCASE_UUID_PATTERN.test(parsed.id)
+      ? `comment id: ${parsed.id}\n`
+      : null;
   } catch {
     return null;
   }
@@ -1505,7 +1528,8 @@ function bridgeResponseBodyLimitError(maxBodyBytes: number): Error {
 }
 
 /**
- * Read the forward response body into a string. The reader bounds the body with
+ * Read the forward response body into one `Buffer`. The reader carries the raw
+ * bytes end to end and holds no separate string copy. It bounds the body with
  * two controls. The per-request `maxBodyBytes` limit rejects a body larger than
  * the configured per-request ceiling. The optional host aggregate byte ledger
  * bounds the retained bytes across all live routes.
@@ -1524,7 +1548,7 @@ async function readBridgeForwardResponseBody(
   response: Response,
   maxBodyBytes: number,
   ledger?: DuplexAggregateByteLedger | null,
-): Promise<string> {
+): Promise<Buffer> {
   const rawContentLength = response.headers.get("content-length");
   if (rawContentLength) {
     const contentLength = Number.parseInt(rawContentLength, 10);
@@ -1534,7 +1558,7 @@ async function readBridgeForwardResponseBody(
   }
 
   if (!response.body) {
-    return "";
+    return Buffer.alloc(0);
   }
 
   const reader = response.body.getReader();
@@ -1579,7 +1603,7 @@ async function readBridgeForwardResponseBody(
       }
       tokens.push(concatToken);
     }
-    return Buffer.concat(chunks, totalBytes).toString("utf8");
+    return Buffer.concat(chunks, totalBytes);
   } finally {
     if (ledger) {
       for (const token of tokens) {
@@ -4225,14 +4249,17 @@ export async function startAdapterExecutionTargetPaperclipBridge(input: {
       path: string;
       query: string;
       headers: Record<string, string>;
-      /** The file bridge passes the whole request body here as one string. */
-      body?: string;
+      /**
+       * The file bridge passes the whole request body here as one string. The
+       * http2 path passes the raw request bytes with no string round trip.
+       */
+      body?: string | Uint8Array;
     },
     signal?: AbortSignal,
     options?: {
       suppressDebugLog?: boolean;
     },
-  ): Promise<{ status: number; headers: Record<string, string>; body: string }> => {
+  ): Promise<{ status: number; headers: Record<string, string>; body: Buffer }> => {
     const method = request.method.trim().toUpperCase() || "GET";
     // The per-request debug log prints the method, the path, and the query. The
     // duplex path suppresses it, so no route or query rides a log line there. The
@@ -4257,14 +4284,22 @@ export async function startAdapterExecutionTargetPaperclipBridge(input: {
     const timeoutSignal = AbortSignal.timeout(forwardTimeoutMs);
     const forwardSignal = signal ? AbortSignal.any([signal, timeoutSignal]) : timeoutSignal;
     // Build the request-body init. A GET or a HEAD carries no body. The file
-    // bridge passes the whole body as one string.
+    // bridge passes the whole body as one string; the http2 path passes the
+    // raw request bytes.
     const forwardInit: RequestInit = {
       method,
       headers,
       signal: forwardSignal,
     };
-    if (method !== "GET" && method !== "HEAD" && typeof request.body === "string") {
-      forwardInit.body = request.body;
+    if (
+      method !== "GET" &&
+      method !== "HEAD" &&
+      (typeof request.body === "string" || request.body instanceof Uint8Array)
+    ) {
+      // `fetch` accepts a `Uint8Array` body at runtime. The DOM `BodyInit` type
+      // pins the typed-array generic to a concrete `ArrayBuffer`, so a
+      // `Buffer`'s wider `ArrayBufferLike` generic needs this assertion.
+      forwardInit.body = request.body as BodyInit;
     }
     const response = await fetch(buildBridgeForwardUrl(hostApiUrl, request), forwardInit);
     if (emitDebugLog) {
@@ -4285,7 +4320,7 @@ export async function startAdapterExecutionTargetPaperclipBridge(input: {
     // non-retryable 504 and marks the outcome indeterminate, exactly like an
     // aborted in-flight forward. The in-sandbox server maps the indeterminate 504
     // to a non-retryable 409 for both the file bridge and the duplex broker.
-    let responseBody: string;
+    let responseBody: Buffer;
     try {
       responseBody = await readBridgeForwardResponseBody(
         response,
@@ -4300,9 +4335,11 @@ export async function startAdapterExecutionTargetPaperclipBridge(input: {
         return {
           status: 502,
           headers: { "content-type": "application/json" },
-          body: JSON.stringify({
-            error: error instanceof Error ? error.message : String(error),
-          }),
+          body: Buffer.from(
+            JSON.stringify({
+              error: error instanceof Error ? error.message : String(error),
+            }),
+          ),
         };
       }
       return {
@@ -4311,11 +4348,13 @@ export async function startAdapterExecutionTargetPaperclipBridge(input: {
           "content-type": "application/json",
           "x-paperclip-bridge-outcome": "indeterminate",
         },
-        body: JSON.stringify({
-          error: error instanceof Error ? error.message : String(error),
-          outcome: "indeterminate",
-          retryable: false,
-        }),
+        body: Buffer.from(
+          JSON.stringify({
+            error: error instanceof Error ? error.message : String(error),
+            outcome: "indeterminate",
+            retryable: false,
+          }),
+        ),
       };
     }
     const commentMarker = postedIssueCommentLogMarker(method, request.path, response.status, responseBody);
@@ -4546,7 +4585,7 @@ export async function startAdapterExecutionTargetPaperclipBridge(input: {
                   path: request.pathname,
                   query: request.query,
                   headers: request.headers,
-                  body: request.body.toString("utf8"),
+                  body: request.body,
                 },
                 undefined,
                 { suppressDebugLog: true },
@@ -4669,7 +4708,13 @@ export async function startAdapterExecutionTargetPaperclipBridge(input: {
       maxBodyBytes,
       getRuntimeParentContext: input.getRuntimeParentContext,
       runtimeSpan: input.runtimeSpan,
-      handleRequest: async (request, options) => forwardBridgeRequest(request, options?.signal),
+      // The file bridge response file holds the body as a JSON string field, so
+      // this call site converts the forwarded bytes to a string here. It is the
+      // one place on the file bridge path that creates a string copy.
+      handleRequest: async (request, options) => {
+        const result = await forwardBridgeRequest(request, options?.signal);
+        return { status: result.status, headers: result.headers, body: result.body.toString("utf8") };
+      },
     });
     server = await startSandboxCallbackBridgeServer({
       runner,