fix(browser): control results can contain corrupted text from malformed UTF-8 (#111898)

* fix(browser): reject malformed UTF-8 control responses

* test(browser): cover malformed UTF-8 error responses

* test(browser): strengthen UTF-8 transport coverage

* fix(browser): preserve retry guidance on decode errors

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>

extensions/browser/src/browser/client-fetch.error-body-boundary.test.ts

@@ -25,6 +25,7 @@ const STREAM_BODY_BYTES = 1024 * 1024;
 const SUCCESS_STREAM_CHUNK = Buffer.alloc(64 * 1024, "x");
 const SUCCESS_STREAM_BODY_BYTES = 33 * 1024 * 1024;
 const BROWSER_SUCCESS_BODY_LIMIT_BYTES = 32 * 1024 * 1024;
+const MALFORMED_UTF8_STATUSES = [200, 401, 408, 500, 504] as const;
 
 function scheduleStreamChunk(writeNext: () => void): void {
   // Separate event-loop turns preserve streaming and backpressure without a wall-clock sleep.
@@ -40,6 +41,8 @@ describe("fetchHttpJson error body boundary", () => {
   let resolveSmallConnectionClosed: () => void;
   let successStreamClosed: Promise<void>;
   let resolveSuccessStreamClosed: () => void;
+  let malformedConnectionClosed: Map<number, Promise<void>>;
+  let resolveMalformedConnectionClosed: Map<number, () => void>;
   let streamCompleted: boolean;
   let successStreamCompleted: boolean;
 
@@ -64,12 +67,35 @@ describe("fetchHttpJson error body boundary", () => {
     successStreamClosed = new Promise<void>((resolve) => {
       resolveSuccessStreamClosed = resolve;
     });
+    malformedConnectionClosed = new Map();
+    resolveMalformedConnectionClosed = new Map();
+    for (const status of MALFORMED_UTF8_STATUSES) {
+      malformedConnectionClosed.set(
+        status,
+        new Promise<void>((resolve) => {
+          resolveMalformedConnectionClosed.set(status, resolve);
+        }),
+      );
+    }
     streamCompleted = false;
     successStreamCompleted = false;
     server = http.createServer((req, res) => {
       if (req.url === "/success-small") {
         res.writeHead(200, { "Content-Type": "application/json" });
-        res.end('{"payload":"control"}');
+        res.end('{"payload":"control 🦞"}');
+        return;
+      }
+      const malformedStatus = Number(req.url?.match(/^\/malformed-utf8\/(\d+)$/)?.[1]);
+      if (MALFORMED_UTF8_STATUSES.some((status) => status === malformedStatus)) {
+        req.socket.once("close", () => resolveMalformedConnectionClosed.get(malformedStatus)?.());
+        res.writeHead(malformedStatus, { "Content-Type": "application/json" });
+        res.end(
+          Buffer.concat([
+            Buffer.from('{"error":"control '),
+            Buffer.from([0xff]),
+            Buffer.from('"}'),
+          ]),
+        );
         return;
       }
       if (req.url === "/success-large") {
@@ -194,10 +220,35 @@ describe("fetchHttpJson error body boundary", () => {
 
   it("preserves a normal successful JSON response", async () => {
     await expect(fetchBrowserJson(`${baseUrl}/success-small`)).resolves.toEqual({
-      payload: "control",
+      payload: "control 🦞",
     });
   });
 
+  it("rejects malformed UTF-8 responses, preserves retry policy, and releases each fetch", async () => {
+    for (const status of MALFORMED_UTF8_STATUSES) {
+      const error = await fetchBrowserJson(`${baseUrl}/malformed-utf8/${status}`).catch(
+        (err: unknown) => err,
+      );
+
+      expect(error).toMatchObject({
+        name: "BrowserServiceError",
+        status,
+      });
+      const message = error instanceof Error ? error.message : "";
+      expect(message).toContain(`Browser control response was not valid UTF-8 (HTTP ${status})`);
+      if (status === 401) {
+        expect(message).toContain("Do NOT retry the browser tool");
+      } else if (status === 408 || status === 504) {
+        expect(message).toContain("Retry the browser tool once");
+      } else {
+        expect(message).not.toContain("Retry the browser tool");
+      }
+      const connectionClosed = malformedConnectionClosed.get(status);
+      expect(connectionClosed).toBeDefined();
+      await expect(connectionClosed).resolves.toBeUndefined();
+    }
+  });
+
   it("preserves a complete diagnostic body within the limit", async () => {
     const error = await fetchBrowserJson(`${baseUrl}/small`).catch((err: unknown) => err);
 

extensions/browser/src/browser/client-fetch.ts

@@ -151,6 +151,18 @@ const BROWSER_ERROR_BODY_LIMIT_BYTES = 16 * 1024;
 // `response/body` supports 5M characters; 32 MiB covers worst-case JSON escaping while staying bounded.
 const BROWSER_SUCCESS_BODY_LIMIT_BYTES = 32 * 1024 * 1024;
 
+function decodeBrowserControlResponseUtf8(body: Uint8Array, status: number): string {
+  try {
+    return new TextDecoder("utf-8", { fatal: true }).decode(body);
+  } catch {
+    throw browserServiceErrorFromPayload(
+      undefined,
+      `Browser control response was not valid UTF-8 (HTTP ${status})`,
+      status,
+    );
+  }
+}
+
 function isRateLimitStatus(status: number): boolean {
   return status === 429;
 }
@@ -378,7 +390,7 @@ async function fetchHttpJson<T>(
       const body = await readResponseWithLimit(res, BROWSER_ERROR_BODY_LIMIT_BYTES).catch(
         () => undefined,
       );
-      const text = body ? new TextDecoder().decode(body) : "";
+      const text = body ? decodeBrowserControlResponseUtf8(body, res.status) : "";
       let parsed: unknown;
       if (text) {
         try {
@@ -393,7 +405,7 @@ async function fetchHttpJson<T>(
       onOverflow: ({ maxBytes }) =>
         new BrowserServiceError(`Browser control response exceeded ${maxBytes} bytes`),
     });
-    return JSON.parse(new TextDecoder().decode(body)) as T;
+    return JSON.parse(decodeBrowserControlResponseUtf8(body, res.status)) as T;
   } finally {
     clearTimeout(t);
     await release?.();