shell(mv): report ENAMETOOLONG instead of panicking when a source name does not fit the path buffers

[robobun]

Reproduction

mkdir -p /tmp/mvp/target && cd /tmp/mvp && bun -e 'const r = await Bun.$`mv ${"a".repeat(5000)} target`.nothrow().quiet(); console.log("exit", r.exitCode);'

panic: range end index 5000 out of range for slice of length 4096
oh no: Bun has crashed.

The process aborts (exit 134). Expected: mv: target/aaaa...: File name too long, a non-zero exit code from mv, and the script keeps running. On Linux the name has to be longer than 4096 bytes, on macOS longer than 1024.

A second shape needs a much shorter name. With a 3700-byte source name (the kernel rejects it, NAME_MAX is 255) and a target directory whose path is about 480 bytes, the abort moves to the error reporting instead:

panic: range end index 4182 out of range for slice of length 4094

mv a b dir/ with one over-long operand hits the same code, so it aborts too.

Cause

When the target is an existing directory, ShellMvBatchedTask::move_in_dir (src/runtime/shell/builtin/mv.rs) does two things with unbounded argv-sized input and fixed-size buffers:

1. It normalizes basename(src) into a PathBuffer (MAX_PATH_BYTES, 4096 on Linux, 1024 on macOS) and only checks the resulting length afterwards. normalize_buf copies with plain slice indexing, so a name that does not fit panics before the check runs.
2. When renameat fails, it builds the target/basename shown in the error message with resolve_path::join_z, which writes into the 4096-byte thread-local join buffer on every platform. A target path plus source name that do not fit together panic there, even when each one fits the path buffer on its own. Because of this, on Linux every name from 4094 bytes up crashed one way or the other.

move_multiple_into_dir goes through the same function, which is the multi-operand case.

Fix

move_in_dir now checks basename(src) against the buffer before normalizing. The bound is the same one the old post-check enforced (len + 1 >= MAX_PATH_BYTES): normalizing never grows a name by more than one byte ("" becomes "."), so a name that passes still leaves room for the NUL. Names that fail get the same ENAMETOOLONG error the old check produced, and that error now goes through the same path as a failed renameat, so it is reported as mv: target/<name>: File name too long like every other length rather than the path-less mv: File name too long the old check printed for names that happened to land exactly on the boundary.

The error path joins target and the name with join_spill, the existing helper that falls back to a caller-owned Vec when the thread-local buffer is too small (Bun.Glob and the rm fix in #37521 use it the same way). The join result is boxed into the error right away, so the spill vector is only live inside the closure and is never allocated when the path fits.

The limit enforced here is only the size of mv's own scratch buffer; whether a shorter name is acceptable to the filesystem (NAME_MAX) is still left to the kernel, as before. Behavior for every name that fits is unchanged: the old and new binaries print identical messages and exit codes for "", ., .., /, dir/, nested and plain files, a missing source and a 1100-byte name. The exit status of a failing mv is still the raw errno; #32278 changes that across builtins, so it is left alone here and the new test only asserts that mv failed. The same scratch-buffer pattern in mkdir and touch is tracked separately.

Verification

New case in test/js/bun/shell/commands/mv.test.ts. It runs mv in a child process (so the abort shows up as a failed assertion instead of taking the runner down) and covers the three shapes above: a 5000-byte name into a directory, a 3700-byte name into a two-level 481-byte directory (fits the Linux path buffer, overflows the join buffer; the same sizes stay creatable under macOS's 1024-byte PATH_MAX), and mv short.txt <5000-byte name> target, which additionally checks that short.txt was moved before the bad operand was reported. Each of the three crashes the unfixed binary on its own. On POSIX it asserts the exact mv: <target>/<name>: File name too long message; on Windows, where the errno for such a name is up to the OS, it only asserts that mv reported a failure.

• USE_SYSTEM_BUN=1 bun test test/js/bun/shell/commands/mv.test.ts -t "path buffers": fails, the child aborts with the first panic above.
• bun bd test test/js/bun/shell/commands/mv.test.ts: 13 pass.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 3 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: 670f8899-20b2-4b9c-b2f2-23248c24a465

📥 Commits

Reviewing files that changed from the base of the PR and between da3851e and bb01c9c.

📒 Files selected for processing (2)

• src/runtime/shell/builtin/mv.rs
• test/js/bun/shell/commands/mv.test.ts

---

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

[robobun]

Status: reproduced on the release build (1.4.0-canary.1, Linux x64) with the three shapes in the description; each one aborts the process with panic: range end index N out of range for slice of length 4096 (or 4094 for the join-buffer shape). With this branch all three report mv: <target>/<name>: File name too long and the script continues. bun bd test test/js/bun/shell/commands/mv.test.ts passes (13 tests); the new case fails against the unfixed binary. Waiting on CI.

[claude]

LGTM — focused crash fix that moves the length check before the unchecked buffer write and swaps join_z for join_spill on the error path.

What was reviewed:

• Verified normalize_buf_z and join_spill signatures/semantics in resolve_path.rs; with_path boxes the slice immediately so the spill Vec lifetime is contained to the closure.
• Traced the three test shapes (5000-byte name, 3700-byte name into 481-byte dir, multi-operand) through both the Linux 4096-byte and macOS 1024-byte MAX_PATH_BYTES branches — each hits the intended check and the join-buffer spill.
• Checked the bound base.len() + 1 >= MAX_PATH_BYTES matches the old post-check and leaves room for normalize_buf_z's trailing NUL.
• Test follows harness conventions (subprocess isolation for the abort, Buffer.alloc, tempDir, concurrent pipe drain, exit code asserted last).

Extended reasoning...

Overview

The PR fixes two panic sites in ShellMvBatchedTask::move_in_dir (src/runtime/shell/builtin/mv.rs) reached when mv is given a source name longer than the fixed-size path buffers, and adds a subprocess-isolated regression test in test/js/bun/shell/commands/mv.test.ts.

The fix is a small reordering plus a helper swap: (1) check basename(src).len() against MAX_PATH_BYTES before calling normalize_buf_z (which writes into the buffer with unchecked slice indexing), and (2) build the target/basename string in the error message with resolve_path::join_spill (falls back to a caller-owned Vec) instead of join_z (fixed 4096-byte thread-local buffer). Both helpers already exist and are used the same way elsewhere (rm, Bun.Glob).

Security risks

None. This turns a user-reachable panic (DoS on the Bun process) into a recoverable ENAMETOOLONG error, which is strictly a hardening. No new I/O, no new trust boundaries, no path traversal surface — the only path handling added is for the error message string, which is boxed into bun_sys::Error and printed.

Level of scrutiny

Medium. Shell builtins are user-facing runtime code and the change touches a size-bounds check on argv-sized input, so I traced the bound arithmetic carefully: the pass condition base.len() + 1 < MAX_PATH_BYTES guarantees normalize_buf_z has room for the normalized name plus its NUL terminator (normalization never grows a bare basename beyond +1, and the old code enforced the identical bound post-hoc). The error-path change is confined to a map_err closure; with_path immediately clones the joined slice into a Box, so the spill Vec and thread-local buffer are both safe to drop/reuse afterward. Behavior for every name that fits is byte-identical to before — same normalize call, same do_rename, same error format.

Other factors

• The test is well-constructed per REVIEW.md: runs the crashing input in a spawned child so the unfixed abort fails the assertion instead of killing the runner; covers all three crash shapes plus the multi-operand path; asserts the exact POSIX error string and a positive side-effect (shortMoved: true); loosens only the Windows errno text; uses Buffer.alloc(n, fill) over .repeat(), tempDir, {...bunEnv}, concurrent pipe drain, and asserts exitCode last.
• The 240-byte directory segments were chosen to stay under NAME_MAX (255) and keep the nested path creatable under macOS's 1024-byte PATH_MAX, while still overflowing the 4096-byte join buffer when combined with the 3700-byte name — I walked the arithmetic for both platforms.
• PR description documents USE_SYSTEM_BUN=1 failure and bun bd test pass, and explicitly scopes out the sibling mkdir/touch pattern and the errno-as-exit-code cleanup (#32278) as tracked separately.
• No prior human or bot reviews on this PR to consider.