From d7bba6672f9aea90a5ac445d5c7902d0ce8cd923 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 00:51:34 +0000 Subject: [PATCH 1/4] install: make bins of symlink-installed packages executable The symlink install backend (file: folders outside the project, --backend symlink) fills node_modules/ with one symlink per file. Since the bin chmod became lchmod, it landed on that symlink and the file behind it stayed non-executable, so node_modules/.bin/ could not be run. The hoisted installer now tells the bin linker which directory the package was installed from (the file: folder, or the cache). When the bin target is a symlink whose real path lies inside that directory, it is one the installer wrote, and the file behind it is lchmod'ed too. Symlinks pointing anywhere else, relative symlinks, and packages installed as a single directory symlink (workspaces, link:, the root) are left alone as before. --- src/install/PackageInstaller.rs | 40 +++- src/install/bin.rs | 84 +++++--- src/install/isolated_install/Installer.rs | 2 + src/runtime/cli/link_command.rs | 1 + src/runtime/cli/unlink_command.rs | 1 + .../install/symlink-path-traversal.test.ts | 195 ++++++++++++++++++ 6 files changed, 289 insertions(+), 34 deletions(-) diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index 6c96867f593b..54bc6573e6fe 100644 --- a/src/install/PackageInstaller.rs +++ b/src/install/PackageInstaller.rs @@ -5,7 +5,9 @@ use bun_core::fmt::PathSep; use bun_core::{Global, Output}; use bun_core::{ZStr, strings}; use bun_paths::resolve_path::{dirname, join_abs_string_z, join_z_buf}; -use bun_paths::{AbsPath, AutoAbsPath, MAX_PATH_BYTES, PathBuffer, SEP, platform}; +use bun_paths::{ + AbsPath, AutoAbsPath, AutoAbsPathChecked, MAX_PATH_BYTES, PathBuffer, SEP, platform, +}; use bun_semver::String; use bun_sys::{self as Syscall, Dir, Fd}; @@ -587,11 +589,17 @@ impl<'a> PackageInstaller<'a> { let pkg_resolutions_lists = pkgs.items_resolutions(); let pkg_resolutions_buffer = lockfile.buffers.resolutions.as_slice(); let pkg_names = pkgs.items_name(); + let pkg_resolutions = pkgs.items_resolution(); let completed_trees = &self.completed_trees; let tree = &mut self.trees[tree_id as usize]; let mut deferred: Vec = Vec::new(); + // Backing storage for `bin::Linker::installed_from`; the cache path is + // resolved at most once per tree. + let mut real_folder_buf = bun_paths::path_buffer_pool::get(); + let mut real_cache_dir: Option = None; + while let Some(dep_id) = tree.binaries.remove_or_null() { debug_assert!((dep_id as usize) < lockfile.buffers.dependencies.as_slice().len()); let package_id = lockfile.buffers.resolutions.as_slice()[dep_id as usize]; @@ -604,6 +612,7 @@ impl<'a> PackageInstaller<'a> { .slice(string_buf); let package_name_ = strings::StringOrTinyString::init(alias); let mut target_package_name = package_name_; + let mut target_package_id = package_id; let mut can_retry_without_native_binlink_optimization = false; let mut target_node_modules_path_opt: Option = None; let mut defer_this_bin = false; @@ -669,6 +678,7 @@ impl<'a> PackageInstaller<'a> { pkg_names[replacement_pkg_id as usize].slice(string_buf); target_package_name = strings::StringOrTinyString::init(replacement_name); + target_package_id = replacement_pkg_id; can_retry_without_native_binlink_optimization = true; } } @@ -696,6 +706,32 @@ impl<'a> PackageInstaller<'a> { }; loop { + let installed_from: Option<&[u8]> = { + let target_resolution = &pkg_resolutions[target_package_id as usize]; + if target_resolution.tag == resolution::Tag::Folder { + // The folder resolution is relative to the root package, + // which is also how `PackageInstall` opens it. + let mut folder = AutoAbsPathChecked::init_top_level_dir(); + match folder.join(&[target_resolution.folder().slice(string_buf)]) { + Ok(()) => { + Syscall::realpath(folder.slice_z(), &mut real_folder_buf).ok() + } + Err(_) => None, + } + } else if target_resolution.tag.can_enqueue_install_task() { + if real_cache_dir.is_none() { + real_cache_dir = + AbsPath::init_fd_path(manager.get_cache_directory()).ok(); + } + real_cache_dir.as_ref().map(AbsPath::slice) + } else { + // Workspaces, `link:` and the root itself are installed as + // one symlink to their own directory; a symlink found at a + // bin target there is the package's own. + None + } + }; + // `node_modules_path` (mut) and `target_node_modules_path` // (read-only) refer to the same buffer when no replacement is // set. Derive both from a single `*mut` so the read pointer @@ -724,6 +760,7 @@ impl<'a> PackageInstaller<'a> { abs_target_buf: link_target_buf, abs_dest_buf: link_dest_buf, rel_buf: link_rel_buf, + installed_from, err: None, skipped_due_to_missing_bin: false, }; @@ -742,6 +779,7 @@ impl<'a> PackageInstaller<'a> { ); } target_package_name = package_name_; + target_package_id = package_id; target_node_modules_path_opt = None; continue; } diff --git a/src/install/bin.rs b/src/install/bin.rs index 7bb19bf33bbf..9267612c6353 100644 --- a/src/install/bin.rs +++ b/src/install/bin.rs @@ -831,6 +831,14 @@ pub struct Linker<'a> { pub abs_dest_buf: &'a mut [u8], pub rel_buf: &'a mut [u8], + /// Real path of the directory the target package's files were installed + /// from: its `file:` folder, or the cache. The symlink install backend + /// (`file:` folders, `--backend symlink`) fills the package directory with + /// one symlink per file pointing in here, and the executable bit has to + /// land on the file behind such a link (see `make_executable`). `None` + /// leaves a symlink at a bin target alone. + pub installed_from: Option<&'a [u8]>, + pub err: Option, pub skipped_due_to_missing_bin: bool, } @@ -953,10 +961,45 @@ impl<'a> Linker<'a> { #[cfg(not(windows))] { + Self::make_executable(self.installed_from, abs_target); Self::try_normalize_shebang(abs_target); } } + /// `lchmod` so that a symlink shipped by the package as its bin target + /// cannot redirect the chmod to an arbitrary file. The one symlink that is + /// followed is the installer's own per-file link (see `installed_from`), + /// recognized by the file behind it lying inside that directory; the file + /// itself is again chmodded without following anything. + #[cfg(not(windows))] + fn make_executable(installed_from: Option<&[u8]>, abs_target: &ZStr) { + let mode = 0o777 & !(UMASK.load(Ordering::Acquire) as Mode); + let _ = sys::lchmod(abs_target, mode); + + let Some(installed_from) = installed_from else { + return; + }; + let mut link_buf = path::path_buffer_pool::get(); + let Ok(link_len) = sys::readlink(abs_target, link_buf.as_mut_slice()) else { + return; + }; + // `sys::readlink` writes the NUL at `link_len`. + let link_target = ZStr::from_buf(&link_buf[..], link_len); + if !path::is_absolute(link_target.as_bytes()) { + return; + } + let mut real_buf = path::path_buffer_pool::get(); + let Ok(real_target) = sys::realpath(link_target, &mut *real_buf) else { + return; + }; + if resolve_path::is_parent_or_equal(installed_from, real_target) + != resolve_path::ParentEqual::Parent + { + return; + } + let _ = sys::lchmod(link_target, mode); + } + #[cfg(not(windows))] fn try_normalize_shebang(abs_target: &ZStr) { let mut shebang_buf = [0u8; 2048]; @@ -1258,10 +1301,6 @@ impl<'a> Linker<'a> { #[cfg(not(windows))] fn create_symlink(&mut self, abs_target: &ZStr, abs_dest: &ZStr, global: bool) { - // hoisted from `defer { if (this.err == null) chmod }` — scopeguard - // cannot capture `&mut self.err` without conflicting with the body's writes, - // so each return path calls `Self::chmod_on_ok` explicitly instead. - let abs_dest_dir = resolve_path::dirname::(abs_dest.as_bytes()); let rel_target = resolve_path::relative_buf_z(self.rel_buf, abs_dest_dir, abs_target.as_bytes()); @@ -1272,7 +1311,6 @@ impl<'a> Linker<'a> { sys::Result::Err(err) => { if err.get_errno() != sys::Errno::EEXIST && err.get_errno() != sys::Errno::ENOENT { self.err = Some(err.into()); - Self::chmod_on_ok(self.err, abs_target); return; } @@ -1280,7 +1318,6 @@ impl<'a> Linker<'a> { if err.get_errno() == sys::Errno::ENOENT { if global { self.err = Some(err.into()); - Self::chmod_on_ok(self.err, abs_target); return; } @@ -1291,27 +1328,18 @@ impl<'a> Linker<'a> { let _ = sys::Dir::cwd().make_path(self.node_modules_path.slice()); self.node_modules_path.set_length(node_modules_path_save); - match sys::symlink_running_executable(rel_target, abs_dest) { - sys::Result::Err(real_error) => { - // It was just created, no need to delete destination and symlink again - self.err = Some(real_error.into()); - Self::chmod_on_ok(self.err, abs_target); - return; - } - sys::Result::Ok(()) => { - Self::chmod_on_ok(self.err, abs_target); - return; - } + // `.bin` was just created, so the destination cannot exist yet: + // no need to delete it and symlink again on failure. + if let Err(real_error) = sys::symlink_running_executable(rel_target, abs_dest) { + self.err = Some(real_error.into()); } + return; } // beyond this error can only be `.EXIST` debug_assert!(err.get_errno() == sys::Errno::EEXIST); } - sys::Result::Ok(()) => { - Self::chmod_on_ok(self.err, abs_target); - return; - } + sys::Result::Ok(()) => return, } // delete and try again @@ -1319,16 +1347,6 @@ impl<'a> Linker<'a> { if let Err(err) = sys::symlink_running_executable(rel_target, abs_dest) { self.err = Some(err.into()); } - Self::chmod_on_ok(self.err, abs_target); - } - - #[cfg(not(windows))] - fn chmod_on_ok(err: Option, abs_target: &ZStr) { - // hoisted from `defer` block in create_symlink - if err.is_none() { - let mode = 0o777 & !(UMASK.load(Ordering::Acquire) as Mode); - let _ = sys::lchmod(abs_target, mode); - } } #[cfg(not(windows))] @@ -1592,7 +1610,7 @@ impl<'a> Linker<'a> { // is called while `abs_target` / `abs_dest` borrow `self.abs_target_buf` // / `self.abs_dest_buf`. `link_bin_or_create_shim` never reads or writes // those two buffers (it only touches `rel_buf`, `node_modules_path`, `seen`, `err`, - // `skipped_due_to_missing_bin`). Detach the `abs_dest` borrow via a raw + // `skipped_due_to_missing_bin`, `installed_from`). Detach the `abs_dest` borrow via a raw // pointer so borrowck allows the disjoint access; the SAFETY invariant // is that `abs_dest_buf` is not aliased mutably for the lifetime of the // detached slice. `package_dir` (`abs_target_buf[0..package_dir_len]`) @@ -1810,7 +1828,7 @@ impl<'a> Linker<'a> { // SAFETY: result lives in `self.abs_target_buf`, which // `link_bin_or_create_shim` does not write to (only // `rel_buf`/`node_modules_path`/`seen`/`err`/ - // `skipped_due_to_missing_bin` are touched). + // `skipped_due_to_missing_bin`/`installed_from` are touched). ZStr::from_raw(r.as_bytes().as_ptr(), r.len()) }; diff --git a/src/install/isolated_install/Installer.rs b/src/install/isolated_install/Installer.rs index ce346a116b37..bbbe7c3721f4 100644 --- a/src/install/isolated_install/Installer.rs +++ b/src/install/isolated_install/Installer.rs @@ -1841,6 +1841,7 @@ impl Task { abs_target_buf: &mut *abs_target_buf, abs_dest_buf: &mut *abs_dest_buf, rel_buf: &mut *rel_buf, + installed_from: None, err: None, skipped_due_to_missing_bin: false, }; @@ -2403,6 +2404,7 @@ impl<'a> Installer<'a> { abs_target_buf: &mut *link_target_buf, abs_dest_buf: &mut *link_dest_buf, rel_buf: &mut *link_rel_buf, + installed_from: None, err: None, skipped_due_to_missing_bin: false, }; diff --git a/src/runtime/cli/link_command.rs b/src/runtime/cli/link_command.rs index a52f8c3a3429..27847445da18 100644 --- a/src/runtime/cli/link_command.rs +++ b/src/runtime/cli/link_command.rs @@ -261,6 +261,7 @@ fn link(ctx: command::Context) -> crate::Result<()> { abs_target_buf: link_target_buf.as_mut_slice(), abs_dest_buf: link_dest_buf.as_mut_slice(), rel_buf: link_rel_buf.as_mut_slice(), + installed_from: None, err: None, skipped_due_to_missing_bin: false, }; diff --git a/src/runtime/cli/unlink_command.rs b/src/runtime/cli/unlink_command.rs index cfababcd13c5..c7d843f82ae8 100644 --- a/src/runtime/cli/unlink_command.rs +++ b/src/runtime/cli/unlink_command.rs @@ -210,6 +210,7 @@ fn unlink(ctx: &mut ContextData) -> crate::Result<()> { abs_target_buf: link_target_buf.as_mut_slice(), abs_dest_buf: link_dest_buf.as_mut_slice(), rel_buf: link_rel_buf.as_mut_slice(), + installed_from: None, err: None, skipped_due_to_missing_bin: false, }; diff --git a/test/cli/install/symlink-path-traversal.test.ts b/test/cli/install/symlink-path-traversal.test.ts index 9d2400d10d28..e7a1f8525513 100644 --- a/test/cli/install/symlink-path-traversal.test.ts +++ b/test/cli/install/symlink-path-traversal.test.ts @@ -868,6 +868,201 @@ it.skipIf(isWindows)( 60000, ); +// The symlink install backend (used for `file:` folders outside the project and +// for `--backend symlink`) fills node_modules/ with one symlink per file. +// A symlink cannot carry the executable bit, so the bin linker has to set it on +// the file behind the installer's own link (npm likewise chmods the file in a +// `file:` folder), while still refusing to follow a symlink that the installer +// did not create (the two tests above). +async function runBun(cwd: string, args: string[], extraEnv: Record = {}) { + await using proc = spawn({ + cmd: [bunExe(), ...args], + cwd, + stdout: "pipe", + stderr: "pipe", + env: { ...env, ...extraEnv }, + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; +} + +const isOwnerExecutable = async (path: string) => ((await stat(path)).mode & 0o100) !== 0; + +it.skipIf(isWindows)("makes the files behind a symlink-installed file: dependency's bins executable", async () => { + using dir = tempDir("file-dep-bin-executable", { + "app/bunfig.toml": `[install]\nlinker = "hoisted"\n`, + "app/package.json": JSON.stringify({ + name: "file-dep-bin-app", + version: "1.0.0", + dependencies: { "file-dep-with-bins": "file:../dep" }, + }), + "dep/package.json": JSON.stringify({ + name: "file-dep-with-bins", + version: "1.0.0", + bin: { "file-dep-cli": "cli.js", "file-dep-nested-cli": "bin/nested.js" }, + }), + "dep/cli.js": `#!/usr/bin/env node\nconsole.log("cli");\n`, + "dep/bin/nested.js": `#!/usr/bin/env node\nconsole.log("nested");\n`, + "dep/lib.js": `module.exports = 1;\n`, + }); + const appDir = join(String(dir), "app"); + const depDir = join(String(dir), "dep"); + const depFiles = ["cli.js", "bin/nested.js", "lib.js", "package.json"]; + const executableDepFiles = async () => + Object.fromEntries(await Promise.all(depFiles.map(async f => [f, await isOwnerExecutable(join(depDir, f))]))); + + expect(await executableDepFiles()).toEqual({ + "cli.js": false, + "bin/nested.js": false, + "lib.js": false, + "package.json": false, + }); + + expect(await runBun(appDir, ["install"])).toMatchObject({ exitCode: 0 }); + + // The layout this is about: the package directory holds the installer's + // per-file symlinks, and node_modules/.bin points at them. + expect((await lstat(join(appDir, "node_modules", "file-dep-with-bins", "cli.js"))).isSymbolicLink()).toBe(true); + expect((await lstat(join(appDir, "node_modules", "file-dep-with-bins", "bin", "nested.js"))).isSymbolicLink()).toBe( + true, + ); + + // Only the bin targets become executable, in the folder itself. + expect(await executableDepFiles()).toEqual({ + "cli.js": true, + "bin/nested.js": true, + "lib.js": false, + "package.json": false, + }); + + expect(await runBun(appDir, ["run", "file-dep-cli"])).toMatchObject({ stdout: "cli\n", exitCode: 0 }); + expect(await runBun(appDir, ["run", "file-dep-nested-cli"])).toMatchObject({ stdout: "nested\n", exitCode: 0 }); +}); + +it.skipIf(isWindows)("makes the cached file behind a --backend symlink bin executable", async () => { + // `createTarball` writes every file with mode 0644, so the extracted cache + // entry is not executable and the bin linker is what has to make it so. + const tarball = createTarball([ + { + name: "package/package.json", + type: "file", + content: JSON.stringify({ + name: "symlink-backend-pkg", + version: "1.0.0", + bin: { "symlink-backend-cli": "cli.js" }, + }), + }, + { name: "package/cli.js", type: "file", content: `#!/usr/bin/env node\nconsole.log("from the cache");\n` }, + ]); + using dir = tempDir("symlink-backend-bin-executable", { + "bunfig.toml": `[install]\nlinker = "hoisted"\n`, + "package.json": JSON.stringify({ + name: "symlink-backend-app", + version: "1.0.0", + dependencies: { "symlink-backend-pkg": "file:./symlink-backend-pkg-1.0.0.tgz" }, + }), + "symlink-backend-pkg-1.0.0.tgz": Buffer.from(tarball), + }); + const installDir = String(dir); + const cacheDir = join(installDir, ".bun-cache"); + + expect( + await runBun(installDir, ["install", "--backend", "symlink"], { BUN_INSTALL_CACHE_DIR: cacheDir }), + ).toMatchObject({ exitCode: 0 }); + + const installedCli = join(installDir, "node_modules", "symlink-backend-pkg", "cli.js"); + const cachedCli = await readlink(installedCli); + expect(cachedCli.startsWith(`${await realpath(cacheDir)}/`)).toBe(true); + expect(await isOwnerExecutable(cachedCli)).toBe(true); + + expect(await runBun(installDir, ["run", "symlink-backend-cli"])).toMatchObject({ + stdout: "from the cache\n", + exitCode: 0, + }); +}); + +it.skipIf(isWindows)( + "does not change permissions of files reached through bin target symlinks the installer did not create", + async () => { + // An npm package installed with `--backend symlink` is left alone by later + // installs as long as its package.json still reads back, while its bins are + // linked again every time. Swapping the installer's links for foreign ones + // in between therefore puts a symlink the installer did not write at a bin + // target of a package whose other files legitimately are installer links. + // Both links lead outside the cache the package was installed from, so + // neither destination may be chmodded. + // For registry packages the bin field is read from the registry manifest. + const manifest = { + name: "relinked-bin-pkg", + version: "1.0.0", + bin: { "relinked-abs": "abs.js", "relinked-rel": "rel.js" }, + }; + const tarball = createTarball([ + { name: "package/package.json", type: "file", content: JSON.stringify(manifest) }, + { name: "package/abs.js", type: "file", content: `#!/usr/bin/env node\n` }, + { name: "package/rel.js", type: "file", content: `#!/usr/bin/env node\n` }, + ]); + using registry = Bun.serve({ + port: 0, + fetch(req) { + const { pathname } = new URL(req.url); + if (pathname === "/relinked-bin-pkg") { + return Response.json({ + name: manifest.name, + "dist-tags": { latest: manifest.version }, + versions: { + [manifest.version]: { + ...manifest, + dist: { tarball: `http://localhost:${registry.port}/relinked-bin-pkg-1.0.0.tgz` }, + }, + }, + }); + } + if (pathname === "/relinked-bin-pkg-1.0.0.tgz") { + return new Response(tarball, { headers: { "Content-Type": "application/octet-stream" } }); + } + return new Response("Not Found", { status: 404 }); + }, + }); + using dir = tempDir("relinked-bin-target-test", { + "bunfig.toml": `[install]\nlinker = "hoisted"\nregistry = "http://localhost:${registry.port}/"\n`, + "package.json": JSON.stringify({ + name: "relinked-bin-app", + version: "1.0.0", + dependencies: { "relinked-bin-pkg": "1.0.0" }, + }), + "victim-abs.txt": "reached through an absolute symlink", + "victim-rel.txt": "reached through a relative symlink", + }); + const installDir = String(dir); + const installEnv = { BUN_INSTALL_CACHE_DIR: join(installDir, ".bun-cache") }; + const victimAbs = join(installDir, "victim-abs.txt"); + const victimRel = join(installDir, "victim-rel.txt"); + await chmod(victimAbs, 0o600); + await chmod(victimRel, 0o600); + + expect(await runBun(installDir, ["install", "--backend", "symlink"], installEnv)).toMatchObject({ exitCode: 0 }); + + const pkgDir = join(installDir, "node_modules", "relinked-bin-pkg"); + expect((await lstat(join(pkgDir, "package.json"))).isSymbolicLink()).toBe(true); + await rm(join(pkgDir, "abs.js")); + await rm(join(pkgDir, "rel.js")); + await symlink(victimAbs, join(pkgDir, "abs.js")); + await symlink(join("..", "..", "victim-rel.txt"), join(pkgDir, "rel.js")); + await rm(join(installDir, "node_modules", ".bin"), { recursive: true }); + + expect(await runBun(installDir, ["install", "--backend", "symlink"], installEnv)).toMatchObject({ exitCode: 0 }); + + // The bins were linked again, against the swapped-in symlinks. + expect((await readdir(join(installDir, "node_modules", ".bin"))).sort()).toEqual(["relinked-abs", "relinked-rel"]); + expect(await readlink(join(pkgDir, "abs.js"))).toBe(victimAbs); + expect(await readlink(join(pkgDir, "rel.js"))).toBe(join("..", "..", "victim-rel.txt")); + + expect((await stat(victimAbs)).mode & 0o777).toBe(0o600); + expect((await stat(victimRel)).mode & 0o777).toBe(0o600); + }, +); + it.skipIf(isWindows)( "skips a package bin entry whose name contains a NUL byte and links the remaining entries", async () => { From e77cf7336d3b59db40650f2643bdc476dcfe9ca7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 05:20:43 +0000 Subject: [PATCH 2/4] install: keep the bin linker comments to one line each --- src/install/PackageInstaller.rs | 9 ++------- src/install/bin.rs | 16 +++------------- 2 files changed, 5 insertions(+), 20 deletions(-) diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index 54bc6573e6fe..7b037cbaced4 100644 --- a/src/install/PackageInstaller.rs +++ b/src/install/PackageInstaller.rs @@ -595,8 +595,6 @@ impl<'a> PackageInstaller<'a> { let tree = &mut self.trees[tree_id as usize]; let mut deferred: Vec = Vec::new(); - // Backing storage for `bin::Linker::installed_from`; the cache path is - // resolved at most once per tree. let mut real_folder_buf = bun_paths::path_buffer_pool::get(); let mut real_cache_dir: Option = None; @@ -709,8 +707,7 @@ impl<'a> PackageInstaller<'a> { let installed_from: Option<&[u8]> = { let target_resolution = &pkg_resolutions[target_package_id as usize]; if target_resolution.tag == resolution::Tag::Folder { - // The folder resolution is relative to the root package, - // which is also how `PackageInstall` opens it. + // Folder resolutions are relative to the root package. let mut folder = AutoAbsPathChecked::init_top_level_dir(); match folder.join(&[target_resolution.folder().slice(string_buf)]) { Ok(()) => { @@ -725,9 +722,7 @@ impl<'a> PackageInstaller<'a> { } real_cache_dir.as_ref().map(AbsPath::slice) } else { - // Workspaces, `link:` and the root itself are installed as - // one symlink to their own directory; a symlink found at a - // bin target there is the package's own. + // Directory-symlinked packages: never chmod through their own links. None } }; diff --git a/src/install/bin.rs b/src/install/bin.rs index 9267612c6353..22a35cbede46 100644 --- a/src/install/bin.rs +++ b/src/install/bin.rs @@ -831,12 +831,7 @@ pub struct Linker<'a> { pub abs_dest_buf: &'a mut [u8], pub rel_buf: &'a mut [u8], - /// Real path of the directory the target package's files were installed - /// from: its `file:` folder, or the cache. The symlink install backend - /// (`file:` folders, `--backend symlink`) fills the package directory with - /// one symlink per file pointing in here, and the executable bit has to - /// land on the file behind such a link (see `make_executable`). `None` - /// leaves a symlink at a bin target alone. + /// Real path the target package was installed from; see `make_executable`. pub installed_from: Option<&'a [u8]>, pub err: Option, @@ -966,11 +961,7 @@ impl<'a> Linker<'a> { } } - /// `lchmod` so that a symlink shipped by the package as its bin target - /// cannot redirect the chmod to an arbitrary file. The one symlink that is - /// followed is the installer's own per-file link (see `installed_from`), - /// recognized by the file behind it lying inside that directory; the file - /// itself is again chmodded without following anything. + /// Follows only the symlink backend's own link into `installed_from`, never a package's. #[cfg(not(windows))] fn make_executable(installed_from: Option<&[u8]>, abs_target: &ZStr) { let mode = 0o777 & !(UMASK.load(Ordering::Acquire) as Mode); @@ -1328,8 +1319,7 @@ impl<'a> Linker<'a> { let _ = sys::Dir::cwd().make_path(self.node_modules_path.slice()); self.node_modules_path.set_length(node_modules_path_save); - // `.bin` was just created, so the destination cannot exist yet: - // no need to delete it and symlink again on failure. + // It was just created, no need to delete destination and symlink again if let Err(real_error) = sys::symlink_running_executable(rel_target, abs_dest) { self.err = Some(real_error.into()); } From e3802d1b6188d4582fa9778ebeff7c3e5a30a719 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 08:02:46 +0000 Subject: [PATCH 3/4] test: cover the native binlink redirect and retry for symlink-installed packages A file: folder listed in nativeDependencies links its bin either from the platform package (installed from the cache) or, when that package has no bin file, from the folder itself after the retry. The bin linker has to be told which of the two directories the linked file lives in; each test fails if the corresponding target_package_id update is dropped, and both fail without the fix. --- .../bun-install-native-binlink.test.ts | 119 ++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/test/cli/install/bun-install-native-binlink.test.ts b/test/cli/install/bun-install-native-binlink.test.ts index 71df3ee19c83..24eca327d927 100644 --- a/test/cli/install/bun-install-native-binlink.test.ts +++ b/test/cli/install/bun-install-native-binlink.test.ts @@ -569,6 +569,125 @@ describe.concurrent("native binlink altpath", () => { } }); +// A `file:` folder outside the project is installed as one symlink per file, so the +// executable bit has to be set on the file behind the bin target. With a +// nativeDependencies package that file belongs to whichever package the bin ends up +// linked from: the platform package (installed from the cache) when the redirect +// succeeds, the folder itself when the platform package has no bin file and the linker +// retries without the redirect. +// +// POSIX-only: the Windows bin linker writes shims and never chmods anything. +describe.concurrent.skipIf(isWindows)("symlink-installed nativeDependencies package", () => { + async function setup(local: { + name: string; + bin: Record; + target: string; + files: Record; + }) { + const { packageDir } = await verdaccio.createTestDir({ + files: { + "app/package.json": JSON.stringify({ + name: "test-app", + version: "1.0.0", + dependencies: { [local.name]: `file:../${local.name}` }, + nativeDependencies: [local.name], + }), + [`${local.name}/package.json`]: JSON.stringify({ + name: local.name, + version: "1.0.0", + bin: local.bin, + optionalDependencies: { [local.target]: "1.0.0" }, + }), + ...Object.fromEntries( + Object.entries(local.files).map(([file, contents]) => [`${local.name}/${file}`, contents]), + ), + }, + }); + const appDir = join(packageDir, "app"); + await verdaccio.writeBunfig(appDir, { linker: "hoisted" }); + const env = { ...bunEnv, BUN_INSTALL_CACHE_DIR: join(appDir, ".bun-cache") }; + env.BUN_TMPDIR = env.TMPDIR = env.TEMP = join(appDir, ".bun-tmp"); + + async function install(...args: string[]) { + await using proc = spawn({ + cmd: [bunExe(), "install", ...args], + cwd: appDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toMatchObject({ exitCode: 0 }); + } + + async function runBin(name: string) { + await using proc = spawn({ + cmd: [join(appDir, "node_modules", ".bin", name)], + cwd: appDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [out, err, code] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { out: out.trim(), err, code }; + } + + return { + localDir: join(packageDir, local.name), + appDir, + binDir: join(appDir, "node_modules", ".bin"), + install, + runBin, + }; + } + + test("chmods the platform package's file in the cache when it is installed with --backend symlink", async () => { + const { localDir, appDir, binDir, install, runBin } = await setup({ + name: "local-native-binlink", + bin: { "local-native-cmd": "bin/main.js" }, + target: "test-native-binlink-target", + files: { "bin/main.js": `#!/usr/bin/env node\nconsole.log("FAIL: main package bin");\n` }, + }); + + await install("--backend", "symlink"); + + // The bin is redirected into the platform package, whose files are symlinks into + // the cache; the cached `bin/main.js` is 0644 in the fixture tarball. + const cachedBin = readBinTarget(binDir, "local-native-cmd"); + expect(cachedBin.startsWith(realpathSync(join(appDir, ".bun-cache")) + sep)).toBeTrue(); + expect(cachedBin).toContain("test-native-binlink-target@1.0.0"); + expect(cachedBin).toEndWith(join("bin", "main.js")); + expect(statSync(cachedBin).mode & 0o111).not.toBe(0); + // The folder's own bin was not linked, so it is left alone. + expect(statSync(join(localDir, "bin", "main.js")).mode & 0o111).toBe(0); + + expect(await runBin("local-native-cmd")).toEqual({ + out: "SUCCESS: Using platform-specific bin (test-native-binlink-target)", + err: "", + code: 0, + }); + }); + + test("chmods the folder's own bin when the platform package has no bin file", async () => { + const { localDir, binDir, install, runBin } = await setup({ + name: "local-native-fallback", + bin: { "local-fallback-cmd": "cli.js" }, + target: "test-native-binlink-fallback-target", + files: { "cli.js": `#!/usr/bin/env node\nconsole.log("SUCCESS: Using main package bin");\n` }, + }); + const folderBin = join(localDir, "cli.js"); + expect(statSync(folderBin).mode & 0o111).toBe(0); + + await install(); + + expect(readBinTarget(binDir, "local-fallback-cmd")).toBe(realpathSync(folderBin)); + expect(statSync(folderBin).mode & 0o111).not.toBe(0); + expect(await runBin("local-fallback-cmd")).toEqual({ out: "SUCCESS: Using main package bin", err: "", code: 0 }); + }); +}); + // The bin linker must not create a `node_modules/.bin` entry (nor chmod or rewrite the // target) when a package's bin path resolves through an in-package symlink to a location // outside the package directory. Bins that resolve inside the package must still link, From 902a2a773c5867878320b1c1a3c48b58687434f2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 08:58:16 +0000 Subject: [PATCH 4/4] install: state which folder packages reach the bin linker --- src/install/PackageInstaller.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index 7b037cbaced4..4dde6809acd3 100644 --- a/src/install/PackageInstaller.rs +++ b/src/install/PackageInstaller.rs @@ -707,7 +707,7 @@ impl<'a> PackageInstaller<'a> { let installed_from: Option<&[u8]> = { let target_resolution = &pkg_resolutions[target_package_id as usize]; if target_resolution.tag == resolution::Tag::Folder { - // Folder resolutions are relative to the root package. + // Folders with bins are root- or workspace-declared, hence root-relative. let mut folder = AutoAbsPathChecked::init_top_level_dir(); match folder.join(&[target_resolution.folder().slice(string_buf)]) { Ok(()) => {