diff --git a/Cargo.lock b/Cargo.lock index d6de440ec9d4..7b5654882f96 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -168,7 +168,6 @@ version = "0.0.0" dependencies = [ "bun_alloc", "bun_options_types", - "bun_url", ] [[package]] diff --git a/src/api/Cargo.toml b/src/api/Cargo.toml index 890d219c438a..4d765b6897aa 100644 --- a/src/api/Cargo.toml +++ b/src/api/Cargo.toml @@ -12,4 +12,3 @@ workspace = true [dependencies] bun_alloc.workspace = true bun_options_types.workspace = true -bun_url.workspace = true diff --git a/src/api/lib.rs b/src/api/lib.rs index 14249de1ba5f..5ea6fdaf5719 100644 --- a/src/api/lib.rs +++ b/src/api/lib.rs @@ -2,7 +2,7 @@ #![warn(unused_must_use)] //! Re-exports of the install config types (`BunInstall`, `NpmRegistry`, …) //! whose canonical definitions live in `bun_options_types::schema::api`, plus -//! the registry-URL parser shared by the bunfig and npmrc loaders. +//! the `Parser` handle used by the bunfig and npmrc loaders. // ────────────────────────────────────────────────────────────────────────── // Re-exports — canonical definitions live in `bun_options_types::schema::api`. @@ -19,8 +19,6 @@ pub use bun_options_types::schema::api::{ /// `Parser` lives in a sibling module of `NpmRegistry`; the canonical path /// is `bun_api::npm_registry::Parser`. pub mod npm_registry { - use bun_url::URL; - pub use super::NpmRegistry; // `Parser` stays generic over `L` (Log) / `S` (Source) so this leaf @@ -38,24 +36,7 @@ pub mod npm_registry { &mut self, str: &[u8], ) -> Result { - let url = URL::parse(str); - let mut registry = NpmRegistry::default(); - - // Token - if url.username.is_empty() && !url.password.is_empty() { - registry.token = Box::<[u8]>::from(url.password); - registry.url = url.href_without_auth(); - } else if !url.username.is_empty() && !url.password.is_empty() { - registry.username = Box::<[u8]>::from(url.username); - registry.password = Box::<[u8]>::from(url.password); - - registry.url = url.href_without_auth(); - } else { - // Do not include a trailing slash. There might be parameters at the end. - registry.url = Box::<[u8]>::from(url.href); - } - - Ok(registry) + Ok(NpmRegistry::from_url(str)) } } } diff --git a/src/ini/lib.rs b/src/ini/lib.rs index c8b8f1e57c36..fed27f26583d 100644 --- a/src/ini/lib.rs +++ b/src/ini/lib.rs @@ -1303,16 +1303,12 @@ mod draft { configs } - fn has_credentials(registry: &NpmRegistry) -> bool { - !registry.token.is_empty() || !registry.username.is_empty() || !registry.password.is_empty() - } - pub fn apply_registry_auth(install: &mut BunInstall, auth: &[RegistryAuth]) { if auth.is_empty() { return; } if let Some(registry) = install.default_registry.as_mut() { - if !has_credentials(registry) { + if !registry.has_credentials() { for item in auth { let matched = item.matches(if registry.url.is_empty() { bun_install_types::NodeLinker::npm::Registry::DEFAULT_URL.as_bytes() @@ -1327,7 +1323,7 @@ mod draft { } if let Some(scoped) = install.scoped.as_mut() { for registry in scoped.scopes.values_mut() { - if has_credentials(registry) { + if registry.has_credentials() { continue; } for item in auth { diff --git a/src/install/npm.rs b/src/install/npm.rs index 3af91eae2421..d8ff6b674e4f 100644 --- a/src/install/npm.rs +++ b/src/install/npm.rs @@ -351,6 +351,15 @@ pub mod registry { } } + // The config loaders split literal strings; an expanded $ENV_VAR is split here. + let from_url = api::NpmRegistry::from_url(®istry.url); + registry.url = from_url.url; + if !registry.has_credentials() { + registry.token = from_url.token; + registry.username = from_url.username; + registry.password = from_url.password; + } + // `url` borrows the owned `registry_url` buffer for the duration // of parsing. The final href is moved into `Scope.url: OwnedURL` // (owned `Box<[u8]>`). diff --git a/src/options_types/schema.rs b/src/options_types/schema.rs index 2b59b98a391a..ffde83bb9211 100644 --- a/src/options_types/schema.rs +++ b/src/options_types/schema.rs @@ -152,6 +152,31 @@ pub mod api { pub email: Box<[u8]>, } + impl NpmRegistry { + pub fn from_url(str: &[u8]) -> NpmRegistry { + let url = bun_url::URL::parse(str); + let mut registry = NpmRegistry::default(); + + if url.username.is_empty() && !url.password.is_empty() { + registry.token = Box::from(url.password); + registry.url = url.href_without_auth(); + } else if !url.username.is_empty() && !url.password.is_empty() { + registry.username = Box::from(url.username); + registry.password = Box::from(url.password); + registry.url = url.href_without_auth(); + } else { + // Do not include a trailing slash. There might be parameters at the end. + registry.url = Box::from(url.href); + } + + registry + } + + pub fn has_credentials(&self) -> bool { + !self.token.is_empty() || !self.username.is_empty() || !self.password.is_empty() + } + } + /// Per-scope npm registry overrides, keyed by scope name. #[derive(Default)] pub struct NpmRegistryMap { diff --git a/src/runtime/cli/publish_command.rs b/src/runtime/cli/publish_command.rs index ac6e1736eb84..b408185fc92f 100644 --- a/src/runtime/cli/publish_command.rs +++ b/src/runtime/cli/publish_command.rs @@ -868,6 +868,7 @@ impl PublishCommand { let registry_url = registry.url.url(); if registry.token.is_empty() + && registry.auth.is_empty() && (registry_url.password.is_empty() || registry_url.username.is_empty()) { return Err(PublishError::NeedAuth); diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts index 279851e5dd27..0f1002823852 100644 --- a/test/cli/install/bun-install.test.ts +++ b/test/cli/install/bun-install.test.ts @@ -854,6 +854,182 @@ describe.concurrent("bun-install", () => { expect(exitCode).toBe(0); }); + // `registry = "https://user:pass@host/"` written literally is split into + // credentials while bunfig.toml is parsed. A `registry = "$ENV_VAR"` value is + // only expanded later, in `Scope::from_api`, so the credentials inside the + // variable's URL have to be split out there. + describe("credentials embedded in a registry URL taken from an env var", () => { + const tgz = join(import.meta.dir, "registry", "packages", "no-deps", "no-deps-1.0.0.tgz"); + const integrity = "sha512-v4w12JRjUGvfHDUP8vFDwu0gUWu04j0cv9hLb1Abf9VdaXu4XcrddYFTMVBVvmldKViGWH7jrb6xPJRF0wq6gw=="; + const basic = `Basic ${Buffer.from("alice:s3cret").toString("base64")}`; + + async function installWith(opts: { + bunfig: (registryOrigin: string) => string; + env?: (registryOrigin: string) => Record; + files?: (registryOrigin: string) => Record; + dependency?: string; + }) { + const requests: { path: string; authorization: string | null }[] = []; + await using registry = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + fetch(req) { + const { pathname } = new URL(req.url); + requests.push({ path: pathname, authorization: req.headers.get("authorization") }); + if (pathname.endsWith(".tgz")) { + return new Response(Bun.file(tgz)); + } + const name = decodeURIComponent(pathname.slice(1)); + if (name !== "no-deps" && name !== "@myorg/pkg") { + return new Response("not found", { status: 404 }); + } + return Response.json({ + name, + "dist-tags": { latest: "1.0.0" }, + versions: { + "1.0.0": { + name, + version: "1.0.0", + dist: { integrity, tarball: `http://127.0.0.1:${registry.port}/${name}/-/pkg-1.0.0.tgz` }, + }, + }, + }); + }, + }); + const origin = `127.0.0.1:${registry.port}`; + const dependency = opts.dependency ?? "no-deps"; + + using dir = tempDir("registry-url-credentials", { + "package.json": JSON.stringify({ name: "app", version: "1.0.0", dependencies: { [dependency]: "1.0.0" } }), + "bunfig.toml": opts.bunfig(origin), + ...opts.files?.(origin), + }); + + await using proc = spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env: { ...env, BUN_INSTALL_CACHE_DIR: join(String(dir), ".cache"), ...opts.env?.(origin) }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { requests, stdout, stderr, exitCode }; + } + + const basicAuthForms: [name: string, opts: Parameters[0]][] = [ + [ + "install.registry = $ENV_VAR", + { + bunfig: () => `[install]\nregistry = "$MY_REG"\n`, + env: origin => ({ MY_REG: `http://alice:s3cret@${origin}/` }), + }, + ], + [ + "install.registry = $ENV_VAR, set in the project's .env", + { + bunfig: () => `[install]\nregistry = "$MY_REG"\n`, + files: origin => ({ ".env": `MY_REG=http://alice:s3cret@${origin}/\n` }), + }, + ], + [ + "install.registry = { url = $ENV_VAR }", + { + bunfig: () => `[install]\nregistry = { url = "$MY_REG" }\n`, + env: origin => ({ MY_REG: `http://alice:s3cret@${origin}/` }), + }, + ], + [ + "install.scopes entry = $ENV_VAR", + { + bunfig: () => `[install.scopes]\n"@myorg" = "$MY_REG"\n`, + env: origin => ({ MY_REG: `http://alice:s3cret@${origin}/` }), + dependency: "@myorg/pkg", + }, + ], + ]; + + for (const [name, opts] of basicAuthForms) { + it(`sends user:pass@ from the URL as Basic auth (${name})`, async () => { + const { requests, stdout, stderr, exitCode } = await installWith(opts); + const manifestPath = opts.dependency === "@myorg/pkg" ? "/@myorg%2fpkg" : "/no-deps"; + const tarballPath = `/${opts.dependency ?? "no-deps"}/-/pkg-1.0.0.tgz`; + expect({ requests, stderr }).toEqual({ + requests: [ + { path: manifestPath, authorization: basic }, + { path: tarballPath, authorization: basic }, + ], + stderr: expect.not.stringContaining("s3cret"), + }); + expect(stdout).toContain("1 package installed"); + expect(exitCode).toBe(0); + }); + } + + it("sends :token@ from the URL as a Bearer token", async () => { + const { requests, stdout, stderr, exitCode } = await installWith({ + bunfig: () => `[install]\nregistry = "$MY_REG"\n`, + env: origin => ({ MY_REG: `http://:tok-from-env@${origin}/` }), + }); + expect({ requests, stderr }).toEqual({ + requests: [ + { path: "/no-deps", authorization: "Bearer tok-from-env" }, + { path: "/no-deps/-/pkg-1.0.0.tgz", authorization: "Bearer tok-from-env" }, + ], + stderr: expect.not.stringContaining("tok-from-env"), + }); + expect(stdout).toContain("1 package installed"); + expect(exitCode).toBe(0); + }); + + it("does not print the credentials with the request URL", async () => { + const { requests, stderr, exitCode } = await installWith({ + bunfig: () => `[install]\nregistry = "$MY_REG"\n`, + env: origin => ({ MY_REG: `http://alice:s3cret@${origin}/` }), + dependency: "not-on-this-registry", + }); + expect({ requests, stderr }).toEqual({ + requests: [{ path: "/not-on-this-registry", authorization: basic }], + stderr: expect.stringMatching(/error: GET http:\/\/127\.0\.0\.1:\d+\/+not-on-this-registry - 404/), + }); + expect(stderr).not.toContain("s3cret"); + expect(exitCode).toBe(1); + }); + + // Either kind of configured credential has to win: `from_api` sends a token + // in preference to a username/password pair, so a token taken from the URL + // would otherwise displace an explicitly configured pair. + const explicitWins: [name: string, bunfig: string, urlUserinfo: string, expected: string][] = [ + [ + "configured token vs user:pass@ in the URL", + `[install]\nregistry = { url = "$MY_REG", token = "configured-token" }\n`, + "alice:s3cret", + "Bearer configured-token", + ], + [ + "configured username/password vs :token@ in the URL", + `[install]\nregistry = { url = "$MY_REG", username = "configured-user", password = "configured-pass" }\n`, + ":s3cret", + `Basic ${Buffer.from("configured-user:configured-pass").toString("base64")}`, + ], + ]; + + for (const [name, bunfig, urlUserinfo, expected] of explicitWins) { + it(`credentials configured explicitly override the ones inside the URL (${name})`, async () => { + const { requests, stderr, exitCode } = await installWith({ + bunfig: () => bunfig, + env: origin => ({ MY_REG: `http://${urlUserinfo}@${origin}/` }), + dependency: "not-on-this-registry", + }); + expect({ requests, stderr }).toEqual({ + requests: [{ path: "/not-on-this-registry", authorization: expected }], + stderr: expect.stringMatching(/error: GET http:\/\/127\.0\.0\.1:\d+\/+not-on-this-registry - 404/), + }); + expect(stderr).not.toContain("s3cret"); + expect(exitCode).toBe(1); + }); + } + }); + it("--silent suppresses verbose output even when RUNNER_DEBUG is set", async () => { using dir = tempDir("install-silent-verbose", { "package.json": JSON.stringify({ name: "app", dependencies: {} }), diff --git a/test/cli/install/bun-publish.test.ts b/test/cli/install/bun-publish.test.ts index c7f9df4fc406..9126d43d8d98 100644 --- a/test/cli/install/bun-publish.test.ts +++ b/test/cli/install/bun-publish.test.ts @@ -1236,6 +1236,45 @@ test("dist.tarball in the published manifest does not include userinfo from the expect(exitCode).toBe(0); }); +test("user:pass@ in a registry url taken from an env var is sent as Basic auth", async () => { + const requests: { method: string; pathname: string; authorization: string | null }[] = []; + using mock = Bun.serve({ + port: 0, + fetch(req) { + requests.push({ + method: req.method, + pathname: new URL(req.url).pathname, + authorization: req.headers.get("authorization"), + }); + return new Response("OK", { status: 200 }); + }, + }); + + using packageDir = tempDir("publish-env-registry-userinfo", { + "package.json": JSON.stringify({ name: "env-userinfo-pkg", version: "1.0.0" }), + "bunfig.toml": `[install]\nregistry = "$PUBLISH_REGISTRY"\n`, + }); + + const { out, err, exitCode } = await publish( + { ...env, PUBLISH_REGISTRY: `http://pubuser:hunter2@localhost:${mock.port}/` }, + String(packageDir), + ); + expect({ requests, err }).toEqual({ + requests: [ + { + method: "PUT", + pathname: "/env-userinfo-pkg", + authorization: `Basic ${Buffer.from("pubuser:hunter2").toString("base64")}`, + }, + ], + err: expect.not.stringContaining("error:"), + }); + expect(out).toContain(`Registry: http://localhost:${mock.port}/\n`); + expect(out).toContain(" + env-userinfo-pkg@1.0.0"); + expect(out).not.toContain("hunter2"); + expect(exitCode).toBe(0); +}); + describe("--tolerate-republish", async () => { test("republishing normally fails", async () => { const { packageDir, packageJson } = await registry.createTestDir(); diff --git a/test/cli/install/redacted-config-logs.test.ts b/test/cli/install/redacted-config-logs.test.ts index f6ca0cea2070..6c49d43123b4 100644 --- a/test/cli/install/redacted-config-logs.test.ts +++ b/test/cli/install/redacted-config-logs.test.ts @@ -3,10 +3,12 @@ import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe, tempDir, tmpdirSync } from "harness"; import { join } from "path"; -test("registry url password is masked in request error output", async () => { +test("registry url password is sent as Basic auth and left out of request error output", async () => { + const authorizations: (string | null)[] = []; await using server = Bun.serve({ port: 0, - fetch() { + fetch(req) { + authorizations.push(req.headers.get("authorization")); return new Response(JSON.stringify({ error: "unauthorized" }), { status: 401, headers: { "content-type": "application/json" }, @@ -32,12 +34,36 @@ test("registry url password is masked in request error output", async () => { const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(err).toContain(`401 Unauthorized: http://user:**********@${server.hostname}:${server.port}/is-number`); + expect(authorizations).toEqual([`Basic ${Buffer.from("user:secretpass").toString("base64")}`]); + expect(err).toContain(`401 Unauthorized: http://${server.hostname}:${server.port}/is-number`); expect(err).not.toContain("secretpass"); expect(out).not.toContain("secretpass"); expect(exitCode).toBe(1); }); +test("url password is masked in the verbose request line", async () => { + await using server = Bun.serve({ + port: 0, + fetch() { + return new Response("ok"); + }, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `await fetch("http://user:secretpass@${server.hostname}:${server.port}/pkg")`], + env: { ...bunEnv, NO_COLOR: "1", BUN_CONFIG_VERBOSE_FETCH: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + + const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(err).toContain(`GET http://user:**********@${server.hostname}:${server.port}/pkg`); + expect(err).not.toContain("secretpass"); + expect(out).not.toContain("secretpass"); + expect(exitCode).toBe(0); +}); + test("registry port is not mistaken for a credential when the package is scoped", async () => { await using server = Bun.serve({ port: 0,