Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
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
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}***` };
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

// 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