diff --git a/crates/volta-core/src/command.rs b/crates/volta-core/src/command.rs index ece081278..780604391 100644 --- a/crates/volta-core/src/command.rs +++ b/crates/volta-core/src/command.rs @@ -2,6 +2,9 @@ use std::ffi::OsStr; use std::process::Command; use cfg_if::cfg_if; +use log::debug; + +use crate::error::Fallible; cfg_if! { if #[cfg(windows)] { @@ -28,3 +31,56 @@ cfg_if! { } } } + +/// Rebuild command against given PATH +#[cfg(unix)] +pub fn command_on_path>(command: Command, path: S) -> Fallible { + debug!("PATH: {}", path.as_ref().to_string_lossy()); + let mut command = command; + command.env("PATH", path.as_ref()); + Ok(command) +} + +/// Rebuild command against given PATH +/// +/// On Windows, we need to explicitly use an absolute path to the executable, +/// otherwise the executable will not be located properly, even if we've set the PATH. +/// see: https://github.com/rust-lang/rust/issues/37519 +/// +/// This function will try to find the executable in the given path and rebuild +/// the command with the absolute path to the executable. +#[cfg(windows)] +pub fn command_on_path>(command: Command, path: S) -> Fallible { + debug!("PATH: {}", path.as_ref().to_string_lossy()); + let args = command.get_args().collect::>(); + // cmd /c [...other] + // args_idx 0 1 2.. + let name = args.get(1).expect("A command always has a name"); + + let mut paths = which::which_in_global(name, Some(&path)).map_err(|_| { + crate::error::ErrorKind::BinaryNotFound { + name: name.to_string_lossy().to_string(), + } + })?; + + paths.next().map_or( + Err(crate::error::ErrorKind::BinaryNotFound { + name: name.to_string_lossy().to_string(), + } + .into()), + |exe| { + let mut new_command = create_command(exe); + let envs = command + .get_envs() + .filter_map(|(k, maybe_v)| Some(k).zip(maybe_v)) + .collect::>(); + + // The args will be the command name and any additional args. + new_command.args(&args[2..]); + new_command.envs(envs); + new_command.env("PATH", path.as_ref()); + + Ok(new_command) + }, + ) +} diff --git a/crates/volta-core/src/error/kind.rs b/crates/volta-core/src/error/kind.rs index 6add88efa..df736cee4 100644 --- a/crates/volta-core/src/error/kind.rs +++ b/crates/volta-core/src/error/kind.rs @@ -329,6 +329,9 @@ pub enum ErrorKind { /// Thrown when unable to parse the platform.json file ParsePlatformError, + /// Thrown when unable to parse the RECURSION_ENV_VAR value + ParseRecursionEnvError, + /// Thrown when unable to parse a tool spec (`[@]`) ParseToolSpecError { tool_spec: String, @@ -408,6 +411,9 @@ pub enum ErrorKind { file: PathBuf, }, + /// Thrown when recursion limit is reached + RecursionLimit, + /// Thrown when unable to read the user Path environment variable from the registry #[cfg(windows)] ReadUserPathError, @@ -1114,6 +1120,10 @@ Please ensure the version of Node is correct." {}", REPORT_BUG_CTA ), + ErrorKind::ParseRecursionEnvError => write!( + f, + "Could not parse RECURSION_ENV_VAR value." + ), ErrorKind::ParseToolSpecError { tool_spec } => write!( f, "Could not parse tool spec `{}` @@ -1246,6 +1256,10 @@ from {} file.display(), PERMISSIONS_CTA ), + ErrorKind::RecursionLimit => write!( + f, + "Recursive call limit reached." + ), #[cfg(windows)] ErrorKind::ReadUserPathError => write!( f, @@ -1533,6 +1547,7 @@ impl ErrorKind { ErrorKind::ParseNpmManifestError => ExitCode::UnknownError, ErrorKind::ParsePackageConfigError => ExitCode::UnknownError, ErrorKind::ParsePlatformError => ExitCode::ConfigurationError, + ErrorKind::ParseRecursionEnvError => ExitCode::UnknownError, ErrorKind::PersistInventoryError { .. } => ExitCode::FileSystemError, ErrorKind::PnpmVersionNotFound { .. } => ExitCode::NoVersionMatch, ErrorKind::ProjectLocalBinaryExecError { .. } => ExitCode::ExecutionFailure, @@ -1549,6 +1564,7 @@ impl ErrorKind { ErrorKind::ReadNpmManifestError => ExitCode::UnknownError, ErrorKind::ReadPackageConfigError { .. } => ExitCode::FileSystemError, ErrorKind::ReadPlatformError { .. } => ExitCode::FileSystemError, + ErrorKind::RecursionLimit => ExitCode::ExecutionFailure, #[cfg(windows)] ErrorKind::ReadUserPathError => ExitCode::EnvironmentError, ErrorKind::RegistryFetchError { .. } => ExitCode::NetworkError, diff --git a/crates/volta-core/src/run/executor.rs b/crates/volta-core/src/run/executor.rs index 175223ed4..eeb13396d 100644 --- a/crates/volta-core/src/run/executor.rs +++ b/crates/volta-core/src/run/executor.rs @@ -6,8 +6,8 @@ use std::os::unix::process::ExitStatusExt; use std::os::windows::process::ExitStatusExt; use std::process::{Command, ExitStatus}; -use super::RECURSION_ENV_VAR; -use crate::command::create_command; +use super::{RECURSION_ENV_VAR, RECURSION_LIMIT}; +use crate::command::{command_on_path, create_command}; use crate::error::{Context, ErrorKind, Fallible}; use crate::layout::volta_home; use crate::platform::{CliPlatform, Platform, System}; @@ -175,7 +175,7 @@ impl ToolCommand { } /// Runs the command, returning the `ExitStatus` if it successfully launches - pub fn execute(mut self, session: &mut Session) -> Fallible { + pub fn execute(self, session: &mut Session) -> Fallible { let (path, on_failure) = match self.kind { ToolKind::Node => super::node::execution_context(self.platform, session)?, ToolKind::Npm => super::npm::execution_context(self.platform, session)?, @@ -191,11 +191,24 @@ impl ToolCommand { ToolKind::Bypass(command) => (System::path()?, ErrorKind::BypassError { command }), }; - self.command.env(RECURSION_ENV_VAR, "1"); - self.command.env("PATH", path); + // Do recursive call limit check + let recursion = match std::env::var(RECURSION_ENV_VAR) { + Err(_) => 1u8, + Ok(var) => var + .parse::() + .with_context(|| ErrorKind::ParseRecursionEnvError)?, + }; + if recursion > RECURSION_LIMIT { + return Err(ErrorKind::RecursionLimit.into()); + } - pass_control_to_shim(); - self.command.status().with_context(|| on_failure) + command_on_path(self.command, path) + .and_then(|mut command| { + command.env(RECURSION_ENV_VAR, (recursion + 1).to_string()); + pass_control_to_shim(); + command.status().with_context(|| ErrorKind::BinaryExecError) + }) + .with_context(|| on_failure) } } @@ -274,17 +287,17 @@ impl PackageInstallCommand { /// Runs the install command, applying the necessary modifications to install into the Volta /// data directory - pub fn execute(mut self, session: &mut Session) -> Fallible { + pub fn execute(self, session: &mut Session) -> Fallible { let _lock = VoltaLock::acquire(); let image = self.platform.checkout(session)?; let path = image.path()?; - self.command.env(RECURSION_ENV_VAR, "1"); - self.command.env("PATH", path); - self.installer.setup_command(&mut self.command); + let mut command = command_on_path(self.command, path)?; + + command.env(RECURSION_ENV_VAR, "1"); + self.installer.setup_command(&mut command); - let status = self - .command + let status = command .status() .with_context(|| ErrorKind::BinaryExecError)?; @@ -350,20 +363,19 @@ impl PackageLinkCommand { /// directory. /// /// This will also check for some common failure cases and alert the user - pub fn execute(mut self, session: &mut Session) -> Fallible { + pub fn execute(self, session: &mut Session) -> Fallible { self.check_linked_package(session)?; let image = self.platform.checkout(session)?; let path = image.path()?; - self.command.env(RECURSION_ENV_VAR, "1"); - self.command.env("PATH", path); + let mut command = command_on_path(self.command, path)?; + + command.env(RECURSION_ENV_VAR, "1"); let package_root = volta_home()?.package_image_dir(&self.tool); - PackageManager::Npm.setup_global_command(&mut self.command, package_root); + PackageManager::Npm.setup_global_command(&mut command, package_root); - self.command - .status() - .with_context(|| ErrorKind::BinaryExecError) + command.status().with_context(|| ErrorKind::BinaryExecError) } /// Check for possible failure cases with the linked package: @@ -465,19 +477,19 @@ impl PackageUpgradeCommand { /// /// Will also check for common failure cases, such as non-existant package or wrong package /// manager - pub fn execute(mut self, session: &mut Session) -> Fallible { + pub fn execute(self, session: &mut Session) -> Fallible { self.upgrader.check_upgraded_package()?; let _lock = VoltaLock::acquire(); let image = self.platform.checkout(session)?; let path = image.path()?; - self.command.env(RECURSION_ENV_VAR, "1"); - self.command.env("PATH", path); - self.upgrader.setup_command(&mut self.command); + let mut command = command_on_path(self.command, path)?; + + command.env(RECURSION_ENV_VAR, "1"); + self.upgrader.setup_command(&mut command); - let status = self - .command + let status = command .status() .with_context(|| ErrorKind::BinaryExecError)?; diff --git a/crates/volta-core/src/run/mod.rs b/crates/volta-core/src/run/mod.rs index 89278eba7..bf371b1f6 100644 --- a/crates/volta-core/src/run/mod.rs +++ b/crates/volta-core/src/run/mod.rs @@ -32,6 +32,7 @@ mod yarn; /// Note: This is explicitly _removed_ when calling a command through `volta run`, as that will /// never happen due to the Volta environment. const RECURSION_ENV_VAR: &str = "_VOLTA_TOOL_RECURSION"; +const RECURSION_LIMIT: u8 = 10; const VOLTA_BYPASS: &str = "VOLTA_BYPASS"; /// Execute a shim command, based on the command-line arguments to the current process diff --git a/crates/volta-core/src/tool/package/install.rs b/crates/volta-core/src/tool/package/install.rs index 4416e0e34..de2ba16fc 100644 --- a/crates/volta-core/src/tool/package/install.rs +++ b/crates/volta-core/src/tool/package/install.rs @@ -1,7 +1,7 @@ use std::path::PathBuf; use super::manager::PackageManager; -use crate::command::create_command; +use crate::command::{command_on_path, create_command}; use crate::error::{Context, ErrorKind, Fallible}; use crate::platform::Image; use crate::style::progress_spinner; @@ -17,6 +17,7 @@ pub(super) fn run_global_install( staging_dir: PathBuf, platform_image: &Image, ) -> Fallible<()> { + let path = platform_image.path()?; let mut command = create_command("npm"); command.args([ "install", @@ -26,7 +27,9 @@ pub(super) fn run_global_install( "--no-audit", ]); command.arg(&package); - command.env("PATH", platform_image.path()?); + + command = command_on_path(command, path)?; + PackageManager::Npm.setup_global_command(&mut command, staging_dir); debug!("Installing {} with command: {:?}", package, command); diff --git a/tests/acceptance/volta_bypass.rs b/tests/acceptance/volta_bypass.rs index 6cba3d10c..e2ab0b9c7 100644 --- a/tests/acceptance/volta_bypass.rs +++ b/tests/acceptance/volta_bypass.rs @@ -15,19 +15,10 @@ fn shim_skips_platform_checks_on_bypass() { ) .build(); - #[cfg(unix)] assert_that!( s.process(shim_exe()), execs() .with_status(ExitCode::ExecutionFailure as i32) .with_stderr_contains("VOLTA_BYPASS is enabled[..]") ); - - #[cfg(windows)] - assert_that!( - s.process(shim_exe()), - execs() - .with_status(ExitCode::UnknownError as i32) - .with_stderr_contains("[..]is not recognized as an internal or external command[..]") - ); }