Skip to content

install: send credentials embedded in a tarball URL as Basic authorization - #39025

Closed
robobun wants to merge 1 commit into
mainfrom
farm/a6acfa69/tarball-url-credentials
Closed

robobun wants to merge 1 commit into
mainfrom
farm/a6acfa69/tarball-url-credentials

Conversation

@robobun

@robobun robobun commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • A dependency declared as a tarball URL with credentials, "no-deps": "http://carol:s3cret@127.0.0.1:PORT/cdn/no-deps-1.0.0.tgz", is downloaded without an Authorization header. A server that needs the credentials answers 401 and the install fails with error: GET http://carol:s3cret@127.0.0.1:PORT/cdn/no-deps-1.0.0.tgz - 401 (bun 1.4.0 and main, hoisted and isolated linker). npm 11 installs the same package.json and sends Authorization: Basic Y2Fyb2w6czNjcmV0 (base64("carol:s3cret")).
  • The username-only form (http://token@127.0.0.1:PORT/x.tgz) does not reach the server at all: the request goes to the hostname token@127.0.0.1.
  • Cause: NetworkTask::for_tarball (src/install/NetworkTask.rs) only ever attaches the registry scope's configured token or _auth, and only for npm packages whose tarball is on the registry's origin. Nothing reads the URL's userinfo, and the HTTP client does not either (it only turns the userinfo of a proxy URL into Proxy-Authorization). The misrouted username-only form comes from bun_url::URL::parse, which takes token@127.0.0.1 for the hostname when the userinfo has no : and a port follows (tracked separately; this change no longer depends on how that parser splits the authority).
  • Found while fixing the isolated store names of these URLs (install: leave URL credentials out of isolated store entry names #39014), not from a user report.

Fix

  • for_tarball splits the userinfo off the request URL before anything else looks at it (split_url_userinfo: the authority runs from :// to the first /, ? or #, the userinfo is everything in it up to the last @, so the @ of /@scope/pkg/-/pkg.tgz is not one) and sends it as Authorization: Basic base64(userinfo) (basic_authorization_from_userinfo, which appends : when the userinfo has no password). The request URL is the URL without the userinfo.
  • Precedence: when the registry scope's credentials apply to the tarball (npm package, tarball on the registry origin, scope has a token or _auth), they are still sent and the URL's are not. This is npm's order as well: npm-registry-fetch sets the header from the config, and node only derives Authorization from the URL's auth when the request has no such header.
  • Why Basic of the userinfo as written: it is what npm sends, checked against npm 11.16 for user:pass, user (sent as user:), :pass and a percent-encoded password (sent undecoded); transcript below. It is also how bun already treats credentials written into a registry URL (install: send credentials embedded in --registry and registry env var URLs #38796 stores the username and password bytes as given). pnpm was not available offline. Two spellings are deliberately not npm's: a second : inside the password is sent as written where npm percent-encodes it, and :token@ in a tarball URL is Basic like npm rather than the Bearer that bun's registry URLs make of it; both are called out in the test table and in the code comment.
  • Why the URL is requested without the userinfo: bun_url::URL::origin includes the userinfo and the HTTP client compares origins to decide whether Authorization follows a redirect, so with the userinfo left in, a redirect to the same host would drop the header (verified: with only the header added, the redirect test below fails with authorization: null on the second hop). It also fixes the username-only form without touching bun_url, and the GET <url> - 401 line now prints the URL without the credentials. The cross-host rule is unchanged: the HTTP client strips the header on a redirect to another origin, same as for the registry token (test included).
  • Behavior outside the request is unchanged: package.json, the lockfile, the task id and the cache key still use the URL as written. Documented in docs/pm/cli/add.mdx.
  • Verified with test/cli/install/bun-install.test.ts, describe("credentials embedded in a tarball URL"): 12 tests, 10 of which fail on the unfixed build (the two guards, scoped path and registry credentials taking precedence, pass on both). The rest of the file is unchanged: the remaining failures locally are the tests that need the public internet, identical with the unmodified binary.
  • cargo check -p bun_install, cargo clippy -p bun_install, rustfmt and prettier are clean.

Background

  • Tarball dependency: a package.json entry whose version is an http(s):// URL ending in .tgz/.tar.gz/.tar. Bun downloads it directly; no registry manifest is involved, so the registry's configured credentials never applied to it (Authorization::NoAuthorization at the call sites in PackageManagerEnqueue.rs). Registry packages reach the same for_tarball through their manifest's dist.tarball URL with AllowAuthorization, which is the case where the registry scope's credentials can apply.
  • Userinfo: the user:password@ part of a URL's authority (RFC 3986 section 3.2.1). HTTP never puts it on the wire; clients that honor it (npm through minipass-fetch, curl, browsers) convert it into Authorization: Basic base64(user:password).
  • NetworkTask::for_tarball builds one HTTP request per tarball download: url_buf (the request URL) and header_buf (the headers, built with HeaderBuilder in two passes, count then append, because the buffer is allocated exactly once in between). Retries reuse the same request, so the header is sent on every attempt.
  • bun_url::URL::parse is the allocation-free splitter the HTTP client and for_tarball use; it is not a WHATWG parser. Its origin is a prefix of the input string, which is why it still contains the userinfo.
npm 11.16 against a local server logging the Authorization header (same tarball, same package.json shape)
userinfo in the dependency URL   header npm sent
carol:s3cret@                    Basic Y2Fyb2w6czNjcmV0      = base64("carol:s3cret")
carol@                           Basic Y2Fyb2w6              = base64("carol:")
:s3cret@                         Basic OnMzY3JldA==          = base64(":s3cret")
carol:s3%40cret@                 Basic Y2Fyb2w6czMlNDBjcmV0  = base64("carol:s3%40cret"), not decoded
carol:s3:cret@                   Basic Y2Fyb2w6czMlM0FjcmV0  = base64("carol:s3%3Acret"), bun sends base64("carol:s3:cret")
carol:s3cret@ + 302 to same host Basic ... on both hops

bun 1.4.0-canary.1 (eabb96d) for the first row: the server logs auth=null and bun prints error: GET http://carol:s3cret@127.0.0.1:PORT/cdn/no-deps-1.0.0.tgz - 401.

…ation

A dependency declared as http://user:pass@host/pkg.tgz (or a registry
dist.tarball URL of that shape) was requested without any Authorization
header. NetworkTask::for_tarball now splits the userinfo off the request
URL and sends it as Authorization: Basic base64(user:pass), the header
npm sends for such URLs. Credentials configured for the registry scope
still take precedence when they apply to the tarball. The request URL no
longer carries the userinfo, so the HTTP client's same-origin check keeps
the header across a redirect within the host and error output prints the
URL without the credentials.
@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 16 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 634e1f05-7d46-477e-a7ca-d42e94fcf8ac

📥 Commits

Reviewing files that changed from the base of the PR and between 88a6398 and 25f7523.

📒 Files selected for processing (3)
  • docs/pm/cli/add.mdx
  • src/install/NetworkTask.rs
  • test/cli/install/bun-install.test.ts

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator Author

Status: reproduced and fixed, waiting for CI.

  • Reproduced on bun 1.4.0-canary.1 (eabb96d) and main: a local server logging the Authorization header receives null for a http://carol:s3cret@127.0.0.1:PORT/cdn/no-deps-1.0.0.tgz dependency and the install fails with a 401; npm 11.16 sends Basic Y2Fyb2w6czNjcmV0 for the same package.json.
  • Fix in src/install/NetworkTask.rs (for_tarball); tests in test/cli/install/bun-install.test.ts, describe("credentials embedded in a tarball URL"): 10 of the 12 fail on the unfixed build and all 12 pass with it.

@claude claude 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.

I reviewed this PR and didn't find any bugs. Because it changes how Authorization headers are derived and sent for tarball downloads (credential handling, precedence vs. registry auth, redirect behavior), a human look would still be worthwhile.

Checked split_url_userinfo against scoped paths, empty userinfo, and multi-@ authorities — bounding on first /?# then last @ handles them.
Checked the HeaderBuilder two-pass count/append stays balanced for the URL-auth branch (debug_assert_eq!(content.len, content.cap) holds; npm-auth-type is correctly omitted).
Checked lifetimes: userinfo borrows the old url_buf and is fully consumed into a fresh Vec before the box is reassigned.
Checked that registry-scope credentials still win (header_count == 0 gate) and cross-origin redirects still strip the header (URL is requested without userinfo, so the existing origin compare works).

Extended reasoning...

Overview

The PR makes NetworkTask::for_tarball in src/install/NetworkTask.rs extract the userinfo from a tarball URL, send it as Authorization: Basic base64(userinfo), and request the URL without the userinfo. Two new helpers (split_url_userinfo, basic_authorization_from_userinfo) plus ~25 lines of integration in for_tarball. It adds 12 subprocess tests in test/cli/install/bun-install.test.ts and a doc line in docs/pm/cli/add.mdx.

Security risks

This is credential-handling code. The risks I looked for: leaking credentials to the wrong host (guarded — the existing HTTP client strips Authorization on cross-origin redirects, and stripping userinfo from the request URL means the origin comparison now works correctly for same-host redirects too), leaking registry credentials to a URL-specified host (unchanged — the origin match against the registry scope still gates send_auth, and registry credentials take precedence over URL ones when both apply), and printing credentials in error output (improved — the 401 error line now shows the URL without userinfo). The credentials sent are only those the user explicitly wrote in the URL for that host, or that a registry embedded in its own dist.tarball (in which case the registry already knows them). No new leak surface identified.

Level of scrutiny

High. Even though the change is small and closely follows npm's documented behavior (with a transcript against npm 11.16), it decides when and how an Authorization header is attached. Per the approval guidelines, auth-sensitive code should have a human sign-off regardless of apparent correctness.

Other factors

The implementation is careful: split_url_userinfo bounds the authority at the first /?# before searching for the last @, so /@scope/pkg paths are not misread (tested). The HeaderBuilder count/append two-pass stays byte-exact for the new branch (only Authorization counted and appended; npm-auth-type omitted for URL creds). Borrow of the old url_buf via userinfo ends before reassignment. Tests use local servers, port: 0, tempDir, await using, and concurrent stdout/stderr/exited drains, and cover the userinfo variant matrix, both linkers, redirects (same-host keeps, cross-host drops), the scoped-path guard, error-message redaction, and both precedence directions with registry credentials. No CODEOWNERS entry covers these paths. No prior human review comments to address.

@robobun

robobun commented Aug 15, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 10:28 AM PT - Aug 15th, 2026

❌ @robobun, your commit 25f7523 has some failures in Build #98030 (All Failures)


🧪   To try this PR locally:

bunx bun-pr 39025

That installs a local version of the PR into your bun-39025 executable, so you can run:

bun-39025 --bun

@Jarred-Sumner

Copy link
Copy Markdown
Collaborator

Folded into #38867.

dylan-conway pushed a commit that referenced this pull request Aug 17, 2026
…ile: tarballs relative to their folder package (#38867)

This PR bundles three independent install fixes (each was reviewed on
its own PR; folded here so they land together):

1. bound the resolution part of isolated store entry names (this PR's
original change)
2. send credentials embedded in a tarball URL as Basic authorization
(from #39025)
3. read `file:` tarballs relative to the `file:` folder package that
declares them (from #39017)

Rebased on main after #39014: the isolated-store test file now computes
expected git entry names through the same resolution cut
(`storeEntryName`), since a `git+http+++host+port+repo.git+<url
hash>+<sha>` resolution passes 80 bytes.

---

## 1. install: bound the resolution part of isolated store entry names

#### Problem

- With `linker = "isolated"`, a trusted git dependency with any
lifecycle script makes `bun install` fail on Windows with `error: Failed
to run script prepare due to error ENOENT` (exit 1). The same project
installs fine with the hoisted linker. This is the Windows failure of
the isolated case in #38810's test, which that PR currently skips on
Windows.
- A store entry is named `<name>@<resolution>` (`StoreKeyFormatter`,
`src/install/isolated_install/Store.rs`). For git, github, tarball and
folder dependencies the resolution is the whole URL or path with
separators turned into `+` (`src/install/repository.rs`
`StorePathFormatter`, `src/install/resolution.rs` `StorePathFormatter`),
plus the commit for git: a git dependency checked out from a temp
directory gets an entry like
`dep@git+file++++C++Users+AZUREU~1+AppData+Local+Temp+lc-repro+dep-repo+3c6955c70dbe1ff186a126c93c5e77c3ae7bd6b4`
(111 bytes here, 170+ in #38810's test), and the name grows with the
repository path.
- The package directory,
`<project>\node_modules\.bun\<entry>\node_modules\<name>`, is the cwd of
the package's lifecycle scripts (`Scripts::get_list` ->
`lifecycle_script_runner.rs` `SpawnOptions.cwd`). bun's own file
operations use long-path-capable NT paths, so the entry is created and
linked, but `CreateProcessW` does not accept a current directory longer
than `MAX_PATH`, and the spawn (libuv's `uv_spawn` on Windows) reports
that as ENOENT. Measured on Windows x64 with the released
`1.4.0-canary.1`: a package directory of 238 characters runs the script,
one of 268 characters fails as above.
- The same unbounded name also fails on every platform once it passes
`NAME_MAX`: a tarball URL or folder path of 250+ bytes makes the install
fail with `ENAMETOOLONG: File name too long: failed to link package`
(the case of #37470).

#### Fix

- `StoreKeyFormatter` now writes the resolution through
`write_resolution`: a resolution of at most 80 bytes
(`MAX_RESOLUTION_LEN`) is written unchanged; a longer one is written as
its first 63 bytes (backed up to a UTF-8 character boundary) followed by
`+` and the 16 hex digit wyhash of the full text (seed 0, so
`Bun.hash(text)` reproduces it), which is 80 bytes again. The `name@`
prefix and the `+<peer hash>` suffix are unchanged. A cut git entry
looks like
`dep@git+file++++C++Users+AZUREU~1+AppData+Local+Temp+lc-repro-aaaaa+ef7ad81a431ad661`
(the 268 character case from the measurement above; its package
directory is now 206 characters).
- Why this is correct:
- Every path that contains an entry name is formatted through this one
`Display` impl: the project store directory and the dependency symlinks
into it (`Installer.rs` `append_store_path` /
`append_store_node_modules_path` / `link_to_hidden_node_modules`), the
lifecycle script cwd (same `append_store_path`), the global store
directory name and the entry hash seeded from the name (`Installer.rs`
`append_global_store_entry_path`, `isolated_install.rs` entry hash),
`bun pm prune`'s set of expected directories (`prune.rs`
`push_store_entry_names`) and `bun pm licenses`'s lookup
(`pm_licenses_command.rs` `BunStore::lookup`, via `fmt_store_key`). They
all see the same cut name; nothing persists or parses the old one (the
lockfile stores resolutions, and an existing entry is detected by
re-deriving its name).
- The cut name stays a function of the resolution alone, so repeated
installs find the same entry, and two resolutions that agree on the
first 63 bytes still get distinct entries through the hash. The sink
hashes the text in whatever chunks the formatters write it in (the path
formatters write one character at a time); `Wyhash`'s streaming form is
chunk-invariant (`test_iterative_chunked_matches_oneshot` in
`src/wyhash/lib.rs`), which is what lets the tests compute the expected
names with `Bun.hash`.
- The cut happens inside the resolution only, so `prune.rs`
(`split_store_key`, `store_has_entries`, `store_link_target`, which
split the name at `@`) and `pm licenses` (which matches `<key>` or
`<key>+<peer hash>`) keep working without changes.
- Resolutions of 80 bytes or less are byte for byte what they were, so
versions and `github:` shorthands (`github+owner+repo+<sha>` is about 60
to 75 bytes) are unaffected on upgrade. Most full git URLs are over 80
bytes (`git+https+++github.com+` is 23 bytes and the commit adds 41), so
those entries are re-created once under the cut name on the first
install after upgrading, and the old directories stay until `bun pm
prune`, as for any re-resolved entry. The cut keeps the URL itself
readable:
`git-pkg@git+file++++tmp+licenses-git-repo_eXITMt+491eb29762d8d19b570b43+f87e35b31f207dd4`.
- 80 is the tunable here. The package directory is `<project> + 34 + 2 *
<name> + <resolution>` (+17 with peers) characters, so with 80 it fits
`MAX_PATH` whenever `<project> + 2 * <name>` is at most 145 (128 with
peers); #38810's case (61 character temp dir, 25 character name) ends up
at about 225 instead of about 290. It also makes the entry directory
name fit `NAME_MAX` together with every suffix the installers append
(`+<peer hash>`, `-<entry hash>`, `.tmp-<hex>`) for names up to 119
bytes, and with the peer suffix alone for names up to 157 bytes. For
comparison, pnpm's `virtual-store-dir-max-length` defaults to 120 bytes
for the whole directory name; 80 bytes of resolution plus a typical name
lands in the same range.
- #37470 (the same mechanism with a 200 byte limit on the whole
`name@resolution`, for the `NAME_MAX` case only) is closed in favor of
this PR: that limit does not help the `MAX_PATH` case (the failing names
above are 130 to 180 bytes), and a cut through `name@` would no longer
work with `bun pm prune` / `pm licenses` (see the `prune.rs` bullet
above). Its repro installs with this branch and its deep-directory case
is carried over below. What it covered and this PR does not is the name
part: a name over 119 bytes (157 in the project store) can still produce
an entry over `NAME_MAX` once its resolution is cut; if that should be
covered too, it would be the same sink applied to the name separately,
keeping the `@`. #36973 (documenting the layout) would need a sentence
about this rule; this PR adds a short one to
`docs/pm/isolated-installs.mdx`. #38810's Windows skip of its isolated
case can be removed once both land. #39014 changes the same text (it
removes URL credentials, and the hash is of the text as formatted), so
landing it in the same release as this avoids renaming those entries
twice.
- Verification, `long store entry names` in
`test/cli/install/isolated-install.test.ts`:
- a folder resolution of exactly 80 bytes is kept verbatim and one of 81
bytes is cut to the name computed with `Bun.hash`; two resolutions of
one package that only differ after the cut point get separate entries
that each resolve to their own package; a second install finds the same
entries
- a cut entry with a resolved peer gets `+<peer hash>` appended after
the cut name and links the peer
- a folder name made of 2 byte characters, positioned so that byte 63
falls inside a character, is cut at byte 62 and still installs (without
the character boundary loop this case panics with `a formatting trait
implementation returned an error`; the test only asserts the shape
because the spelling of non-ASCII bytes in these names is #32304's
subject)
- (carried over from #37470) a local tarball and a folder three 85 byte
directories deep, whose unbounded entry names would be 277 bytes,
install; the top-level symlinks and the `.bun/node_modules` fallback
link point at the cut names, and a second install finds the same entries
- a tarball URL of about 290 bytes installs with `install.globalStore`
enabled: the local entry has the cut name, links to `links/<cut
name>-<entry hash>`, and the package imports at runtime
- the reported case: a trusted `git+file://` dependency whose repository
directory name alone is 60 characters runs its postinstall script with
the isolated linker; the entry has the cut name
- Without the `Store.rs` change the first three tests and the git one
fail on the entry name (on Windows the git one fails with `error: Failed
to run script postinstall due to error ENOENT`, the reported message)
and the deep-directory and tarball ones exit 1 with `ENAMETOOLONG`; all
of them pass with `bun bd test` on Linux (the five original ones also on
a local Windows x64 build; the deep-directory one on the Windows lanes
in CI). #37470's repro from its description (`./x/../` x130 tarball
spec) installs with this branch with an 84 byte entry.
- Also run with the fix: the rest of `isolated-install.test.ts`,
`bun-pm-licenses.test.ts` (its `git dependency is listed (isolated)`
case now goes through a cut name on every platform, since the repo lives
in a temp dir), `bun-prune.test.ts`, `public-hoist-pattern.test.ts`,
`isolated-relink.test.ts`, `bun-install-native-binlink.test.ts`,
`config-version.test.ts`, and the PowerShell reproduction from the
report on Windows x64 (package directory of 268 characters before, 206
after, script runs).

#### Background

- Isolated linker: instead of hoisting, every package is materialized
once under `node_modules/.bun/<entry>/node_modules/<name>` and
everything else symlinks to it. `<entry>` is `<name>@<resolution>`, plus
`+<16 hex>` when the package was installed for a specific set of peer
dependencies. With `install.globalStore`, `node_modules/.bun/<entry>` is
itself a symlink to `<cache>/links/<entry>-<16 hex entry hash>`.
- Resolution: the lockfile's description of where a package came from.
It is the version for registry packages and the spec itself (folder
path, tarball URL, git URL plus commit) for everything else, which is
why only those entries get long.
- `MAX_PATH`: Windows' 260 character limit for paths passed to Win32
APIs. Applications can opt out of it for file operations (bun does, by
using `\\?\` / NT paths), but the current directory handed to
`CreateProcess` is still limited to it, and libuv maps the resulting
Win32 error to ENOENT.
- `NAME_MAX`: the 255 byte limit on one path component on Linux and
macOS (255 UTF-16 units on NTFS); exceeding it fails with `ENAMETOOLONG`
regardless of how long the whole path is.

<details>
<summary>Windows measurement with the released build
(1.4.0-canary.1)</summary>

Git repo and project created under `%TEMP%` with `linker = "isolated"`,
`trustedDependencies: ["dep"]` and `"prepare": "echo prepared >
prepared.txt"`; the temp directory name was lengthened to move the
package directory across the limit.

```
store entry: dep@git+file++++C++Users+AZUREU~1+AppData+Local+Temp+lc-repro-aaaaaaaaaaaaaaaaaaa+dep-repo+d1a8f7c4111533cb16b52a9edcb086329e6472b5
pkg dir len: 238
prepared.txt exists: True
exit: 0

store entry: dep@git+file++++C++Users+AZUREU~1+AppData+Local+Temp+lc-repro-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+dep-repo+30b6ecc8d4d0e4fb2f3df2402556222bf6a5b7dc
pkg dir len: 268
pkg dir exists: True
prepared.txt exists: False
error: Failed to run script prepare due to error ENOENT
exit: 1
```

Same second layout with this PR's build:

```
store entry: dep@git+file++++C++Users+AZUREU~1+AppData+Local+Temp+lc-repro-aaaaa+ef7ad81a431ad661
pkg dir len: 206
prepared.txt exists: True
exit: 0
```

</details>

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 1 · Platform-specific test(s) that do not
run on this machine. Deferring to CI, which covers all platforms:
test/cli/install/bun-install.test.ts
test/cli/install/isolated-install.test.ts

<!-- robobun:evidence:end -->

---

## 2. install: send credentials embedded in a tarball URL as Basic
authorization (from #39025)

#### Problem
- A dependency declared as a tarball URL with credentials, `"no-deps":
"http://carol:s3cret@127.0.0.1:PORT/cdn/no-deps-1.0.0.tgz"`, is
downloaded without an `Authorization` header. A server that needs the
credentials answers 401 and the install fails with `error: GET
http://carol:s3cret@127.0.0.1:PORT/cdn/no-deps-1.0.0.tgz - 401` (bun
1.4.0 and `main`, hoisted and isolated linker). npm 11 installs the same
package.json and sends `Authorization: Basic Y2Fyb2w6czNjcmV0`
(`base64("carol:s3cret")`).
- The username-only form (`http://token@127.0.0.1:PORT/x.tgz`) does not
reach the server at all: the request goes to the hostname
`token@127.0.0.1`.
- Cause: `NetworkTask::for_tarball` (`src/install/NetworkTask.rs`) only
ever attaches the registry scope's configured token or `_auth`, and only
for npm packages whose tarball is on the registry's origin. Nothing
reads the URL's userinfo, and the HTTP client does not either (it only
turns the userinfo of a proxy URL into `Proxy-Authorization`). The
misrouted username-only form comes from `bun_url::URL::parse`, which
takes `token@127.0.0.1` for the hostname when the userinfo has no `:`
and a port follows (tracked separately; this change no longer depends on
how that parser splits the authority).
- Found while fixing the isolated store names of these URLs (#39014),
not from a user report.

#### Fix
- `for_tarball` splits the userinfo off the request URL before anything
else looks at it (`split_url_userinfo`: the authority runs from `://` to
the first `/`, `?` or `#`, the userinfo is everything in it up to the
last `@`, so the `@` of `/@scope/pkg/-/pkg.tgz` is not one) and sends it
as `Authorization: Basic base64(userinfo)`
(`basic_authorization_from_userinfo`, which appends `:` when the
userinfo has no password). The request URL is the URL without the
userinfo.
- Precedence: when the registry scope's credentials apply to the tarball
(npm package, tarball on the registry origin, scope has a token or
`_auth`), they are still sent and the URL's are not. This is npm's order
as well: npm-registry-fetch sets the header from the config, and node
only derives `Authorization` from the URL's `auth` when the request has
no such header.
- Why Basic of the userinfo as written: it is what npm sends, checked
against npm 11.16 for `user:pass`, `user` (sent as `user:`), `:pass` and
a percent-encoded password (sent undecoded); transcript below. It is
also how bun already treats credentials written into a registry URL
(#38796 stores the username and password bytes as given). pnpm was not
available offline. Two spellings are deliberately not npm's: a second
`:` inside the password is sent as written where npm percent-encodes it,
and `:token@` in a tarball URL is Basic like npm rather than the Bearer
that bun's registry URLs make of it; both are called out in the test
table and in the code comment.
- Why the URL is requested without the userinfo: `bun_url::URL::origin`
includes the userinfo and the HTTP client compares origins to decide
whether `Authorization` follows a redirect, so with the userinfo left
in, a redirect to the same host would drop the header (verified: with
only the header added, the redirect test below fails with
`authorization: null` on the second hop). It also fixes the
username-only form without touching `bun_url`, and the `GET <url> - 401`
line now prints the URL without the credentials. The cross-host rule is
unchanged: the HTTP client strips the header on a redirect to another
origin, same as for the registry token (test included).
- Behavior outside the request is unchanged: `package.json`, the
lockfile, the task id and the cache key still use the URL as written.
Documented in `docs/pm/cli/add.mdx`.
- Verified with `test/cli/install/bun-install.test.ts`,
`describe("credentials embedded in a tarball URL")`: 12 tests, 10 of
which fail on the unfixed build (the two guards, scoped path and
registry credentials taking precedence, pass on both). The rest of the
file is unchanged: the remaining failures locally are the tests that
need the public internet, identical with the unmodified binary.
- `cargo check -p bun_install`, `cargo clippy -p bun_install`, rustfmt
and prettier are clean.

#### Background
- Tarball dependency: a `package.json` entry whose version is an
`http(s)://` URL ending in `.tgz`/`.tar.gz`/`.tar`. Bun downloads it
directly; no registry manifest is involved, so the registry's configured
credentials never applied to it (`Authorization::NoAuthorization` at the
call sites in `PackageManagerEnqueue.rs`). Registry packages reach the
same `for_tarball` through their manifest's `dist.tarball` URL with
`AllowAuthorization`, which is the case where the registry scope's
credentials can apply.
- Userinfo: the `user:password@` part of a URL's authority (RFC 3986
section 3.2.1). HTTP never puts it on the wire; clients that honor it
(npm through minipass-fetch, curl, browsers) convert it into
`Authorization: Basic base64(user:password)`.
- `NetworkTask::for_tarball` builds one HTTP request per tarball
download: `url_buf` (the request URL) and `header_buf` (the headers,
built with `HeaderBuilder` in two passes, `count` then `append`, because
the buffer is allocated exactly once in between). Retries reuse the same
request, so the header is sent on every attempt.
- `bun_url::URL::parse` is the allocation-free splitter the HTTP client
and `for_tarball` use; it is not a WHATWG parser. Its `origin` is a
prefix of the input string, which is why it still contains the userinfo.

<details>
<summary>npm 11.16 against a local server logging the Authorization
header (same tarball, same package.json shape)</summary>

```
userinfo in the dependency URL   header npm sent
carol:s3cret@                    Basic Y2Fyb2w6czNjcmV0      = base64("carol:s3cret")
carol@                           Basic Y2Fyb2w6              = base64("carol:")
:s3cret@                         Basic OnMzY3JldA==          = base64(":s3cret")
carol:s3%40cret@                 Basic Y2Fyb2w6czMlNDBjcmV0  = base64("carol:s3%40cret"), not decoded
carol:s3:cret@                   Basic Y2Fyb2w6czMlM0FjcmV0  = base64("carol:s3%3Acret"), bun sends base64("carol:s3:cret")
carol:s3cret@ + 302 to same host Basic ... on both hops
```

bun 1.4.0-canary.1 (eabb96d) for the first row: the server logs
`auth=null` and bun prints `error: GET
http://carol:s3cret@127.0.0.1:PORT/cdn/no-deps-1.0.0.tgz - 401`.
</details>

---

## 3. install: read `file:` tarballs relative to the `file:` folder
package that declares them (from #39017)

#### Problem

- A `file:` folder dependency whose own package.json declares a local
tarball fails to install (released 1.4.0 canary and main, both linkers):
project package.json `{"dependencies":{"lib":"file:./vendor/lib"}}`,
`vendor/lib/package.json` declaring `"tool": "file:./tool.tgz"`,
`vendor/lib/tool.tgz` present:
  ```
  error: ENOENT extracting tarball from tool
  error: tool@file:./tool.tgz failed to resolve
  ```
- The path is read as `<project>/tool.tgz`. If that file happens to
exist it is installed as `tool` instead of the folder's copy, with no
error. The directory form of the same declaration (`"tool":
"file:./tool"`) is already resolved relative to `vendor/lib`.
- Cause: `enqueue_local_tarball`
(`src/install/PackageManager/PackageManagerEnqueue.rs`) picks the
directory a local tarball path is relative to, and the only declarer it
looked at was a workspace (`get_workspace_pkg_if_workspace_dep`). Every
other declarer, including a `file:` folder package, fell through to the
top-level dir.
- The same choice was also wrong in the other direction: a root
`overrides` / `resolutions` entry or catalog entry pointing a dependency
at `file:./x.tgz` was read relative to the workspace when the dependency
it applied to was declared by a workspace member, so `overrides: { bar:
"file:./bar.tgz" }` with `bar.tgz` in the project root failed with the
same ENOENT as soon as a workspace depended on `bar`. This is #25835
(`overrides`) and #25752 (`catalogs`); both reproduce as reported on the
released build and install with this change. The directory form of an
override is already resolved relative to the project (`Folder` arm of
`get_or_put_resolved_package`).

Fixes #25835
Fixes #25752

#### Fix

- `enqueue_local_tarball` now takes the base directory from
`local_tarball_base_dir`: the directory of the declaring package when it
is a workspace or a `file:` folder package and that package's own
specifier is the tarball path being read; the top-level dir in every
other case.
- Why the declaring package: a path in a package.json means a file next
to that package.json, which is what npm does for `file:` and what bun
already does for workspace declarers and for the directory form. A
`file:` folder package is read from the project like a workspace is, and
its `Resolution::Folder` payload is its directory relative to the
top-level dir (`folder_resolver.rs`, `NewResolver { folder_path: rel
}`), the same shape as a workspace's `Resolution::Workspace` payload, so
both are joined the same way. The only other `Resolution::Folder`
packages are the stubs created for `file:` directories declared by
something other than the root or a workspace (`Folder` arm of
`get_or_put_resolved_package`); those carry no dependency list, so they
are never the declarer of an edge.
- Why the "own specifier" condition: `overrides`, `resolutions` and
catalogs are only parsed from the root package.json (`Package.rs`,
`FEATURES.is_main`), and applying one leaves the declaring package's
stored edge untouched (the replacement is local to
`enqueue_dependency_with_main_and_success_fn`). So when the stored
edge's specifier is not the path being read, the root wrote the path and
the top-level dir is the only directory it can mean. Without this
condition, root overrides applied to a folder-declared dependency, which
work today only because of the bug, would start being read from the
folder.
- Why it is decided from the edge and not from the resolve pass's
`version_was_replaced`: `enqueue_local_tarball` is also reached from
`enqueue_tarball_for_reading` when a project with a `bun.lock` is
installed into an empty cache. The lockfile row keeps the path as
declared (`"tool": ["bar@./tool.tgz", ...]` under `"lib": [..., {
"dependencies": { "tool": "file:./tool.tgz" } }]`), and the edge's
declarer and specifier are available there too, so both passes compute
the same directory. Lockfile format is unchanged and existing lockfiles
keep working; the path in the row is joined onto a different directory
only for declarers that previously failed or installed the wrong file.
- Declarers extracted from the cache (registry, git, tarball packages)
still fall through to the top-level dir, as before; #38986 is changing
what happens to those separately.
`Lockfile::get_parent_pkg_of_dependency` is the same helper that PR
adds.
- Left as is: the lockfile identity of a local tarball is still the path
as written (`bar@./tool.tgz`), so two project packages declaring the
same relative path to two different files still share one row and one
read, the limitation workspaces already have. A package.json inside a
folder dependency that worked around this bug by writing a
project-relative path (`file:./vendor/lib/tool.tgz`) will now need the
path relative to itself, which is what npm requires for it as well.
- Verified with `test/cli/install/bun-install.test.ts`, `describe("file:
tarball declared by a file: folder dependency")`: the folder's tarball
is installed with the hoisted and with the isolated linker, and a root
override supplying the path is read from the project. Each test plants a
different tarball at the other candidate path and runs a fresh install
followed by `--frozen-lockfile` into an emptied cache, so both the
resolve pass and the install from `bun.lock` have to read the right
file. `test/cli/install/bun-workspaces.test.ts`, `relative tarballs >
from a root override / catalog entry applied to a workspace dependency`,
covers #25835 and #25752 the same way. The four folder/workspace tests
install the wrong tarball without the `src/` change (checked against a
debug build of main and against the released build); the
override-on-folder test passes before and after and pins that case.
- Also run with this change: `bun-workspaces.test.ts` (74 pass),
`overrides.test.ts` + `nested-overrides.test.ts` (151 pass),
`bun-lock.test.ts` (40 pass), `bun-install.test.ts -t
"tarball|tgz|file:|folder|override|resolutions"` (51 pass; `should treat
non-GitHub http(s) URLs as tarballs` fails identically on the released
build, it needs network), `isolated-install.test.ts`, `bun-add.test.ts`
and `bun-install-registry.test.ts` filtered to tarball/file tests (all
pass). `cargo clippy -p bun_install` is clean.

#### Background

- A `file:` dependency on a directory is a `Tag::Folder` dependency; one
on a `.tgz` is a `Tag::Tarball` dependency with a local URI. The latter
resolves to `Resolution::LocalTarball(<path as written>)`; the path
string is both the task id for reading it and the package's identity in
`bun.lock`.
- bun reads the package.json of the root, of workspace members and of
`file:` folder dependencies from disk (`Package::parse`), and stores for
each of the latter two a `Resolution::Workspace` / `Resolution::Folder`
whose payload is the package directory relative to the top-level dir.
Packages from the registry, git or a tarball get their dependency lists
from the manifest or the extracted archive instead and have no directory
in the project while they resolve.
- Dependency edges live in one flat buffer and every package owns a
contiguous slice of it, which is how the package that declared an edge
is found. The edge stores the specifier as declared; overrides,
resolutions and catalogs replace it only for the duration of resolving
that edge.
- `enqueue_local_tarball` is called from two places: the `Tarball` arm
of the resolve pass (no lockfile row yet, the tarball is read to learn
the package's name and dependencies) and `enqueue_tarball_for_reading`
during install (a row exists in `bun.lock`, but the extracted package is
missing from the cache). It computes the on-disk path on the main thread
and hands it to a thread pool task, which only reads the file.

Closes #39025
Closes #39017

---------

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants