install: make bins of symlink-installed packages executable

[robobun]

Problem

• Hoisted linker: the bins of a file: folder dependency that lives outside the project (file:../dep, which is also what an absolute file: path is stored as) get a node_modules/.bin/<name> link, but the file it points at stays 0644. bun run <name> prints error: Script not found "<name>" and running the link directly fails with Permission denied. The same happens to every package installed with --backend symlink. npm makes the file executable (bin-links fix-bin); bun did too before the chmod was hardened.
• Cause: both cases are installed by the symlink backend (PackageInstall::install_with_symlink), which fills node_modules/<pkg> with one symlink per file pointing into the folder or the cache. Since Hardening: input validation and bounds tightening across 36 subsystems (round 4) #31339 the bin linker sets the mode with lchmod (src/install/bin.rs, chmod_on_ok), so a package cannot redirect the chmod through a symlink it ships. On these installs the bin target is the installer's own symlink: the lchmod is refused (Linux, EOPNOTSUPP) or lands on the link itself (macOS), and the real file keeps its mode.
• Not affected: file:./vendor/dep (hardlinked on Linux, cloned on macOS, so the chmod reaches a real file) and the isolated linker (copies into the store).

Fix

• PackageInstaller::link_tree_bins gives the linker installed_from, the real path of the directory the linked package's files were installed from: realpath(<root>/<folder>) for Folder resolutions (the only folder packages that carry a bin are the ones the root or a workspace declares, whose resolution Package::parse stores root-relative; transitive folder packages get a name-only lockfile entry and never reach the bin linker), the cache directory (via its fd, the same canonical form install_with_symlink writes into the links) for registry/git/tarball resolutions, and None for workspaces, link: and the root, which are installed as a single directory symlink. It follows the native binlink redirect (target_package_id), so it describes the package whose directory the bin is actually linked from.
• Linker::make_executable still lchmods the bin target. If the target is a symlink and installed_from is set, it reads the link, requires an absolute target whose realpath is strictly inside installed_from, and lchmods that path as well.
• Why this is correct: a link meeting those conditions is what the symlink backend writes (absolute, into the directory it was installing from, to a regular file it enumerated), so the file behind it is the package's own content, the same file the copy and hardlink backends chmod today (with the default hardlink backend the cache file's mode already changes, it is the same inode). The property from Hardening: input validation and bounds tightening across 36 subsystems (round 4) #31339 is kept: a symlink shipped by a package either fails the containment check or points at the package's own files; realpath resolves directory components, so a symlinked intermediate directory cannot escape; and the second chmod is again an lchmod, so nothing beyond the installer's one link is ever followed. Workspaces and link: packages keep the current behaviour (existing test does not change permissions of a file reached through a symlinked bin target).
• installed_from is derived from the lockfile at link time rather than recorded while installing because bins are also relinked for packages that were already installed and are skipped.
• The six chmod_on_ok calls spread over create_symlink's return paths become one call after the link is created; the condition (no error) is unchanged.
• Verified with the new tests (test/cli/install/symlink-path-traversal.test.ts and test/cli/install/bun-install-native-binlink.test.ts):
  • file:../dep with a top-level and a nested bin target: both files in the folder become executable, package.json/lib.js do not, and both bins run through bun run
  • --backend symlink with a local tarball whose bin is 0644: the file in the cache becomes executable and the bin runs
  • guard: a registry package installed with --backend symlink, its two bin links replaced by an absolute and a relative symlink to 0600 files outside the cache, reinstalled (package is kept, bins are relinked): both files stay 0600
  • native binlink: a file:../<pkg> folder listed in nativeDependencies, against the existing fixtures. With --backend symlink the bin is redirected into the platform package and the file behind it in the cache (0644 in the fixture tarball) becomes executable; with the fallback fixture, whose platform package has no bin file, the folder's own bin becomes executable after the retry. These pin the two target_package_id updates: removing the redirect assignment fails only the first, removing the retry reset fails only the second
  • the first two and both native binlink tests fail on the released bun (1.4.0), all pass with the debug build; the rest of the file, shebang-normalize, bun-install-native-binlink, the binaries block of bun-install-registry and bun-link pass as well (the last test in bun-link times out under the debug build on main too: the failing install's debug stack dump alone takes about 4s)
  • cargo check -p bun_install for x86_64-pc-windows-msvc and x86_64-apple-darwin, clippy clean

Background

• Install backends: a package's files get into node_modules/<pkg> by clonefile (macOS), hardlink (Linux), copyfile, or, for file: folders whose path starts with .. (and all transitive folder deps) and for --backend symlink, by creating one symlink per file that points at the absolute path of the source file in the folder or in the cache. Workspaces and link: deps are different again: node_modules/<pkg> itself is one symlink to the package directory.
• Bin linking: after a tree is installed, link_tree_bins creates node_modules/.bin/<name> pointing at ../<pkg>/<target> and makes the target executable (0777 & ~umask), because tarballs and checkouts do not reliably carry the executable bit. lchmod changes the mode of the path itself and never follows a symlink in the final component (fchmodat2(AT_SYMLINK_NOFOLLOW) on Linux, lchmod on macOS), which is what Hardening: input validation and bounds tightening across 36 subsystems (round 4) #31339 relies on to stop a package from chmodding an arbitrary file through a symlink declared as its bin.
• realpath resolves every symlink in a path, so comparing its result against a directory answers "is the file really inside this directory" regardless of symlinked directories along the way; bin.rs already uses the same technique to check multi-component bin targets (resolved_target_parent_escapes_package_dir).

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 19 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: 3b54a920-00a8-4e13-a9f9-8b853abdb44a

📥 Commits

Reviewing files that changed from the base of the PR and between c418051 and 902a2a7.

📒 Files selected for processing (7)

• src/install/PackageInstaller.rs
• src/install/bin.rs
• src/install/isolated_install/Installer.rs
• src/runtime/cli/link_command.rs
• src/runtime/cli/unlink_command.rs
• test/cli/install/bun-install-native-binlink.test.ts
• test/cli/install/symlink-path-traversal.test.ts

---

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

[robobun]

Reproduced on the released bun 1.4.0 with a file:../dep dependency whose bin is 0644: node_modules/.bin/depcli is created but the file behind it stays 0644, so bun run depcli reports Script not found (same with bun install --backend symlink and a fresh cache). Covered by the first two tests added to test/cli/install/symlink-path-traversal.test.ts and the two added to test/cli/install/bun-install-native-binlink.test.ts; all four fail on the released binary and pass with this branch.

State: the fix is unchanged since the rebase onto main; the later pushes shortened comments, added the native-binlink tests and corrected one comment after review (902a2a7). CI for 902a2a7: every Linux and Windows job passed with no failing tests (the entries the build lists all passed on retry). The two macOS test jobs have been waiting for an agent since the build started, so the build is not green yet; they are the only lanes that have not run on this branch, and I am leaving the build queued rather than pushing again, which would send them to the back of the queue. Ready for review.

[robobun]

^{Updated 7:05 AM PT - Aug 15th, 2026}

❌ @robobun, your commit 902a2a7 has some failures in Build #97766 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 38777

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

bun-38777 --bun

[claude]

I reviewed this PR and didn't find any bugs. Because it deliberately relaxes the #31339 lchmod hardening (following one installer-created symlink to chmod the file behind it), a human look at the containment reasoning in make_executable would still be worthwhile.

What was reviewed

• make_executable's containment check: absolute-only readlink result, realpath before is_parent_or_equal, and the second chmod is again lchmod so a package-shipped symlink at the resolved path is not followed.
• installed_from derivation in link_tree_bins: Folder → realpath(top_level/<folder>), cache-backed tags → whole cache dir via fd path, workspace/link/root → None; tracks target_package_id across the native-binlink redirect and its retry.
• The chmod_on_ok hoist out of create_symlink: every previous call site was gated on self.err.is_none(), and the new call in link_bin_or_create_shim sits after the same if self.err.is_some() { return } guard.
• Ruled out: a cached package whose bin file is itself a symlink escaping the cache — realpath(link_target) resolves it and fails the ParentEqual::Parent check; a relative foreign link — rejected by the is_absolute gate.

Extended reasoning...

Overview

Fixes bins of symlink-installed packages (file:../dep, --backend symlink) staying non-executable after #31339 switched the bin-target chmod to lchmod. Adds an installed_from: Option<&[u8]> field to bin::Linker, threads it from PackageInstaller::link_tree_bins (derived per-package from the resolution tag), and uses it in a new make_executable to follow exactly one symlink hop when — and only when — the file behind it realpaths to inside the directory the installer copied from. The six chmod_on_ok calls in create_symlink collapse to one call in link_bin_or_create_shim, gated identically (after the self.err.is_some() early return). Isolated installer, bun link, bun unlink pass None and keep the old behaviour. Three new tests: two positive (file:../dep, --backend symlink tarball) and one guard (swapped-in absolute and relative foreign links stay 0600).

Security risks

This is the security-relevant part of the change: it re-introduces a symlink-follow into a path that #31339 hardened to never follow. The invariant being relied on is that the installer's own per-file links are the only absolute symlinks at a bin target whose realpath lands strictly inside installed_from. I traced the adversarial cases: (a) a package that ships its bin as a symlink to an outside file — for cache-backed installs the installer link points at <cache>/<pkg>/<bin>, whose realpath follows the shipped link out of the cache and fails containment; for Folder installs the folder is user-controlled, not package-controlled; (b) a foreign relative link swapped in — rejected by is_absolute; (c) a foreign absolute link into the cache — passes containment, but the second lchmod refuses to follow it, so at worst a file already inside the cache/folder gets +x, which the copy/hardlink backends already do to the same inode. is_parent_or_equal requires Parent (not Equal), and installed_from is a realpath (Folder) or an fd-derived canonical path (cache), so prefix-matching against another realpath output is a real containment test. I did not find a bypass, but the argument is subtle enough that a maintainer who owns #31339 should confirm it.

Level of scrutiny

High. This is bun install chmod logic on a path that was previously the subject of a security fix, and the change's correctness rests on a filesystem-containment argument rather than a simple type or flag. It is not a mechanical change.

Other factors

The PR description is unusually thorough and directly states the security reasoning; the guard test exercises the exact scenario the containment check exists for (absolute and relative foreign links after a re-install that skips the package but relinks bins). The chmod_on_ok refactor is behaviour-preserving on inspection. Buffer lifetimes for installed_from (real_folder_buf from the pool, real_cache_dir: Option<AbsPath>) live outside the per-dependency loop and are re-derived each iteration, so the retry path does not read stale slices. can_enqueue_install_task covers exactly the resolution tags that go through the cache (Npm/LocalTarball/RemoteTarball/Git/Github).

[robobun]

Agreed that the containment step in make_executable is the part worth a maintainer's eyes; the argument for it is spelled out in the Fix section of the description. In short: the second chmod only happens for an absolute link whose realpath is strictly inside the directory the package was installed from, and it is an lchmod again, so the installer's own per-file link is the only symlink ever followed. The last new test in symlink-path-traversal.test.ts pins the rejecting side (absolute and relative foreign links at a bin target of a package that gets relinked without being reinstalled), and the pre-existing workspace test pins that packages installed as a directory symlink are still never followed.

[robobun]

Two follow-up pushes, neither changes behaviour:

• Rebased onto main. The only red test in the previous run, test/js/node/test/parallel/test-http-chunk-problem.js, was failing on main at this branch's old base and passes on current main (and locally on this branch), so the rebase is there to get a clean run rather than a retrigger. Main's trailing-slash bin fix that landed in between composes with this change: abs_target no longer ends in a separator, and the containment check here goes through realpath, which resolves the same components the kernel resolves for the final lchmod, so what gets chmodded is always either the link itself or the vetted file.
• The comments flagged by the comment lint are now one line each; the reasoning they carried stays in the PR description. The flagged blocks no longer exist in the diff.

[claude]

I reviewed this PR and the bug hunting system found no issues. The fix and its containment argument look correct, but since it deliberately relaxes the #31339 lchmod hardening (following one installer-written symlink to chmod the file behind it), a maintainer should sign off on the security reasoning in make_executable.

What was reviewed:

• make_executable: readlink → absolute-only → realpath containment against installed_from → second lchmod on the raw link target (never follows further); the create_symlink refactor preserves chmod-only-on-success.
• installed_from derivation in link_tree_bins: Folder → realpath of the folder, cache-backed tags (can_enqueue_install_task) → cache dir via fd, workspace/link:/root → None; tracks target_package_id across the native-binlink redirect and its retry reset.
• New tests pin both directions (folder + --backend symlink bins become executable; foreign absolute/relative links at a relinked bin target stay 0600) and the pre-existing workspace/symlinked-bin-target tests still hold.

Extended reasoning...

Overview

This PR restores the executable bit on bin targets of packages installed via the symlink backend (file:../dep folders and --backend symlink), which #31339 broke by switching the bin-target chmod to lchmod. It threads a new installed_from: Option<&[u8]> through bin::Linker, computed per-dependency in PackageInstaller::link_tree_bins, and adds Linker::make_executable which — after the unchanged lchmod(abs_target) — reads the installer's own per-file symlink, requires it to be absolute and to realpath strictly inside installed_from, then lchmods that path. The six chmod_on_ok calls scattered across create_symlink's return paths are folded into one call in link_bin_or_create_shim after the self.err check, preserving the on-success-only condition. All other Linker construction sites (isolated installer, bun link/unlink) pass installed_from: None, so their behaviour is unchanged.

Security risks

This is squarely security-sensitive: it re-introduces a symlink-following chmod that #31339 removed to close a path-traversal chmod. The gate is: (1) installed_from is only set for resolutions the installer itself materialized as per-file symlinks (folder or cache-backed), never for workspace/link: directory symlinks; (2) the followed link must be absolute (what install_with_symlink writes); (3) its realpath must be a strict child of installed_from; (4) the second chmod is again lchmod, so nothing beyond the one installer-written hop is followed. I couldn't construct a bypass — a package-shipped symlink at the bin target either fails the absolute check, fails realpath containment, or is the final component of the second lchmod and therefore not followed. The cache-dir granularity (whole cache rather than per-package) is coarse, but the first hop is written by the installer, not the package, so it can only point at that package's own cache entry. TOCTOU between realpath and lchmod isn't a privilege boundary here (same user owns both sides). Still, this is exactly the class of change the review guidelines say a maintainer must sign off on.

Level of scrutiny

High. src/install/bin.rs is the layer that enforces the CVE-2019-16775-class bin-link protections, and this PR intentionally softens one of them. The description's argument is thorough and the tests cover both the enabling and refusing sides, but the design decision (follow one hop, gated by realpath containment against a lockfile-derived source directory) deserves human confirmation.

Other factors

The comment-cop lint flags were addressed in a follow-up push (comments now one line each). The one CI failure on the earlier build (test-http-chunk-problem.js) was a main break at the old base, since rebased. Three new tests are added alongside the existing traversal tests; the guard test reproduces the relink-without-reinstall case where a foreign symlink sits at a bin target of a package whose other files are legitimate installer links. Cross-target cargo check (Windows/macOS) reported clean.

[robobun]

One note on the cache granularity mentioned above, since it is the one deliberate coarsening: bins are also relinked for packages that were installed on an earlier run and are skipped this time, and the package's cache subpath is not available at that point, so the prefix is the cache directory rather than the package's own entry. What that admits is a file bun itself extracted or checked out into the cache, which the hardlink backend already chmods on every install (same inode). The first hop is the installer's link, so in practice it only ever points at the package's own entry anyway.

[robobun]

Self-review of the diff turned up one gap, now closed in e3802d1: the two target_package_id updates in link_tree_bins (follow the native binlink redirect, undo it on retry) were not pinned by any test. bun-install-native-binlink.test.ts now installs a file:../<pkg> folder listed in nativeDependencies against the existing fixtures. With --backend symlink the bin is redirected into the platform package and the file behind it in the cache has to be made executable (the fixture tarball ships it as 0644); with the fallback fixture, whose platform package has no bin file, the folder's own bin has to be made executable after the retry. Checked by mutation: dropping the redirect assignment fails only the first test, dropping the retry reset fails only the second, and both fail on the released bun. Nothing about the fix itself came out of the review.

[claude]

Beyond the inline nit, I also traced the new make_executable containment step for the #31339 property: abs_target under the symlink backend is always the installer's own per-file link, realpath(link_target) resolves any further hop before the is_parent_or_equal check, and the second chmod is lchmod so a swapped final component is not followed. I also compared the create_symlink refactor's return paths against the removed chmod_on_ok calls — the self.err.is_some() early return preserves the same gating. Since this deliberately relaxes a security hardening (even with a well-argued containment check), a maintainer sign-off on the containment argument is still worthwhile.

Extended reasoning...

This PR touches the bin-linker chmod path that #31339 hardened from chmod to lchmod specifically to stop packages from redirecting the executable-bit write through a shipped symlink. The change re-introduces a follow-one-symlink step, gated on a realpath containment check against the directory the package was installed from. The argument is sound as far as I traced it (installer's own link is the only first hop, realpath resolves intermediate directories, second write is lchmod, workspaces/link: get None so the pre-existing test still holds), and the negative test pins the rejecting side. But it is a deliberate, reasoned relaxation of a security check in bun install, which per the review guidelines is exactly the category that should get human eyes rather than bot approval.

[robobun]

Heads-up from #38814: once it lands, folder packages declared by other local file: packages also carry a bin and reach the bin linker (today they are name-only rows). Their resolutions are root-relative as well (Package::parse normalizes them the same way as root-declared ones), so the installed_from = realpath(<root>/<folder>) computation here covers them unchanged; only the prose claiming transitive folder packages never reach the bin linker would need dropping. Under the hoisted linker they are exactly the per-file-symlink case this PR fixes, so the two changes compose: #38814 creates the link, this makes its target executable.