fix: Prevent fork bomb on Windows

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<S: AsRef<OsStr>>(command: Command, path: S) -> Fallible<Command> {
+    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<S: AsRef<OsStr>>(command: Command, path: S) -> Fallible<Command> {
+    debug!("PATH: {}", path.as_ref().to_string_lossy());
+    let args = command.get_args().collect::<Vec<_>>();
+    //          cmd /c <name> [...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::<Vec<_>>();
+
+            // 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)
+        },
+    )
+}

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 (`<tool>[@<version>]`)
     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,

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<ExitStatus> {
+    pub fn execute(self, session: &mut Session) -> Fallible<ExitStatus> {
         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::<u8>()
+                .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<ExitStatus> {
+    pub fn execute(self, session: &mut Session) -> Fallible<ExitStatus> {
         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<ExitStatus> {
+    pub fn execute(self, session: &mut Session) -> Fallible<ExitStatus> {
         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<ExitStatus> {
+    pub fn execute(self, session: &mut Session) -> Fallible<ExitStatus> {
         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)?;
 

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

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);

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[..]")
-    );
 }