install: percent-decode the credentials written into a registry or tarball URL

Closed
RO
25d ago
+101-13

Problem

  • Credentials in a URL are percent-encoded (RFC 3986 section 3.2.1): a password p@ss:w is written p%40ss%3Aw. bun sends them as written, Basic base64("us%25er:p%40ss%3Aw"), and gets a 401. curl and node's http.get send base64("us%er:p@ss:w"). A password that needs encoding cannot be used at all.
  • Neither builder decodes: basic_authorization_from_userinfo (src/install/NetworkTask.rs, tarball URLs, #38867) and NpmRegistry::from_url (src/options_types/schema.rs, registry URLs).

Fix

  • PercentEncoding::decode_lenient_alloc (src/url/lib.rs) decodes every %XX and keeps any other byte, a stray % included. That is WHATWG percent-decode and what curl does. A raw % keeps working. A password that itself contains %40 is now written %2540, as for curl.
  • from_url decodes username, password and the :token@ form. The config keys of the same names are not URL text and stay as written. The tarball path splits at the colon written in the URL, then decodes, as curl and node do.
  • Only the header changes. The lockfile, store names and printed URLs keep the URL as written.
  • Verified: 6 new tests in test/cli/install/config-precedence.test.ts and 5 rows of the tarball table in test/cli/install/bun-install.test.ts fail on main. The other credential suites still pass (Notes).

Background

  • Userinfo is the user:password@ part of a URL. @, :, / and % in it are written as %XX. The consumer decodes them. Basic authorization is base64(user ":" password) of the real bytes.
  • npm 11 sends the userinfo undecoded (minipass-fetch passes the encoded WHATWG getters to node's auth option). #38867 matched that. Such passwords fail with npm too, so nothing depends on it.
  • from_url splits a registry URL string (bunfig, .npmrc, --registry, env vars) into username/password or token, which Scope::from_api (src/install/npm.rs) sends as Basic or Bearer. A tarball dependency has no scope, so for_tarball builds its own header.
Notes

Request log of a loopback stub for http://us%25er:p%40ss%3Aw@127.0.0.1:PORT/...:

main, tarball dependency:            Basic dXMlMjVlcjpwJTQwc3MlM0F3   = us%25er:p%40ss%3Aw
main, [install.scopes] url:          Basic dXMlMjVlcjpwJTQwc3MlM0F3
curl 8.14, node http.get:            Basic dXMlZXI6cEBzczp3           = us%er:p@ss:w
this branch, both:                   Basic dXMlZXI6cEBzczp3

curl edge cases, checked against the same stub and encoded in the test table: carol:50%off%2F2 sends carol:50%off/2, carol:s3cret% sends carol:s3cret%, car%3Aol sends car:ol: (the username is split off before decoding), lowercase hex decodes. The trailing % row passes on main too. It documents the lenient rule. node's urlToHttpOptions uses decodeURIComponent and throws on a stray %. curl keeps it. This change follows curl, which keeps today's behavior for such input.

Compatibility: the only input whose header changes is a userinfo that contains a valid %XX sequence. On main it was sent literally. Now it decodes, and the literal spelling is %25XX. That is the same rule every other client applies to the same URL, and the same URL did not work with npm 11 either (it sends the text undecoded, like main), so no URL that works today across tools changes meaning.

The new config-precedence tests cover the bunfig string form, a [install.scopes] object with a url (the reported registry case), .npmrc registry=, --registry and BUN_CONFIG_REGISTRY, all with percent%2Duser:p%40ss%3Aw%25rd against a verdaccio user whose password is p@ss:w%rd, plus a :token@ whose every byte is written as %xx. On main each fails with error: GET http://localhost:PORT//@needs-auth%2ftest-pkg - 401.

bun-install.test.ts as a whole has 14 failures in this sandbox with and without the change: 13 tests that fetch from bitbucket, gitlab or some.url, and should support --registry CLI flag, whose listener binds localhost to ::1 here while the client connects to 127.0.0.1 (node behaves the same in this sandbox). config-precedence.test.ts passes completely (57 tests).

The proxy header already decodes (build_proxy_authorization, src/http/AsyncHTTP.rs, strict decode_alloc). decode_lenient_alloc is a separate function rather than a third mode of decode_fault_tolerant, whose fault tolerant mode is the %PUBLIC_URL% special case used by URLPath. The strict callers (DirectoryRoute, fetch, package_json, the proxy header) are unchanged.

$VAR expansion: Scope::from_api still expands a username, password or token that is exactly $NAME, as it did with the undecoded text. A registry URL that itself comes from a $VAR bunfig value is still not split at all (noted in #38796, unchanged here). pm whoami prints the decoded username, since it reads Scope.user.

Overlap: #39063 rewrites the same three assignments in from_url (username without a password). Whichever lands second keeps the decode calls on the same lines. #33617 (fetch) also decodes, so the Authorization builders agree once both land.

Also run with the fix: the rest of both test files, npmrc.test.ts, redacted-config-logs.test.ts, and the credential tests of bun-publish, bun-audit, isolated-install and pm whoami. cargo clippy and cargo fmt --check on bun_url, bun_options_types and bun_install are clean, as are prettier and the byte-search source lint. Docs: one sentence each in docs/pm/cli/add.mdx and docs/pm/scopes-registries.mdx.