chatgpt-web: address PR #1596 review feedback (#1597)

Integrated into release/v3.7.0. Thank you for the contribution @trader-payne!

open-sse/executors/chatgpt-web.ts

@@ -152,6 +152,16 @@ interface SessionResponse {
   user?: { id?: string };
 }
 
+// Session-token family — NextAuth uses one of these depending on token size:
+//   __Secure-next-auth.session-token            (unchunked, < 4KB)
+//   __Secure-next-auth.session-token.0          (chunked, first piece)
+//   __Secure-next-auth.session-token.N          (chunked, additional pieces)
+// Rotation can change the shape (unchunked → chunked or vice versa). When
+// that happens, every old family member must be dropped — keeping the stale
+// variant alongside the new one would send both, and depending on parser
+// precedence the server could read the stale value and fail auth.
+const SESSION_TOKEN_FAMILY_RE = /^__Secure-next-auth\.session-token(?:\.\d+)?$/;
+
 /**
  * Merge any rotated session-token chunks from a Set-Cookie response into the
  * original cookie blob, preserving every other cookie the caller pasted
@@ -186,9 +196,9 @@ function mergeRefreshedCookie(
   }
 
   const pairs = blob.split(/;\s*/).filter(Boolean);
-  const replaced = new Set<string>();
   const result: string[] = [];
   let mutated = false;
+  let droppedStale = false;
   for (const pair of pairs) {
     const eqIdx = pair.indexOf("=");
     if (eqIdx < 0) {
@@ -197,23 +207,22 @@ function mergeRefreshedCookie(
     }
     const name = pair.slice(0, eqIdx).trim();
     const value = pair.slice(eqIdx + 1);
-    if (refreshed.has(name)) {
-      const newValue = refreshed.get(name)!;
-      if (newValue !== value) mutated = true;
-      result.push(`${name}=${newValue}`);
-      replaced.add(name);
-    } else {
-      result.push(`${name}=${value}`);
+    // Drop ALL session-token-family members from the original — we'll
+    // append the refreshed set below. This handles unchunked→chunked and
+    // chunked→unchunked rotations, where keeping the old name would leave
+    // the stale token visible alongside the new one.
+    if (SESSION_TOKEN_FAMILY_RE.test(name)) {
+      if (!refreshed.has(name) || refreshed.get(name) !== value) mutated = true;
+      droppedStale = true;
+      continue;
     }
+    result.push(`${name}=${value}`);
   }
-  // Append any rotated chunks that weren't in the original (e.g., the user
-  // pasted only `.0` but rotation returned `.0` and `.1`).
+  // Append the full refreshed family.
   for (const [name, value] of refreshed) {
-    if (!replaced.has(name)) {
-      result.push(`${name}=${value}`);
-      mutated = true;
-    }
+    result.push(`${name}=${value}`);
   }
+  if (!droppedStale) mutated = true; // refreshed chunks were entirely new
   return mutated ? result.join("; ") : null;
 }
 

open-sse/services/chatgptTlsClient.ts

@@ -228,8 +228,16 @@ async function tlsFetchStreaming(
   // request fully completes (full body written).
   const requestPromise = client.request(url, streamOpts);
 
-  // Wait briefly for the file to appear so we can detect early errors.
-  const ready = await waitForFile(path, 5_000);
+  // Wait for the file to exist AND have at least one byte. tls-client-node
+  // creates the output file when the request starts, but the file can be
+  // empty for a brief window before the first body chunk lands — peeking
+  // during that window would return "" and misclassify the response as
+  // non-SSE, dropping us into the buffered-wait branch and silently turning
+  // a streaming request into a buffered one. Waiting for content avoids
+  // that race; if the request actually fails before producing any bytes,
+  // the timeout falls through to the requestPromise drain below (returning
+  // the real upstream status).
+  const ready = await waitForContent(path, 5_000, requestPromise);
   if (!ready) {
     const r = await requestPromise.catch(
       (e) => ({ status: 502, headers: {}, body: String(e) }) as TlsResponseLike
@@ -305,15 +313,39 @@ async function readFirstBytes(path: string, n: number): Promise<string> {
   }
 }
 
-async function waitForFile(path: string, timeoutMs: number): Promise<boolean> {
+/**
+ * Wait for the streaming output file to exist AND contain at least one byte.
+ * Returns false if the request settles before any bytes arrive (so the caller
+ * can drain `requestPromise` and surface the real upstream status). Returns
+ * true as soon as the file has data — even one byte is enough for the SSE
+ * heuristic to give a useful answer.
+ */
+async function waitForContent(
+  path: string,
+  timeoutMs: number,
+  requestPromise: Promise<TlsResponseLike>
+): Promise<boolean> {
+  let requestSettled = false;
+  requestPromise.then(
+    () => {
+      requestSettled = true;
+    },
+    () => {
+      requestSettled = true;
+    }
+  );
   const start = Date.now();
   while (Date.now() - start < timeoutMs) {
     try {
-      await stat(path);
-      return true;
+      const s = await stat(path);
+      if (s.size > 0) return true;
     } catch {
-      await sleep(25);
+      // file doesn't exist yet
     }
+    // If the request finished without producing any bytes, no point waiting
+    // out the rest of the timeout — let the caller drain it.
+    if (requestSettled) return false;
+    await sleep(25);
   }
   return false;
 }

tests/unit/chatgpt-web.test.ts

@@ -949,6 +949,102 @@ test("Cookie rotation: full DevTools blob keeps cf_clearance/__cf_bm/_cfuvid", a
   }
 });
 
+test("Cookie rotation: unchunked → chunked drops stale unchunked variant", async () => {
+  // When the original was unchunked (< 4KB session token) and rotation
+  // returns chunked (.0/.1), the stale unchunked entry must NOT survive in
+  // the merged blob — otherwise both old and new session-token cookies are
+  // sent on the next request and depending on parser precedence the server
+  // could read the stale value.
+  reset();
+  const m = installMockFetch({
+    session: {
+      status: 200,
+      body: {
+        accessToken: "jwt-abc",
+        expires: new Date(Date.now() + 3600_000).toISOString(),
+        user: { id: "user-1" },
+      },
+      setCookie:
+        "__Secure-next-auth.session-token.0=NEW0; Path=/; HttpOnly, " +
+        "__Secure-next-auth.session-token.1=NEW1; Path=/; HttpOnly",
+    },
+  });
+  try {
+    let refreshed = null;
+    const executor = new ChatGptWebExecutor();
+    await executor.execute({
+      model: "gpt-5.3-instant",
+      body: { messages: [{ role: "user", content: "hi" }] },
+      stream: false,
+      credentials: {
+        apiKey: "__Secure-next-auth.session-token=UNCHUNKED_OLD; cf_clearance=CFCLEAR",
+      },
+      signal: AbortSignal.timeout(10_000),
+      log: null,
+      onCredentialsRefreshed: (creds) => {
+        refreshed = creds;
+      },
+    });
+    assert.ok(refreshed);
+    // Stale unchunked variant must NOT appear (whole or in part).
+    assert.doesNotMatch(
+      refreshed.apiKey,
+      /__Secure-next-auth\.session-token=UNCHUNKED_OLD/,
+      "stale unchunked session-token must be dropped"
+    );
+    // Non-session-token cookies preserved.
+    assert.match(refreshed.apiKey, /cf_clearance=CFCLEAR/);
+    // New chunks present.
+    assert.match(refreshed.apiKey, /session-token\.0=NEW0/);
+    assert.match(refreshed.apiKey, /session-token\.1=NEW1/);
+  } finally {
+    m.restore();
+  }
+});
+
+test("Cookie rotation: chunked → unchunked drops stale chunks", async () => {
+  // Reverse case: original is chunked, rotation goes back to unchunked.
+  reset();
+  const m = installMockFetch({
+    session: {
+      status: 200,
+      body: {
+        accessToken: "jwt-abc",
+        expires: new Date(Date.now() + 3600_000).toISOString(),
+        user: { id: "user-1" },
+      },
+      setCookie: "__Secure-next-auth.session-token=NEW_UNCHUNKED; Path=/; HttpOnly",
+    },
+  });
+  try {
+    let refreshed = null;
+    const executor = new ChatGptWebExecutor();
+    await executor.execute({
+      model: "gpt-5.3-instant",
+      body: { messages: [{ role: "user", content: "hi" }] },
+      stream: false,
+      credentials: {
+        apiKey:
+          "__Secure-next-auth.session-token.0=OLD0; " +
+          "__Secure-next-auth.session-token.1=OLD1; " +
+          "cf_clearance=CFCLEAR",
+      },
+      signal: AbortSignal.timeout(10_000),
+      log: null,
+      onCredentialsRefreshed: (creds) => {
+        refreshed = creds;
+      },
+    });
+    assert.ok(refreshed);
+    assert.doesNotMatch(refreshed.apiKey, /OLD0/, "stale chunk .0 dropped");
+    assert.doesNotMatch(refreshed.apiKey, /OLD1/, "stale chunk .1 dropped");
+    assert.match(refreshed.apiKey, /session-token=NEW_UNCHUNKED/);
+    assert.match(refreshed.apiKey, /cf_clearance=CFCLEAR/);
+  } finally {
+    m.restore();
+  }
+});
+
 test("Cookie rotation: returns null when Set-Cookie has no session-token", async () => {
   // When NextAuth doesn't rotate (Set-Cookie sets only unrelated cookies, or
   // returns the same session-token value), the callback shouldn't fire.