From 99331f777e6267387c1f697c26b207e2faf7abcb Mon Sep 17 00:00:00 2001 From: Payne Date: Sat, 25 Apr 2026 18:13:46 +0300 Subject: [PATCH] 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 | 37 +++++++---- open-sse/services/chatgptTlsClient.ts | 44 ++++++++++-- tests/unit/chatgpt-web.test.ts | 96 +++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 20 deletions(-) diff --git a/open-sse/executors/chatgpt-web.ts b/open-sse/executors/chatgpt-web.ts index d7592e7048..f8df8ebbc9 100644 --- a/open-sse/executors/chatgpt-web.ts +++ b/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(); 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; } diff --git a/open-sse/services/chatgptTlsClient.ts b/open-sse/services/chatgptTlsClient.ts index 9d66d44323..a7e5c1c603 100644 --- a/open-sse/services/chatgptTlsClient.ts +++ b/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 { } } -async function waitForFile(path: string, timeoutMs: number): Promise { +/** + * 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 +): Promise { + 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; } diff --git a/tests/unit/chatgpt-web.test.ts b/tests/unit/chatgpt-web.test.ts index 4a16192145..5a489a897a 100644 --- a/tests/unit/chatgpt-web.test.ts +++ b/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.