Skip to content

fix: strip proxy-authorization header on cross-host redirect - #196

Open
mahirhir wants to merge 1 commit into
npm:mainfrom
mahirhir:fix-strip-proxy-authorization-on-cross-host-redirect
Open

mahirhir wants to merge 1 commit into
npm:mainfrom
mahirhir:fix-strip-proxy-authorization-on-cross-host-redirect

Conversation

@mahirhir

Copy link
Copy Markdown

When a request is redirected to a different hostname, minipass-fetch already drops the authorization and cookie headers so credentials are not forwarded to the new host (added in #45). The proxy-authorization header is a credential header in the same class, but it is not dropped, so it stays on the redirected request and reaches the new host.

This adds proxy-authorization to that same strip, right next to authorization and cookie. The redirect condition and everything else are unchanged.

The other redirect-following clients already treat proxy-authorization as part of this boundary:

  • undici removes authorization, cookie, and proxy-authorization on a cross-origin redirect.
  • follow-redirects drops Authorization, Proxy-Authorization, and Cookie across host/scheme changes (it added Proxy-Authorization for CVE-2024-28849).
  • @microsoft/kiota-http-fetchlibrary added Proxy-Authorization to its cross-origin scrub recently (CVE-2026-49336).

Scope: this is the same threat model as the existing authorization/cookie strip, that is, a caller that sets a Proxy-Authorization header and then follows a redirect to a different host. It is defence in depth for parity with the clients above, not a new class of exploit.

The existing "redirect to different host strips headers" test now also sends a proxy-authorization header and asserts it is absent on the redirected request. Without the change the test fails, because the header reaches the new host.

When a request redirects to a different hostname, minipass-fetch already
drops the authorization and cookie headers (npm#45) so credentials are not
forwarded to the new host. The proxy-authorization header is in the same
class but was left in place, so it survives onto the redirected request.

Drop it alongside authorization and cookie, matching undici, follow-redirects
(CVE-2024-28849), and @microsoft/kiota-http-fetchlibrary (CVE-2026-49336).
@mahirhir
mahirhir requested a review from a team as a code owner June 29, 2026 17:35
@mahirhir

Copy link
Copy Markdown
Author

No review on this since June. It is a one-line change to credential handling, so rather than argue from the spec I compared what the two nearest implementations do, with file and line for each.

The block on main:

// lib/index.js:219
// if the redirect is to a new hostname, strip the authorization and cookie headers
if (parsedOriginal.hostname !== parsedRedirect.hostname) {
  requestOpts.headers.delete('authorization')
  requestOpts.headers.delete('cookie')
}

Which header names each project removes when it strips:

minipass-fetch  lib/index.js:223                      authorization, cookie
node-fetch      src/index.js:210                      authorization, www-authenticate, cookie, cookie2
undici          lib/handler/redirect-handler.js:190   authorization, cookie, proxy-authorization

stripped by node-fetch or undici but not here: www-authenticate, cookie2, proxy-authorization

The two precedents disagree with each other, which is worth knowing. But this list is a strict subset of both, so whichever one you consider authoritative, something is missing. proxy-authorization is the one undici names explicitly, and it is the one this PR adds.

It matters because of who the header is for. authorization is a credential for the origin, and a redirect to another origin is at least a plausible recipient. proxy-authorization is a credential for the proxy the client was configured with; the destination host is never its intended audience. Forwarding it hands a proxy credential to whoever controls the redirect target.

A second gap, which this PR does not address. The condition compares hostnames only, while node-fetch also requires the same protocol:

redirect                                       minipass strips   node-fetch strips
different host       a.example -> b.other      true              true
same host            a.example -> a.example    false             false
same host, https -> http                       false             true
same host -> subdomain                         true              false

Row three is the one I would flag: a redirect from https://a.example to http://a.example keeps authorization and cookie, and sends them in clear text. Row four is the opposite disagreement, where node-fetch treats a subdomain as trusted and this code does not, which is the more conservative choice and probably deliberate.

I have deliberately not touched either in this PR, because changing the condition is a policy decision about what counts as the same origin, while adding a name to the list is not. Happy to open a separate one for the protocol case if you want it, or to fold it in here if you would rather have one change.

One thing I am not claiming:

$ gh pr checks 196 --repo npm/minipass-fetch
no checks reported on the 'fix-strip-proxy-authorization-on-cross-host-redirect' branch

Nothing runs on this branch. The table above is a comparison of source I fetched from each project, not a live interception test; the existing redirect to different host strips headers test in this repo is what actually exercises the behaviour, and this PR adds the header to both its reqheaders and its badheaders.

Happy to rebase, or to take the one line directly and close this. No attribution needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant