From 6bb12c191f5380fad321322fb90216ae0dc36985 Mon Sep 17 00:00:00 2001 From: SapirBaruch Date: Wed, 3 Jun 2026 11:05:51 +0300 Subject: [PATCH] fix: custom auth headers not stripped on cross-origin redirects (#10892) Co-authored-by: Jason Saayman Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 --- PRE_RELEASE_CHANGELOG.md | 20 ++++ README.md | 6 + THREATMODEL.md | 4 +- index.d.cts | 1 + index.d.ts | 1 + lib/adapters/http.js | 73 +++++++++++- tests/unit/adapters/http.test.js | 162 +++++++++++++++++++++++++- tests/unit/prototypePollution.test.js | 29 +++++ 8 files changed, 291 insertions(+), 5 deletions(-) diff --git a/PRE_RELEASE_CHANGELOG.md b/PRE_RELEASE_CHANGELOG.md index 5822d329..3e01d873 100644 --- a/PRE_RELEASE_CHANGELOG.md +++ b/PRE_RELEASE_CHANGELOG.md @@ -2,6 +2,26 @@ ## Unreleased +## Security Fixes + +- **HTTP Adapter Redirects:** Added a Node.js `sensitiveHeaders` request config option that strips caller-selected custom secret headers from cross-origin redirects. (**#10892**) + +## Docs updates + +- docs/pages/misc/security.md LN29 : | [`sensitiveHeaders`](/pages/advanced/request-config#sensitiveheaders) | Custom authentication headers such as `X-API-Key` can be forwarded to a different origin when a trusted server redirects there. | List custom secret-bearing headers in `sensitiveHeaders` so the Node.js adapter strips them on cross-origin redirects. | +- docs/pages/advanced/request-config.md LN269 : | ### `sensitiveHeaders` + +The `sensitiveHeaders` property is an optional array of custom secret-bearing header names to remove when axios follows a redirect to a different origin. Matching is case-insensitive. Same-origin redirects keep these headers. + +This only applies to redirects followed by the Node.js HTTP adapter. If `maxRedirects` is set to 0, `sensitiveHeaders` is not used. + +```js +axios.get('https://api.example.com/users', { + headers: { 'X-API-Key': 'secret' }, + sensitiveHeaders: ['X-API-Key'] +}); +``` + ## New Features - **HTTP Adapter - Zstandard:** Added automatic zstd decompression on Node.js versions that support it. `zstd` is only advertised in the default `Accept-Encoding` header when `transitional.advertiseZstdAcceptEncoding: true` is set. (**#6792**) diff --git a/README.md b/README.md index 1760cf55..06c8a668 100644 --- a/README.md +++ b/README.md @@ -898,6 +898,12 @@ These config options are available for requests. Only `url` is required. Request // If set to 0, Axios follows no redirects. maxRedirects: 21, // default + // `sensitiveHeaders` (Node only option) lists custom secret-bearing headers + // to remove from cross-origin redirects. Matching is case-insensitive. + // Same-origin redirects keep these headers. If `maxRedirects` is 0, this + // option is not used. + sensitiveHeaders: ['X-API-Key'], + // `beforeRedirect` defines a function that Axios calls before redirect. // Use this to adjust the request options upon redirecting, // to inspect the latest response headers, diff --git a/THREATMODEL.md b/THREATMODEL.md index 60ff5c21..94ed3c1c 100644 --- a/THREATMODEL.md +++ b/THREATMODEL.md @@ -104,8 +104,8 @@ The runtime model is general by design. axios is a transport library and cannot | **Description** | Caller sets `Authorization: Bearer …` and requests `https://api.trusted.com/x`. Server responds `302 Location: https://evil.com/`. Does the bearer token go to evil.com? | | **Likelihood** | Medium | | **Impact** | High (full credential theft) | -| **Mitigations** | • Node adapter delegates to `follow-redirects@^1.15.11`, which strips `Authorization`, `Cookie`, and `Proxy-Authorization` on cross-host redirects and on HTTPS to HTTP downgrades.
• `maxRedirects` defaults to 5; set to `0` to handle redirects manually.
• `beforeRedirect` callback allows custom inspection.
• Browser adapters (XHR/fetch) delegate to the browser, which applies its own cross-origin credential rules. | -| **Residual risk** | Low. We inherit `follow-redirects`' security posture. It is a critical transitive dependency, and its CVEs are our CVEs. | +| **Mitigations** | • Node adapter delegates to `follow-redirects@^1.16.0`, which strips `Authorization`, `Cookie`, and `Proxy-Authorization` on cross-host redirects and on HTTPS→HTTP downgrades.
• `sensitiveHeaders` lets callers list custom secret-bearing headers (for example `X-API-Key`) that axios strips on cross-origin redirects.
• `maxRedirects` defaults to 5; set to `0` to handle redirects manually.
• `beforeRedirect` callback allows custom inspection.
• Browser adapters (XHR/fetch) delegate to the browser, which applies its own cross-origin credential rules. | +| **Residual risk** | Low for standard credential headers and configured custom secret headers. We inherit `follow-redirects`' security posture - it is a critical transitive dependency and its CVEs are our CVEs. Callers must list any custom secret headers they want stripped. | --- diff --git a/index.d.cts b/index.d.cts index 837558ee..3288c0e7 100644 --- a/index.d.cts +++ b/index.d.cts @@ -559,6 +559,7 @@ declare namespace axios { }; formDataHeaderPolicy?: 'legacy' | 'content-only'; redact?: string[]; + sensitiveHeaders?: string[]; } // Alias diff --git a/index.d.ts b/index.d.ts index 7304f5fa..31f05e9f 100644 --- a/index.d.ts +++ b/index.d.ts @@ -452,6 +452,7 @@ export interface AxiosRequestConfig { }; formDataHeaderPolicy?: 'legacy' | 'content-only'; redact?: string[]; + sensitiveHeaders?: string[]; } // Alias diff --git a/lib/adapters/http.js b/lib/adapters/http.js index 8f4a19ea..1876d38f 100755 --- a/lib/adapters/http.js +++ b/lib/adapters/http.js @@ -154,8 +154,8 @@ const flushOnFinish = (stream, [throttled, flush]) => { const http2Sessions = new Http2Sessions(); /** - * If the proxy, auth, or config beforeRedirects functions are defined, call them - * with the options object. + * If the proxy, auth, sensitive header, or config beforeRedirects functions are defined, + * call them with the options object. * * @param {Object} options - The options object that was passed to the request. * @@ -168,11 +168,39 @@ function dispatchBeforeRedirect(options, responseDetails, requestDetails) { if (options.beforeRedirects.auth) { options.beforeRedirects.auth(options); } + if (options.beforeRedirects.sensitiveHeaders) { + options.beforeRedirects.sensitiveHeaders(options, requestDetails); + } if (options.beforeRedirects.config) { options.beforeRedirects.config(options, responseDetails, requestDetails); } } +function stripMatchingHeaders(headers, sensitiveSet) { + if (!headers) { + return; + } + + Object.keys(headers).forEach((header) => { + if (sensitiveSet.has(header.toLowerCase())) { + delete headers[header]; + } + }); +} + +function isSameOriginRedirect(redirectOptions, requestDetails) { + if (!requestDetails) { + return false; + } + + try { + return new URL(requestDetails.url).origin === new URL(redirectOptions.href).origin; + } catch (e) { + // If origin comparison fails, treat the redirect as unsafe. + return false; + } +} + /** * If the proxy or config afterRedirects functions are defined, call them with the options * @@ -864,6 +892,7 @@ export default isHttpAdapterSupported && transport = isHttpsRequest ? https : http; isNativeTransport = true; } else { + options.sensitiveHeaders = []; if (config.maxRedirects) { options.maxRedirects = config.maxRedirects; } @@ -888,6 +917,45 @@ export default isHttpAdapterSupported && } }; } + const sensitiveHeaders = own('sensitiveHeaders'); + if (sensitiveHeaders != null) { + if (!utils.isArray(sensitiveHeaders)) { + return reject( + new AxiosError( + 'sensitiveHeaders must be an array of strings', + AxiosError.ERR_BAD_OPTION_VALUE, + config + ) + ); + } + + const sensitiveSet = new Set(); + for (const header of sensitiveHeaders) { + if (!utils.isString(header)) { + return reject( + new AxiosError( + 'sensitiveHeaders must be an array of strings', + AxiosError.ERR_BAD_OPTION_VALUE, + config + ) + ); + } + + sensitiveSet.add(header.toLowerCase()); + } + + if (sensitiveSet.size) { + options.sensitiveHeaders = Array.from(sensitiveSet); + options.beforeRedirects.sensitiveHeaders = function beforeRedirectSensitiveHeaders( + redirectOptions, + requestDetails + ) { + if (!isSameOriginRedirect(redirectOptions, requestDetails)) { + stripMatchingHeaders(redirectOptions.headers, sensitiveSet); + } + }; + } + } transport = isHttpsRequest ? httpsFollow : httpFollow; } } @@ -1254,3 +1322,4 @@ export default isHttpAdapterSupported && }; export const __setProxy = setProxy; +export const __isSameOriginRedirect = isSameOriginRedirect; diff --git a/tests/unit/adapters/http.test.js b/tests/unit/adapters/http.test.js index 8e103c09..32c919dd 100644 --- a/tests/unit/adapters/http.test.js +++ b/tests/unit/adapters/http.test.js @@ -10,7 +10,7 @@ import { } from '../../setup/server.js'; import axios from '../../../index.js'; import AxiosError from '../../../lib/core/AxiosError.js'; -import { __setProxy } from '../../../lib/adapters/http.js'; +import { __isSameOriginRedirect, __setProxy } from '../../../lib/adapters/http.js'; import HttpsProxyAgent from 'https-proxy-agent'; import http from 'http'; import https from 'https'; @@ -632,6 +632,166 @@ describe('supports http with nodejs', () => { await stopHTTPServer(proxy); }); + it('should strip sensitiveHeaders on cross-origin redirect', async () => { + let capturedHeaders; + + // destination server — different port means different origin + const destination = await startHTTPServer((req, res) => { + capturedHeaders = req.headers; + res.statusCode = 200; + res.end('ok'); + }); + + // origin server — redirects to destination (cross-origin) + const origin = await startHTTPServer((req, res) => { + res.setHeader('Location', `http://localhost:${destination.address().port}/dest`); + res.statusCode = 302; + res.end(); + }); + + try { + await axios.get(`http://localhost:${origin.address().port}/src`, { + maxRedirects: 5, + headers: { 'X-API-Key': 'secret', 'X-Other': 'keep' }, + sensitiveHeaders: ['X-API-Key'], + }); + + assert.strictEqual(capturedHeaders['x-api-key'], undefined, 'X-API-Key should be stripped'); + assert.strictEqual(capturedHeaders['x-other'], 'keep', 'X-Other should be preserved'); + } finally { + await stopHTTPServer(origin); + await stopHTTPServer(destination); + } + }); + + it('should preserve sensitiveHeaders on same-origin redirect', async () => { + let capturedHeaders; + let requestCount = 0; + + const server = await startHTTPServer((req, res) => { + requestCount++; + if (requestCount === 1) { + res.setHeader('Location', '/dest'); + res.statusCode = 302; + res.end(); + } else { + capturedHeaders = req.headers; + res.statusCode = 200; + res.end('ok'); + } + }); + + try { + await axios.get(`http://localhost:${server.address().port}/src`, { + maxRedirects: 5, + headers: { 'X-API-Key': 'secret' }, + sensitiveHeaders: ['X-API-Key'], + }); + + assert.strictEqual(capturedHeaders['x-api-key'], 'secret', 'X-API-Key should be preserved on same-origin redirect'); + } finally { + await stopHTTPServer(server); + } + }); + + it('should strip sensitiveHeaders case-insensitively on cross-origin redirect', async () => { + let capturedHeaders; + + const destination = await startHTTPServer((req, res) => { + capturedHeaders = req.headers; + res.statusCode = 200; + res.end('ok'); + }); + + const origin = await startHTTPServer((req, res) => { + res.setHeader('Location', `http://localhost:${destination.address().port}/dest`); + res.statusCode = 302; + res.end(); + }); + + try { + await axios.get(`http://localhost:${origin.address().port}/src`, { + maxRedirects: 5, + // Header sent with mixed casing; sensitiveHeaders list uses different casing + headers: { 'X-Api-Key': 'secret' }, + sensitiveHeaders: ['x-api-key'], + }); + + assert.strictEqual(capturedHeaders['x-api-key'], undefined, 'X-Api-Key should be stripped case-insensitively'); + } finally { + await stopHTTPServer(origin); + await stopHTTPServer(destination); + } + }); + + it('should strip sensitiveHeaders configured on an instance', async () => { + let capturedHeaders; + + const destination = await startHTTPServer((req, res) => { + capturedHeaders = req.headers; + res.statusCode = 200; + res.end('ok'); + }); + + const origin = await startHTTPServer((req, res) => { + res.setHeader('Location', `http://localhost:${destination.address().port}/dest`); + res.statusCode = 302; + res.end(); + }); + + const client = axios.create({ + headers: { 'X-API-Key': 'secret', 'X-Other': 'keep' }, + sensitiveHeaders: ['X-API-Key'], + }); + + try { + await client.get(`http://localhost:${origin.address().port}/src`, { + maxRedirects: 5, + }); + + assert.strictEqual(capturedHeaders['x-api-key'], undefined, 'X-API-Key should be stripped'); + assert.strictEqual(capturedHeaders['x-other'], 'keep', 'X-Other should be preserved'); + } finally { + await stopHTTPServer(origin); + await stopHTTPServer(destination); + } + }); + + it('should reject invalid sensitiveHeaders config', async () => { + await assert.rejects( + axios.get('http://localhost:1/', { sensitiveHeaders: 'X-API-Key' }), + (error) => { + assert.strictEqual(error.code, AxiosError.ERR_BAD_OPTION_VALUE); + assert.strictEqual(error.message, 'sensitiveHeaders must be an array of strings'); + return true; + } + ); + + await assert.rejects( + axios.get('http://localhost:1/', { sensitiveHeaders: [null] }), + (error) => { + assert.strictEqual(error.code, AxiosError.ERR_BAD_OPTION_VALUE); + assert.strictEqual(error.message, 'sensitiveHeaders must be an array of strings'); + return true; + } + ); + }); + + it('should fail closed when sensitiveHeaders redirect origin cannot be parsed', () => { + assert.strictEqual( + __isSameOriginRedirect( + { href: 'http://localhost/final' }, + { url: 'http://localhost/start' } + ), + true + ); + assert.strictEqual( + __isSameOriginRedirect({ href: 'http://[::1' }, { url: 'http://localhost/start' }), + false + ); + assert.strictEqual(__isSameOriginRedirect({ href: 'http://localhost/final' }), false); + }); + it('should wrap HTTP errors and keep stack', async () => { const server = await startHTTPServer( (req, res) => { diff --git a/tests/unit/prototypePollution.test.js b/tests/unit/prototypePollution.test.js index c533991c..ce2b0cbb 100644 --- a/tests/unit/prototypePollution.test.js +++ b/tests/unit/prototypePollution.test.js @@ -28,6 +28,7 @@ describe('Prototype Pollution Protection', () => { delete Object.prototype.baseURL; delete Object.prototype.socketPath; delete Object.prototype.beforeRedirect; + delete Object.prototype.sensitiveHeaders; delete Object.prototype.insecureHTTPParser; delete Object.prototype.adapter; delete Object.prototype.httpAgent; @@ -498,6 +499,34 @@ describe('Prototype Pollution Protection', () => { } }, 10000); + it('should not pick up Object.prototype.sensitiveHeaders during redirects', async () => { + Object.prototype.sensitiveHeaders = ['X-Secret']; + let capturedHeaders; + + const target = await startServer((req, res) => { + capturedHeaders = req.headers; + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end('{"ok":true}'); + }); + const { port: targetPort } = target.address(); + + const redirector = await startServer((req, res) => { + res.writeHead(302, { Location: `http://127.0.0.1:${targetPort}/final` }); + res.end(); + }); + const { port: redirectorPort } = redirector.address(); + + try { + await axios.get(`http://127.0.0.1:${redirectorPort}/start`, { + headers: { 'X-Secret': 'keep' }, + }); + assert.strictEqual(capturedHeaders['x-secret'], 'keep'); + } finally { + await stopServer(redirector); + await stopServer(target); + } + }, 10000); + it('should not enable insecureHTTPParser via Object.prototype', async () => { // A raw TCP server emits a response that uses LF-only line terminators // instead of CRLF. Node's strict HTTP parser rejects this payload with