Repository navigation
Conversation
[Medium impact] The changed code appears safe to merge; no actionable issues were found. SummaryThe fetch adapter now rejects non-empty
Acknowledged context:
|
There was a problem hiding this comment.
3 issues found and verified against the latest diff
Confidence score: 3/5
- In
lib/adapters/fetch.js, an invalid non-nullsensitiveHeadersvalue can bypass the check and let fetch send secrets through redirects. Reject malformed values before redirect handling. PRE_RELEASE_DOCS.mdoverstates the rejection rule: it should apply only to an ownsensitiveHeadersproperty, since inherited properties are ignored. Clarify that behavior.PRE_RELEASE_DOCS.mdpoints to a nonexistent security-docs section, so release prep may miss the existing page. Change the target tomisc/security.mdand use its matching translation target.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="PRE_RELEASE_DOCS.md">
<violation number="1" location="PRE_RELEASE_DOCS.md:28">
P2: The security documentation target points to a nonexistent section, so release preparation will miss the existing security page. Replace `advanced/security.md` with `misc/security.md` and use the corresponding translated paths.</violation>
<violation number="2" location="PRE_RELEASE_DOCS.md:29">
P2: Qualify this rejection rule as applying to an own `sensitiveHeaders` property and document that inherited properties are ignored. Otherwise the release documentation can claim rejection for configurations the adapter intentionally treats as absent.</violation>
</file>
<file name="lib/adapters/fetch.js">
<violation number="1" location="lib/adapters/fetch.js:239">
P2: Custom agent: **Flag Security Vulnerabilities**
A non-null invalid value such as `sensitiveHeaders: 'X-API-Key'` bypasses this check, so fetch silently sends the secret through redirects without protection. Reject malformed values here, as the HTTP adapter does.</violation>
</file>
Reply to a comment to ask cubic a question or push back. It learns from your replies.
View guided diff | Re-trigger cubic
| - **Source:** [PR #11425](https://github.com/axios/axios/pull/11425). | ||
| - **Status:** Pending. | ||
| - **Docs targets:** `README.md` Request config and custom secret-bearing header example; `docs/pages/advanced/request-config.md`, `docs/pages/advanced/fetch-adapter.md`, and `docs/pages/advanced/security.md`, with their Spanish, French, and Chinese translations; `THREATMODEL.md` T-R2. | ||
| - **Required content:** Clarify that custom redirect-header stripping is supported only by the Node HTTP adapter, rather than by every adapter running in Node. A non-empty array with `adapter: 'fetch'` rejects with `AxiosError.ERR_NOT_SUPPORT`, including with custom `env.fetch` implementations. Omitted, `null`, and empty arrays remain no-ops. Fetch delegates redirects to the runtime and cannot strip caller-selected headers between hops. Callers requiring this protection should explicitly select `adapter: 'http'` where available or avoid sending the custom secret header; do not recommend removing `sensitiveHeaders` while retaining the secret header. |
There was a problem hiding this comment.
P2: Qualify this rejection rule as applying to an own sensitiveHeaders property and document that inherited properties are ignored. Otherwise the release documentation can claim rejection for configurations the adapter intentionally treats as absent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At PRE_RELEASE_DOCS.md, line 29:
<comment>Qualify this rejection rule as applying to an own `sensitiveHeaders` property and document that inherited properties are ignored. Otherwise the release documentation can claim rejection for configurations the adapter intentionally treats as absent.</comment>
<file context>
@@ -20,6 +20,16 @@ Do not store raw diffs or line-number-only instructions here; prefer stable sect
+- **Source:** The Fetch `sensitiveHeaders` entry in `PRE_RELEASE_CHANGELOG.md`.
+- **Status:** Pending.
+- **Docs targets:** `README.md` Request config and custom secret-bearing header example; `docs/pages/advanced/request-config.md`, `docs/pages/advanced/fetch-adapter.md`, and `docs/pages/advanced/security.md`, with their Spanish, French, and Chinese translations; `THREATMODEL.md` T-R2.
+- **Required content:** Clarify that custom redirect-header stripping is supported only by the Node HTTP adapter, rather than by every adapter running in Node. A non-empty array with `adapter: 'fetch'` rejects with `AxiosError.ERR_NOT_SUPPORT`, including with custom `env.fetch` implementations. Omitted, `null`, and empty arrays remain no-ops. Fetch delegates redirects to the runtime and cannot strip caller-selected headers between hops. Callers requiring this protection should explicitly select `adapter: 'http'` where available or avoid sending the custom secret header; do not recommend removing `sensitiveHeaders` while retaining the secret header.
+- **Examples:** Add `adapter: 'http'` to the existing `X-API-Key`/`sensitiveHeaders` example. Show checking `axios.isAxiosError(error)` and `error.code === 'ERR_NOT_SUPPORT'` when handling an unsupported configuration.
+- **Notes:** No declaration shape or runtime export changes are required. Document the intentional compatibility change: previously accepted fetch requests with this option now reject rather than proceeding without the requested protection.
</file context>
| - **Required content:** Clarify that custom redirect-header stripping is supported only by the Node HTTP adapter, rather than by every adapter running in Node. A non-empty array with `adapter: 'fetch'` rejects with `AxiosError.ERR_NOT_SUPPORT`, including with custom `env.fetch` implementations. Omitted, `null`, and empty arrays remain no-ops. Fetch delegates redirects to the runtime and cannot strip caller-selected headers between hops. Callers requiring this protection should explicitly select `adapter: 'http'` where available or avoid sending the custom secret header; do not recommend removing `sensitiveHeaders` while retaining the secret header. | |
| - **Required content:** Clarify that custom redirect-header stripping is supported only by the Node HTTP adapter, rather than by every adapter running in Node. An own, non-empty `sensitiveHeaders` array with `adapter: 'fetch'` rejects with `AxiosError.ERR_NOT_SUPPORT`, including with custom `env.fetch` implementations; inherited properties are ignored. Omitted, `null`, and empty arrays remain no-ops. Fetch delegates redirects to the runtime and cannot strip caller-selected headers between hops. Callers requiring this protection should explicitly select `adapter: 'http'` where available or avoid sending the custom secret header; do not recommend removing `sensitiveHeaders` while retaining the secret header. |
| - **Change:** The fetch adapter rejects non-empty `sensitiveHeaders` arrays before sending a request instead of silently ignoring them. | ||
| - **Source:** [PR #11425](https://github.com/axios/axios/pull/11425). | ||
| - **Status:** Pending. | ||
| - **Docs targets:** `README.md` Request config and custom secret-bearing header example; `docs/pages/advanced/request-config.md`, `docs/pages/advanced/fetch-adapter.md`, and `docs/pages/advanced/security.md`, with their Spanish, French, and Chinese translations; `THREATMODEL.md` T-R2. |
There was a problem hiding this comment.
P2: The security documentation target points to a nonexistent section, so release preparation will miss the existing security page. Replace advanced/security.md with misc/security.md and use the corresponding translated paths.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At PRE_RELEASE_DOCS.md, line 28:
<comment>The security documentation target points to a nonexistent section, so release preparation will miss the existing security page. Replace `advanced/security.md` with `misc/security.md` and use the corresponding translated paths.</comment>
<file context>
@@ -20,6 +20,16 @@ Do not store raw diffs or line-number-only instructions here; prefer stable sect
+- **Change:** The fetch adapter rejects non-empty `sensitiveHeaders` arrays before sending a request instead of silently ignoring them.
+- **Source:** The Fetch `sensitiveHeaders` entry in `PRE_RELEASE_CHANGELOG.md`.
+- **Status:** Pending.
+- **Docs targets:** `README.md` Request config and custom secret-bearing header example; `docs/pages/advanced/request-config.md`, `docs/pages/advanced/fetch-adapter.md`, and `docs/pages/advanced/security.md`, with their Spanish, French, and Chinese translations; `THREATMODEL.md` T-R2.
+- **Required content:** Clarify that custom redirect-header stripping is supported only by the Node HTTP adapter, rather than by every adapter running in Node. A non-empty array with `adapter: 'fetch'` rejects with `AxiosError.ERR_NOT_SUPPORT`, including with custom `env.fetch` implementations. Omitted, `null`, and empty arrays remain no-ops. Fetch delegates redirects to the runtime and cannot strip caller-selected headers between hops. Callers requiring this protection should explicitly select `adapter: 'http'` where available or avoid sending the custom secret header; do not recommend removing `sensitiveHeaders` while retaining the secret header.
+- **Examples:** Add `adapter: 'http'` to the existing `X-API-Key`/`sensitiveHeaders` example. Show checking `axios.isAxiosError(error)` and `error.code === 'ERR_NOT_SUPPORT'` when handling an unsupported configuration.
</file context>
| - **Docs targets:** `README.md` Request config and custom secret-bearing header example; `docs/pages/advanced/request-config.md`, `docs/pages/advanced/fetch-adapter.md`, and `docs/pages/advanced/security.md`, with their Spanish, French, and Chinese translations; `THREATMODEL.md` T-R2. | |
| - **Docs targets:** `README.md` Request config and custom secret-bearing header example; `docs/pages/advanced/request-config.md`, `docs/pages/advanced/fetch-adapter.md`, and `docs/pages/misc/security.md`, with their Spanish, French, and Chinese translations; `THREATMODEL.md` T-R2. |
|
|
||
| // Native fetch handles redirects without exposing each hop to strip custom headers. | ||
| const sensitiveHeaders = own('sensitiveHeaders'); | ||
| if (utils.isArray(sensitiveHeaders) && sensitiveHeaders.length > 0) { |
There was a problem hiding this comment.
P2: Custom agent: Flag Security Vulnerabilities
A non-null invalid value such as sensitiveHeaders: 'X-API-Key' bypasses this check, so fetch silently sends the secret through redirects without protection. Reject malformed values here, as the HTTP adapter does.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At lib/adapters/fetch.js, line 239:
<comment>A non-null invalid value such as `sensitiveHeaders: 'X-API-Key'` bypasses this check, so fetch silently sends the secret through redirects without protection. Reject malformed values here, as the HTTP adapter does.</comment>
<file context>
@@ -234,6 +234,16 @@ const factory = (env) => {
+ // Native fetch handles redirects without exposing each hop to strip custom headers.
+ const sensitiveHeaders = own('sensitiveHeaders');
+ if (utils.isArray(sensitiveHeaders) && sensitiveHeaders.length > 0) {
+ throw new AxiosError(
+ 'sensitiveHeaders is not supported by the fetch adapter; it is only honored by the http adapter',
</file context>
There was a problem hiding this comment.
2 existing issues remain and 1 new issue found across 5 files
Confidence score: 3/5
- In
lib/adapters/fetch.js, malformedsensitiveHeadersvalues can leave secret headers exposed across fetch redirects. Reject every non-null value that isn’t an array. - In
tests/unit/adapters/fetch.test.js, failure to start the second server can leavetargetlistening. Protect both server acquisitions with cleanup. - In
PRE_RELEASE_DOCS.md, the security documentation target points to a nonexistent path, which could leave the actual summary and translations stale. Update it todocs/pages/misc/....
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/unit/adapters/fetch.test.js">
<violation number="1" location="tests/unit/adapters/fetch.test.js:58">
P2: The test leaks `target` when the second server cannot start because acquisition happens before `try`. Start both servers inside the protected setup or add an outer cleanup path so setup failures cannot leave a listening server behind.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.
View guided diff | Re-trigger cubic
| describe('sensitiveHeaders', () => { | ||
| it('rejects before sending a secret to a cross-origin redirect', async () => { | ||
| const received = []; | ||
| const target = await startHTTPServer((req, res) => { |
There was a problem hiding this comment.
P2: The test leaks target when the second server cannot start because acquisition happens before try. Start both servers inside the protected setup or add an outer cleanup path so setup failures cannot leave a listening server behind.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At tests/unit/adapters/fetch.test.js, line 58:
<comment>The test leaks `target` when the second server cannot start because acquisition happens before `try`. Start both servers inside the protected setup or add an outer cleanup path so setup failures cannot leave a listening server behind.</comment>
<file context>
@@ -52,6 +52,103 @@ const createBrokenDOMExceptionLikeError = () =>
+ describe('sensitiveHeaders', () => {
+ it('rejects before sending a secret to a cross-origin redirect', async () => {
+ const received = [];
+ const target = await startHTTPServer((req, res) => {
+ received.push(req.headers['x-api-key']);
+ res.end('ok');
</file context>
Summary
The fetch adapter accepts
sensitiveHeaderswithout applying it. A caller protectingX-API-Keycan therefore send that header to a different origin after a native fetch redirect. Reject non-empty arrays withAxiosError.ERR_NOT_SUPPORTbefore dispatch, explaining that the Node HTTP adapter is required for this protection.Linked issue
No public issue. Reproduced against the current
v1.xbase: a local cross-origin redirect received the configured test secret before the fix; after the fix, both servers receive zero requests.Changes
sensitiveHeadersproperty and reject before signal subscription or request construction.Validation
Environment: macOS arm64, Node 22.22.3, npm 10.9.8, Vitest 4.1.12. Browser tests used Playwright Chromium, Firefox, and WebKit installed with
npx playwright install chromium firefox webkit.npm ci --ignore-scripts: passed.npm run lint: passed.npm run build: passed.npm run test:vitest:unit -- tests/unit/adapters/fetch.test.js tests/unit/adapters/http.test.js -t sensitiveHeaders: 13 passed, including all seven new Node tests and the existing HTTP adapter tests.npm run test:vitest:browser:headless -- tests/browser/fetchSensitiveHeaders.browser.test.js --browser.api.host=127.0.0.1: 12 passed, across all three browsers. Proxy environment variables were unset for browser runs to keep loopback traffic local.npm run test:vitest:unit: 1,655 passed, 57 failed, 34 unhandled errors. Failures include fixed-portEADDRINUSEerrors on port 8020, timing/child-process assertions, and existing defensive tests. Three failing cases were independently reproduced in an untouched copy of the base: inheritedSymbol.iteratorrequest headers, inheritedinsecureHTTPParser, and the connection-refusal error assertion.npm run test:vitest:browser:headless -- --browser.api.host=127.0.0.1: 1,073 passed, 4 failed, 3 skipped. Remaining failures are the Firefox native XHR progress tests and ReDoS timing assertion, plus a WebKit localized Date-header assertion.git diff --check: passed. No dependency, workflow, version, or generated bundle changes.The full suites are not green in this local environment, so this PR remains a draft pending successful full-suite validation. The final follow-up commit only adds this PR's links to the prerelease notes; its Markdown formatting and diff checks passed.
GitHub Actions for the latest head,
861f104644a311c28a3c9436098c3badadb76f7a, remain unverified: the CI diagnostics integration could not connect to GitHub. The initial PR status at93f2338listed only in-progress review/security bot checks, which are not build or runtime-matrix evidence.Compatibility and documentation
Patch security hardening with an intentional behavior change: fetch requests using a non-empty
sensitiveHeadersarray now reject instead of proceeding without the requested protection. Omitted,null, and empty arrays retain their existing behavior. Native and custom fetch implementations are covered.PRE_RELEASE_DOCS.mdrecords the HTTP-adapter limitation, error handling, and safe migration guidance for README/docs/translations and the threat model. Public declarations and exports are unchanged; CJS/ESM type and packed-package compatibility suites are not applicable because this PR does not change types, exports, packaging, or build configuration.Checklist
v1.x), and rationale checkedPRE_RELEASE_CHANGELOG.mdentry addedAI assistance was used for this change and description. A maintainer needs to apply
ai-assisted: creating the PR succeeded, but GitHub denied this account permission to assign the requested label.