install: stop panicking on package labels longer than the stack buffers

Merged
RO
3mo ago
+301-86

Repro

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.