ai slop

[robobun]

This PR has been marked as AI slop and the description has been updated to avoid confusion or misleading reviewers.

Many AI PRs are fine, but sometimes they submit a PR too early, fail to test if the problem is real, fail to reproduce the problem, or fail to test that the problem is fixed. If you think this PR is not AI slop, please leave a comment.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6f33c555-7197-4516-bf7a-4c815d1cafba

📥 Commits

Reviewing files that changed from the base of the PR and between 7eb4c67 and 9a2d732.

📒 Files selected for processing (2)

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

---

Walkthrough

Lockfile loading now validates deserialized slice and tree indices before use. ExternalSlice accessors clamp disk-derived ranges, Tree::folder_name guards invalid dependency IDs, and install tests cover crafted bun.lockb corruption cases.

Changes

Lockfile Corruption Hardening

Layer / File(s)	Summary
ExternalSlice bounds clamping src/install_types/resolver_hooks.rs	ExternalSlice::get and mut_ clamp both slice endpoints against the input buffer, and in_bounds is documented as the precheck used by lockfile loading.
Lockfile integrity checks src/install/lockfile/bun.lockb.rs, src/install/lockfile/Tree.rs	bun.lockb::load rejects out-of-bounds dependency, resolution, and hoisted-tree references, and Tree::folder_name returns empty for invalid dependency IDs.
Corruption rejection tests test/cli/install/bun-lockb.test.ts	The install test suite mutates crafted bun.lockb fields and asserts that install ignores the lockfile, exits successfully, and still installs pkg-a.

Possibly related PRs

• oven-sh/bun#31008 — Also hardens bun.lockb install handling for corrupted out-of-range IDs and extends corruption-focused install tests.

Suggested reviewers

• Jarred-Sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and accurately summarizes the main change: rejecting out-of-bounds bun.lockb ExternalSlice offsets.
Description check	✅ Passed	The description includes the problem, fix, repro, and verification, so it is mostly complete despite not using the template headings.
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.}

[robobun]

^{Updated 12:57 PM PT - Jun 24th, 2026}

✅ @robobun, your commit 9a2d73297516d1e5920ac10ac7a469207039d367 passed in Build #64545! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 32679

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

bun-32679 --bun

[claude]

Thanks for adding the tree[1].dependency_id and tree[1].parent cases — coverage now matches all five guards. I didn't find anything else, but since this touches ExternalSlice::get/mut_ (used by every lockfile/manifest caller) and the bun.lockb loader on the install hot path, it's worth a quick human pass.

Extended reasoning...

Overview

The PR hardens the binary lockfile (bun.lockb) loader against crafted/out-of-range (off, len) pairs and tree indices that previously reached slice indexing and panicked in release builds. Four files touched:

• src/install_types/resolver_hooks.rs: ExternalSlice::get/mut_ now clamp off as well as end; new in_bounds() helper.
• src/install/lockfile/bun.lockb.rs: new validation block in load() rejecting out-of-range per-package dependencies/resolutions slices, per-tree dependencies slices, tree.dependency_id, and tree.parent. Mirrors the existing meta.id check just above it.
• src/install/lockfile/Tree.rs: folder_name treats an out-of-range dependency_id like the invalid sentinel.
• test/cli/install/bun-lockb.test.ts: six new it.concurrent cases, one per guard.

Security risks

This is input-validation hardening for data parsed off disk (a tampered bun.lockb shipped in a cloned repo). The change is strictly defensive: previously-panicking inputs now degrade to empty slices or surface as InvalidLockfile (warn + re-resolve). No new attack surface is introduced. in_bounds computes off as usize + len as usize from two u32s, which cannot overflow on Bun's 64-bit targets.

Level of scrutiny

Medium-high. The logic is simple and follows the established meta.id pattern in the same function, but ExternalSlice::get/mut_ is a low-level primitive shared by every lockfile and npm-manifest-cache caller, and bun.lockb load() runs on every bun install in repos with a binary lockfile. A human should confirm the new rejection conditions can't trip on any legitimately-produced lockfile shape (I traced tree.parent / tree.dependency_id provenance through process_subtree and believe they cannot, but it's worth a second pair of eyes).

Other factors

• My earlier coverage comment (guards 4 and 5 untested) was addressed in 7944d46: the fixture now uses nested folder deps to produce a non-root tree, and two new cases corrupt tree[1].dependency_id and tree[1].parent. The author confirmed each guard is load-bearing by deletion testing. Thread is resolved.
• No CODEOWNERS cover these paths.
• The bug-hunting system found no issues on the current revision.
• CI build #64541 is in progress.

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@src/install/lockfile/bun.lockb.rs`:
- Around line 430-435: The comment in the lockfile safety check is too long and
should be reduced to 3 lines while preserving the core invariant. Shorten the
explanatory block near the dependency validation logic in bun.lockb.rs, keeping
only the essential note that ExternalSlice offsets and tree.dependency_id are
validated here to avoid invalid buffer access, and move any extra historical
context to the PR description.

In `@test/cli/install/bun-lockb.test.ts`:
- Around line 180-185: Shorten the block comment in the bun-lockb test to 3
lines or fewer while preserving the durable invariant about rejecting
out-of-bounds memcpy-derived offsets/indices at load time. Update the comment
near the test case setup in bun-lockb.test.ts to remove the detailed bug history
and keep only the essential note that tampered bun.lockb values must be rejected
so install warns and re-resolves instead of aborting.

🪄 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: 5366ad51-3c08-4919-b790-969068e20120

📥 Commits

Reviewing files that changed from the base of the PR and between 942c222 and 7eb4c67.

📒 Files selected for processing (4)

• src/install/lockfile/Tree.rs
• src/install/lockfile/bun.lockb.rs
• src/install_types/resolver_hooks.rs
• test/cli/install/bun-lockb.test.ts

[github-actions]

This PR has been closed because it was flagged as AI slop.

Many AI-generated PRs are fine, but this one was identified as having one or more of the following issues:

• Fails to verify the problem actually exists
• Fails to test that the fix works
• Makes incorrect assumptions about the codebase
• Submits changes that are incomplete or misleading

If you believe this was done in error, please leave a comment explaining why.