Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion src/api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,3 @@ workspace = true
[dependencies]
bun_alloc.workspace = true
bun_options_types.workspace = true
bun_url.workspace = true
23 changes: 2 additions & 21 deletions src/api/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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
Expand All @@ -38,24 +36,7 @@ pub mod npm_registry {
&mut self,
str: &[u8],
) -> Result<NpmRegistry, bun_alloc::AllocError> {
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))
}
}
}
8 changes: 2 additions & 6 deletions src/ini/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 {
Expand Down
15 changes: 15 additions & 0 deletions src/install/npm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,21 @@
}
}

// The config loaders only split `user:pass@` / `:token@` out of
// literal registry strings; an expanded `$ENV_VAR` gets the same
// split here. Explicitly configured credentials take precedence.
Comment thread
robobun marked this conversation as resolved.
Outdated
let from_url = api::NpmRegistry::from_url(&registry.url);
registry.url = from_url.url;
if registry.token.is_empty() {
registry.token = from_url.token;
}
if registry.username.is_empty() {
registry.username = from_url.username;
}
if registry.password.is_empty() {
registry.password = from_url.password;
}

Check failure on line 367 in src/install/npm.rs

View check run for this annotation

Claude / Claude Code Review

URL-embedded :token@ overrides explicitly-configured username/password

The per-field fill lets a URL-embedded `:token@` populate `registry.token` even when `username`/`password` were explicitly configured (e.g. `{ url = "$MY_REG", username = "...", password = "..." }` with `MY_REG=http://:t@host/`); the now-non-empty token then short-circuits the `'outer` block at line 382 so `Scope.auth` is never built, and Bearer is sent instead of the previously-working Basic — contradicting the "Explicitly configured credentials take precedence" comment. Gating the three fills
Comment thread
claude[bot] marked this conversation as resolved.

// `url` borrows the owned `registry_url` buffer for the duration
// of parsing. The final href is moved into `Scope.url: OwnedURL`
// (owned `Box<[u8]>`).
Expand Down
25 changes: 25 additions & 0 deletions src/options_types/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions src/runtime/cli/publish_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
156 changes: 156 additions & 0 deletions test/cli/install/bun-install.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,162 @@ 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<string, string>;
files?: (registryOrigin: string) => Record<string, string>;
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<typeof installWith>[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);
});

it("credentials configured explicitly override the ones inside the URL", async () => {
const { requests, stderr, exitCode } = await installWith({
bunfig: () => `[install]\nregistry = { url = "$MY_REG", token = "configured-token" }\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: "Bearer configured-token" }],
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: {} }),
Expand Down
39 changes: 39 additions & 0 deletions test/cli/install/bun-publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading
Loading