Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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 mordant-baseline.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"defaulted_failure:src/bun_core/string/immutable.rs" = 1

[bun_install]
"defaulted_failure:src/install/npm.rs" = 1
"unread_none:src/install/PackageInstall.rs" = 1

[bun_jsc]
Expand Down
14 changes: 12 additions & 2 deletions src/install/npm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2606,10 +2606,20 @@
}
}

if let Some(shasum_str) = dist.get(b"shasum").and_then(|v| v.as_str()) {
package_version.integrity =
Integrity::parse_sha_sum(shasum_str).unwrap_or_default();
match Integrity::parse_sha_sum(shasum_str) {
Ok(integrity) => package_version.integrity = integrity,
Err(_) => log.add_warning_fmt(
None,
bun_ast::Loc::EMPTY,
format_args!(
"Malformed shasum in registry metadata for {}@{}; its tarball will not be verified",
bstr::BStr::new(expected_name),
bstr::BStr::new(version_name),
),
),
}
}

Check warning on line 2622 in src/install/npm.rs

View check run for this annotation

Claude / Claude Code Review

Malformed dist.integrity sibling still installs unverified with no warning

The sibling case 6 lines above — a non-empty malformed `dist.integrity` with no (or empty) `dist.shasum` — still falls through to `Tag::UNKNOWN` and installs unverified with nothing on stderr. `Integrity::parse` returns UNKNOWN (not `Err`) on every failure path, so `is_supported()` is false and we don't `break 'integrity`; the `shasum` block is then skipped (absent) or hits the `Ok(UNKNOWN)` arm (empty), never the new `Err` arm. The `bun.lock.rs` precedent this PR cites already guards this with
Comment thread
claude[bot] marked this conversation as resolved.
}
}

Expand Down
56 changes: 51 additions & 5 deletions test/cli/install/bun-install-tarball-integrity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -649,9 +649,10 @@ describe.concurrent("tarball integrity metadata forms", () => {
tgz,
sha512: "sha512-" + createHash("sha512").update(tgz).digest("base64"),
sha384: "sha384-" + createHash("sha384").update(tgz).digest("base64"),
shasum: createHash("sha1").update(tgz).digest("hex"),
};
}
function serveManifest(integrity: string, tgz: Buffer) {
function serveManifest(dist: { integrity?: string; shasum?: string }, tgz: Buffer) {
const server = Bun.serve({
port: 0,
hostname: "127.0.0.1",
Expand All @@ -666,7 +667,7 @@ describe.concurrent("tarball integrity metadata forms", () => {
name: "pkg",
version: "1.0.0",
dist: {
integrity,
...dist,
tarball: `http://127.0.0.1:${server.port}/pkg/-/pkg-1.0.0.tgz`,
},
},
Expand Down Expand Up @@ -700,7 +701,7 @@ describe.concurrent("tarball integrity metadata forms", () => {
const real = buildTarball(Buffer.from('{"name":"pkg","version":"1.0.0"}\n'));
const other = buildTarball(Buffer.from('{"name":"other","version":"9.9.9"}\n'));

await using server = serveManifest(`${other.sha512} ${real.sha384}`, real.tgz);
await using server = serveManifest({ integrity: `${other.sha512} ${real.sha384}` }, real.tgz);
using dir = projectDir("integrity-multi-hash", server.port);

await using proc = spawn({
Expand All @@ -720,7 +721,7 @@ describe.concurrent("tarball integrity metadata forms", () => {
const real = buildTarball(Buffer.from('{"name":"pkg","version":"1.0.0"}\n'));
const other = buildTarball(Buffer.from('{"name":"other","version":"9.9.9"}\n'));

await using server = serveManifest(`${real.sha512} ${other.sha384}`, real.tgz);
await using server = serveManifest({ integrity: `${real.sha512} ${other.sha384}` }, real.tgz);
using dir = projectDir("integrity-multi-hash-lock", server.port);

await using proc = spawn({
Expand All @@ -743,7 +744,7 @@ describe.concurrent("tarball integrity metadata forms", () => {
const real = buildTarball(Buffer.from('{"name":"pkg","version":"1.0.0"}\n'));
const other = buildTarball(Buffer.from('{"name":"other","version":"9.9.9"}\n'));

await using server = serveManifest(`${other.sha512}?vcs=git`, real.tgz);
await using server = serveManifest({ integrity: `${other.sha512}?vcs=git` }, real.tgz);
using dir = projectDir("integrity-option-suffix", server.port);

await using proc = spawn({
Expand All @@ -758,6 +759,51 @@ describe.concurrent("tarball integrity metadata forms", () => {
expect(stdout).not.toContain("1 package installed");
expect(exitCode).not.toBe(0);
});

it("verifies the tarball against the manifest shasum when there is no integrity field", async () => {
const real = buildTarball(Buffer.from('{"name":"pkg","version":"1.0.0"}\n'));
const other = buildTarball(Buffer.from('{"name":"other","version":"9.9.9"}\n'));

await using server = serveManifest({ shasum: other.shasum }, real.tgz);
using dir = projectDir("integrity-shasum-mismatch", server.port);

await using proc = spawn({
cmd: [bunExe(), "install"],
cwd: String(dir),
env: { ...env, BUN_INSTALL_CACHE_DIR: join(String(dir), ".cache") },
stdout: "pipe",
stderr: "pipe",
});
const [stderr, stdout, exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]);
expect(stderr).not.toContain("Malformed shasum");
expect(stderr + stdout).toContain("Integrity check failed");
expect(stdout).not.toContain("1 package installed");
expect(exitCode).not.toBe(0);
});

it("warns when the manifest shasum is malformed instead of silently skipping verification", async () => {
const real = buildTarball(Buffer.from('{"name":"pkg","version":"1.0.0"}\n'));

// Same length as a sha1 hex digest, but not hex.
await using server = serveManifest({ shasum: Buffer.alloc(40, "x").toString() }, real.tgz);
using dir = projectDir("integrity-shasum-malformed", server.port);

await using proc = spawn({
cmd: [bunExe(), "install", "--save-text-lockfile"],
cwd: String(dir),
env: { ...env, BUN_INSTALL_CACHE_DIR: join(String(dir), ".cache") },
stdout: "pipe",
stderr: "pipe",
});
const [stderr, stdout, exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]);
expect(stderr).toContain(
"warn: Malformed shasum in registry metadata for pkg@1.0.0; its tarball will not be verified",
);
expect(stdout).toContain("1 package installed");
// Nothing usable was advertised, so the lockfile carries no pin for it.
expect(await file(join(String(dir), "bun.lock")).text()).not.toMatch(/sha\d+-/);
expect(exitCode).toBe(0);
});
});

describe.concurrent.each(["hoisted", "isolated"] as const)("tarball download failure (%s)", linker => {
Expand Down
Loading