From 6362ebf5bcd8972a710d8b917eab485195bcd33d Mon Sep 17 00:00:00 2001 From: fordlyn Date: Wed, 6 May 2026 16:08:29 +0800 Subject: [PATCH] fix(package): repair moved global bin symlinks Repair Unix package bin symlinks after moving a global package install from the staging directory into the final package image directory. Some packages rewrite npm global bin entries during postinstall to point at absolute paths inside the staging directory. After Volta persists that directory into tools/image/packages, those symlinks become stale and the generated Volta shim fails to execute. Only symlinks in the package bin directory that point back into the staging directory are rewritten. Regular files, relative symlinks, and external symlinks are left unchanged. Also clear _VOLTA_TOOL_RECURSION in test sandboxes so smoke and acceptance tests do not accidentally inherit a parent shim-recursion state. Co-authored-by: Codex --- Cargo.lock | 16 +- crates/volta-core/src/tool/package/mod.rs | 219 +++++++++++++++++++++- tests/acceptance/support/sandbox.rs | 1 + tests/smoke/direct_install.rs | 74 ++++++++ tests/smoke/support/temp_project.rs | 1 + 5 files changed, 307 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e1402e365..3f634b69e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "adler" @@ -1657,7 +1657,7 @@ dependencies = [ "volta-core", "volta-migrate", "which", - "winreg", + "winreg 0.53.0", ] [[package]] @@ -1701,7 +1701,7 @@ dependencies = [ "volta-layout", "walkdir", "which", - "winreg", + "winreg 0.55.0", ] [[package]] @@ -2015,6 +2015,16 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "winreg" +version = "0.55.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb5a765337c50e9ec252c2069be9bf91c7df47afb103b642ba3a53bf8101be97" +dependencies = [ + "cfg-if", + "windows-sys 0.59.0", +] + [[package]] name = "winsafe" version = "0.0.19" diff --git a/crates/volta-core/src/tool/package/mod.rs b/crates/volta-core/src/tool/package/mod.rs index c8ae39fc5..181d1359e 100644 --- a/crates/volta-core/src/tool/package/mod.rs +++ b/crates/volta-core/src/tool/package/mod.rs @@ -1,5 +1,9 @@ use std::fmt::{self, Display}; +#[cfg(unix)] +use std::fs; use std::fs::create_dir_all; +#[cfg(unix)] +use std::io; use std::path::{Path, PathBuf}; use std::process::Command; @@ -299,9 +303,90 @@ where rename(staging_dir, &package_dir).with_context(|| ErrorKind::SetupToolImageError { tool: package_name.into(), version: package_version.to_string(), - dir: package_dir, + dir: package_dir.clone(), })?; + repair_moved_bin_symlinks(staging_dir, &package_dir, package_name, package_version)?; + + Ok(()) +} + +#[cfg(unix)] +fn repair_moved_bin_symlinks( + staging_dir: &Path, + package_dir: &Path, + package_name: &str, + package_version: V, +) -> Fallible<()> +where + V: Display, +{ + let bin_dir = package_dir.join("bin"); + let entries = match fs::read_dir(&bin_dir) { + Ok(entries) => entries, + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(()), + Err(error) => { + return Err(error).with_context(|| ErrorKind::ReadDirError { + dir: bin_dir.clone(), + }) + } + }; + + for entry in entries { + let entry = entry.with_context(|| ErrorKind::ReadDirError { + dir: bin_dir.clone(), + })?; + let metadata = + fs::symlink_metadata(entry.path()).with_context(|| ErrorKind::ReadDirError { + dir: bin_dir.clone(), + })?; + + if !metadata.file_type().is_symlink() { + continue; + } + + let target = + fs::read_link(entry.path()).with_context(|| ErrorKind::SetupToolImageError { + tool: package_name.into(), + version: package_version.to_string(), + dir: package_dir.to_owned(), + })?; + + if let Ok(relative_target) = target.strip_prefix(staging_dir) { + let repaired_target = package_dir.join(relative_target); + if !repaired_target.exists() { + return Err(ErrorKind::SetupToolImageError { + tool: package_name.into(), + version: package_version.to_string(), + dir: package_dir.to_owned(), + } + .into()); + } + + crate::fs::remove_file_if_exists(entry.path())?; + crate::fs::symlink_file(repaired_target, entry.path()).with_context(|| { + ErrorKind::SetupToolImageError { + tool: package_name.into(), + version: package_version.to_string(), + dir: package_dir.to_owned(), + } + })?; + } + } + + Ok(()) +} + +#[cfg(windows)] +fn repair_moved_bin_symlinks( + _staging_dir: &Path, + _package_dir: &Path, + _package_name: &str, + _package_version: V, +) -> Fallible<()> +where + V: Display, +{ Ok(()) } @@ -323,3 +408,135 @@ fn link_package_to_shared_dir(package_name: &str, manager: PackageManager) -> Fa name: package_name.into(), }) } + +#[cfg(all(test, unix))] +mod tests { + use super::repair_moved_bin_symlinks; + use crate::fs::symlink_file; + use std::fs; + use tempfile::tempdir; + + #[test] + fn repairs_bin_symlink_that_points_into_staging_dir() { + let temp = tempdir().expect("create temp dir"); + let staging_dir = temp + .path() + .join("tmp") + .join("image") + .join("packages") + .join("staging"); + let package_dir = temp + .path() + .join("tools") + .join("image") + .join("packages") + .join("volta-test"); + let staging_target = staging_dir.join("lib/node_modules/volta-test/bin/native"); + let final_target = package_dir.join("lib/node_modules/volta-test/bin/native"); + let bin_link = package_dir.join("bin").join("volta-test"); + + fs::create_dir_all(staging_target.parent().unwrap()).expect("create staging target dir"); + fs::create_dir_all(final_target.parent().unwrap()).expect("create final target dir"); + fs::create_dir_all(bin_link.parent().unwrap()).expect("create package bin dir"); + fs::write(&staging_target, "#!/usr/bin/env node\n").expect("write staging target"); + fs::write(&final_target, "#!/usr/bin/env node\n").expect("write final target"); + symlink_file(&staging_target, &bin_link).expect("create staging symlink"); + + repair_moved_bin_symlinks(&staging_dir, &package_dir, "volta-test", "1.0.0") + .expect("repair moved symlink"); + + assert_eq!( + fs::read_link(&bin_link).expect("read repaired link"), + final_target + ); + } + + #[test] + fn leaves_non_symlink_bin_entries_unchanged() { + let temp = tempdir().expect("create temp dir"); + let staging_dir = temp + .path() + .join("tmp") + .join("image") + .join("packages") + .join("staging"); + let package_dir = temp + .path() + .join("tools") + .join("image") + .join("packages") + .join("volta-test"); + let bin_file = package_dir.join("bin").join("volta-test"); + + fs::create_dir_all(bin_file.parent().unwrap()).expect("create package bin dir"); + fs::write(&bin_file, "#!/usr/bin/env node\n").expect("write bin file"); + + repair_moved_bin_symlinks(&staging_dir, &package_dir, "volta-test", "1.0.0") + .expect("repair moved symlink"); + + assert_eq!( + fs::read_to_string(&bin_file).expect("read bin file"), + "#!/usr/bin/env node\n" + ); + } + + #[test] + fn leaves_symlink_targets_outside_staging_dir_unchanged() { + let temp = tempdir().expect("create temp dir"); + let staging_dir = temp + .path() + .join("tmp") + .join("image") + .join("packages") + .join("staging"); + let package_dir = temp + .path() + .join("tools") + .join("image") + .join("packages") + .join("volta-test"); + let external_target = temp.path().join("external").join("native"); + let bin_link = package_dir.join("bin").join("volta-test"); + + fs::create_dir_all(external_target.parent().unwrap()).expect("create external target dir"); + fs::create_dir_all(bin_link.parent().unwrap()).expect("create package bin dir"); + fs::write(&external_target, "#!/usr/bin/env node\n").expect("write external target"); + symlink_file(&external_target, &bin_link).expect("create external symlink"); + + repair_moved_bin_symlinks(&staging_dir, &package_dir, "volta-test", "1.0.0") + .expect("repair moved symlink"); + + assert_eq!( + fs::read_link(&bin_link).expect("read unchanged link"), + external_target + ); + } + + #[test] + fn errors_when_repaired_symlink_target_does_not_exist() { + let temp = tempdir().expect("create temp dir"); + let staging_dir = temp + .path() + .join("tmp") + .join("image") + .join("packages") + .join("staging"); + let package_dir = temp + .path() + .join("tools") + .join("image") + .join("packages") + .join("volta-test"); + let staging_target = staging_dir.join("lib/node_modules/volta-test/bin/native"); + let bin_link = package_dir.join("bin").join("volta-test"); + + fs::create_dir_all(staging_target.parent().unwrap()).expect("create staging target dir"); + fs::create_dir_all(bin_link.parent().unwrap()).expect("create package bin dir"); + fs::write(&staging_target, "#!/usr/bin/env node\n").expect("write staging target"); + symlink_file(&staging_target, &bin_link).expect("create staging symlink"); + + let result = repair_moved_bin_symlinks(&staging_dir, &package_dir, "volta-test", "1.0.0"); + + assert!(result.is_err()); + } +} diff --git a/tests/acceptance/support/sandbox.rs b/tests/acceptance/support/sandbox.rs index 25e6ab176..2f3cc5ee9 100644 --- a/tests/acceptance/support/sandbox.rs +++ b/tests/acceptance/support/sandbox.rs @@ -857,6 +857,7 @@ impl Sandbox { .env("PATH", &self.path) .env("VOLTA_POSTSCRIPT", volta_postscript()) .env_remove("VOLTA_SHELL") + .env_remove("_VOLTA_TOOL_RECURSION") .env_remove("MSYSTEM"); // assume cmd.exe everywhere on windows // overrides for env vars diff --git a/tests/smoke/direct_install.rs b/tests/smoke/direct_install.rs index 9b712e471..b06c09782 100644 --- a/tests/smoke/direct_install.rs +++ b/tests/smoke/direct_install.rs @@ -3,6 +3,43 @@ use hamcrest2::assert_that; use hamcrest2::prelude::*; use test_support::matchers::execs; +const ABSOLUTE_BIN_PACKAGE_JSON: &str = r#"{ + "name": "volta-absolute-bin", + "version": "1.0.0", + "bin": { + "volta-absolute-bin": "bin/wrapper.js" + }, + "scripts": { + "postinstall": "node scripts/postinstall.js" + } +}"#; + +const ABSOLUTE_BIN_WRAPPER: &str = r#"#!/usr/bin/env node +console.log("wrapper bin"); +"#; + +const ABSOLUTE_BIN_NATIVE: &str = r#"#!/usr/bin/env node +console.log("absolute bin successful"); +"#; + +const ABSOLUTE_BIN_POSTINSTALL: &str = r#"const fs = require("fs"); +const path = require("path"); + +const bin = path.join(process.env.npm_config_prefix, "bin", "volta-absolute-bin"); +const target = path.join(__dirname, "..", "bin", "native.js"); + +try { + fs.unlinkSync(bin); +} catch (error) { + if (error.code !== "ENOENT") { + throw error; + } +} + +fs.chmodSync(target, 0o755); +fs.symlinkSync(target, bin); +"#; + #[test] fn npm_global_install() { let p = temp_project().build(); @@ -42,6 +79,43 @@ fn npm_global_install() { ); } +#[test] +fn npm_global_install_repairs_absolute_bin_symlink() { + let p = temp_project() + .project_file("absolute-bin/package.json", ABSOLUTE_BIN_PACKAGE_JSON) + .project_file("absolute-bin/bin/wrapper.js", ABSOLUTE_BIN_WRAPPER) + .project_file("absolute-bin/bin/native.js", ABSOLUTE_BIN_NATIVE) + .project_file( + "absolute-bin/scripts/postinstall.js", + ABSOLUTE_BIN_POSTINSTALL, + ) + .build(); + + // Have to install node to ensure npm is available + assert_that!(p.volta("install node@14.3.0"), execs().with_status(0)); + + let package_path = p.root().join("absolute-bin"); + assert_that!( + p.npm(&format!("pack {}", package_path.display())), + execs().with_status(0) + ); + + let tarball_path = p.root().join("volta-absolute-bin-1.0.0.tgz"); + assert_that!( + p.npm(&format!("install --global {}", tarball_path.display())), + execs().with_status(0) + ); + + assert!(p.shim_exists("volta-absolute-bin")); + assert!(p.package_is_installed("volta-absolute-bin")); + assert_that!( + p.exec_shim("volta-absolute-bin", ""), + execs() + .with_status(0) + .with_stdout_contains("absolute bin successful") + ); +} + #[test] fn yarn_global_add() { let p = temp_project().build(); diff --git a/tests/smoke/support/temp_project.rs b/tests/smoke/support/temp_project.rs index 4be937647..ec6aad4b1 100644 --- a/tests/smoke/support/temp_project.rs +++ b/tests/smoke/support/temp_project.rs @@ -289,6 +289,7 @@ impl TempProject { .env("VOLTA_HOME", volta_home(self.root())) .env("VOLTA_INSTALL_DIR", cargo_dir()) .env_remove("VOLTA_NODE_VERSION") + .env_remove("_VOLTA_TOOL_RECURSION") .env_remove("MSYSTEM"); // assume cmd.exe everywhere on windows // overrides for env vars