Skip to content

fix: custom auth headers not stripped on cross-origin redirects#10892

Merged
jasonsaayman merged 9 commits into
axios:v1.xfrom
sapirbaruch:fix/v1x-sensitive-headers-cross-origin-redirect
Jun 3, 2026
Merged

fix: custom auth headers not stripped on cross-origin redirects#10892
jasonsaayman merged 9 commits into
axios:v1.xfrom
sapirbaruch:fix/v1x-sensitive-headers-cross-origin-redirect

Conversation

@sapirbaruch

@sapirbaruch sapirbaruch commented May 14, 2026

Copy link
Copy Markdown
Contributor

Found this while auditing redirect behavior in a service that uses X-API-Key for authentication. Standard headers like Authorization are stripped correctly on cross-origin redirects, but any custom auth headers survive the redirect to the new origin.

This extends the existing cross-origin header-stripping logic to also cover a configurable list of sensitive header names via the new sensitiveHeaders config option. On a cross-origin redirect, any header in that list is removed from the outgoing request.

Refs #10711.


Summary by cubic

Fixes a security bug where custom auth headers could leak on cross-origin redirects in the Node HTTP adapter. Adds a case-insensitive sensitiveHeaders option with fail-closed origin checks and prototype‑pollution guards.

Description

  • Summary of changes

    • Add options.beforeRedirects.sensitiveHeaders to strip caller‑listed headers on cross‑origin redirects (case‑insensitive).
    • Implement isSameOriginRedirect (fail closed on parse errors) and export __isSameOriginRedirect for tests.
    • Validate sensitiveHeaders as an array of strings; reject with ERR_BAD_OPTION_VALUE otherwise.
    • Ignore Object.prototype.sensitiveHeaders when reading config.
    • Add sensitiveHeaders?: string[] to index.d.ts and index.d.cts.
    • Update threat model to follow-redirects@^1.16.0.
    • Move usage notes to PRE_RELEASE_CHANGELOG.md; keep README snippet.
  • Reasoning

    • Prevent leaking custom secrets (for example X-API-Key) to other origins while preserving same‑origin behavior.
  • Additional context

    • Node HTTP adapter only; unused when maxRedirects is 0.

Docs

Add /docs/ updates with sensitiveHeaders examples and security notes. Keep them aligned with the README snippet and the new entry in PRE_RELEASE_CHANGELOG.md.

Testing

Added/updated unit tests for:

  • Cross‑origin stripping, same‑origin preservation, and case‑insensitive matching.
  • Instance‑level config and invalid config rejection.
  • Fail‑closed origin parsing via __isSameOriginRedirect.
  • Prototype‑pollution guard to ignore Object.prototype.sensitiveHeaders.

Semantic version impact

Patch: backward‑compatible security hardening behind an optional Node‑only config.

Written for commit d493090. Summary will update on new commits.

Review in cubic

@sapirbaruch sapirbaruch requested a review from jasonsaayman as a code owner May 14, 2026 08:44

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

@jasonsaayman

Copy link
Copy Markdown
Member

@sapirbaruch this fails ci, please check linting

Comment thread lib/adapters/http.js Outdated
Comment thread lib/adapters/http.js
Comment thread lib/adapters/http.js Outdated
@jasonsaayman jasonsaayman added status::changes-requested A reviewer requested changes to the PR status::failing-ci The PR is failing CI labels May 24, 2026
sapirbaruch and others added 3 commits May 24, 2026 20:11
…ders

- Fix delete logic to iterate Object.keys(headers) and compare
  lowercased, so headers like X-API-Key are stripped regardless
  of the casing preserved by follow-redirects
- Add sensitiveHeaders?: string[] to AxiosRequestConfig in both
  index.d.ts and index.d.cts
- Add three HTTP adapter tests: cross-origin strips, same-origin
  preserves, and case-insensitive matching

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 8 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread THREATMODEL.md Outdated
jasonsaayman and others added 4 commits June 1, 2026 18:43
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Resolved conflicts in README.md and THREATMODEL.md:
- README.md: kept sensitiveHeaders docs block, adopted upstream's
  updated beforeRedirect comment and fuller security note
- THREATMODEL.md: kept PR's version (follow-redirects@^1.16.0,
  sensitiveHeaders mitigation row)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@jasonsaayman jasonsaayman self-requested a review June 3, 2026 07:24
@sapirbaruch

Copy link
Copy Markdown
Contributor Author

Hi @jasonsaayman — all three points from your review have been addressed in the subsequent commits:

  1. Case-insensitive header matching: stripMatchingHeaders now iterates Object.keys(redirectOptions.headers) and compares each key via .toLowerCase() against the sensitive set, so X-API-Key, x-api-key, and x-Api-Key are all caught.

  2. TypeScript types: sensitiveHeaders?: string[] is now in both index.d.ts and index.d.cts.

  3. Tests: tests/unit/adapters/http.test.js now covers cross-origin strip, same-origin preserve, case-insensitive matching, instance-level config, invalid config rejection, and fail-closed origin parsing. All 816 tests pass and npm run lint exits clean.

Happy to address any further feedback.

@jasonsaayman

Copy link
Copy Markdown
Member

Hi @jasonsaayman — all three points from your review have been addressed in the subsequent commits:

  1. Case-insensitive header matching: stripMatchingHeaders now iterates Object.keys(redirectOptions.headers) and compares each key via .toLowerCase() against the sensitive set, so X-API-Key, x-api-key, and x-Api-Key are all caught.
  2. TypeScript types: sensitiveHeaders?: string[] is now in both index.d.ts and index.d.cts.
  3. Tests: tests/unit/adapters/http.test.js now covers cross-origin strip, same-origin preserve, case-insensitive matching, instance-level config, invalid config rejection, and fail-closed origin parsing. All 816 tests pass and npm run lint exits clean.

Happy to address any further feedback.

Thanks nothing further to do, I am trying to make sure we dont release docs on un-released improvements so thats why im making some adjustments.

@jasonsaayman jasonsaayman merged commit 6bb12c1 into axios:v1.x Jun 3, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commit::fix The PR is related to a bugfix priority::medium A medium priority status::changes-requested A reviewer requested changes to the PR status::failing-ci The PR is failing CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants