which: return lengths from the search helpers, forbid unsafe

[robobun]

What

which() built its Option<&'a ZStr> result inside the search itself, so every caller that wanted to retry after a miss had to reborrow the buffer behind the borrow checker's back. which_for_spawn reborrowed the PathBuffer through a raw pointer before its cwd attempt, and the two lookup loops in which_win did the same with the WPathBuffer once per $PATH segment because search_bin / search_bin_in_path returned Option<&mut [u16]>. The absolute-path arm additionally built its result with ZStr::from_raw_mut while the sibling arms in the same function already used the safe ZStr::from_buf.

The search now returns the length it wrote:

// before
pub fn which<'a>(buf: &'a mut PathBuffer, ..) -> Option<&'a ZStr> { /* whole search */ }
fn search_bin(buf: &mut WPathBuffer, ..) -> Option<&mut [u16]>
fn search_bin_in_path<'a>(buf: &'a mut WPathBuffer, ..) -> Option<&'a mut [u16]>

// after
pub fn which<'a>(buf: &'a mut PathBuffer, ..) -> Option<&'a ZStr> {
    let len = which_len(buf, path, cwd, bin)?;
    Some(ZStr::from_buf(&buf[..], len))
}
fn which_len(buf: &mut PathBuffer, ..) -> Option<usize>   // writes path + NUL, returns len
fn search_bin(buf: &mut WPathBuffer, ..) -> Option<usize>
fn search_bin_in_path(buf: &mut WPathBuffer, ..) -> Option<usize>

which() and which_for_spawn() keep their signatures, so none of the external call sites change. which_for_spawn's cwd attempt is now if let Some(len) = which_len(..) { return Some(ZStr::from_buf(&buf[..], len)); }, and the three result sites in which_win slice buf[..len] themselves. This removes the 4 unsafe blocks in the crate (the only ones it had), and src/which/lib.rs is now #![forbid(unsafe_code)]. One file, 30 insertions, 39 deletions.

Why

The raw-pointer reborrows existed only to work around NLL rejecting "return the borrow on hit, keep using the buffer on miss"; returning the length instead expresses that shape directly, and the borrow that becomes the returned &ZStr / &[u16] is created exactly once, on the returning path, where the compiler can check it. It is zero-cost: the lengths were already computed by is_valid and search_bin, ZStr::from_buf is debug-assert only (it replaces an unchecked from_raw_mut with identical release codegen), and each &buf[..len] in which_win replaces the identical &mut buf[..path_size] slice search_bin used to build, so no checks, copies or branches are added on any path. which_len is private with a single caller on POSIX.

Part of a series of small type-system hardening changes; each PR stands alone.

Verification

cargo check and cargo clippy are clean for the touched crates. Debug build succeeds. bun bd test test/js/bun/util/which.test.ts test/js/bun/spawn/spawn-path.test.ts test/js/bun/shell/exec.test.ts test/js/bun/shell/commands/which.test.ts: 31 pass, 0 fail (31 tests across 4 files).

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 18 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: af862722-efd1-4b4a-9f31-158cf7c9a0f5

📥 Commits

Reviewing files that changed from the base of the PR and between 97e21e5 and e4415d5.

📒 Files selected for processing (1)

• src/which/lib.rs

---

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

[claude]

LGTM — mechanical refactor that swaps raw-pointer reborrows for length-returning helpers and forbids unsafe in the crate.

What was reviewed

• Traced every Some(len) return in which_len back to a preceding buf[len] = 0 write (Windows convert path, POSIX absolute, and both is_valid arms) so ZStr::from_buf's NUL precondition holds on all paths.
• Confirmed search_bin/search_bin_in_path/which_win have no callers outside this file, and the public which/which_for_spawn signatures are unchanged.
• Checked ZStr::from_buf in src/bun_core/util.rs — safe fn with debug-asserts only, so the from_raw_mut → from_buf swap is behavior-identical in release.

Extended reasoning...

Overview

Single-file refactor of src/which/lib.rs (30 insertions, 39 deletions). The internal search helpers (search_bin, search_bin_in_path, and the body of which now extracted as which_len) previously returned borrows into the caller's buffer (Option<&mut [u16]> / Option<&ZStr>), which forced NLL-workaround raw-pointer reborrows at three retry sites plus one ZStr::from_raw_mut in the absolute-path arm. The helpers now return Option<usize> (the length written into buf), and each caller slices buf[..len] on the returning path only. All four unsafe blocks in the crate are gone and #[forbid(unsafe_code)] is added at the top.

Security risks

None introduced. This is strictly a reduction in unsafe surface: raw-pointer reborrows that the borrow checker could not verify are replaced with length returns the compiler checks end-to-end. I verified every Some(len) path in which_len writes buf[len] = 0 before returning, so the ZStr::from_buf(&buf[..], len) at the two call sites (which and which_for_spawn's Windows cwd probe) upholds the NUL invariant on all branches. ZStr::from_buf itself (src/bun_core/util.rs:122) is a safe fn with debug-asserts and identical release codegen to the removed from_raw_mut, so there is no behavior or performance change.

Level of scrutiny

Low-to-medium. The change is a type-level restructuring with no new logic, no new branches, and net-negative lines. Public signatures (which, which_for_spawn) are unchanged, so no external call sites are affected — confirmed via grep that which_win/search_bin/search_bin_in_path are referenced only within this file. The #[forbid(unsafe_code)] attribute means the compiler now enforces the crate stays unsafe-free. Not CODEOWNER-gated.

Other factors

The PR description reports 31 passing tests across which.test.ts, spawn-path.test.ts, exec.test.ts, and shell/commands/which.test.ts, plus clean cargo check/clippy. The bug-hunting pass found nothing. The Windows posix_to_platform_in_place call in the cwd arm still receives a &mut buf[..len] slice equivalent to the old search_bin_in_path return, and the $PATH-loop arm returns &buf[..len] matching the previous &*bin_path. This is exactly the kind of small type-system hardening change the description frames it as.

[github-actions]

This PR may be a duplicate of:

1. Enable -Zpolonius=next and remove NLL borrow-checker workarounds #36951 - Removes the same three raw-pointer reborrow unsafe blocks in src/which/lib.rs (which_for_spawn and both search_bin_in_path call sites in which_win) as NLL borrowck workarounds, via -Zpolonius=next instead of returning lengths.

🤖 Generated with Claude Code

[robobun]

Overlaps with one hunk of #36951 but is not the same change: #36951 keeps this code shape and makes it compile by switching the whole workspace to the polonius borrow checker (31 files), while this PR restructures the helpers to return lengths so the reborrows are unnecessary under the current borrow checker, and then forbids unsafe in the crate. Whichever lands first, the other still applies: if #36951 goes in first this PR reduces to the length-returning shape plus the forbid attribute; if this goes in first, #36951 simply loses its src/which/lib.rs hunk.

[robobun]

^{Updated 1:45 PM PT - Aug 11th, 2026}

✅ @robobun, your commit e4415d5ad455ac2e5250e84fe6491faebb840db6 passed in Build #92420! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 37595

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

bun-37595 --bun