patch: guard against crafted patches that panic the applier

[robobun]

Repro

Two inputs crash bun install (and patchInternals.apply) on current canary:

// 1) file-creation hunk with an empty body
const patch = [
  "diff --git a/newfile.txt b/newfile.txt",
  "new file mode 100644",
  "--- /dev/null",
  "+++ b/newfile.txt",
  "@@ -0,0 +0,0 @@",
  "",
].join("\n");
patchInternals.apply(patch, dir);
// panic: index out of bounds: the len is 0 but the index is 0

// 2) header claims more deletions than the target file has
// target.txt = "only line\n"
const patch = [
  "diff --git a/target.txt b/target.txt",
  "--- a/target.txt",
  "+++ b/target.txt",
  "@@ -1,10 +1,1 @@",
  " only line",
  "-x","-x","-x","-x","-x","-x","-x","-x","-x",
  "",
].join("\n");
patchInternals.apply(patch, dir);
// panic: called `Result::unwrap()` on an `Err` value: TryFromIntError(NegOverflow)

Both patches parse successfully (they pass verify_integrity), so they reach PatchFile::apply. A malicious patchedDependencies entry can crash bun install this way.

Cause

• PatchFile::apply for FileCreation indexed hunk.parts[0] unconditionally. A header like @@ -0,0 +0,0 @@ with no body produces a hunk whose parts vec is empty (the integrity check passes because both lengths are 0).
• apply_patch precomputed the capacity for the lines vec as usize::try_from(i64(count) + patched_len - original_len).unwrap(). When the header's original.len exceeds the actual file's line count, that goes negative and the usize conversion panics before the real per-part bounds checks run.

Fix

• Use hunk.parts.first() and treat an empty parts vec the same as hunk: None: the newly-created file stays empty.
• The lines_count value is only a Vec::with_capacity hint; switch to saturating arithmetic so a lying header clamps to 0 instead of panicking. The existing bounds checks during the splice then surface the mismatch as a catchable EINVAL.

Verification

bun bd test test/js/bun/patch/patch.test.ts: 25 pass, including the two new apply > malformed patches do not crash > ... cases. Both new tests fail with exitCode: 132 / panic output under USE_SYSTEM_BUN=1. test/regression/issue/patch-bounds-check.test.ts still passes (3/3).

Fixes #21932 (crash in PatchFile.apply during bun install when a patchedDependencies entry targets a package version whose files no longer match the hunk headers).

[robobun]

^{Updated 10:17 AM PT - Jun 24th, 2026}

❌ @robobun, your commit a125b21 has 1 failures in Build #64514 (All Failures):

• test/js/bun/terminal/terminal.test.ts - code 1 on 🍎 14 x64

---

🧪   To try this PR locally:

bunx bun-pr 32668

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

bun-32668 --bun

[github-actions]

Found 1 issue this PR may fix:

1. Crash when patching in bun install #21932 - Crash in PatchFile.apply during bun install with patchedDependencies — the stack trace points to the exact apply code path this PR hardens against empty hunk parts and oversized deletion counts.

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

Fixes #21932

🤖 Generated with Claude Code

[coderabbitai]

Walkthrough

Two safety fixes in the Rust patch engine: PatchFile::apply now guards against an empty hunk parts vector using parts.first(), and apply_patch replaces i64-cast arithmetic with saturating add/sub for capacity hints. Two process-stability tests verify both cases.

Changes

Malformed patch safety fixes

Layer / File(s)	Summary
Safe empty-hunk and saturating arithmetic in patch engine src/patch/lib.rs	FileCreation switches from hunk.parts[0] to hunk.parts.first() with a guard for the empty-body case, and the capacity-hint block replaces i64 conversions with direct saturating_add/saturating_sub on no_newline_at_end_of_file as usize.
Process-stability tests for malformed patches test/js/bun/patch/patch.test.ts	New describe.concurrent("malformed patches do not crash") suite spawns Bun processes to assert that an empty-parts file-creation hunk succeeds with an empty output file, and a line-count underflow hunk fails with EINVAL and no stderr.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: preventing crafted patches from crashing the applier.
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.
Description check	✅ Passed	The PR description clearly explains the fixes and includes verification results, even though it uses different headings than the template.

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

---

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

[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/js/bun/patch/patch.test.ts`:
- Line 545: The test fixture setup still uses tempDirWithFiles for directories
that need disposal, so those temp directories can leak in persistent runners.
Update the affected cases in patch.test.ts to use tempDir with using for the
fixture directory, and when spawning the process pass String(dir) so the
temporary path is used correctly. Make the same change in the other matching
test case as well, keeping the cleanup behavior aligned with tempDir and the
existing test helpers.

🪄 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: 4661a69e-e766-4a78-8352-3214731e3ed4

📥 Commits

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

📒 Files selected for processing (2)

• src/patch/lib.rs
• test/js/bun/patch/patch.test.ts

[robobun]

Linked #21932 in the description: that crash is PatchFile.apply segfaulting when a patchedDependencies patch is applied against a package version whose files no longer match the hunk line counts, which is the negative-capacity path this PR replaces with saturating arithmetic.

Resolved the coderabbit note about tempDir: keeping tempDirWithFiles to match every other test in this file (and its Windows path-normalization wrapper).

---

CI status: the diff is green; test/js/bun/patch/patch.test.ts passes on every lane. The remaining red on build 64514 is test/js/bun/terminal/terminal.test.ts timing out on darwin 14 x64 ("creates subprocess with terminal attached", 90s timeout), which does not touch the patch applier. The earlier run (64503) failed only on a darwin artifact-download timeout before any tests ran. Ready for a maintainer to merge.

[claude]

LGTM — minimal defensive hardening that swaps two panic paths for graceful handling, with regression tests for both.

Extended reasoning...

Overview

This PR touches two files: src/patch/lib.rs (the Rust patch applier used by bun install for patchedDependencies) and test/js/bun/patch/patch.test.ts. The Rust change is two surgical edits: (1) replace unconditional hunk.parts[0] indexing in the FileCreation arm with hunk.parts.first() + a let-else continue, so a zero-body @@ -0,0 +0,0 @@ hunk yields an empty file instead of an index-out-of-bounds panic; (2) replace the usize::try_from(i64 - i64).unwrap() capacity precomputation in apply_patch with saturating_add/saturating_sub, so a header that overstates deletions clamps to 0 instead of panicking on negative-to-usize conversion. Two new subprocess tests cover each case.

Security risks

None introduced — this is hardening that removes two DoS-via-panic vectors reachable from untrusted patchedDependencies input. The file-creation path still goes through the existing is_safe_patch_path guard before openat, and the empty-parts continue lands after the file is already created/truncated, which is the correct semantics for a 0-line "new file" hunk. The saturated lines_count is used only as a Vec::with_capacity hint; correctness is enforced by the per-part bounds checks that already return EINVAL during the splice loop.

Level of scrutiny

Low. Both edits are behavior-preserving for well-formed patches: .first() is identical to [0] when the vec is non-empty, and saturating arithmetic is identical to the old i64 path when no underflow occurs. The only new behavior is on previously-panicking inputs, where we now get an empty file or a catchable EINVAL.

Other factors

All prior review threads are resolved (CodeRabbit withdrew its tempDir suggestion; my earlier cargo fmt nit was addressed by the autofix commit b74309c). The bug-hunting system found nothing. The musl CI build failures point at scripts/build/ci.ts build infra, not at this change, and a retrigger commit is already on the branch. No CODEOWNERS entry covers src/patch/.