install: leave URL credentials out of isolated store entry names

[robobun]

Problem

• With --linker isolated, a dependency declared as a tarball URL with credentials, e.g. "direct": "http://carol:s3cret@127.0.0.1:PORT/cdn/direct-1.0.0.tgz?token=npm_a1b2...", is installed into a store directory literally named node_modules/.bun/no-deps@http+++carol+s3cret@127.0.0.1+PORT+cdn+direct-1.0.0.tgz+token=npm_a1b2... (reproduced on 1.4.0-canary.1 and current main). A git dependency such as git+https://carol:s3cret@host/org/repo.git gets repo@git+https+++carol+s3cret@host+org+repo.git+<commit>.
• That directory name is part of the realpath of every file in the package, so the password and the token show up in stack traces, import.meta.url, bun pm licenses paths, the --verbose link failure messages, and ls node_modules/.bun. The output redaction in install: redact credentials in tarball and git URLs printed as resolutions and specifiers #38977 cannot cover this: these paths really exist on disk.
• Cause: the entry name is <name>@<resolution store path> (src/install/isolated_install/Store.rs StoreKeyFormatter). For a remote tarball the store path was the whole URL with / \ : # ? turned into + (src/install/resolution.rs StorePathFormatter, via bun_semver's String::fmt_store_path in src/semver/lib.rs), and for git it was the whole repository URL plus the commit (src/install/repository.rs StorePathFormatter). Nothing removed the userinfo or the query string.

Fix

• src/install/resolution.rs: new fmt_store_url, used for the remote tarball store path and, in src/install/repository.rs, for the repository part of the git store path. It writes the URL without its userinfo and without its query string, and when either of them was present it appends +<16 hex wyhash of the complete URL>. The git commit suffix is unchanged and still follows. Examples: http://carol:s3cret@h/p.tgz?token=x becomes no-deps@http+++h+p.tgz+<url hash>; the same URL without credentials stays no-deps@http+++h+p.tgz; a credentialed git URL becomes repo@git+https+++host+org+repo.git+<url hash>+<commit>.
• src/semver/lib.rs: String::fmt_store_path now delegates to a byte-slice fmt_store_path, so the two kept pieces of the URL are spelled exactly the way the whole URL was before (one character mapping, no copy of it).
• Why this is correct:
  • The userinfo and the query string are the two places a URL carries credentials (user:password@, a bare token as the username, ?token= / signed URLs); host, path and scheme are not secret, so they stay readable. The userinfo is delimited with RFC 3986's rule, the same one find_url_password in bun_core uses: the authority runs from scheme:// (or from the start of an scp-like user@host:path) to the first /, ? or #, and the userinfo is everything in it up to the last @. The whole userinfo goes, not only the password, because https://TOKEN@host/... is a documented way of passing tokens. bun_url::URL::parse was not used for this because it does not recognize the userinfo of user@host:port at all.
  • The name stays a function of the resolution alone, so a second install (whose resolution comes from the lockfile, which still stores the full URL) derives the same name and finds the same entry; the tests check this.
  • Whenever something is removed, the hash of the complete URL is appended, so two resolutions that used to get different names still get different names: pkg.tgz?v=1 and pkg.tgz?v=2 are different packages and must not share a directory, and even for git, where the commit usually disambiguates, resolved can be empty for packages migrated from pnpm/yarn lockfiles. Without either part there is nothing to disambiguate, so no hash is appended and every existing entry for a plain tarball, git+file://, github: or registry package keeps its name byte for byte.
  • Entries whose URL has a userinfo or a query string are renamed once by this change (that includes the common git+ssh://git@host/... form, whose git@ is not a secret but cannot be told apart from a token); the next bun install links them under the new name and bun pm prune removes the old directories, since it removes every store entry the lockfile does not produce. The lockfile itself is untouched.
  • The hash sits inside the resolution part, so the consumers that read names back keep working: bun pm prune splits at @ (src/install/prune.rs split_store_key; the names now also contain no second @), bun pm licenses re-derives the name through the same formatter (src/runtime/cli/pm_licenses_command.rs), and the global store derives its links/<name>-<entry hash> directory from the same name, so it is fixed as well. install: bound isolated store entry names; tarball URL credentials; file: tarballs relative to their folder package #38867 (bounding long names) would apply on top of this name.
• Verification, test/cli/install/isolated-install.test.ts, describe store entry names of URL dependencies:
  • tarball dependencies with a plain URL (name unchanged), a password, a token in the query string, and both plus a fragment: exact entry name computed with Bun.hash, the node_modules link points into it, bun.lock still contains the full URL, the package imports at runtime, and a second install keeps the name
  • two tarball URLs differing only in ?v= get two entries and each alias resolves to its own version
  • with install.globalStore, the links/ directory name is built from the credential-free name
  • git dependencies served over git's dumb HTTP protocol from a local bare repository: plain URL (name unchanged), password, and a token as the username, each with the exact name including the commit, the lockfile resolution, and a second install
  • the username-only tarball form (http://token@host:port/x.tgz) is exercised through the git cases only: the tarball downloader currently sends that URL with the userinfo still in the Host header and gets a 400, which is tracked separately (as is the fact that the downloader never sends URL credentials at all); neither affects this change
  • the existing Bun isolated linker misresolves packages installed from tarball URLs with query strings #36987 test in the same file asserted the old +x=y name and now asserts the hashed one; its point (no literal ? in the name, package resolves at runtime) is unchanged
  • On the released build, 8 of these 10 tests fail with the old names (the two plain URL cases pass by design); all pass with bun bd test. Also run with the change: the rest of isolated-install.test.ts (75 pass), bun-pm-licenses.test.ts (79 pass, it asserts the unchanged name of a plain tarball entry), bun-prune.test.ts (109 pass), bun-install-git-deps.test.ts (7 pass), cargo clippy and cargo fmt --check on bun_install and bun_semver, and test/internal/source-lints.

Background

• Isolated linker: every package is materialized once under node_modules/.bun/<entry>/node_modules/<name> and everything that depends on it gets a symlink to that directory. <entry> is <package name>@<store path of the resolution>, optionally followed by +<peer hash>; with install.globalStore the entry is itself a symlink into <cache>/links/<entry>-<entry hash>.
• Resolution: the lockfile's record of where a package came from. For registry packages it is the version, which is why their entries read name@1.2.3; for tarball and git dependencies it is the URL as written in package.json (git: plus the resolved commit), which is what became the directory name here.
• Store path: the spelling of a resolution as one path component, done by replacing /, \, :, # and ? with + (bun_semver's StorePathFormatter); http://h/p.tgz reads http+++h+p.tgz.
• Userinfo: the user:password@ part of a URL's authority (scheme://userinfo@host:port/path?query#fragment).
• wyhash: bun's default 64-bit hash (bun_wyhash::hash, what Bun.hash() computes), which is how the tests compute the expected names; the store already uses it for the peer hash suffix.

Reproduction on the released build

package.json with {"dependencies": {"direct": "http://carol:s3cret@127.0.0.1:PORT/cdn/direct-1.0.0.tgz?token=npm_a1b2c3d4e5f6"}}, a Bun.serve answering that path with a tarball, bunfig.toml with install.linker = "isolated":

$ bun install        # 1.4.0-canary.1
+ direct@http://carol:s3cret@127.0.0.1:35767/cdn/direct-1.0.0.tgz?token=npm_a1b2c3d4e5f6
$ ls node_modules/.bun
no-deps@http+++carol+s3cret@127.0.0.1+35767+cdn+direct-1.0.0.tgz+token=npm_a1b2c3d4e5f6
node_modules

With this change the entry is no-deps@http+++127.0.0.1+35767+cdn+direct-1.0.0.tgz+<16 hex>. The cache folder for the same tarball was already credential-free (@T@<hash>).

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 28 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: 80826fa2-3791-4581-90ff-5d4981baaa1a

📥 Commits

Reviewing files that changed from the base of the PR and between 732491c and 3ad5fbb.

📒 Files selected for processing (4)

• src/install/repository.rs
• src/install/resolution.rs
• src/semver/lib.rs
• test/cli/install/isolated-install.test.ts

---

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

[robobun]

Status: reproduced on 1.4.0-canary.1 with a local tarball server (ls node_modules/.bun shows no-deps@http+++carol+s3cret@127.0.0.1+PORT+cdn+direct-1.0.0.tgz+token=...), fix and tests in this PR; the new tests fail on the released build with the old names and pass with the debug build. Waiting for CI.

The one design call to look at is the third and fourth bullets under Fix: URLs with a userinfo or a query string are renamed once (this includes git+ssh://git@... dependencies, since a username cannot be told apart from a token), plain URLs keep their names.

[robobun]

^{Updated 4:39 AM PT - Aug 15th, 2026}

@robobun, your commit 3ad5fbb is building: #97981

[claude]

I reviewed this PR and didn't find any bugs. Because it changes the on-disk store naming scheme (renaming every existing entry whose URL carries userinfo, including the very common git+ssh://git@… form) and hand-parses URL authority/query boundaries for a credential-redaction purpose, a maintainer look at those design choices would still be worthwhile.

What was reviewed:

• fmt_store_url parsing: authority-start via first : + //, authority-end via first /?#, userinfo via last @ in authority, query via first ? after host — matches RFC 3986 and bun_core::find_url_password; scp-like user@host:path handled by authority_start = 0.
• Uniqueness preserved: hash of the full URL is appended whenever anything is stripped, so ?v=1 vs ?v=2 still separate; plain URLs keep their existing names byte-for-byte (no hash appended).
• Consumers: bun pm prune's split_store_key splits at the first non-leading @ and the new names contain none in the version part; bun pm licenses and the global store re-derive names through the same formatter.
• bun_semver::StorePathFormatter field change (str+buf → bytes) — the only external constructor was String::fmt_store_path, updated in the same commit; install/lib.rs has its own unrelated StorePathFormatter.

Extended reasoning...

Overview

The PR stops the isolated linker from writing URL-embedded credentials into directory names under node_modules/.bun. It adds fmt_store_url in src/install/resolution.rs (used for remote-tarball resolutions and, via src/install/repository.rs, for the repo part of git resolutions), which emits the URL with its userinfo and query string removed and, when either was present, appends a 16-hex wyhash of the complete URL. src/semver/lib.rs refactors StorePathFormatter to accept a raw byte slice via a free fmt_store_path so the two kept URL pieces are spelled with the exact same character mapping as before. ~220 lines of new tests in test/cli/install/isolated-install.test.ts cover tarball and git dependencies with plain URLs (name unchanged), passwords, query-string tokens, both together, query-string-only disambiguation, the global store, and lockfile round-tripping.

Security risks

The change is redactive: it removes secrets from disk paths rather than introducing any new trust boundary. The URL is still stored in full in the lockfile (unchanged, tested), so no functional auth is affected. The hand-rolled URL parsing is only used to derive a directory name — if it mis-splits, the worst case is a longer or shorter readable prefix, and the appended full-URL hash still keeps distinct resolutions in distinct directories. I did not find a way for two previously-distinct resolutions to collide onto the same store entry.

Level of scrutiny

Medium-high. The Rust change itself is small and self-contained (one new formatter, two call-site swaps, one struct-field refactor with no external constructors), but it is a deliberate change to the isolated store's on-disk naming, which has downstream consumers (bun pm prune, bun pm licenses, the global store, #38867's length bounding) and a one-time migration effect. In particular, every git+ssh://git@host/… dependency — a very common form where git@ is not secret — will be renamed with a hash suffix on the next install. The PR calls this out and argues it cannot be distinguished from a token-as-username, which is reasonable, but it is exactly the kind of tradeoff a maintainer should sign off on.

Other factors

Test coverage is thorough: exact expected names computed with Bun.hash, symlink targets, lockfile contents, runtime import, and second-install stability are all asserted; the two "plain URL" cases prove existing names are byte-identical. The PR description states the rest of isolated-install.test.ts, bun-pm-licenses.test.ts, and bun-prune.test.ts still pass. No prior review comments to address. Given the on-disk layout change and the credential-handling context, deferring to a human rather than auto-approving.

[robobun]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.