install: fail closed on unusable npm integrity (pin computed sha512; reject invalid manifest/lockfile hashes) - #31327
Code review found 2 important issues
Found 5 candidates, confirmed 6. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 2 |
| 🟡 Nit | 4 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/install/lockfile/bun.lock.rs:2670-2699 |
Empty-integrity frozen-lockfile check retroactively breaks existing v2 lockfiles; gating comment's premise is false |
| 🔴 Important | src/install/npm.rs:2571-2594 |
Tag::INVALID rejects placeholder integrity strings, breaking minimum-release-age.test.ts on all CI platforms |
| 🟡 Nit | src/install/integrity.rs:290-296 |
Tag::INVALID doc comment claims "Never written to disk" but it is serialized to the manifest cache |
Annotations
Check failure on line 2699 in src/install/lockfile/bun.lock.rs
claude / Claude Code Review
Empty-integrity frozen-lockfile check retroactively breaks existing v2 lockfiles; gating comment's premise is false
The comment says this check is "Gated to v2+ so lockfiles written before the back-fill existed… keep loading silently", but `Version::CURRENT` was already `V2` on the base commit (ae4b17de introduced V2 on 2026-07-24; this PR's base is 59242d6c) — so pre-PR Bun already writes v2 lockfiles, and per this PR's own Face B those v2 lockfiles persist `""` for npm packages whose registry omits integrity. After upgrading, `bun install --frozen-lockfile` / `--production` on such a lockfile fails with "Pa
Check failure on line 2594 in src/install/npm.rs
claude / Claude Code Review
Tag::INVALID rejects placeholder integrity strings, breaking minimum-release-age.test.ts on all CI platforms
The new `Tag::INVALID` path breaks `test/cli/install/minimum-release-age.test.ts` on all 9 CI platforms (see robobun's comment): its mock registry serves ~48 placeholder `dist.integrity` values like `"sha512-fake1=="` which now trip `had_invalid_integrity` and cause `Package::from_npm` to fail every install with `error: Registry provided an invalid integrity hash for <pkg>@<ver>`. Per REVIEW.md ("When changing output/defaults/messages, grep the suite for assertions on the old behavior and update
Check warning on line 296 in src/install/integrity.rs
claude / Claude Code Review
Tag::INVALID doc comment claims "Never written to disk" but it is serialized to the manifest cache
The "Never written to disk" claim here (and in the PR description's "runtime sentinel only … so it never reaches disk") is not quite true: `npm.rs:2589-2594` sets `Tag::INVALID` on `PackageVersion.integrity`, and `PackageVersion` is byte-serialized to the on-disk manifest cache via `Serializer::write_array(writer, &this.package_versions, …)` (`npm.rs:1038`). It correctly never reaches a *lockfile* (`from_npm` replaces it before append; `Display` emits nothing), but discriminant 5 does land in `~