install: report ENAMETOOLONG for a --cwd value that does not fit the path buffer

[robobun]

Problem

• Every package manager subcommand (bun install, add, remove, update, pm, ...) aborts when its --cwd value is at least PATH_MAX bytes long (4096 on Linux, 1024 on macOS), instead of printing an error:

  $ bun install --cwd "$(head -c 5000 /dev/zero | tr '\0' a)"
  panic: range end index 5000 out of range for slice of length 4096
  $ bun install --cwd "./$(head -c 5000 /dev/zero | tr '\0' a)"
  panic: range end index 5014 out of range for slice of length 4095
  $ bun install --cwd "$(head -c 4096 /dev/zero | tr '\0' a)"
  panic: index out of bounds: the len is 4096 but the index is 4096
  

  (exit 134). A value one byte shorter already printed error: failed to change directory to "...": ENAMETOOLONG and exited 1.
• Cause: the --cwd block in src/install/PackageManager/CommandLineArguments.rs (lines 1298-1329 on main) stages the value in a PathBuffer of exactly PATH_MAX bytes with no length check. A value not starting with . is copied in with copy_from_slice and then NUL-terminated one byte further; a value starting with . is resolved with the unchecked join_abs_string_buf, which panics inside the normalizer when the result does not fit the buffer.
• The runtime's global bun --cwd flag has the same bug in separate code (src/runtime/cli/Arguments.rs); that one is fixed in cli: stop aborting on --cwd and --tsconfig-override values longer than the path join buffer #38368. This PR only covers the package manager's own flag.

Fix

• The block moves into change_directory. The .-prefixed branch resolves with join_abs_string_buf_checked into PATH_MAX - 1 bytes of the buffer; the other branch copies the value only when it leaves that last byte free. Both keep the last byte for the NUL terminator chdir reads. When the value does not fit, the existing failed to change directory to "<value>": <errno> message is printed with ENAMETOOLONG and the process exits 1, the same as when chdir itself fails.
• Why ENAMETOOLONG is the right answer rather than some new error: PATH_MAX counts the terminator, so a path of PATH_MAX or more bytes is one chdir would reject with ENAMETOOLONG if it could be passed at all. The check only turns away values that could never have been entered, and the user sees the same line either side of the buffer boundary (the PATH_MAX - 1 control in the test is the kernel's own rejection, printed through the unchanged chdir error path).
• Why the checked join rather than a length check on the argument in the . branch: that branch already normalizes (--cwd ./moo/.. enters the parent), and join_abs_string_buf_checked fails only when the normalized result does not fit, so that behaviour is kept. Mapping its None to ENAMETOOLONG is how the lockfile code already treats it (src/install/lockfile/Package/WorkspaceMap.rs).
• Messages for failures that are not about length are unchanged: a missing directory still prints the value as given ("/nonexistent") or, for a . value, the resolved path ("/tmp/proj/nonexistent").
• Verified with the new --cwd that does not fit the path buffer block in test/cli/install/bun-install.test.ts: PATH_MAX bytes, PATH_MAX + 1000 bytes, the ./ form, and bun add, plus PATH_MAX - 1 as the kernel-rejected control. On the current release build the four new lengths abort with the panics above and the control passes; with this change all five pass, as does the existing should handle --cwd test.
• Checked by hand with the debug build that --cwd moo, ./moo, moo/, ., ./moo/.., an absolute path, "", a missing directory and ./<5000 bytes>/.. (normalizes to the cwd and succeeds) behave as before.
• The test block is skipped on Windows: PathBuffer there is about 96 KiB, longer than any command line, so the overflow is unreachable.

Background

• PathBuffer is a [u8; MAX_PATH_BYTES] scratch buffer, where MAX_PATH_BYTES is the platform PATH_MAX (4096 on Linux, 1024 on macOS and the BSDs, 32767 * 3 + 1 on Windows). PATH_MAX includes the NUL terminator, so the longest path a syscall accepts is PATH_MAX - 1 bytes.
• join_abs_string_buf(cwd, buf, parts) is path.resolve into a caller buffer; it assumes the result fits. join_abs_string_buf_checked is the variant for user-controlled input: it normalizes into heap scratch when needed and returns None only if the normalized result does not fit buf.
• Output::err_generic + Global::crash() is how this argument parser reports every invalid flag value: it prints error: ... and exits 1.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 27 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: 85bf1d5f-7143-4a09-9ab8-ff82da234560

📥 Commits

Reviewing files that changed from the base of the PR and between abe3007 and eb150a6.

📒 Files selected for processing (2)

• src/install/PackageManager/CommandLineArguments.rs
• test/cli/install/bun-install.test.ts

---

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

[robobun]

^{Updated 4:05 AM PT - Aug 14th, 2026}

❌ @robobun, your commit eb150a6 has some failures in Build #95575 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 38375

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

bun-38375 --bun

[robobun]

Status: reproduced and fixed; review feedback addressed; waiting on CI.

• Reproduced on the current release build: bun install --cwd "$(head -c 5000 /dev/zero | tr '\0' a)" aborts with panic: range end index 5000 out of range for slice of length 4096 (exit 134); the ./ form and an exactly 4096-byte value abort as well.
• Fix in src/install/PackageManager/CommandLineArguments.rs, tests in test/cli/install/bun-install.test.ts (--cwd that does not fit the path buffer): the four over-long cases fail on the release build and pass with this branch.
• Follow-ups from review: Android uses the 4096-byte PATH_MAX in the test, the child's stdout is drained and asserted empty, and the doc comment is one line.
• The runtime's global bun --cwd flag is separate code and is handled in cli: stop aborting on --cwd and --tsconfig-override values longer than the path join buffer #38368.

[claude]

LGTM — straightforward panic-to-error fix with correct bounds handling. The Android PATH_MAX nit in the test is latent (CI does not execute tests on Android).

What was reviewed:

• Both buffer paths keep the last byte for the NUL: . branch passes buf2[..len-1] to the checked join so buf2[len] = 0 stays in bounds; non-. branch gates on arg.len() < buf.len().
• getcwd failure still propagates via ?, and the chdir error message format / exit code are unchanged for existing cases.
• Test covers the exact-PATH_MAX boundary, the kernel-rejected PATH_MAX-1 control, the ./ normalizer path, and a second subcommand.

Extended reasoning...

Overview

This PR fixes a panic in the package manager's --cwd flag handling (src/install/PackageManager/CommandLineArguments.rs) when the value is ≥ PATH_MAX bytes. The inline block is extracted into change_directory(), the .-prefixed branch switches from join_abs_string_buf to join_abs_string_buf_checked writing into PATH_MAX-1 bytes, and the plain-copy branch adds an arg.len() < buf.len() guard. Overflow now prints the existing failed to change directory to "…": ENAMETOOLONG line and exits 1 instead of aborting. A new describe.concurrent block in bun-install.test.ts covers five cases (kernel-rejected control, exact boundary, over-length, ./ prefix, bun add).

Security risks

None. This is CLI argument validation on the local process; the only effect is turning a controlled Rust panic (already a process abort) into a clean error message + exit 1. No new trust boundary, no new syscall, no change to what chdir is called with for previously-valid inputs.

Level of scrutiny

Moderate — native buffer handling, but mechanical. I traced both branches for off-by-one: the checked join is given a buf2.len()-1 slice, so the returned len satisfies len ≤ buf2.len()-1 and buf2[len] = 0 is the last byte at worst; the copy branch's < (not ≤) leaves exactly the NUL slot. ZStr::from_buf's len < buf.len() and buf[len] == 0 preconditions hold on both. The .map(|j| j.len()) drops the borrow before the mutable NUL write, so no aliasing issue. getcwd failure still short-circuits via ? up through parse(), preserving the original propagation.

Other factors

The one finding is a latent test-constant mismatch on Android (isLinux excludes it but MAX_PATH_BYTES is 4096 there); Android is a cross-compile-only target with no test execution in CI, so it cannot fail today. The PR description is thorough, cites the sibling fix (#38368) for the runtime's own --cwd, and the test's PATH_MAX-1 control row proves the assertion path is shared with the pre-existing kernel rejection. No prior reviews on the PR.

[claude]

LGTM — the panic is now a clean ENAMETOOLONG exit, and the earlier Android PATH_MAX and doc-comment-length feedback have both been addressed. The remaining inline nit (piped-but-undrained stdout in the test) has no functional impact on this path.

Extended reasoning...

Overview

Extracts the package manager's --cwd handling into change_directory() in src/install/PackageManager/CommandLineArguments.rs, replacing the unchecked copy_from_slice/join_abs_string_buf into a PATH_MAX-sized PathBuffer with length-checked equivalents. Oversized values now print the existing failed to change directory to "…": ENAMETOOLONG line and exit 1 instead of aborting with a slice-index panic. A five-row it.each in test/cli/install/bun-install.test.ts covers PATH_MAX - 1 (kernel-rejected control), exactly PATH_MAX, PATH_MAX + 1000, the ./ form, and bun add.

Security risks

None. The change turns a user-reachable panic (release-build abort on adversarial CLI input, exit 134) into a recoverable error, which is exactly what REVIEW.md asks for. No new syscalls, no privilege- or trust-boundary changes; the argument was already being passed to chdir before.

Level of scrutiny

Low-to-medium. This is a focused ~45-line refactor of one CLI-parsing block plus a test. The non-. branch is a strict-less-than length check leaving one byte for the NUL; the . branch uses join_abs_string_buf_checked into buf2[..len-1] and maps None to ENAMETOOLONG (matching the precedent cited in WorkspaceMap.rs). The .map(|joined| joined.len()) releases the mutable borrow of buf2 before re-borrowing it for the NUL write and ZStr::from_buf, so the borrow structure is sound. Error reporting reuses the exact Output::err_generic + Global::crash() pattern every other invalid-flag branch in this file uses.

Other factors

• My prior review flagged the test's PATH_MAX = isLinux ? 4096 : 1024 as wrong for Android; commit 35b9e40 fixed it to isLinux || isAndroid and updated the comment — resolved.
• The comment-cop bot flagged an over-long doc comment; commit 8c2fecb shortened it to one line — addressed (thread not marked resolved but the code is fixed).
• The one CI failure (test/bake/deinitialization.test.ts segfault on Windows x64) is in an unrelated subsystem; the new test is skipIf(isWindows).
• The remaining inline finding is a harness-convention nit: stdout: "pipe" is requested but never drained. There is no deadlock risk here because the child writes only to stderr and crashes before any install output, so it does not block merge.

[claude]

LGTM — both earlier nits (Android PATH_MAX, stdout drain) and the comment-cop note are addressed in the latest commits.

What was reviewed:

• change_directory: verified arg.len() < buf.len() and buf2[..buf2.len()-1] both leave the NUL slot in-bounds for ZStr::from_buf's len < buf.len() precondition on every branch.
• Checked that join_abs_string_buf_checked → None → ENAMETOOLONG matches the existing WorkspaceMap.rs treatment; error-message shape is unchanged for non-length failures.
• Test: five rows hit the three code paths (kernel reject, copy-branch overflow, checked-join overflow) plus a second subcommand; describe.concurrent, pipes drained, Windows skipped for the stated reason.

Extended reasoning...

Overview

Fixes a panic in the package-manager --cwd handling (bun install/add/remove/update/pm/...) when the argument is ≥ PATH_MAX bytes: the value was staged in a fixed PathBuffer with no length check, so copy_from_slice / the unchecked join_abs_string_buf panicked. The block is extracted into change_directory, which uses join_abs_string_buf_checked for the .-prefixed branch and an explicit arg.len() < buf.len() guard for the other, mapping overflow to the existing failed to change directory to "...": ENAMETOOLONG message + exit 1. A five-row it.each in bun-install.test.ts covers exact-PATH_MAX, over-PATH_MAX, the ./ variant, a second subcommand, and a PATH_MAX-1 control that reaches the kernel.

Security risks

None. The change hardens CLI argument handling — it turns a reachable panic on adversarial input into a clean error. No new syscalls, no path traversal surface (the value was already passed to chdir), no privilege changes.

Level of scrutiny

Low-to-medium. ~45 lines of Rust extracted from an inline block with two length guards added; the success path is byte-for-byte the same behavior. I traced both branches against ZStr::from_buf's len < buf.len() / buf[len] == 0 preconditions and the join_abs_string_buf_checked contract in resolve_path.rs — every index write is provably in-bounds. The ? on getcwd propagates identically to before.

Other factors

I left two nits on earlier revisions (Android PATH_MAX value, undrained stdout pipe); both are fixed in commits 35b9e40 and eb150a6. The comment-cop bot's paragraph-comment complaint was addressed in 8c2fecb (doc comment is now one line). The bug-hunting system found nothing on this revision. The one CI failure (test/bake/deinitialization.test.ts segfault on Windows x64) is in an unrelated dev-server test and predates the follow-up commits. No CODEOWNERS on the touched paths.