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/PackageManager/PackageManagerOptions.rs b/src/install/PackageManager/PackageManagerOptions.rs index da7ddb805c55..6ff59e745914 100644 --- a/src/install/PackageManager/PackageManagerOptions.rs +++ b/src/install/PackageManager/PackageManagerOptions.rs @@ -607,23 +607,18 @@ impl Options { if !registry_.is_empty() && (registry_.starts_with(b"https://") || registry_.starts_with(b"http://")) { - let prev_scope = self.scope.clone(); - let prev_url = prev_scope.url.url(); - let new_url = bun_url::URL::parse(registry_); - let token = if bun_core::without_trailing_slash(new_url.host) - == bun_core::without_trailing_slash(prev_url.host) - && (new_url.is_https() || !prev_url.is_https()) - { - prev_scope.token - } else { - Box::default() - }; - // Default (empty strings) is the zero value for Api::NpmRegistry. - let api_registry = Api::NpmRegistry { - url: registry_.into(), - token, - ..Default::default() - }; + let mut api_registry = Api::NpmRegistry::from_url(registry_); + // Credentials in the URL win, as they do for `registry=` in .npmrc. + if !api_registry.has_credentials() { + let prev_url = self.scope.url.url(); + let new_url = bun_url::URL::parse(&api_registry.url); + if bun_core::without_trailing_slash(new_url.host) + == bun_core::without_trailing_slash(prev_url.host) + && (new_url.is_https() || !prev_url.is_https()) + { + api_registry.token = core::mem::take(&mut self.scope.token); + } + } self.scope = Npm::registry::Scope::from_api(b"", api_registry, env)?; break; } @@ -631,6 +626,32 @@ impl Options { } } + if let Some(cli) = &maybe_cli { + if !cli.registry.is_empty() { + let api_registry = Api::NpmRegistry::from_url(cli.registry); + if api_registry.has_credentials() { + self.scope = Npm::registry::Scope::from_api(b"", api_registry, env)?; + } else { + let new_url = bun_url::URL::parse(&api_registry.url); + let same_origin = { + let prev_url = self.scope.url.url(); + bun_core::without_trailing_slash(new_url.host) + == bun_core::without_trailing_slash(prev_url.host) + && (new_url.is_https() || !prev_url.is_https()) + }; + if !same_origin { + self.scope.token = Box::default(); + self.scope.auth = Box::default(); + self.scope.user = Box::default(); + } + let href = api_registry.url; + self.scope.url_hash = + Npm::registry::Scope::hash(bun_core::without_trailing_slash(&href)); + self.scope.url = bun_url::OwnedURL::from_href(href); + } + } + } + { const TOKEN_KEYS: [&[u8]; 3] = [ b"BUN_CONFIG_TOKEN", @@ -687,25 +708,6 @@ impl Options { self.enable .set(Enable::ONLY_MISSING, cli.only_missing || cli.analyze); - if !cli.registry.is_empty() { - let new_url = bun_url::URL::parse(cli.registry); - let same_origin = { - let prev_url = self.scope.url.url(); - bun_core::without_trailing_slash(new_url.host) - == bun_core::without_trailing_slash(prev_url.host) - && (new_url.is_https() || !prev_url.is_https()) - }; - if !same_origin { - self.scope.token = Box::default(); - self.scope.auth = Box::default(); - self.scope.user = Box::default(); - } - let href: Box<[u8]> = cli.registry.into(); - self.scope.url_hash = - Npm::registry::Scope::hash(bun_core::without_trailing_slash(&href)); - self.scope.url = bun_url::OwnedURL::from_href(href); - } - if let Some(cache_dir) = cli.cache_dir { self.cache_directory = cache_dir; } 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-publish.test.ts b/test/cli/install/bun-publish.test.ts index c7f9df4fc406..8d7728ec6b87 100644 --- a/test/cli/install/bun-publish.test.ts +++ b/test/cli/install/bun-publish.test.ts @@ -807,6 +807,95 @@ describe("--dry-run", async () => { }); }); +describe.concurrent("credentials in the registry url", () => { + // Records every request; `bun publish` against it must send exactly one authenticated PUT. + function registryMock() { + const requests: { method: string; pathname: string; authorization: string | null }[] = []; + const server = 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 }); + }, + }); + return { + requests, + port: server.port, + [Symbol.dispose]: () => server.stop(true), + }; + } + + async function packageDirFor(name: string) { + const packageDir = tmpdirSync(); + await write(join(packageDir, "package.json"), JSON.stringify({ name, version: "1.0.0" })); + return packageDir; + } + + const basicAuth = `Basic ${Buffer.from("pubuser:hunter2").toString("base64")}`; + + test("--registry with user:pass@ publishes with Basic auth", async () => { + using mock = registryMock(); + const packageDir = await packageDirFor("userinfo-flag-pkg"); + + const { out, err, exitCode } = await publish( + env, + packageDir, + "--registry", + `http://pubuser:hunter2@localhost:${mock.port}/`, + ); + expect(err).not.toContain("error:"); + expect(out).toContain(`Registry: http://localhost:${mock.port}/\n`); + expect(out).toContain(" + userinfo-flag-pkg@1.0.0"); + expect(out).not.toContain("hunter2"); + expect(err).not.toContain("hunter2"); + expect(mock.requests).toEqual([{ method: "PUT", pathname: "/userinfo-flag-pkg", authorization: basicAuth }]); + expect(exitCode).toBe(0); + }); + + test("--registry with :token@ publishes with a Bearer token", async () => { + using mock = registryMock(); + const packageDir = await packageDirFor("userinfo-token-pkg"); + + const { out, err, exitCode } = await publish( + env, + packageDir, + "--registry", + `http://:publish-token@localhost:${mock.port}/`, + ); + expect(err).not.toContain("error:"); + expect(out).toContain(" + userinfo-token-pkg@1.0.0"); + expect(out).not.toContain("publish-token"); + expect(mock.requests).toEqual([ + { method: "PUT", pathname: "/userinfo-token-pkg", authorization: "Bearer publish-token" }, + ]); + expect(exitCode).toBe(0); + }); + + test.each(["npm_config_registry", "NPM_CONFIG_REGISTRY", "BUN_CONFIG_REGISTRY"])( + "%s with user:pass@ publishes with Basic auth", + async key => { + using mock = registryMock(); + const packageDir = await packageDirFor("userinfo-env-pkg"); + + const { out, err, exitCode } = await publish( + { ...env, [key]: `http://pubuser:hunter2@localhost:${mock.port}/` }, + packageDir, + ); + expect(err).not.toContain("error:"); + expect(out).toContain(`Registry: http://localhost:${mock.port}/\n`); + expect(out).toContain(" + userinfo-env-pkg@1.0.0"); + expect(out).not.toContain("hunter2"); + expect(err).not.toContain("hunter2"); + expect(mock.requests).toEqual([{ method: "PUT", pathname: "/userinfo-env-pkg", authorization: basicAuth }]); + expect(exitCode).toBe(0); + }, + ); +}); + describe("lifecycle scripts", async () => { const script = `const fs = require("fs"); fs.writeFileSync(process.argv[2] + ".txt", \` diff --git a/test/cli/install/config-precedence.test.ts b/test/cli/install/config-precedence.test.ts index c5d77e070fcf..0a2bed979602 100644 --- a/test/cli/install/config-precedence.test.ts +++ b/test/cli/install/config-precedence.test.ts @@ -515,6 +515,117 @@ describe.concurrent("bun install config precedence", () => { expect(exitCode).toBe(0); }); + // `http://user:pass@host/` in a registry URL is sent as Basic auth and `http://:token@host/` as a Bearer + // token, however the URL reaches bun. The .npmrc / bunfig string forms already did this; these cover + // --registry and the registry env vars, which used to drop the credentials. + const basicAuth = `Basic ${Buffer.from("config-precedence:verysecure").toString("base64")}`; + const withUserinfo = (capture: { port: number }, userinfo: string) => `http://${userinfo}@localhost:${capture.port}/`; + + test("--registry with user:pass@ in the URL sends Basic auth", async () => { + using capture = capturingRegistry(); + using dir = tempDir("config-precedence", { + "project/package.json": packageJson({ "@needs-auth/test-pkg": "1.0.0" }), + }); + const { stderr, exitCode } = await install(String(dir), [ + "--registry", + withUserinfo(capture, "config-precedence:verysecure"), + ]); + expect(stderr).not.toContain("error:"); + expect(stderr).not.toContain("verysecure"); + expect(new Set(capture.authorizations)).toStrictEqual(new Set([basicAuth])); + expect(installed(String(dir), "@needs-auth", "test-pkg")).toBe(true); + expect(exitCode).toBe(0); + }); + + test("--registry with :token@ in the URL sends it as a Bearer token", async () => { + using capture = capturingRegistry(); + using dir = tempDir("config-precedence", { + "project/package.json": packageJson({ "@needs-auth/test-pkg": "1.0.0" }), + }); + const { stderr, exitCode } = await install(String(dir), ["--registry", withUserinfo(capture, `:${authToken}`)]); + expect(stderr).not.toContain("error:"); + expect(new Set(capture.authorizations)).toStrictEqual(new Set([`Bearer ${authToken}`])); + expect(installed(String(dir), "@needs-auth", "test-pkg")).toBe(true); + expect(exitCode).toBe(0); + }); + + test("BUN_CONFIG_TOKEN beats credentials embedded in the --registry URL", async () => { + using capture = capturingRegistry(); + using dir = tempDir("config-precedence", { + "project/package.json": packageJson({ "@needs-auth/test-pkg": "1.0.0" }), + }); + const { stderr, exitCode } = await install(String(dir), ["--registry", withUserinfo(capture, ":wrong-token")], { + BUN_CONFIG_TOKEN: authToken, + }); + expect(stderr).not.toContain("error:"); + expect(new Set(capture.authorizations)).toStrictEqual(new Set([`Bearer ${authToken}`])); + expect(installed(String(dir), "@needs-auth", "test-pkg")).toBe(true); + expect(exitCode).toBe(0); + }); + + test.each(["BUN_CONFIG_REGISTRY", "NPM_CONFIG_REGISTRY", "npm_config_registry"])( + "%s with user:pass@ in the URL sends Basic auth", + async key => { + using capture = capturingRegistry(); + using dir = tempDir("config-precedence", { + "project/package.json": packageJson({ "@needs-auth/test-pkg": "1.0.0" }), + }); + const { stderr, exitCode } = await install(String(dir), [], { + [key]: withUserinfo(capture, "config-precedence:verysecure"), + }); + expect(stderr).not.toContain("error:"); + expect(stderr).not.toContain("verysecure"); + expect(new Set(capture.authorizations)).toStrictEqual(new Set([basicAuth])); + expect(installed(String(dir), "@needs-auth", "test-pkg")).toBe(true); + expect(exitCode).toBe(0); + }, + ); + + test("BUN_CONFIG_REGISTRY with :token@ in the URL sends it as a Bearer token", async () => { + using capture = capturingRegistry(); + using dir = tempDir("config-precedence", { + "project/package.json": packageJson({ "@needs-auth/test-pkg": "1.0.0" }), + }); + const { stderr, exitCode } = await install(String(dir), [], { + BUN_CONFIG_REGISTRY: withUserinfo(capture, `:${authToken}`), + }); + expect(stderr).not.toContain("error:"); + expect(new Set(capture.authorizations)).toStrictEqual(new Set([`Bearer ${authToken}`])); + expect(installed(String(dir), "@needs-auth", "test-pkg")).toBe(true); + expect(exitCode).toBe(0); + }); + + test("user:pass@ in --registry replaces the .npmrc _authToken for the same host", async () => { + using capture = capturingRegistry(); + using dir = tempDir("config-precedence", { + "project/.npmrc": `registry=${capture.url}\n//localhost:${capture.port}/:_authToken=token-from-npmrc\n`, + "project/package.json": packageJson({ "@needs-auth/test-pkg": "1.0.0" }), + }); + const { stderr, exitCode } = await install(String(dir), [ + "--registry", + withUserinfo(capture, "config-precedence:verysecure"), + ]); + expect(stderr).not.toContain("error:"); + expect(new Set(capture.authorizations)).toStrictEqual(new Set([basicAuth])); + expect(installed(String(dir), "@needs-auth", "test-pkg")).toBe(true); + expect(exitCode).toBe(0); + }); + + test("user:pass@ in BUN_CONFIG_REGISTRY replaces the .npmrc _authToken for the same host", async () => { + using capture = capturingRegistry(); + using dir = tempDir("config-precedence", { + "project/.npmrc": `registry=${capture.url}\n//localhost:${capture.port}/:_authToken=token-from-npmrc\n`, + "project/package.json": packageJson({ "@needs-auth/test-pkg": "1.0.0" }), + }); + const { stderr, exitCode } = await install(String(dir), [], { + BUN_CONFIG_REGISTRY: withUserinfo(capture, "config-precedence:verysecure"), + }); + expect(stderr).not.toContain("error:"); + expect(new Set(capture.authorizations)).toStrictEqual(new Set([basicAuth])); + expect(installed(String(dir), "@needs-auth", "test-pkg")).toBe(true); + expect(exitCode).toBe(0); + }); + test("BUN_CONFIG_TOKEN beats the _authToken in ~/.npmrc", async () => { using capture = capturingRegistry(); using dir = tempDir("config-precedence", { @@ -528,6 +639,38 @@ describe.concurrent("bun install config precedence", () => { expect(exitCode).toBe(0); }); + test.each(["BUN_CONFIG_TOKEN", "NPM_CONFIG_TOKEN"])( + "%s applies to the registry passed with --registry", + async key => { + using capture = capturingRegistry(); + using dir = tempDir("config-precedence", { + "project/package.json": packageJson({ "@needs-auth/test-pkg": "1.0.0" }), + }); + const { stderr, exitCode } = await install(String(dir), ["--registry", capture.url], { [key]: authToken }); + expect(stderr).not.toContain("error:"); + expect(new Set(capture.authorizations)).toStrictEqual(new Set([`Bearer ${authToken}`])); + expect(installed(String(dir), "@needs-auth", "test-pkg")).toBe(true); + expect(exitCode).toBe(0); + }, + ); + + test("--registry drops the _authToken of the .npmrc registry but keeps BUN_CONFIG_TOKEN", async () => { + using dead = deadRegistry(); + using capture = capturingRegistry(); + using dir = tempDir("config-precedence", { + "project/.npmrc": `registry=${dead.url}\n//localhost:${dead.server.port}/:_authToken=token-for-dead\n`, + "project/package.json": packageJson({ "@needs-auth/test-pkg": "1.0.0" }), + }); + const { stderr, exitCode } = await install(String(dir), ["--registry", capture.url], { + BUN_CONFIG_TOKEN: authToken, + }); + expect(stderr).not.toContain("error:"); + expect(dead.hits).toBe(0); + expect(new Set(capture.authorizations)).toStrictEqual(new Set([`Bearer ${authToken}`])); + expect(installed(String(dir), "@needs-auth", "test-pkg")).toBe(true); + expect(exitCode).toBe(0); + }); + test("NPM_CONFIG_REGISTRY beats registry= in project .npmrc", async () => { using dead = deadRegistry(); using dir = tempDir("config-precedence", { 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,