From fcf7380d130fad516e2b360836b4da957e37cfd7 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Thu, 30 Apr 2026 13:55:56 -0700 Subject: [PATCH] fix(fetch): drop authorization and recompute client cert on cross-origin redirect --- packages/playwright-core/src/server/fetch.ts | 14 ++++++-- tests/library/client-certificates.spec.ts | 34 ++++++++++++++++++++ tests/library/global-fetch.spec.ts | 22 +++++++++++++ 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts index 54fc6c1cdee16..a8543273ffce4 100644 --- a/packages/playwright-core/src/server/fetch.ts +++ b/packages/playwright-core/src/server/fetch.ts @@ -414,7 +414,6 @@ export abstract class APIRequestContext extends SdkObject { headers, agent: options.agent, maxRedirects: options.maxRedirects - 1, - ...getMatchingTLSOptionsForOrigin(this._defaultOptions().clientCertificates, url.origin), __testHookLookup: options.__testHookLookup, }; // rejectUnauthorized = undefined is treated as true in node 12. @@ -438,6 +437,15 @@ export abstract class APIRequestContext extends SdkObject { if (headers['host']) headers['host'] = locationURL.host; + // Drop credentials scoped to the original origin on cross-origin redirects. + if (locationURL.origin !== url.origin) + removeHeader(headers, 'authorization'); + + // Client certificates are origin-scoped — pick them based on the redirect + // target, not the original URL. + Object.assign(redirectOptions, + getMatchingTLSOptionsForOrigin(this._defaultOptions().clientCertificates, locationURL.origin)); + notifyRequestFinished(); fulfill(this._sendRequest(progress, locationURL, redirectOptions, postData)); request.destroy(); @@ -767,7 +775,9 @@ function getHeader(headers: HeadersObject, name: string) { } function removeHeader(headers: { [name: string]: string }, name: string) { - delete headers[name]; + const existing = Object.entries(headers).find(pair => pair[0].toLowerCase() === name.toLowerCase()); + if (existing) + delete headers[existing[0]]; } function setBasicAuthorizationHeader(headers: { [name: string]: string }, credentials: HTTPCredentials) { diff --git a/tests/library/client-certificates.spec.ts b/tests/library/client-certificates.spec.ts index 833bbf448cab1..5b3229eca20fb 100644 --- a/tests/library/client-certificates.spec.ts +++ b/tests/library/client-certificates.spec.ts @@ -156,6 +156,40 @@ test.describe('fetch', () => { await request.dispose(); }); + test('should not leak client certificate to cross-origin redirect target', async ({ playwright, startCCServer, asset }) => { + const targetURL = await startCCServer(); + + // Standalone HTTPS server (not cert-required) that 302-redirects to the cert-required server. + const redirectServer = createHttpsServer({ + key: fs.readFileSync(asset('client-certificates/server/server_key.pem')), + cert: fs.readFileSync(asset('client-certificates/server/server_cert.pem')), + }, (req, res) => { + res.writeHead(302, { Location: targetURL }); + res.end(); + }); + await new Promise(f => redirectServer.listen(0, '127.0.0.1', () => f())); + const redirectAddr = redirectServer.address() as net.AddressInfo; + // Use 'localhost' for the redirect origin so it differs from the target's '127.0.0.1' origin + // even though both resolve to the loopback interface and share the same trusted CA. + const redirectURL = `https://localhost:${redirectAddr.port}/redir`; + + const request = await playwright.request.newContext({ + ignoreHTTPSErrors: true, + // Cert is scoped to the redirect *origin*, not the target — it must not follow the redirect. + clientCertificates: [{ + origin: new URL(redirectURL).origin, + certPath: asset('client-certificates/client/trusted/cert.pem'), + keyPath: asset('client-certificates/client/trusted/key.pem'), + }], + }); + const response = await request.get(redirectURL); + expect(response.url()).toBe(targetURL); + expect(response.status()).toBe(401); + expect(await response.text()).toContain('you need to provide a client certificate'); + await request.dispose(); + await new Promise(f => redirectServer.close(() => f())); + }); + test('pass with trusted client certificates in pfx format', async ({ playwright, startCCServer, asset }) => { const serverURL = await startCCServer(); const request = await playwright.request.newContext({ diff --git a/tests/library/global-fetch.spec.ts b/tests/library/global-fetch.spec.ts index 4b57ceeddae2a..c307d3d83cf1a 100644 --- a/tests/library/global-fetch.spec.ts +++ b/tests/library/global-fetch.spec.ts @@ -90,6 +90,28 @@ it('should propagate extra http headers with redirects', async ({ playwright, se await request.dispose(); }); +it('should preserve authorization on same-origin redirect but strip on cross-origin', async ({ playwright, server }) => { + server.setRedirect('/same/redirect', '/same/dest'); + server.setRedirect('/cross/redirect', server.CROSS_PROCESS_PREFIX + '/cross/dest'); + const request = await playwright.request.newContext({ + extraHTTPHeaders: { 'Authorization': 'Bearer secret' }, + }); + + const [sameDestReq] = await Promise.all([ + server.waitForRequest('/same/dest'), + request.get(`${server.PREFIX}/same/redirect`), + ]); + expect(sameDestReq.headers['authorization']).toBe('Bearer secret'); + + const [crossDestReq] = await Promise.all([ + server.waitForRequest('/cross/dest'), + request.get(`${server.PREFIX}/cross/redirect`), + ]); + expect(crossDestReq.headers['authorization']).toBeUndefined(); + + await request.dispose(); +}); + it('should support global httpCredentials option', async ({ playwright, server }) => { server.setAuth('/empty.html', 'user', 'pass'); const request1 = await playwright.request.newContext();