install: keep BUN_CONFIG_TOKEN / NPM_CONFIG_TOKEN when --registry points at a different host - #38764
install: keep BUN_CONFIG_TOKEN / NPM_CONFIG_TOKEN when --registry points at a different host#38764robobun wants to merge 1 commit into
Conversation
…nts at a different host The token env vars were applied to the default registry scope before the --registry flag was processed, so the host-mismatch check that drops the credentials configured for the previous registry also dropped the env token, and the install failed with a 401. Apply the --registry override before reading the token env vars: credentials from .npmrc / bunfig.toml are still dropped when the host changes, while the env token, which is not tied to a host, applies to the registry that was actually selected.
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
Updated 10:50 PM PT - Aug 14th, 2026
✅ @robobun, your commit 6f6616d3c11b4d787107ef15bebf47b35e75e982 passed in 🧪 To try this PR locally: bunx bun-pr 38764That installs a local version of the PR into your bun-38764 --bun |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — a clean reorder that restores the 1.3.14 behavior for BUN_CONFIG_TOKEN/NPM_CONFIG_TOKEN + --registry.
What was reviewed:
- Traced
Options::loadordering: nothing between the new and old--registrypositions readsself.scope, so only the intended token interaction changes. - Verified the moved block is byte-identical apart from borrowing
&maybe_cli(cli.registry: &'static [u8]isCopy, so.into()still yieldsBox<[u8]>). - Checked the #36165 guarantee still holds —
.npmrchost-bound creds are dropped on mismatch; the second new test asserts exactly this. - Confirmed token precedence stays config < env < CLI (
cli.tokenstill applied last).
Extended reasoning...
Overview
Moves the --registry handling in Options::load (src/install/PackageManager/PackageManagerOptions.rs) to run before the BUN_CONFIG_TOKEN/NPM_CONFIG_TOKEN/npm_config_token env-var loop, so the host-mismatch credential drop introduced in #36165 no longer wipes an env-provided token. The moved block is identical except that it now borrows &maybe_cli (the later block still consumes it). Three new tests in test/cli/install/config-precedence.test.ts cover both env vars with --registry and the combined case where an .npmrc token for a different host must still be dropped while the env token survives.
Security risks
This is auth-adjacent, so I checked whether the reorder can leak a token to an unintended host. It cannot: the env token has no host binding by design and is documented as "the token for the default registry"; --registry sets that default, and both are explicitly supplied by the user in the same invocation. This restores 1.3.14 behavior. Host-bound credentials from .npmrc/bunfig are still cleared on mismatch (verified by the new "drops the _authToken … but keeps BUN_CONFIG_TOKEN" test, which asserts dead.hits === 0 and that only Bearer <env-token> reaches the target). No https→http downgrade or credential-carry concern is introduced.
Level of scrutiny
Medium. The change is a ~20-line block move plus a 3-line comment; no new logic. I walked every statement between the new and old positions (BUN_CONFIG_YARN_LOCKFILE, retry count, async_http::load_env, the SKIP_*/NO_VERIFY env flags, the Subcommand::Update cache toggle, and the ANALYZE/ONLY_MISSING flags) — none read or write self.scope, so the only behavioral change is the intended one. Token precedence remains config < env < CLI because cli.token is still applied after the env loop.
Other factors
The PR description cites the exact regression point (#36165), the failing repro on canary, and the additional test files that were re-run (npmrc.test.ts, bun-install-registry.test.ts, bun-publish.test.ts). The new tests follow the file's existing capturingRegistry/deadRegistry patterns exactly and assert on the actual Authorization header set, not just exit code. The bug hunting system found no issues.
|
Folded into #38796 (the |
… URLs (#38796) ### Problem - A registry URL with credentials in it (`http://user:pass@host/`, or `http://:token@host/`) passed as `--registry`, or through `BUN_CONFIG_REGISTRY` / `NPM_CONFIG_REGISTRY` / `npm_config_registry`, sends no `Authorization` header. Against a registry that logs the header (bun 1.4.0, also `main`): - `bun publish --registry http://alice:s3cret@127.0.0.1:PORT/` publishes with exit 0 and the registry sees `PUT /ui-pkg authorization=null`; same with `npm_config_registry=...`. - `bun pm view somepkg --registry http://alice:s3cret@...` and `bun install --registry http://alice:s3cret@...`: `GET /somepkg authorization=null`. - The same URL as `registry=` in `.npmrc` sends `Authorization: Basic YWxpY2U6czNjcmV0` on the manifest and the tarball. npm 11 sends Basic for the flag and the env var too (`minipass-fetch` passes URL userinfo as node's `auth` option). - Because the credentials stay inside the stored registry URL, they end up in the manifest URL and `bun install` prints them on failure: `error: GET http://alice:s3cret@127.0.0.1:PORT/somepkg - 401`. - Cause: the config file string forms go through the userinfo splitter (`parse_registry_url_string_impl`, `src/api/lib.rs`), which moves `user:pass` into `NpmRegistry.username`/`password` (or `token`) and strips it from the URL. The flag and env var paths did not: - `src/install/PackageManager/PackageManagerOptions.rs`, registry env vars: built `Api::NpmRegistry { url: <raw value> }` and called `Scope::from_api`, which does not read URL userinfo. - same file, `--registry`: stored the raw href into `self.scope.url` directly. - `src/runtime/cli/publish_command.rs` `publish()`: the "missing authentication" check accepted userinfo in the URL as credentials, so publish went ahead and sent the PUT without a header. The HTTP client (`src/http`) never reads URL userinfo. - Found while looking at the publish pre-flight check (#38776), not from a user report; the effects above are what the repro shows. - Also fixed here (folded in from #38764, cdca4ff): `BUN_CONFIG_TOKEN` / `NPM_CONFIG_TOKEN` were applied before `--registry`, so the host-mismatch check in the `--registry` block dropped them too and `BUN_CONFIG_TOKEN=... bun install --registry <other host>` failed with a 401. ### Fix - `NpmRegistry::from_url` (`src/options_types/schema.rs`) is the splitter, moved next to the type so `bun_install` can call it; `bun_api`'s `parse_registry_url_string_impl` (the bunfig and `.npmrc` entry point) delegates to it, so there is one parse for every registry URL string. `bun_api` loses its now unused `bun_url` dependency, and `bun_ini`'s private `has_credentials` helper is replaced by the method the new call sites needed. - The registry env vars and `--registry` run their value through `from_url`: - credentials in the URL: the scope is rebuilt with `Scope::from_api`, exactly as for a config file registry string, so `Scope.auth` (Basic) or `Scope.token` (Bearer) is populated and the stored URL no longer contains the userinfo. They replace whatever the config files set up for that registry, which is the rule `.npmrc` already applies (a `registry=` line with userinfo wins over `//host/:_authToken=`: `apply_registry_auth` in `src/ini/lib.rs` skips registries that already have credentials). - no credentials in the URL: unchanged (env var keeps a same-host token, `--registry` keeps same-origin credentials and drops them otherwise); `from_url` returns the input href unchanged in that case. - The `--registry` block now runs before the token env vars are read (cdca4ff), so the env token survives a registry switch and layers on top of both overrides. Resulting order, lowest to highest: config files, credentials in the `--registry` / registry env var URL, `BUN_CONFIG_TOKEN` / `NPM_CONFIG_TOKEN`, `--token`. - `publish()` also accepts `Scope.auth` in its pre-flight check, which is where the Basic credentials land. Same one-line hunk as #38776 (the config file side of that check, reported in #17531); it merges in either order and closing those reports is left to #38776. - Why the split belongs at the string entry points rather than only inside `Scope::from_api`: the loaders need the split result before a scope exists. `.npmrc` loading decides whether host-keyed `_authToken` entries apply by looking at the parsed registry's credentials, and the two override layers decide whether to carry configured credentials over the same way. So the parse-time split is required regardless, and this change wires in the two string entry points that were missing from it. Two ways a URL with userinfo can still reach `from_api` unsplit remain: the bunfig object form `registry = { url = "http://u:p@host/" }` (documented with separate `username`/`password`/`token` fields) and a `registry = "$VAR"` whose value carries userinfo, since `$VAR` is only expanded inside `from_api`. Both need a split inside `from_api` itself, which is tracked separately and kept out of this change because #38183 is rewriting that function's tail; this is also why the publish check keeps its URL userinfo clause. - Tests (every new case fails on the released bun and passes with this change): - `test/cli/install/config-precedence.test.ts`: `bun install` with `--registry` and with each of the three env vars, user:pass form (Basic arrives on every request and verdaccio accepts it, `@needs-auth/test-pkg` installs, password not in stderr) and `:token@` form (Bearer); URL credentials replace a same-host `.npmrc` `_authToken` (flag and env var); from cdca4ff: `BUN_CONFIG_TOKEN` beats a token in the `--registry` URL, `BUN_CONFIG_TOKEN` / `NPM_CONFIG_TOKEN` apply to the `--registry` registry, and `--registry` drops the `.npmrc` token for the previous host but keeps `BUN_CONFIG_TOKEN`. - `test/cli/install/bun-publish.test.ts`, `credentials in the registry url`: `--registry` user:pass (PUT carries Basic, summary line has no userinfo), `--registry` `:token@` (Bearer; previously "missing authentication"), the three env vars. - `test/cli/install/redacted-config-logs.test.ts`: the `pm view` test that asserted the password is masked in the 401 line now asserts Basic is sent and the printed URL has no userinfo (the masked form depended on this bug); a verbose `fetch()` case keeps the URL password masking of `redacted_npm_url` covered. - Also run with the debug build: all of `bun-publish.test.ts`, `config-precedence.test.ts`, `redacted-config-logs.test.ts`, `npmrc.test.ts`, the `.env` registry override and env var priority tests in `bun-install-registry.test.ts`; `cargo clippy` on the touched crates; the source lints. - Overlap with open work: #37095 adds the same `from_url` while deleting `bun_api` and `schema.rs` (whichever lands second rebases onto the other's copy; the call sites are the same either way); #38183 touches the same `--registry` block for URL normalization. ### Background - `Api::NpmRegistry` is the parsed config for one registry: `url` plus optional `username`/`password`/`token`, produced by bunfig and `.npmrc` loading. `Scope::from_api` (`src/install/npm.rs`) turns it into a `Scope`, the resolved registry the package manager uses: the URL, `token` (sent as `Authorization: Bearer`) and `auth` (base64 `user:password`, sent as `Authorization: Basic`; `user` keeps the plain pair for `pm whoami`). Requests only ever look at `Scope.token` / `Scope.auth`; a URL's userinfo is never sent by the HTTP client. - `Options::load` builds the default scope in layers: config files, the registry env vars, `--registry`, the token env vars, then the remaining CLI flags (`--token`). The registry env vars and `--registry` are the two layers that accept a registry URL string without going through the config parsers, which is why they are the ones changed. - `URL::href_without_auth()` is the existing helper the splitter uses to rebuild the URL without its userinfo. For a registry with no path it yields `http://host//`; every consumer strips or collapses the extra slash (this pre-dates the change and applies to the `.npmrc` form today), so it is left alone here. <details> <summary>Probe against a mock registry (released bun 1.4.0 vs this branch)</summary> ``` # released bun publish --registry http://alice:s3cret@127.0.0.1:PORT/ -> exit 0, REQ PUT /ui-pkg authorization=null npm_config_registry=http://alice:s3cret@... bun publish -> exit 0, REQ PUT /ui-pkg authorization=null bun pm view somepkg version --registry http://alice:s3cret@... -> REQ GET /somepkg authorization=null bun install --registry http://alice:s3cret@... (registry answers 401) error: GET http://alice:s3cret@127.0.0.1:PORT/somepkg - 401 .npmrc registry=http://alice:s3cret@... bun install -> REQ GET /somepkg authorization=Basic YWxpY2U6czNjcmV0 npm view somepkg --registry http://alice:s3cret@... -> REQ GET /somepkg authorization=Basic YWxpY2U6czNjcmV0 # this branch bun publish --registry http://alice:s3cret@... -> Registry: http://127.0.0.1:PORT/ REQ PUT /ui-pkg authorization=Basic YWxpY2U6czNjcmV0 BUN_CONFIG_REGISTRY=http://alice:s3cret@... bun publish -> REQ PUT /ui-pkg authorization=Basic YWxpY2U6czNjcmV0 bun publish --registry http://:tok123@... -> REQ PUT /ui-pkg authorization=Bearer tok123 bun pm view somepkg version --registry http://alice:s3cret@... -> REQ GET /somepkg authorization=Basic YWxpY2U6czNjcmV0 bun pm whoami --registry http://alice:s3cret@... -> alice bun publish --registry http://alice@... (no password) -> error: missing authentication (unchanged) bunfig install.registry = "http://alice:s3cret@..." (string form) -> still Basic, via the delegating parser ``` </details> --------- Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
2 participants
Problem
NPM_CONFIG_TOKEN=<token> bun install --registry=https://npm.corp/(same withBUN_CONFIG_TOKEN) sends noAuthorizationheader and fails witherror: GET http://reg:4873/left-pad-x - 401/left-pad-x@^1.0.0 failed to resolve. 1.3.14 sendsBearer <token>and installs.Options::load(src/install/PackageManager/PackageManagerOptions.rs) applied the token env vars to the default registry scope, and only afterwards processed--registry. Since Robustness pass across install, css, ffi, crypto, spawn, shell, and node compat #36165 that branch compares the flag's host with the host of the registry configured so far (registry.npmjs.org when nothing is configured) and clearstoken/auth/useron a mismatch, which also wiped the env token.BUN_CONFIG_REGISTRY,.npmrcorbunfig.tomlwas unaffected because those are applied before the env token.Fix
--registry(URL switch plus the host-mismatch credential drop from Robustness pass across install, css, ffi, crypto, spawn, shell, and node compat #36165) before readingBUN_CONFIG_TOKEN/NPM_CONFIG_TOKEN/npm_config_token, so the env token is applied to the registry that was actually selected..npmrc/bunfig.tomlare bound to the registry they were declared for and are still dropped when--registrynames a different host; the env token is not bound to a host, so applying it after every registry override is the behavior the env var documents ("auth token for the default registry"). Precedence between the token sources is unchanged: config < env < CLI.BUN_CONFIG_TOKENandNPM_CONFIG_TOKENwith--registry, and.npmrctoken for registry A +--registryB + env token: B receives only the env token, A receives nothing). All three fail on the current canary (1.4.0-canary.1, 401 as above) and pass with this change.--registry override(the Robustness pass across install, css, ffi, crypto, spawn, shell, and node compat #36165 test), test/cli/install/bun-install-registry.test.ts env token / registry override tests, test/cli/install/bun-publish.test.ts.Background
npm::registry::Scope) holds the URL plus the credentials used for every unscoped package request;Options::loadbuilds it in layers:.npmrc/bunfig.toml, thenBUN_CONFIG_REGISTRY-style env vars, then CLI flags..npmrccredentials are declared per host (//host/:_authToken=...) and bunfig tokens sit next to a URL, so they are only valid for that host;BUN_CONFIG_TOKEN/NPM_CONFIG_TOKENcarry no host and have always meant "the token for whatever the default registry is"..npmrc//host/:_authTokenline whose host matches only the--registry(orBUN_CONFIG_REGISTRY) URL is not picked up; the auth lines are matched against the config-file registries when the files are loaded and are not kept around for the overrides.