Skip to content

install: fail instead of panicking when a local tarball or workspace: path does not fit the path buffer - #37462

Open
robobun wants to merge 4 commits into
mainfrom
farm/02b98cc5/install-tarball-path-too-long
Open

install: fail instead of panicking when a local tarball or workspace: path does not fit the path buffer#37462
robobun wants to merge 4 commits into
mainfrom
farm/02b98cc5/install-tarball-path-too-long

Conversation

@robobun

@robobun robobun commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Repro

mkdir probe && cd probe
bun -e 'require("fs").writeFileSync("package.json", JSON.stringify({ name: "probe", dependencies: { dep: "file:./" + "a".repeat(5000) + ".tgz" } }))'
bun install
panic: range end index 5022 out of range for slice of length 4095

bun install aborts (SIGABRT, exit 134). The same happens when the tarball dependency is declared by a workspace package instead of the root package, and for "dep": "workspace:./" + "a".repeat(5000) declared by either. Reproduced with bun 1.4.0-canary.1 on Linux; the buffer is 4096 bytes on Linux, 1024 on the other POSIX platforms and 32767 * 3 + 1 on Windows.

Cause

These specs are stored verbatim and nothing bounds their length. Three sites then join them into a stack PathBuffer with the unchecked join_abs_string_buf, whose normalizer indexes past the buffer when the result does not fit (resolve_path.rs:1067; the "length 4095" is the buffer minus the leading separator):

  • File::read_from_user_input (src/sys/file.rs), used by the local tarball task for dependencies of the root package.
  • enqueue_local_tarball (PackageManagerEnqueue.rs), which joins the workspace directory and the spec on the main thread for tarball dependencies of a workspace package.
  • The workspace: arm of Package::parse_dependency (lockfile/Package.rs), for workspace:<path> specs that do not name a workspace. The file: folder arm next to it already uses the checked join.

Fix

All three use join_abs_string_buf_checked, which still succeeds when an over-long spec normalizes down (x/../x/../...) and returns None only when the normalized path does not fit.

  • read_from_user_input returns ENAMETOOLONG, the same error openat_a already returns for a &[u8] path that does not fit a PathBuffer.
  • The workspace join happens before the task exists, so LocalTarballRequest's (tarball_path, normalize) pair becomes a TarballPath enum: Url (resolve tarball.url against the project root on the worker; this no longer interns a second copy of the same path), Absolute (pre-joined workspace path) and TooLong, which the worker reports as Error::Sys(ENAMETOOLONG).
  • The workspace: arm reports Dependency "x" has an unsafe workspace path and fails the install, mirroring the folder arm's unsafe folder path.
  • openat_a (which both tarball sites end up in for a path of exactly MAX_PATH_BYTES, where the join fits but there is no room for the NUL) built its error from libc::ENAMETOOLONG. On Windows that is the MSVC value 38, and bun_sys::Error decodes the stored value against bun's own errno table, where 38 is ENOSYS; the boundary tests below reported ENOSYS extracting tarball on both Windows lanes. It and the identical guard in write_file_with_path_buffer now use the typed E::ENAMETOOLONG, like the rest of bun_sys. Linux and macOS are unaffected (libc's numbers and bun's table agree there).

Tarballs fail through the task's existing failure path, so both tarball cases print exactly what a too-long path rejected by the OS already prints today:

error: ENAMETOOLONG extracting tarball from dep
error: dep@file:./aaaa... failed to resolve

and bun install exits 1. A path that does not fit PATH_MAX can never be opened, so ENAMETOOLONG is the accurate error for the buffer case too; the kernel's answer is given one layer earlier instead of indexing out of bounds. Using the checked join rather than a length check on the spec keeps ..-heavy specs that normalize to a valid path working, which the positive tests pin down.

Not in this PR

  • bun test: stop panicking on a path argument or tree entry longer than the path buffer #35863 (open) changes the shared primitive to return an empty path on overflow. That would turn these panics into a misleading ENOENT; this PR fixes the call sites so they report the real error either way. The two are independent.
  • folder_resolver::normalize_package_json_path has overflows of its own, reached by file: folder specs whose joined path is within 13 bytes of the buffer size and by any link: spec over 1024 bytes (it normalizes through a 1024 byte thread-local buffer). Different mechanism and function; reported separately, still panics after this PR. The workspace: change here gives those specs the behaviour folder specs have today, no more.
  • The same from_code_int(libc::...) mistake exists in a few Windows-compiled stubs in bun_sys (writev/readv/preadv, get_fcntl_flags, clonefile), where libc::ENOSYS / ENOTSUP decode as ELOOP / EKEYREJECTED. Reported separately; only the two ENAMETOOLONG sites the tests here exercise are changed.
  • Linking a package whose resolution string is over 512 bytes panics in PackageInstaller::install_package_with_name_and_resolution (reproducible with a valid tarball at a 618 byte relative path, no overflow involved). Also reported separately. The positive tests below use --lockfile-only, which stops after the tarball has been read and extracted, and check the lockfile instead of node_modules.

Tests

test/cli/install/bun-workspaces.test.ts, each case for a root dependency and for a workspace dependency:

  • a 100 kB tarball path (longer than the buffer on every platform) fails with ENAMETOOLONG extracting tarball from too-long, exit 1
  • a tarball path whose joined length is one byte below, exactly at and one byte above the buffer size (computed from the temp dir, which is the install's cwd): ENOENT for the one that fits (ENAMETOOLONG on Windows, which rejects anything over 32767 UTF-16 units before looking it up), ENAMETOOLONG for the other two. The workspace row above the limit is what pins TarballPath::TooLong: with the worker handling it like Url, that path fits when joined with the root and comes back ENOENT (verified by making exactly that change locally; the 100 kB case cannot tell the two apart because it overflows both joins)
  • a 100 kB x/../x/../... path that normalizes to an existing tarball resolves (local_tarball entry in the lockfile)
  • a 100 kB workspace: path fails with unsafe workspace path

On the unfixed build (USE_SYSTEM_BUN=1) 7 of the 12 crash with the panics above (both 100 kB tarball cases, root at and above the limit, workspace above the limit, both workspace: cases); the other 5 behave the same before and after. All 75 tests in the file pass with bun bd test on Linux, as do the workspace and tarball path tests in bun-install.test.ts and the local tarball tests in bun-add.test.ts. The 12 new tests also pass with a debug build on Windows x64 (before the openat_a change the two "exactly" rows failed there with ENOSYS, see the Windows lanes of the previous CI run). cargo check -p bun_install passes for x86_64-pc-windows-msvc and x86_64-apple-darwin; clippy is clean for bun_sys and bun_install.

…t the path buffer

Both joins of a `file:` tarball dependency's path used the unchecked
join_abs_string_buf into a stack PathBuffer, so a spec longer than
MAX_PATH_BYTES aborted `bun install` with
"panic: range end index N out of range for slice of length 4095":
File::read_from_user_input for dependencies of the root package, and
enqueue_local_tarball for dependencies of a workspace package.

Use join_abs_string_buf_checked at both sites. read_from_user_input
returns ENAMETOOLONG, which the task already reports as
"ENAMETOOLONG extracting tarball from <name>" (the same error the OS
returns for a path that fits the buffer but is still too long). The
workspace join runs on the main thread before the task exists, so the
request's (tarball_path, normalize) pair becomes a TarballPath enum
with a TooLong state that the worker turns into the same error. The
Url state reads tarball.url directly instead of interning a second
copy of the same path.
@robobun

robobun commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced on bun 1.4.0-canary.1 (Linux x64): a file: tarball dependency with a 5000 byte path, declared by the root package or by a workspace package, and a workspace:./<5000 bytes> dependency all abort bun install with panic: range end index N out of range for slice of length 4095.

Fix: the three joins use join_abs_string_buf_checked. Tarballs fail with ENAMETOOLONG extracting tarball from <name> (exit 1, the output a too-long path rejected by the OS already produces); workspace: paths fail with unsafe workspace path. Tests in test/cli/install/bun-workspaces.test.ts cover root and workspace dependencies: the far case, the three lengths around the buffer size, an over-long spec that normalizes to a valid tarball, and the workspace: case.

The boundary tests also showed that a path of exactly the buffer size was reported as ENOSYS on Windows (openat_a built its error from the MSVC libc::ENAMETOOLONG value); that is fixed in this PR as well and the new tests pass on a Windows x64 debug build.

Two neighbouring pre-existing panics found while testing (the folder_resolver overflows for folder and link: specs, and the 512 byte resolution buffer in the linker) are reported separately and listed under "Not in this PR" in the description.

CI is green on the current head (ba4f41a, Buildkite build 92317).

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Changes

Local tarball installation

Layer / File(s) Summary
Typed tarball path classification
src/install/PackageManager/PackageManagerEnqueue.rs, src/install/PackageManagerTask.rs
Local tarball requests now use TarballPath values for URLs, absolute paths, and overlong paths.
Extraction and checked path resolution
src/install/PackageManagerTask.rs, src/sys/file.rs, src/install/lockfile/Package.rs
Extraction resolves each path variant. Checked joins return ENAMETOOLONG when paths exceed PathBuffer capacity, and unsafe workspace paths return InstallFailed.
Workspace path-length coverage
test/cli/install/bun-workspaces.test.ts
Tests cover overlong paths, boundary lengths, normalization, and oversized workspace: paths on supported platforms.

Possibly related PRs

  • oven-sh/bun#36324: Both PRs add Rust path-length handling that returns ENAMETOOLONG in different filesystem and install paths.
  • oven-sh/bun#36881: Both PRs add checked path-length handling in different subsystems.
  • oven-sh/bun#36977: Both PRs modify local tarball installation code, but this PR handles path representation and length errors while that PR handles offline tarball sourcing.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly and concisely describes the main change: returning an error instead of panicking for overlong local tarball or workspace paths.
Description check ✅ Passed The description thoroughly explains the cause, fix, scope, and verification results, although it does not use the template headings exactly.

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

@robobun

robobun commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — targeted crash fix that turns an out-of-bounds panic on over-long file: tarball paths into a proper ENAMETOOLONG failure, matching what the OS would return anyway.

What was reviewed:

  • Both join sites (read_from_user_input for root deps, enqueue_local_tarball for workspace deps) now use join_abs_string_buf_checked; folder deps already used it, so this closes the gap.
  • TarballPath::Url reading req.tarball.url.slice() on the worker is the same filename-store-backed pattern already used for tarball.name and the git-clone/checkout url fields — not a new cross-thread invariant.
  • read_from_user_input has only one caller (this task), and read_fromopenat_a NUL-terminates internally, so dropping the _z variant is fine.
  • Tests cover root × workspace and too-long × normalizes-down; the --lockfile-only workaround for the pre-existing 512-byte linker panic is documented and scoped.
Extended reasoning...

Overview

Fixes a panic (range end index N out of range for slice of length 4095) when bun install processes a file: tarball dependency whose path does not fit a PathBuffer. Two call sites joined the user-provided spec into a stack buffer with the unchecked join_abs_string_buf / _z; both now use join_abs_string_buf_checked and return ENAMETOOLONG on overflow. LocalTarballRequest's (StringOrTinyString, bool) pair is replaced by a three-variant TarballPath enum (Url / Absolute / TooLong), which also drops a redundant second intern of the same path bytes for root dependencies. Four new tests in bun-workspaces.test.ts.

Security risks

None introduced. This tightens handling of untrusted input (a package.json string) that previously indexed past a stack buffer; the checked join is strictly safer. No new syscalls, no new external inputs.

Level of scrutiny

Medium. bun install is production-critical, and the change touches a struct that crosses the main-thread → ThreadPool-worker boundary. I checked the two things that could go wrong there: (1) TarballPath::Url now reads req.tarball.url on the worker instead of a separate tarball_path copy — but tarball.url is already a StringOrTinyString interned via filename_store_appender(), the exact same storage the old tarball_path used and the same pattern the git-clone/checkout requests already rely on for cross-thread reads; (2) read_from_user_input now returns a non-NUL-terminated slice — but its only consumer, File::read_fromopenat_a, copies into its own buffer and NUL-terminates. read_from_user_input has exactly one caller in the tree.

Other factors

  • Folder dependencies already use join_abs_string_buf_checked (per the PR description, in lockfile/Package.rs), so this brings tarballs to parity rather than introducing a new pattern.
  • The enum's Absolute(StringOrTinyString) variant drops correctly through deinit_payloadManuallyDrop::drop(&mut self.request.local_tarball), same as the old flat field.
  • Tests use test.concurrent.each over root/workspace, assert the exact error string and exit code, and pin the positive normalizes-down case with a lockfile check. The --lockfile-only shortcut is explained inline (separate pre-existing 512-byte linker panic, out of scope) so it won't read as a mystery to the next person.
  • abs_buf was moved inside the labeled block; init_append_if_needed(joined, ...) now runs inside that scope, so the borrow of the stack buffer is still live when copied — the borrow checker enforces this anyway.
  • Cross-platform: PR author reports cargo check passes for Windows and macOS targets; the 100 kB test spec exceeds the buffer on all three platforms.

… the tarball boundary

The workspace: arm of Package::parse_dependency joined the user-written
path with the unchecked join_abs_string_buf, the same way the tarball
sites did, so `workspace:./<5000 bytes>` aborted bun install with the
same panic. Use the checked join and report the dependency like the
Folder arm next to it does.

Tests: both owners for the workspace: case, and a sweep of joined
tarball path lengths one byte below, exactly at and one byte above
MAX_PATH_BYTES for both owners. The workspace row above the limit is
what distinguishes TarballPath::TooLong from resolving the path against
the root, which fits the buffer there and reports ENOENT; the far
100 kB case cannot tell those apart.
Comment thread src/install/PackageManagerTask.rs Outdated
Comment thread src/install/PackageManagerTask.rs Outdated
Comment thread src/install/PackageManagerTask.rs Outdated
@robobun robobun changed the title install: fail with ENAMETOOLONG when a local tarball path does not fit the path buffer install: fail instead of panicking when a local tarball or workspace: path does not fit the path buffer Aug 11, 2026
@robobun

robobun commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 8:27 AM PT - Aug 11th, 2026

@robobun, your commit ba4f41aa2f077a4c54dc5fa029690c97f33fc4ac passed in Build #92317! 🎉


🧪   To try this PR locally:

bunx bun-pr 37462

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

bun-37462 --bun

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs. Because it refactors the cross-thread LocalTarballRequest shape and touches three call sites in the install path, a human look would still be worthwhile.

What was reviewed:

  • TarballPath::Url reads req.tarball.url, which is interned from the same path argument the old normalize=true branch stored — no behaviour change for root tarballs.
  • read_from_user_input switches from _z to _checked; read_fromopenat_a accepts non-sentinel &[u8], so dropping the NUL terminator is fine, and it is the only caller.
  • The workspace: arm mirrors the existing Folder arm's checked-join + InstallFailed pattern verbatim.
  • Boundary tests compute the exact joined length from the temp dir and pin ENOENT vs ENAMETOOLONG at ±1; the positive x/../ test proves the checked join still normalizes.
Extended reasoning...

Overview

This PR converts three out-of-bounds panics in bun install into recoverable errors when a file: tarball or workspace: path exceeds PathBuffer capacity. It swaps unchecked join_abs_string_buf for join_abs_string_buf_checked in File::read_from_user_input (src/sys/file.rs), enqueue_local_tarball (PackageManagerEnqueue.rs) and the workspace: arm of Package::parse_dependency (lockfile/Package.rs). To carry the overflow signal from the main-thread enqueue site to the worker-thread extract, LocalTarballRequest's (tarball_path: StringOrTinyString, normalize: bool) pair is replaced with a TarballPath enum (Url / Absolute / TooLong), and read_and_extract now takes &LocalTarballRequest and matches on it. Twelve new tests in bun-workspaces.test.ts cover root/workspace × far-over / boundary ±1 / normalizing-down / workspace: cases.

Security risks

None. The change tightens validation of user-controlled dependency specs (adversarial input from package.json) so they fail closed with a diagnosable error instead of a slice-index panic. No new surface is added.

Level of scrutiny

Medium-high. The install task path is production-critical and threading-sensitive: enqueue_local_tarball runs on the main thread and the LocalTarball callback runs on a ThreadPool worker that must not touch lockfile.buffers. The refactor changes what the worker reads for the non-workspace case — it now dereferences req.tarball.url (an owned StringOrTinyString interned into filename_store_appender) instead of a separately-interned tarball_path. I traced this and confirmed url is set from the same path argument (line 1895-1899), so the worker still reads only owned data. The Absolute case interns the joined result exactly as the old code did. The _z_checked change in read_from_user_input drops NUL termination, but read_from routes through openat_a which accepts non-sentinel &[u8].

Other factors

The PR description is unusually thorough: it names the exact overflow site (resolve_path.rs:1067), explains why TooLong must be a distinct variant (the workspace-above-limit test row would otherwise pass with ENOENT), lists two adjacent panics deliberately left out of scope, and reports USE_SYSTEM_BUN=1 results (7/12 crash before, 0 after). The workspace: error path copies the existing Folder arm's pattern exactly. The comment-cop bot flagged long comments on TarballPath which were trimmed to one line each in a follow-up commit.

I'm deferring rather than approving because the LocalTarballRequest shape change is a small design decision in a concurrency-sensitive struct — a maintainer glance at whether TarballPath is the shape they want (vs. e.g. threading the error through immediately at enqueue time) seems worthwhile, even though the implementation is sound.

openat_a and write_file_with_path_buffer built the error from
libc::ENAMETOOLONG, which on Windows is the MSVC value 38; Error decodes
the stored value against bun's own errno table, where 38 is ENOSYS, so a
path of exactly MAX_PATH_BYTES was reported as ENOSYS on Windows (the
boundary tests showed this on both Windows lanes). Use the typed
constant like the rest of bun_sys.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and the automated bug hunt found no issues. The three checked-join swaps look correct, tarball.url is owned by the request so reading it on the worker is safe, and the workspace: arm mirrors the existing Folder arm. That said, this refactors the cross-thread LocalTarballRequest shape (introducing the TarballPath enum and deferring the overflow error to the worker) and tweaks two shared bun_sys error sites for Windows, so a human look at the package-manager side would still be worthwhile.

What was reviewed:

  • tarball.url is a StringOrTinyString interned from path on the main thread, not a lockfile-buffer slice — safe for the worker to read in the TarballPath::Url arm.
  • read_from_user_input has exactly one caller (this code path); switching join_abs_string_buf_z_checked and passing &[u8] to read_from is fine because openat_a NUL-terminates internally.
  • deinit_payload for LocalTarball already Rust-drops the whole request, so the new Absolute(StringOrTinyString) variant is released the same way the old tarball_path field was.
Extended reasoning...

Overview

Converts three bun install panics on over-long dependency paths into recoverable errors. Touches: src/sys/file.rs (read_from_user_input uses the checked join and returns ENAMETOOLONG), src/install/PackageManager/PackageManagerEnqueue.rs + src/install/PackageManagerTask.rs (replace the (tarball_path: StringOrTinyString, normalize: bool) pair on LocalTarballRequest with a TarballPath enum { Url, Absolute, TooLong }), src/install/lockfile/Package.rs (workspace: arm uses the checked join and reports "unsafe workspace path"), and src/sys/lib.rs (two from_code_int(libc::ENAMETOOLONG, ..)from_code(E::ENAMETOOLONG, ..) so the Windows path reports the right errno name). ~135 lines of new tests in bun-workspaces.test.ts covering root/workspace × far-overflow/boundary±1/normalizes-down/workspace:.

Security risks

None. This tightens input validation on user-controlled dependency specs — an over-long path that previously panicked (release-build abort) now fails with a proper error and non-zero exit. No new privilege, no new I/O surface; the checked join is already used by the sibling Folder arm.

Level of scrutiny

Medium-high. bun install is production-critical and this reshapes data that crosses the main-thread → ThreadPool-worker boundary. I traced the key invariant: in the new TarballPath::Url case the worker reads req.tarball.url.slice(), which is a StringOrTinyString interned from path via filename_store_appender() at enqueue time (line 1895 of PackageManagerEnqueue.rs) — owned by the request, not a slice into lockfile.buffers.string_bytes, so the existing race-avoidance contract holds. read_from_user_input is single-caller and its return-type contract is unchanged. Drop of the new Absolute(StringOrTinyString) variant goes through the existing ManuallyDrop::drop(&mut self.request.local_tarball) in deinit_payload, same as the old field.

Other factors

Tests are thorough (boundary at ±1 byte, positive normalization case with a real tarball, both root and workspace owners) and the description confirms 7/12 crash on the unfixed build. The comment-cop feedback was addressed. What tips this to defer rather than approve: the TarballPath enum is a small design decision (vs. failing inline on the main thread, or vs. keeping the pair and adding a length check), and the src/sys/lib.rs change alters two cross-platform error sites in a widely-used syscall wrapper — both are reasonable but worth a maintainer's eye on the install/sys ownership boundary.

Jarred-Sumner pushed a commit that referenced this pull request Aug 13, 2026
…rs (#37469)

## Repro

```sh
mkdir probe && cd probe
cp <bun repo>/test/cli/install/bar-0.0.2.tgz .
bun -e 'require("fs").writeFileSync("package.json", JSON.stringify({ name: "foo", dependencies: { bar: "./" + "x/../".repeat(130) + "bar-0.0.2.tgz" } }))'
bun install
```

```
Resolved, downloaded and extracted [1]
panic: unreachable: Error
```

`bun install` aborts (SIGABRT, exit 134) after the tarball has been
resolved and extracted. The spec normalizes to `./bar-0.0.2.tgz`, which
exists; a real 618 byte relative path behaves the same (that is how this
was found, see #37462). The same abort happens for a remote tarball
whose URL is longer than 512 bytes, a `file:` folder at a long path, and
a workspace package whose version has a ~500 byte prerelease tag.
`--lockfile-only` succeeds, so it is the link step. Reproduced with `bun
1.4.0-canary.1` and main.

Top frame: `Result::expect` in
`PackageInstaller::install_package_with_name_and_resolution`
(`src/install/PackageInstaller.rs:1340`), called from `install_package`
/ `hoisted_install::install_hoisted_packages`.

## Cause

While linking each package, the hoisted installer formats the package's
version label into `let mut resolution_buf = [0u8; 512]` with
`buf_print(..).expect("unreachable")`. For npm packages the label is the
version; for tarball, folder and git packages it is the spec they were
resolved from (stored verbatim), and for workspace packages it is the
workspace's own version. Those are user supplied and have no length
bound, so the overflow `buf_print` reports is reachable and the `expect`
turns it into the panic. Both branches (workspace version and
resolution) have it.

The isolated linker already builds this label in a `Vec`
(`Installer::package_patch_info`) and is not affected by this panic. It
fails such installs with `ENAMETOOLONG` instead, because the store
directory name embeds the spec; that is a separate problem and not
touched here.

## Fix

`print_package_version` keeps the 512 byte stack buffer as the
allocation-free path and, only when the label does not fit, formats it
into a `Vec` that lives next to the buffer (the same shape as
`resolve_path::join_z_buf_spill`). The linking loop runs once per
installed package, so the common case still does not allocate.

Spilling is the only correct behavior for this label: it is compared
against the installed `package.json` version and hashed as the version
half of the `name@version` `patchedDependencies` key, so a truncated
label would silently fail verification or miss a patch, and rejecting
the package would refuse a valid install (nothing on disk is named after
the label; the tarball cache folder is a hash of it). The new
patchedDependencies test below checks the spilled label byte for byte by
keying a patch with it.

`bun patch` formats the same labels into 1024 byte buffers with the same
`expect` at four sites in `src/install/PackageManager/patchPackage.rs`,
all reproducible the same way once the install succeeds (verified with a
build that only had the installer change):

* `pkg_info_for_name_and_version` (`bun patch <name>@<version>` compares
the label of every package with that name; panicked with `Resolution
name too long`),
* the multiple-packages-with-this-name loops in `prepare_patch` and
`do_patch_commit` (`bun patch <path>` when another package with the same
name has a long label),
* the `name@label` key in `do_patch_commit`, which is also the patch
file name.

These format into a `Vec` (`print_resolution_label`, reused across a
candidate loop). In `do_patch_commit` the key `Vec` is returned as the
`patch_key` directly and the file name is built from it, replacing the
re-slicing of the shared buffer; the bytes are unchanged (the key was
already valid UTF-8, so the former `BStr` round trip was a copy).
Committing a package whose label is this long still cannot succeed,
since the patch file would be named after the label, but it now exits 1
with an error instead of aborting; today it fails before that at the
diff step for tarball packages (#37124), which this does not change
either way.

## Tests

`test/cli/install/bun-workspaces.test.ts`, "packages whose version label
is longer than 512 bytes" (hoisted linker): local tarball, remote
tarball served from a local `Bun.serve`, workspace package with a long
prerelease version (the other branch), and a `patchedDependencies` entry
keyed by the long spec that must actually be applied.
`test/cli/install/bun-patch.test.ts`, "packages whose label is longer
than 1024 bytes": `bun patch <name>@<label>`, `bun patch <path>` plus
`--commit` when a same-named package has a long label (the long one is
listed first, so the loops format it), and `--commit` of a long-labeled
package exiting 1 without touching `package.json`. The long specs use
`x/../` repeated so nothing long is ever created on disk.

All seven fail on the unfixed build (`panic: unreachable: Error` during
the install) and pass with the fix; with only the installer change
applied, the three `bun patch` tests fail at the `bun patch` step
instead, so each patch site is covered on its own. Both files pass in
full, as does `bun-install-patch.test.ts`; `cargo clippy -p bun_install`
is clean and `cargo check -p bun_install` passes for
`x86_64-pc-windows-msvc` and `x86_64-apple-darwin`.
Jarred-Sumner pushed a commit that referenced this pull request Aug 13, 2026
…ffer (#37531)

## Repro

```sh
mkdir probe && cd probe
bun -e 'require("fs").writeFileSync("package.json", JSON.stringify({ name: "p", workspaces: ["a".repeat(5000)] }))'
bun install
```

```
panic: range end index 5010 out of range for slice of length 4095
```

`bun install` aborts (SIGABRT, exit 134). A glob entry
(`"a".repeat(5000) + "/*"`) aborts the same way with `panic: range end
index 5000 out of range for slice of length 4095`. Reproduced with `bun
1.4.0-canary.1` on Linux, where the buffer is 4096 bytes; it is 1024 on
macOS and 32767 * 3 + 1 on Windows (the glob case is 4096 on every
platform). A 2000 byte entry, which fits the buffer, already fails the
way one would expect:

```
error: ENAMETOOLONG reading package.json for workspace package "aaa..." from "/tmp/probe"
```

## Cause

`WorkspaceMap::process_names_array` joins every entry of the
`workspaces` array with the unchecked join helpers, whose normalizer
indexes past the output buffer when the result does not fit
(`resolve_path.rs`, `buf[buf_i..buf_i + count]`; the "length 4095" is
the buffer minus the leading separator):

* path entries: `join_abs_string_buf_z(project dir, PathBuffer, [entry,
"package.json"])`
* glob entries: `join([pattern, "package.json"])` into the 4096 byte
thread local buffer, before the pattern ever reaches the glob walker
* glob matches: `join_abs_string_buf_z(project dir, PathBuffer, [matched
dir, "package.json"])`. The walker works relative to the project dir, so
it can match a `package.json` whose path relative to the project fits
while the absolute path does not. A member directory whose absolute path
is 4090 bytes long (creatable, it fits PATH_MAX) with `workspaces:
["**"]` panics with `range end index 4102 out of range for slice of
length 4095`.

The folder dependency arm in `lockfile/Package.rs` already uses
`join_abs_string_buf_checked` for the same situation.

## Fix

* The two absolute joins use `join_abs_string_buf_checked`. When it
returns `None` the entry takes the existing error path with
`Error::Sys(ENAMETOOLONG)`, so it prints exactly what an entry the OS
rejects prints today (`ENAMETOOLONG reading package.json for workspace
package "..."`) and `bun install` exits 1. A path that does not fit the
buffer does not fit `PATH_MAX` either, so this is the same answer one
layer earlier. `process_workspace_name` takes `&[u8]` now;
`WorkspacePackageJSONCache::get_with_path` never needed the NUL
terminator.
* The glob pattern join uses `join_spill`, the variant the glob walker
itself uses for long paths, and the pattern is handed to the walker like
any other. The walker already copes with long patterns: a pattern that
matches nothing is skipped, the same as a 2000 byte one is today, and
one that does match (a brace group padded past the buffer size) still
finds its workspace. Rejecting long patterns outright would turn that
second case into an error.

Both joins are checked on the normalized result, so an entry that is
long as written but normalizes to a short path
(`x/../x/../.../pkgs/pkg1`) keeps resolving, same as before.

Related: #37462 fixes the same kind of overflow for local tarball and
`workspace:` dependency specs in other files; #35863 changes the shared
join primitive to return an empty path on overflow, which would turn
these panics into a misleading `Workspace not found`. This PR is
independent of both: the call sites report `ENAMETOOLONG` either way.

## Tests

`test/cli/install/bad-workspace.test.ts` (the existing file for bad
`workspaces` entries), all spawning `bun install` without a registry:

* 100 kB path entry: `ENAMETOOLONG`, exit 1 (every platform)
* path entry whose joined `package.json` path is one byte below /
exactly / one byte above the buffer size: `Workspace not found` (the
path still reaches the OS) / `ENAMETOOLONG` / `ENAMETOOLONG` (POSIX; on
Windows the OS rejects paths long before the buffer size)
* 100 kB brace glob still matches `pkgs/pkg1`; 100 kB glob matching
nothing leaves the other entries alone (every platform)
* 100 kB `x/../` path and glob entries that normalize to `pkgs/pkg1`
resolve (every platform, passes before and after: guards the normalized
length semantics)
* glob match whose absolute `package.json` path does not fit:
`ENAMETOOLONG`, exit 1 (POSIX; the member directory is created at buffer
size - 6 bytes and its `package.json` written relative to the parent
directory)

On the unfixed build (`USE_SYSTEM_BUN=1`) six of these crash with the
panics above (`100035`, `100008`, `100000`, `index 4096` for the NUL
write of the "exactly" row, `4096`, `4102`); the "one byte below" row
and the two normalizing entries pass both ways. With `bun bd test` the
file passes, as do `bun-workspaces.test.ts` (63 tests) and
`test/cli/run/workspaces.test.ts`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant