Skip to content

fix(npm): retain authorization header on http->https redirect#29878

Merged
bartlomieju merged 3 commits intodenoland:mainfrom
nathanwhit:auth-redirect
Mar 12, 2026
Merged

fix(npm): retain authorization header on http->https redirect#29878
bartlomieju merged 3 commits intodenoland:mainfrom
nathanwhit:auth-redirect

Conversation

@nathanwhit
Copy link
Copy Markdown
Member

@nathanwhit nathanwhit commented Jun 24, 2025

Fixes #29869.

Summary

When an npm registry is configured with http:// in .npmrc and the server
redirects to https://, the authorization header was being stripped. This
happened because the previous origin-based check (url.origin() != new_url.origin())
treated scheme changes as cross-origin, since url::Origin includes the scheme.

This caused 401 errors for private registries like GitHub Packages when
configured with http:// URLs.

Changes

  • Extract the redirect auth-stripping logic into a should_strip_auth_on_redirect
    helper function with clear semantics:
    • Strip auth when the host or port changes (true cross-origin redirect)
    • Strip auth on scheme downgrade (https -> http) to avoid leaking
      credentials over plaintext
    • Retain auth on same-host scheme upgrades (http -> https), which are
      safe redirects
  • Add unit tests covering all cases (same-host upgrade, same origin, different
    host, different port, scheme downgrade)

@bartlomieju bartlomieju marked this pull request as ready for review March 12, 2026 10:42
bartlomieju and others added 2 commits March 12, 2026 11:45
Extract redirect auth logic into `should_strip_auth_on_redirect` helper.
Auth headers are now stripped when the host or port changes, or on an
https->http downgrade. Same-host http->https upgrades retain auth.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@kajukitli kajukitli left a comment

Choose a reason for hiding this comment

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

lgtm

the old origin() comparison was too blunt here because it treated a same-host http -> https redirect as cross-origin and dropped auth. the new helper has the right policy: keep auth on same-host upgrades, drop it on host/port changes and on https -> http downgrades.

tests cover the important cases too.

@bartlomieju bartlomieju enabled auto-merge (squash) March 12, 2026 11:04
@bartlomieju bartlomieju merged commit 9d246e7 into denoland:main Mar 12, 2026
112 checks passed
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.

Can't install private npm package with deno (but works with npm)

4 participants