From 06024132b05b21990bbc5957f9e3755e1162f697 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Tue, 3 Nov 2026 17:50:50 -0600 Subject: [PATCH 1/3] fix: don't follow redirects on API requests authenticateWithCode and authenticateWithRefreshToken POST a body containing client_secret. With fetch's default redirect: 'follow', a 307/308 from the API origin re-posts that body to the redirect target (the Authorization header is dropped cross-origin, the body is not). Set redirect: 'manual' so a 3xx fails through the existing error path. Also pass fetchFn through in the worker build and the base WorkOS class (used by createWorkOS). Both ignored it, so a custom fetch was silently dropped on Edge, Cloudflare Workers and createWorkOS clients. --- src/common/net/fetch-client.spec.ts | 23 +++++++++++++++++++++++ src/common/net/fetch-client.ts | 4 ++++ src/factory.spec.ts | 15 +++++++++++++++ src/index.worker.ts | 14 +++++++++----- src/workos.spec.ts | 15 +++++++++++++++ src/workos.ts | 16 ++++++++++------ 6 files changed, 76 insertions(+), 11 deletions(-) diff --git a/src/common/net/fetch-client.spec.ts b/src/common/net/fetch-client.spec.ts index a4c283483..a723e50e3 100644 --- a/src/common/net/fetch-client.spec.ts +++ b/src/common/net/fetch-client.spec.ts @@ -1122,6 +1122,29 @@ describe('request timeout covers the response body (GH-1679)', () => { expect(requestCount).toBe(1); }); + it('does not follow a redirect, so the request body is never re-sent', async () => { + handler = (req, res) => { + if (req.url === '/stolen') { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end('{}'); + return; + } + res.writeHead(307, { location: `${baseURL}/stolen` }); + res.end(); + }; + // Default timeout: the shared 100ms one can retry a slow first request. + const client = new FetchHttpClient(baseURL, { maxRetries: 0 }, fetchImpl); + + await expect( + client.post( + '/user_management/authenticate', + { client_secret: 'sk_test' }, + {}, + ), + ).rejects.toBeInstanceOf(ParseError); + expect(requestCount).toBe(1); + }); + it('does not retry a successful body that times out, even when retries are enabled', async () => { stallBody(200); const { client } = createClient(2); diff --git a/src/common/net/fetch-client.ts b/src/common/net/fetch-client.ts index 1b76c4f41..819f35d77 100644 --- a/src/common/net/fetch-client.ts +++ b/src/common/net/fetch-client.ts @@ -219,6 +219,10 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { }, body: requestBody, signal: abortController.signal, + // The API never redirects. Following one would re-send the body (which + // can carry client_secret) to the redirect target. 'manual' rather than + // 'error': Cloudflare workerd rejects 'error'. + redirect: 'manual', }); if (!res.ok) { diff --git a/src/factory.spec.ts b/src/factory.spec.ts index 311be4005..ca00786f9 100644 --- a/src/factory.spec.ts +++ b/src/factory.spec.ts @@ -73,6 +73,21 @@ describe('createWorkOS', () => { expect(workos.baseURL).toBeDefined(); }); + it('uses the provided fetchFn', async () => { + const fetchFn = jest.fn( + async () => + new Response('{}', { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + const workos = createWorkOS({ apiKey: 'sk_test_123', fetchFn }); + + await workos.get('/path'); + + expect(fetchFn).toHaveBeenCalledTimes(1); + }); + it('has access to all services', () => { const workos = createWorkOS({ apiKey: 'sk_test_123', diff --git a/src/index.worker.ts b/src/index.worker.ts index f1b0b4bbb..f28677a49 100644 --- a/src/index.worker.ts +++ b/src/index.worker.ts @@ -54,11 +54,15 @@ class WorkOSWorker extends WorkOS { headers['Authorization'] = `Bearer ${this.key}`; } - return new FetchHttpClient(this.baseURL, { - ...options.config, - maxRetries: options.maxRetries, - headers, - }); + return new FetchHttpClient( + this.baseURL, + { + ...options.config, + maxRetries: options.maxRetries, + headers, + }, + options.fetchFn, + ); } /** @override */ diff --git a/src/workos.spec.ts b/src/workos.spec.ts index 2e61a1107..37919d263 100644 --- a/src/workos.spec.ts +++ b/src/workos.spec.ts @@ -780,6 +780,21 @@ describe('WorkOS', () => { ).toBeInstanceOf(SubtleCryptoProvider); }); + it('uses the provided fetchFn', async () => { + const fetchFn = jest.fn( + async () => + new Response('{}', { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + const workos = new WorkOSWorker('sk_test_key', { fetchFn }); + + await workos.get('/path'); + + expect(fetchFn).toHaveBeenCalledTimes(1); + }); + it('uses console.warn to emit warnings', () => { const workos = new WorkOSWorker('sk_test_key'); const warnSpy = jest.spyOn(console, 'warn'); diff --git a/src/workos.ts b/src/workos.ts index 72154aada..cec87ef11 100644 --- a/src/workos.ts +++ b/src/workos.ts @@ -202,12 +202,16 @@ export class WorkOS { headers['Authorization'] = `Bearer ${this.key}`; } - return new FetchHttpClient(this.baseURL, { - ...options.config, - timeout: options.timeout, - maxRetries: options.maxRetries, - headers, - }) as HttpClient; + return new FetchHttpClient( + this.baseURL, + { + ...options.config, + timeout: options.timeout, + maxRetries: options.maxRetries, + headers, + }, + options.fetchFn, + ) as HttpClient; } get version() { From 49bf9b0c6f475d19a8d2b0d69981743b39a073e3 Mon Sep 17 00:00:00 2001 From: Nick Nisi Date: Tue, 3 Nov 2026 17:58:50 -0600 Subject: [PATCH 2/3] fix: name redirects in the error and drop the duplicate worker HTTP client A 3xx (or a browser opaqueredirect) now throws an HttpClientError that names the status and Location, surfacing as a GenericServerException, instead of a ParseError from reading an HTML body as JSON. WorkOSWorker's createHttpClient was a copy of the base class method minus timeout. Remove it so the worker build uses the base method, which also makes the timeout option work on Edge and Workers. --- src/common/net/fetch-client.spec.ts | 6 +++- src/common/net/fetch-client.ts | 18 +++++++++++ src/index.worker.ts | 34 --------------------- src/workos.spec.ts | 46 +++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 35 deletions(-) diff --git a/src/common/net/fetch-client.spec.ts b/src/common/net/fetch-client.spec.ts index a723e50e3..53708bcc2 100644 --- a/src/common/net/fetch-client.spec.ts +++ b/src/common/net/fetch-client.spec.ts @@ -1141,7 +1141,11 @@ describe('request timeout covers the response body (GH-1679)', () => { { client_secret: 'sk_test' }, {}, ), - ).rejects.toBeInstanceOf(ParseError); + ).rejects.toMatchObject({ + name: 'HttpClientError', + message: expect.stringContaining(`redirect (307 to ${baseURL}/stolen)`), + response: { status: 307 }, + }); expect(requestCount).toBe(1); }); diff --git a/src/common/net/fetch-client.ts b/src/common/net/fetch-client.ts index 819f35d77..70c979010 100644 --- a/src/common/net/fetch-client.ts +++ b/src/common/net/fetch-client.ts @@ -225,6 +225,24 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { redirect: 'manual', }); + // Browsers report a manual redirect as an opaque response with status 0. + if ( + (res.status >= 300 && res.status < 400) || + res.type === 'opaqueredirect' + ) { + clearTimeout(timeoutId); + const location = res.headers.get('Location'); + const message = `WorkOS API responded with a redirect (${res.status}${location ? ` to ${location}` : ''}). Redirects are not followed; check the apiHostname, port and https options.`; + throw new HttpClientError({ + message, + response: { + status: res.status, + headers: res.headers, + data: { message }, + }, + }); + } + if (!res.ok) { const requestID = res.headers.get('X-Request-ID') ?? ''; // Read the error body under the same deadline, inside the attempt, diff --git a/src/index.worker.ts b/src/index.worker.ts index f28677a49..0604e6f6f 100644 --- a/src/index.worker.ts +++ b/src/index.worker.ts @@ -2,9 +2,6 @@ import { Actions } from './actions/actions'; import { CryptoProvider } from './common/crypto/crypto-provider'; import { SubtleCryptoProvider } from './common/crypto/subtle-crypto-provider'; -import { FetchHttpClient } from './common/net/fetch-client'; -import { HttpClient } from './common/net/http-client'; -import { WorkOSOptions } from './index.worker'; import { Webhooks } from './webhooks/webhooks'; import { WorkOS } from './workos'; @@ -34,37 +31,6 @@ export { } from './factory'; class WorkOSWorker extends WorkOS { - /** @override */ - createHttpClient(options: WorkOSOptions, userAgent: string): HttpClient { - const headers: Record = { - 'User-Agent': userAgent, - }; - - const configHeaders = options.config?.headers; - if ( - configHeaders && - typeof configHeaders === 'object' && - !Array.isArray(configHeaders) && - !(configHeaders instanceof Headers) - ) { - Object.assign(headers, configHeaders); - } - - if (this.key) { - headers['Authorization'] = `Bearer ${this.key}`; - } - - return new FetchHttpClient( - this.baseURL, - { - ...options.config, - maxRetries: options.maxRetries, - headers, - }, - options.fetchFn, - ); - } - /** @override */ createWebhookClient(): Webhooks { return new Webhooks(this); diff --git a/src/workos.spec.ts b/src/workos.spec.ts index 37919d263..fb1fd9fd3 100644 --- a/src/workos.spec.ts +++ b/src/workos.spec.ts @@ -391,6 +391,30 @@ describe('WorkOS', () => { }, 20000); }); + describe('when the api responds with a redirect', () => { + it('throws a GenericServerException that names the redirect', async () => { + const fetchFn = jest.fn( + async () => + new Response(null, { + status: 301, + headers: { Location: 'https://api.workos.com/path' }, + }), + ); + const workos = new WorkOS('sk_test_Sz3IQjepeSWaI4cMS4ms4sMuU', { + fetchFn, + }); + + await expect(workos.post('/path', {})).rejects.toMatchObject({ + name: 'GenericServerException', + status: 301, + message: expect.stringContaining( + 'redirect (301 to https://api.workos.com/path)', + ), + }); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); + }); + describe('when the api responds with a 500 and no error/error_description', () => { it('throws an GenericServerException', async () => { fetchOnce( @@ -795,6 +819,28 @@ describe('WorkOS', () => { expect(fetchFn).toHaveBeenCalledTimes(1); }); + it('honors the timeout option', async () => { + const neverResolvingFetch = jest.fn( + (_url: any, init: any) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + const abortError = new Error('Aborted'); + abortError.name = 'AbortError'; + reject(abortError); + }); + }) as any, + ); + const workos = new WorkOSWorker('sk_test_key', { + timeout: 1, + maxRetries: 0, + fetchFn: neverResolvingFetch as any, + }); + + await expect(workos.post('/path', {})).rejects.toMatchObject({ + status: 408, + }); + }); + it('uses console.warn to emit warnings', () => { const workos = new WorkOSWorker('sk_test_key'); const warnSpy = jest.spyOn(console, 'warn'); From 83437b647b6dd3ef4b08fe8cadc9cf5f73bb37d5 Mon Sep 17 00:00:00 2001 From: "Garen J. Torikian" Date: Wed, 4 Nov 2026 18:08:28 -0500 Subject: [PATCH 3/3] fix: release the connection on a redirect and name opaque redirects The redirect branch threw with the response body still open. On Node that pins the connection until garbage collection, and a proxy that sends redirect headers but stalls the body would hold it indefinitely. Abort the request's controller in a `finally` once the catch has already checked the signal, so the abort releases the socket on every fetch implementation (node-fetch bodies have no `cancel()`) without being reported as a timeout. Reading the body instead would turn a stall into a retried 408 that hides the redirect. A browser reports a manual redirect as an opaque response with status 0 and no headers, which read as "redirect (0)". Say "an opaque redirect" instead, and cover that branch with a test. --- src/common/net/fetch-client.spec.ts | 23 +++++++++++++++++++++++ src/common/net/fetch-client.ts | 20 +++++++++++++++++++- src/workos.spec.ts | 26 ++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/src/common/net/fetch-client.spec.ts b/src/common/net/fetch-client.spec.ts index 53708bcc2..b6aeb6588 100644 --- a/src/common/net/fetch-client.spec.ts +++ b/src/common/net/fetch-client.spec.ts @@ -1149,6 +1149,29 @@ describe('request timeout covers the response body (GH-1679)', () => { expect(requestCount).toBe(1); }); + it('releases the connection when a redirect body stalls instead of waiting on it', async () => { + let connectionClosed!: Promise; + handler = (_req, res) => { + connectionClosed = new Promise((resolve) => + res.on('close', () => resolve()), + ); + res.writeHead(307, { location: `${baseURL}/elsewhere` }); + res.write(''); + // Hold the body open; the client must not read it. + }; + const client = new FetchHttpClient(baseURL, { maxRetries: 0 }, fetchImpl); + + await expect(client.get('/users', {})).rejects.toMatchObject({ + name: 'HttpClientError', + message: expect.stringContaining('redirect (307'), + response: { status: 307 }, + }); + // The request was aborted, so the server sees the connection drop + // rather than the SDK sitting on the deadline. + await connectionClosed; + expect(requestCount).toBe(1); + }); + it('does not retry a successful body that times out, even when retries are enabled', async () => { stallBody(200); const { client } = createClient(2); diff --git a/src/common/net/fetch-client.ts b/src/common/net/fetch-client.ts index 70c979010..888d4ff45 100644 --- a/src/common/net/fetch-client.ts +++ b/src/common/net/fetch-client.ts @@ -206,6 +206,8 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { }); // Set once the headers arrive. let res: Response | undefined; + // Set when the response is a redirect, whose body is never read. + let redirected = false; try { res = await this._fetchFn(url, { @@ -231,8 +233,14 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { res.type === 'opaqueredirect' ) { clearTimeout(timeoutId); + redirected = true; const location = res.headers.get('Location'); - const message = `WorkOS API responded with a redirect (${res.status}${location ? ` to ${location}` : ''}). Redirects are not followed; check the apiHostname, port and https options.`; + // An opaque redirect hides both the status and the Location. + const what = + res.type === 'opaqueredirect' + ? 'an opaque redirect' + : `a redirect (${res.status}${location ? ` to ${location}` : ''})`; + const message = `WorkOS API responded with ${what}. Redirects are not followed; check the apiHostname, port and https options.`; throw new HttpClientError({ message, response: { @@ -310,6 +318,16 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { } throw error; + } finally { + // Release the unread redirect body and its connection. Reading it under + // the deadline would turn a stalled body into a retried 408 that hides + // the redirect, and `body.cancel()` only exists on web streams, whereas + // every fetch implementation honors the signal. This runs after the + // catch above has already checked the signal, so the abort is not + // reported as a timeout. + if (redirected) { + abortController.abort(); + } } } diff --git a/src/workos.spec.ts b/src/workos.spec.ts index fb1fd9fd3..ec4c0e171 100644 --- a/src/workos.spec.ts +++ b/src/workos.spec.ts @@ -413,6 +413,32 @@ describe('WorkOS', () => { }); expect(fetchFn).toHaveBeenCalledTimes(1); }); + + it('throws a GenericServerException for an opaque redirect from a browser fetch', async () => { + // Browsers hide the status and Location of a manual redirect behind + // an opaque response. The Response constructor rejects status 0, so + // this is a bare fake. + const fetchFn = jest.fn( + async () => + ({ + type: 'opaqueredirect', + status: 0, + ok: false, + headers: new Headers(), + body: null, + }) as unknown as Response, + ); + const workos = new WorkOS('sk_test_Sz3IQjepeSWaI4cMS4ms4sMuU', { + fetchFn, + }); + + await expect(workos.post('/path', {})).rejects.toMatchObject({ + name: 'GenericServerException', + status: 0, + message: expect.stringContaining('an opaque redirect'), + }); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); }); describe('when the api responds with a 500 and no error/error_description', () => {