Skip to content

fix: forward git auth to hosted CDN tarball downloads - #2

Open
dyk1454683243-sudo wants to merge 1 commit into
mainfrom
cursor/fix-codeload-authorization-d838
Open

dyk1454683243-sudo wants to merge 1 commit into
mainfrom
cursor/fix-codeload-authorization-d838

Conversation

@dyk1454683243-sudo

Copy link
Copy Markdown
Owner

Fixes npm#331

Cause

When installing a hosted git dependency (git+https://oauth2:TOKEN@github.com/org/private.git), pacote prefers the provider CDN tarball (https://codeload.github.com/.../tar.gz/<sha>) over git clone.

hosted-git-info's tarball() URL does not include git URL userinfo. npm-registry-fetch then looks up credentials by the request host (codeload.github.com), so neither:

  • credentials embedded in the git URL (oauth2:TOKEN@github.com), nor
  • credential-store entries for the git host (//github.com/:_authToken)

are sent. GitHub returns 404 for the private archive, and pacote falls back to git+ssh.

Fix

When GitFetcher shells out to RemoteFetcher for a hosted tarball, compute forceAuth the same way other authenticated pacote fetches do:

  1. Translate git URL userinfo (hosted.auth) to HTTP Basic (percent-decoding first).
  2. Otherwise, if the CDN host has no stored credentials, reuse //<git-host>/ credential-store auth (_authToken → Bearer, _auth / username+password → Basic).
  3. Leave an explicit Authorization header or a CDN-specific token (//codeload.github.com/:_authToken) alone.

npm-registry-fetch then sets Authorization on the CDN request.

Tests

  • Unit tests for URL userinfo, percent-encoding, //github.com/:_authToken, Basic store entries, and CDN-host precedence.
  • Integration tests mock https://codeload.github.com and assert the Authorization header on the tarball GET.

This is distinct from npm#285 / npm#514 (hardlink extract filter).

Open in Web Open in Cursor 

Hosted-git-info tarball URLs (codeload.github.com) omit git URL
userinfo, and npm-registry-fetch looks up credentials by request host.
Pass URL credentials and //<git-host>/ credential-store entries as
forceAuth so the CDN request gets an Authorization header.

Fixes: npm#331

Co-authored-by: David <dyk1454683243-sudo@users.noreply.github.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 20, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-20T15:54:14.105026Z a826ae4 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a826ae4c7f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


const headersHaveAuthorization = (opts) => {
const headers = opts.headers || {}
return !!(headers.authorization || headers.Authorization)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Detect authorization headers case-insensitively

HTTP header names are case-insensitive, but this recognizes only authorization and Authorization. If a caller supplies another valid casing such as AUTHORIZATION while git-host credentials are configured, the helper adds forceAuth; npm-registry-fetch then emits both the caller's header and the forwarded credential as duplicate Authorization fields, which proxies or the CDN may reject or interpret using the wrong credential. Normalize the supplied header names before deciding whether authorization is already present.

Useful? React with 👍 / 👎.

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.

[BUG] CDN download doesn't use provided HTTP Authorization token (even if it's provided hardcoded in the URL)

2 participants