publish: accept basic auth credentials from .npmrc and bunfig.toml - #38776
publish: accept basic auth credentials from .npmrc and bunfig.toml#38776robobun wants to merge 1 commit into
Conversation
`bun publish` returned "missing authentication" unless a bearer token was
configured, even though `_auth`, `username` + `_password`, a registry url
with userinfo, and bunfig's `{ username, password }` all populate
`Scope.auth`, and the publish request builder already sends them as
`Authorization: Basic`. Check `auth` alongside `token` before giving up.
Co-authored-by: Gavin Hailey <gavin.hailey@ibotta.com>
|
Warning Review limit reached
Next review available in: 17 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 11:05 PM PT - Aug 14th, 2026
❌ @robobun, your commit 16af988 has some failures in 🧪 To try this PR locally: bunx bun-pr 38776That installs a local version of the PR into your bun-38776 --bun |
|
Status: ready for review. The diff is green on every CI lane that ran; the build's only red is two macOS jobs that never executed. Reproduced on the released binary ( Tests: CI (build 96994, finished): 177 of 179 jobs passed. Every test failure in the annotations passed on retry or when re-run alone, and none of them is in publish or in this test file. The other two jobs are both Bot reviews: CodeRabbit hit its rate limit (no review); the other automated review found nothing to change. No open review threads. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — one-line fix bringing the pre-flight auth check in line with the header builders that already send Basic <auth>.
What was reviewed:
- Confirmed
construct_publish_headersandcheck_package_version_existsalready branch onregistry.auth, so only the gate was out of step; no otherNeedAuthsites exist. - Tests: hermetic in-process registry on
port: 0,tempDirwith isolatedHOME/USERPROFILE/XDG_CONFIG_HOME, exacttoEqualon the recorded request+header, and the no-credential case still fails with no request sent. - Checked that loosening the check adds no new surface —
.authis only populated from user config thatbun installalready honors.
Extended reasoning...
Overview
One line added to PublishCommand::publish in src/runtime/cli/publish_command.rs: the pre-flight NeedAuth check now also passes when registry.auth (base64 user:password) is non-empty, not just registry.token. A new describe.concurrent("basic auth") block in test/cli/install/bun-publish.test.ts covers four Basic-auth config spellings (.npmrc _auth, .npmrc username+_password, .npmrc registry= with userinfo, bunfig install.registry.{username,password}) plus a negative case that still refuses without credentials.
Security risks
None introduced. This relaxes a client-side pre-flight guard, but only to match what the request builders in the same file already do — construct_publish_headers and check_package_version_exists already emit Authorization: Basic <auth> when token is empty and auth is not. Scope.auth is only populated from the user's own .npmrc/bunfig, and bun install already sends it. No credential leakage: the tests assert the exact header on the wire, and the existing href_without_auth() paths for the summary line and dist.tarball are unchanged.
Level of scrutiny
Low. The production change is a single boolean clause aligning one check with two sibling call sites in the same file. The mechanism is fully explained in the PR description and verified against src/install/npm.rs (Scope.auth is the base64 user:password field). No CODEOWNERS coverage on either file.
Other factors
Test quality is high per the repo's review rules: using for server and tempdir, port: 0, describe.concurrent for independent subprocess tests, {...env, HOME/USERPROFILE/XDG_CONFIG_HOME} pointing at an empty dir so ambient config cannot supply credentials, exact-value assertions (requests.toEqual([...]) with the precise Basic header), stderr checked before exit code, and the negative contract (no credentials → same error, zero requests) pinned. The PR states USE_SYSTEM_BUN=1 fails the four credential cases and the debug build passes all 44 tests in the file. No prior human reviews or outstanding comments.
|
I opened #38782 for the same bug a few minutes after this (same one-line change to the check in Two things from that branch that may be worth lifting here:
|
… 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
Related: #18670 (the
_auth/ Nexus comments on it are this bug; thepublishConfig.registryreport itself is #38322). Redoes #26178, which was closed as stale when the Zig sources went away.Problem
bun publishfails witherror: missing authentication (run `bunx npm login`)and sends no request when the registry credentials are Basic auth rather than a bearer token.bun installandbun pm whoamiaccept the same configuration, andnpm publishworks with the same.npmrc. Same on 1.3.14 and currentmain.bun installsends asAuthorization: Basic:.npmrc//host/:_auth=,.npmrc//host/:username=+//host/:_password=,.npmrcregistry=http://user:pass@host/, and bunfig.tomlinstall.registry = { url, username, password }. Nexus and Artifactory setup snippets commonly hand out the first two.PublishCommand::publish(src/runtime/cli/publish_command.rs:870) only looks atregistry.token(plus userinfo left in the registry URL). Every spelling above ends up inScope.auth(Scope::from_api,src/install/npm.rs:473), which the check ignores, so it returnsNeedAuthbefore any request is built.construct_publish_headers,check_package_version_exists) already sendBasic <auth>whentokenis empty, so only the pre-flight check was out of step.Fix
registry.auth. Nothing else changes: a registry with neither credential still gets the same error and still sends nothing.test/cli/install/bun-publish.test.ts,describe("basic auth"). Each of the four spellings publishes against an in-process registry that records every request, and the test asserts the registry saw exactlyPUT /basic-auth-pkgwithAuthorization: Basic base64(alice:s3cret). A fifth case pins that no credentials still produces the error and no request.HOME/XDG_CONFIG_HOMEpoint at an empty directory so the machine's own config cannot supply credentials.USE_SYSTEM_BUN=1: the four credential cases fail with the error above, the no-credential case passes.bun bd test: all five pass. The whole ofbun-publish.test.ts(44 tests) passes with the debug build.Background
Scope(npm::registry::Scope) is bun's resolved registry: URL plus credentials.tokenholds a bearer token (_authToken, bunfigtoken,NPM_CONFIG_TOKEN);authholds the base64user:passwordthat goes into a Basic header. The.npmrcloader decodes_authinto username/password, andScope::from_apire-encodes any username/password pair intoauth, so all the Basic spellings converge on that one field.--registryornpm_config_registrypasses this check (the URL clause) but never turns into anAuthorizationheader on any command; that is a separate pre-existing issue and is tracked separately..npmrc/bunfigregistry=strings with userinfo are split into username/password at parse time and are covered by this fix.no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/bun-publish.test.ts