install: treat an out-of-range package id in bun.lockb as corruption

[alii]

A bun.lockb damaged by a bad binary-file merge / interrupted write / disk-full / bit-rot made bun install (and --frozen-lockfile) hard-panic (exit 133). Released Bun tolerates the damaged lockfile, re-resolves, and completes the install + rewrites a good lockfile — a deliberate robustness guarantee the port regressed into a crash.

Package::clone indexed package_id_mapping[self.meta.id as usize] with self.meta.id deserialized verbatim from lockfile bytes and never range-checked; on corruption it's garbage and Rust's slice bounds check panics. Zig's ReleaseFast does the same unchecked write (silent UB) and recovers downstream via re-resolution.

Bounds-check self.meta.id against package_id_mapping.len(); out-of-range → return an InvalidLockfile error, which the installer already handles by falling back to a fresh resolution. Verified end-to-end against the saved corrupt lockfile: bun install (and --frozen-lockfile) now completes (81 packages installed, exit 0) exactly like released Bun instead of panicking. Regression test + corrupt-lockfile fixture added.

[robobun]

^{Updated 6:18 PM PT - May 18th, 2026}

✅ @robobun, your commit 94894b08c80ed2283dc0db039c1f1e6b4bd3ace6 passed in Build #55926! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 31008

That installs a local version of the PR into your bun-31008 executable, so you can run:

bun-31008 --bun

[coderabbitai]

Warning

Rate limit exceeded

@robobun has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 5 minutes and 22 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: efda7ee5-df78-4d46-ac24-66e139ca7792

📥 Commits

Reviewing files that changed from the base of the PR and between dcd8ffc and 94894b0.

📒 Files selected for processing (3)

• src/install/lockfile/Package.rs
• src/install/lockfile/bun.lockb.rs
• test/cli/install/bun-lockb.test.ts

Walkthrough

This PR adds a bounds check for self.meta.id in Package<u64>::clone to return InvalidLockfile on out-of-range IDs and adds an integration test that stages a corrupted bun.lockb fixture to verify bun install exits successfully and creates node_modules instead of crashing.

Changes

Lockfile Corruption Handling

Layer / File(s)	Summary
Package ID mapping bounds validation src/install/lockfile/Package.rs	Package<u64>::clone validates self.meta.id against package_id_mapping.len() before indexing, returning Err(bun_core::err!("InvalidLockfile")) if out of bounds.
Integration test for corrupted lockfile handling test/cli/install/bun-lockb.test.ts, test/cli/install/fixtures/corrupt-lockb/package.json	Harness imports tempDir; new test stages a corrupted bun.lockb with a test package.json, runs bun install --prefer-offline --no-progress, asserts exit code is 0 and not 133, and verifies node_modules exists.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding bounds checking for package IDs in corrupted lockfiles to prevent panics.
Description check	✅ Passed	The description provides detailed context about the bug, the fix mechanism, and verification, but lacks explicit sections matching the template structure.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/cli/install/bun-lockb.test.ts`:
- Around line 123-137: The test currently discards child process output by
passing stdout/stderr: "ignore" to spawn, which hides valuable diagnostics;
change the spawn call used in this test (the spawn invocation that returns
exited and the code variable) to capture stdout/stderr (e.g., pipe or collect
streams) and surface them in assertions before or alongside the exit-code checks
so failures include stdout/stderr context; preserve the existing expectations
(expect(code).not.toBe(133), expect(code).toBe(0), and the node_modules exists
check) but reorder or augment the test to first assert or log captured
stdout/stderr when code !== 0 to make CI flakes debuggable.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ce939314-b71a-447e-96ba-1ebde475acc9

📥 Commits

Reviewing files that changed from the base of the PR and between 4c8a21b and db374b3.

⛔ Files ignored due to path filters (1)

• test/cli/install/fixtures/corrupt-lockb/bun.lockb is excluded by !**/bun.lockb

📒 Files selected for processing (3)

• src/install/lockfile/Package.rs
• test/cli/install/bun-lockb.test.ts
• test/cli/install/fixtures/corrupt-lockb/package.json

[github-actions]

Found 2 issues this PR may fix:

1. Random crash parsing lockfile during runtime #16199 - Random crash parsing lockfile during runtime — segfault in Package.parseWithJSON at a low address, consistent with an out-of-range package ID producing a garbage pointer in the lockfile parsing path this PR hardens
2. Bun crashes when I do bun install with a bun.lockb file in same path as package.json #23980 - Bun crashes when running bun install with a bun.lockb file — panic: attempt to use null value in package installation triggered by a corrupted lockfile, which the new bounds check on package_id_mapping would catch earlier

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #16199
Fixes #23980

🤖 Generated with Claude Code

[alii]

@robobun fix review comments

[robobun]

✅ Review comments addressed in 4a8e403:

• Regression test now generates bun.lockb against the local Verdaccio registry (no-deps/a-dep) and corrupts meta[1].id in-place — no more npmjs.org network dependency, no committed binary fixture.
• stdout/stderr piped and asserted (including the Ignoring lockfile warning) before the exit code.
• Moved the meta.id range check into Serializer::load so InvalidLockfile flows into the existing load-error fallback (warn + re-resolve) instead of aborting from clean_with_logger.

Gate verified: without the src fix the test panics with index out of bounds: the len is 3 but the index is 2147483647 at Package.rs:550; with the fix it re-resolves and installs both packages, exit 0.

---

CI: bun-lockb.test.ts passes on every lane. Remaining red on #55926 is the three darwin-*-test-bun lanes expiring waiting for an agent plus a known-flaky spawn-pipe-leak.test.ts RSS-threshold test on Windows (retried → passed) — none touch the lockfile parser. Diff is green; ready for a maintainer to re-run macOS / merge.

[claude]

LGTM — thanks for moving the check into Serializer::load and reworking the test to generate/corrupt the lockfile against local Verdaccio; all prior feedback addressed.

Extended reasoning...

Overview

This PR adds load-time validation that every meta.id in a deserialized bun.lockb is within [0, packages.len()), returning InvalidLockfile if not so the existing LoadResult::Err → report_lockfile_load_error → re-resolve path kicks in instead of a later panic_bounds_check. A second, defense-in-depth check is kept in Package::clone with an updated comment that correctly notes errors at that point abort rather than recover. The regression test now generates a valid lockfile against the local Verdaccio registry (no-deps/a-dep), surgically overwrites meta[1].id with 0x7fffffff, and asserts the install warns "Ignoring lockfile", re-resolves, and exits 0.

Security risks

None. Both source changes are pure additive bounds checks on data already read from disk; they only fire on out-of-range values and return an error that the caller already handles. No new parsing surface, no new trust boundaries, no change to happy-path behaviour.

Level of scrutiny

Lockfile deserialization is a critical install path, but the change here is the most conservative possible: two if id >= len { return Err } guards. The happy path is untouched. The earlier revision had real problems (error returned from a non-recoverable call site, network-dependent test, missing fixture) — all of which I flagged and all of which were fixed in 4a8e403. The author's reply confirms gate testing: without the fix the test panics with index out of bounds: the len is 3 but the index is 2147483647; with it, exit 0.

Other factors

• All three of my prior inline comments are resolved and the responses match the current diff.
• The test's hardcoded binary offsets (header at 42/86/110, 88-byte Meta, column ordering) are brittle to future format changes, but the test self-validates with expect(meta[0].id).toBe(0) / expect(meta[1].id).toBe(1) before corrupting, so format drift will fail loudly at the sanity check rather than silently pass.
• The three CI failures on 4a8e403 (macOS x64 build-cpp, v8-heap-snapshot.test.ts SIGKILL, spawn-stdout-iterate-leak.test.ts) are unrelated to this change.
• No CODEOWNERS entry covers src/install/lockfile/.
• No bugs found by the bug-hunting system on this revision.