Skip to content
5 changes: 1 addition & 4 deletions src/install/PackageManager/PackageManagerOptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -644,10 +644,7 @@ impl Options {
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);
self.scope.set_url(api_registry.url);
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/install/audit_fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ pub struct UnmatchedAdvisory {
}

pub struct UnauditedRegistry {
/// The registry's href without URL credentials or a trailing slash.
pub registry: Box<[u8]>,
pub packages: Vec<Box<[u8]>>,
/// Status code or error name; empty when unknown.
Expand Down Expand Up @@ -297,13 +298,13 @@ pub fn print_unaudited(groups: &[UnauditedRegistry]) {
if group.reason.is_empty() {
bun_core::warn!(
"{} did not answer the audit request; skipped {}",
BStr::new(&group.registry),
bun_core::fmt::redacted_npm_url(&group.registry),
BStr::new(&packages)
);
} else {
bun_core::warn!(
"{} did not answer the audit request ({}); skipped {}",
BStr::new(&group.registry),
bun_core::fmt::redacted_npm_url(&group.registry),
BStr::new(&group.reason),
BStr::new(&packages)
);
Expand Down
8 changes: 7 additions & 1 deletion src/install/audit_fix/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,14 @@ pub(super) fn write(plan: &FixPlan, outcome: Option<&FixOutcome>, dry_run: bool)
out.extend_from_slice(b"],\"unaudited\":[");
for (i, group) in plan.unaudited.iter().enumerate() {
comma(&mut out, i);
let mut registry: Vec<u8> = Vec::new();
let _ = write!(
registry,
"{}",
bun_core::fmt::redacted_npm_url(&group.registry)
);
out.extend_from_slice(b"{\"registry\":");
s(&mut out, &group.registry);
s(&mut out, &registry);
out.extend_from_slice(b",\"packages\":[");
for (j, package) in group.packages.iter().enumerate() {
comma(&mut out, j);
Expand Down
18 changes: 12 additions & 6 deletions src/install/npm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,13 @@ pub mod registry {
bun_semver::semver_string::Builder::string_hash(str)
}

/// Stores the WHATWG serialization (the base `bun_url::join` resolves against) so same-origin checks, concatenated tarball URLs and `url_hash` agree with the requests; credentials must already be split off.
pub fn set_url(&mut self, href: Box<[u8]>) {
self.url = URL::from_string(&bun_core::String::borrow_utf8(&href))
.unwrap_or_else(|_| OwnedURL::from_href(href));
self.url_hash = Self::hash(strings::without_trailing_slash(self.url.href()));
}

pub(crate) fn get_name(name: &[u8]) -> &[u8] {
if name.is_empty() || name[0] != b'@' {
return name;
Expand Down Expand Up @@ -508,16 +515,15 @@ pub mod registry {
registry_url
};

let url_hash = Self::hash(strings::without_trailing_slash(&final_href));

Ok(Scope {
let mut scope = Scope {
name: name.into(),
url: OwnedURL::from_href(final_href),
url_hash,
token: registry.token,
auth,
user,
})
..Default::default()
};
scope.set_url(final_href);
Ok(scope)
}
}

Expand Down
15 changes: 3 additions & 12 deletions src/install_jsc/npm_jsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ impl ManifestBindings {
#[bun_jsc::host_fn]
fn js_parse_manifest(global: &JSGlobalObject, frame: &CallFrame) -> JsResult<JSValue> {
use bstr::BStr;
use bun_core::{String as BunString, strings};
use bun_core::String as BunString;
use bun_install::npm;
use bun_jsc::JsError;
use std::io::Write as _;
Expand Down Expand Up @@ -119,17 +119,8 @@ fn js_parse_manifest(global: &JSGlobalObject, frame: &CallFrame) -> JsResult<JSV
}
};

// The `Scope.url` field
// is `OwnedURL`, which stores only the href buffer and re-derives components
// via `URL::parse` on demand. `load_by_file`/`read_all` only consult
// `scope.url_hash` and `scope.url.href().len()`, so copying the raw href is
// sufficient and drops the unsafe lifetime-extension hack the earlier draft
// needed.
let scope = npm::registry::Scope {
url_hash: npm::registry::Scope::hash(strings::without_trailing_slash(registry.slice())),
url: bun_url::OwnedURL::from_href(Box::from(registry.slice())),
..Default::default()
};
let mut scope = npm::registry::Scope::default();
scope.set_url(Box::from(registry.slice()));

let maybe_package_manifest =
match npm::package_manifest::Serializer::load_by_file(&scope, &manifest_file) {
Expand Down
9 changes: 5 additions & 4 deletions src/runtime/cli/audit_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ fn default_registry_href(pm: &PackageManager) -> &[u8] {

fn report_non_json_response(registry: &[u8]) {
Output::err_generic(
"{s} returned a non-JSON audit response",
(BStr::new(registry),),
"{f} returned a non-JSON audit response",
(bun_core::fmt::redacted_npm_url(registry),),
);
}

Expand Down Expand Up @@ -419,8 +419,9 @@ impl core::fmt::Display for SkipReason {
fn unaudited(request: &AuditRequest, reason: &SkipReason) -> audit_fix::UnauditedRegistry {
let mut reason_text: Vec<u8> = Vec::new();
write!(&mut reason_text, "{reason}").expect("unreachable");
let registry = URL::parse(&request.registry.href).href_without_auth();
audit_fix::UnauditedRegistry {
registry: request.registry.href.clone(),
registry: Box::from(strings::without_trailing_slash(&registry)),
packages: request
.packages
.iter()
Expand Down Expand Up @@ -783,7 +784,7 @@ fn send_audit_request(
reason => {
bun_core::pretty_errorln!(
"<r><red>error<r><d>:<r> <red><b>POST<r><red> {}<d> - {}<r>",
BStr::new(&url_str),
bun_core::fmt::redacted_npm_url(&url_str),
reason
);
}
Expand Down
138 changes: 138 additions & 0 deletions test/cli/install/bun-audit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,144 @@ describe("`bun audit`", () => {
});
});

// Every audit line that names a registry prints it through the same redaction as the install error lines, which
// replaces an npm token or UUID anywhere in the URL with `***`; the skipped-registry record (the warning and the
// `unaudited` entries of `audit fix --json`) additionally leaves out credentials written into the URL itself. Most
// of these tests put the token in the registry path because that reaches the audit command from every config
// source, while `user:password@` is split out of the URL by some of them (`.npmrc`, bunfig registry strings) and
// kept by others (the bunfig object form used below, the env vars today).
describe("`bun audit` with a secret in the registry URL", () => {
const SECRET = "npm_" + "secret".padEnd(36, "0");
const BULK_PATH = "/-/npm/v1/security/advisories/bulk";
const NON_JSON = (registry: string) => `error: ${registry} returned a non-JSON audit response`;

// `url` is what the project is configured with, `printed` is how every audit line must render it.
function secretRegistry(registry: Registry) {
return { url: `${registry.url}${SECRET}/`, printed: `${registry.url}***` };
}

// The bulk endpoint lives under the token path, so the registry answers every request the same way.
function registryAnswering(body: string, init?: ResponseInit) {
return Bun.serve({ port: 0, fetch: () => new Response(body, init) });
}

// `bun audit` only reads bun.lock, so the project never needs an install.
function project(dependencies: Record<string, string>, extraFiles: Record<string, string> = {}) {
return tempDir("audit-registry-secret-", {
"package.json": JSON.stringify({ name: "app", dependencies }),
"bun.lock": JSON.stringify({
lockfileVersion: 1,
workspaces: { "": { name: "app", dependencies } },
packages: Object.fromEntries(
Object.entries(dependencies).map(([name, version]) => [name, [`${name}@${version}`, "", {}, ""]]),
),
}),
...extraFiles,
});
}

async function auditAgainst(dir: string, defaultRegistry: string, ...args: string[]) {
await using proc = spawn({
cmd: [bunExe(), "audit", ...args],
cwd: String(dir),
env: { ...bunEnv, NPM_CONFIG_REGISTRY: defaultRegistry },
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
return { stdout, stderr, exitCode };
}

test.concurrent("the failed POST line masks the secret", async () => {
await using registry = registryAnswering("not found", { status: 404 });
const { url, printed } = secretRegistry(registry);
using dir = project({ "no-deps": "1.0.0" });

const { stdout, stderr, exitCode } = await auditAgainst(dir, url);
expect(normalizeBunSnapshot(stderr)).toBe(`error: POST ${printed}${BULK_PATH} - 404`);
expect(normalizeBunSnapshot(stdout)).toBe("bun audit <version> (<revision>)");
expect(exitCode).toBe(1);
});

test.concurrent("the non-JSON response line masks the secret", async () => {
await using registry = registryAnswering("<html><body>sign in</body></html>");
const { url, printed } = secretRegistry(registry);
using dir = project({ "no-deps": "1.0.0" });

const { stdout, stderr, exitCode } = await auditAgainst(dir, url);
expect(normalizeBunSnapshot(stderr)).toBe(NON_JSON(printed));
expect(normalizeBunSnapshot(stdout)).toBe("bun audit <version> (<revision>)");
expect(exitCode).toBe(1);
});

// A body starting with `{` gets past the response check and is rejected when it is parsed instead; the report,
// --json and fix code paths each report that themselves.
test.concurrent("the unparsable response line masks the secret in every mode", async () => {
const body = "{ not json";
await using registry = registryAnswering(body);
const { url, printed } = secretRegistry(registry);
using dir = project({ "no-deps": "1.0.0" });

const report = await auditAgainst(dir, url);
expect(normalizeBunSnapshot(report.stderr)).toBe(NON_JSON(printed));
expect(normalizeBunSnapshot(report.stdout)).toBe("bun audit <version> (<revision>)");
expect(report.exitCode).toBe(1);

const json = await auditAgainst(dir, url, "--json");
expect(normalizeBunSnapshot(json.stderr)).toBe(NON_JSON(printed));
expect(json.stdout).toBe(body + "\n");
expect(json.exitCode).toBe(1);

const fix = await auditAgainst(dir, url, "fix");
expect(normalizeBunSnapshot(fix.stderr)).toBe(NON_JSON(printed));
expect(normalizeBunSnapshot(fix.stdout)).toBe("bun audit fix <version> (<revision>)");
expect(fix.exitCode).toBe(1);
});

// `bun audit` and `bun audit fix --json` against a project whose only package comes from a scoped registry that
// answers 404, so both commands report that registry as skipped; `printed` is how it must be named.
async function expectSkippedRegistry(dir: string, printed: string) {
const skipped = skippedWarning(printed, "404", "@foo/bar");

const report = await auditAgainst(dir, registryHref(server));
expect(normalizeBunSnapshot(report.stderr)).toBe(skipped);
expect(normalizeBunSnapshot(report.stdout)).toBe(AUDIT_HEADER + noVulnerabilities(0, "1 skipped"));
expect(report.exitCode).toBe(0);

const fix = await auditAgainst(dir, registryHref(server), "fix", "--json");
expect(normalizeBunSnapshot(fix.stderr)).toBe(skipped);
expect(JSON.parse(fix.stdout)).toStrictEqual({
dryRun: false,
fixed: 0,
remaining: 0,
fixes: [],
blocked: [],
unfixable: [],
manifestUnavailable: [],
unmatched: [],
unaudited: [{ registry: printed, packages: ["@foo/bar"], reason: "404" }],
vulnerableAfterInstall: [],
});
expect(fix.exitCode).toBe(0);
}

test.concurrent("the skipped registry warning and the --json unaudited entry mask the secret", async () => {
await using scoped = registryAnswering("not found", { status: 404 });
const { url, printed } = secretRegistry(scoped);
using dir = project({ "@foo/bar": "1.0.0" }, { ".npmrc": `@foo:registry=${url}\n` });

await expectSkippedRegistry(dir, printed);
});

test.concurrent("the skipped registry warning and the --json unaudited entry leave out URL credentials", async () => {
await using scoped = registryAnswering("not found", { status: 404 });
const url = `${scoped.url.protocol}//alice:s3cret@${scoped.url.host}/`;
using dir = project({ "@foo/bar": "1.0.0" }, { "bunfig.toml": `[install.scopes]\nfoo = { url = "${url}" }\n` });

await expectSkippedRegistry(dir, registryHref(scoped));
});
});

describe("`bun audit --prod`", () => {
// pnpm#13605: an optional peer that only a devDependency brought in is not a production dependency.
test.concurrent("bun audit --prod skips a dev-only optional peer of a production package", async () => {
Expand Down
Loading
Loading