Check volta shims for reachability during tool install

Fix #1268

Author: untitaker

Cargo.lock

@@ -53,6 +53,7 @@ chain-map = "0.1.0"
 indexmap = "1.9.1"
 retry = "1.3.1"
 fs2 = "0.4.3"
+which = "4.2.5"
 
 [target.'cfg(windows)'.dependencies]
 winreg = "0.10.1"

crates/volta-core/Cargo.toml

@@ -429,6 +429,11 @@ pub enum ErrorKind {
         name: String,
     },
 
+    /// Thrown when Volta is unable to find the shimmed command.
+    ShimResolveError {
+        name: String,
+    },
+
     /// Thrown when serializnig a bin config to JSON fails
     StringifyBinConfigError,
 
@@ -1260,6 +1265,13 @@ at {}
 {}"#,
                 name, PERMISSIONS_CTA
             ),
+            ErrorKind::ShimResolveError { name } => write!(
+                f,
+                r#"Could not find shim for "{}"
+
+{}"#,
+                name, PERMISSIONS_CTA,
+            ),
             ErrorKind::StringifyBinConfigError => write!(
                 f,
                 "Could not serialize executable configuration.
@@ -1512,6 +1524,7 @@ impl ErrorKind {
             ErrorKind::SetToolExecutable { .. } => ExitCode::FileSystemError,
             ErrorKind::ShimCreateError { .. } => ExitCode::FileSystemError,
             ErrorKind::ShimRemoveError { .. } => ExitCode::FileSystemError,
+            ErrorKind::ShimResolveError { .. } => ExitCode::FileSystemError,
             ErrorKind::StringifyBinConfigError => ExitCode::UnknownError,
             ErrorKind::StringifyPackageConfigError => ExitCode::UnknownError,
             ErrorKind::StringifyPlatformError => ExitCode::UnknownError,

crates/volta-core/src/error/kind.rs

@@ -9,7 +9,7 @@ use crate::error::{Context, ErrorKind, Fallible, VoltaError};
 use crate::fs::{read_dir_eager, symlink_file};
 use crate::layout::{volta_home, volta_install};
 use crate::sync::VoltaLock;
-use log::debug;
+use log::{debug, warn};
 
 pub fn regenerate_shims_for_dir(dir: &Path) -> Fallible<()> {
     // Acquire a lock on the Volta directory, if possible, to prevent concurrent changes
@@ -89,6 +89,23 @@ pub fn create(shim_name: &str) -> Fallible<ShimResult> {
     }
 }
 
+pub fn check_reachable(shim_name: &str) -> Fallible<()> {
+    let shim = volta_home()?.shim_file(shim_name);
+    let resolved = which::which(shim_name).map_err(|e| {
+        VoltaError::from_source(
+            e,
+            ErrorKind::ShimResolveError {
+                name: shim_name.to_string(),
+            },
+        )
+    })?;
+    if resolved != shim {
+        warn!("{} is shadowed by another binary of the same name at {}. Please ensure that {} is at the front of your PATH.", shim_name, resolved.display(), shim.display());
+    }
+
+    Ok(())
+}
+
 pub fn delete(shim_name: &str) -> Fallible<ShimResult> {
     let shim = volta_home()?.shim_file(shim_name);
 

crates/volta-core/src/shim.rs

@@ -7,6 +7,7 @@ use super::{
 use crate::error::{ErrorKind, Fallible};
 use crate::inventory::node_available;
 use crate::session::Session;
+use crate::shim;
 use crate::style::{note_prefix, tool_version};
 use crate::sync::VoltaLock;
 use cfg_if::cfg_if;
@@ -207,6 +208,8 @@ impl Tool for Node {
             info_installed(node_version); // includes node and npm version
         }
 
+        shim::check_reachable("node")?;
+
         if let Ok(Some(project)) = session.project_platform() {
             info_project_version(tool_version("node", &project.node));
         }

crates/volta-core/src/tool/node/mod.rs

@@ -8,6 +8,7 @@ use super::{
 use crate::error::{Context, ErrorKind, Fallible};
 use crate::inventory::npm_available;
 use crate::session::Session;
+use crate::shim;
 use crate::style::{success_prefix, tool_version};
 use crate::sync::VoltaLock;
 use log::info;
@@ -64,6 +65,7 @@ impl Tool for Npm {
             .set_active_npm(Some(self.version.clone()))?;
 
         info_installed(self);
+        shim::check_reachable("npm")?;
 
         if let Ok(Some(project)) = session.project_platform() {
             if let Some(npm) = &project.npm {

crates/volta-core/src/tool/npm/mod.rs

@@ -37,6 +37,7 @@ pub(super) fn write_config_and_shims(
     // Generate the shims and bin configs for each bin provided by the package
     for bin_name in &manifest.bin {
         shim::create(bin_name)?;
+        shim::check_reachable(bin_name)?;
 
         BinConfig {
             name: bin_name.clone(),

crates/volta-core/src/tool/package/configure.rs

@@ -7,6 +7,7 @@ use super::{
 use crate::error::{ErrorKind, Fallible};
 use crate::inventory::yarn_available;
 use crate::session::Session;
+use crate::shim;
 use crate::style::tool_version;
 use crate::sync::VoltaLock;
 use semver::Version;
@@ -63,6 +64,7 @@ impl Tool for Yarn {
             .set_active_yarn(Some(self.version.clone()))?;
 
         info_installed(self);
+        shim::check_reachable("yarn")?;
 
         if let Ok(Some(project)) = session.project_platform() {
             if let Some(yarn) = &project.yarn {

crates/volta-core/src/tool/yarn/mod.rs

@@ -460,6 +460,20 @@ impl SandboxBuilder {
         self.add_exec_dir_to_path()
     }
 
+    /// Add an arbitrary file to the test project within the sandbox,
+    /// give it executable permissions,
+    /// and add its directory to the *front* of PATH, shadowing any volta binaries.
+    pub fn prepend_exec_dir_to_path(mut self) -> Self {
+        if self.has_exec_path {
+            panic!("need to call prepend_exec_dir_to_path before anything else");
+        }
+
+        let exec_path = self.root().join("exec");
+        self.path_dirs.insert(0, exec_path);
+        self.has_exec_path = true;
+        self
+    }
+
     /// Set a package config file for the sandbox (chainable)
     pub fn package_config(mut self, name: &str, contents: &str) -> Self {
         let package_cfg_file = package_config_file(name);

tests/acceptance/support/sandbox.rs

@@ -333,3 +333,21 @@ fn install_yarn_3_without_node_errors() {
             )
     );
 }
+
+#[test]
+fn install_node_with_shadowed_binary() {
+    let s = sandbox()
+        .node_available_versions(NODE_VERSION_INFO)
+        .distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES)
+        .env("VOLTA_LOGLEVEL", "info")
+        .prepend_exec_dir_to_path()
+        .executable_file("node", "echo hello world")
+        .build();
+
+    assert_that!(
+        s.volta("install node"),
+        execs()
+            .with_status(ExitCode::Success as i32)
+            .with_stderr_contains("[..]is shadowed by another binary of the same name at [..]")
+    );
+}