fix: nushell environment variable quoting (#719)

crates/rattler_shell/src/shell/mod.rs

@@ -1,19 +1,23 @@
-//! This module contains the [`Shell`] trait and implementations for various shells.
+//! This module contains the [`Shell`] trait and implementations for various
+//! shells.
 
-use crate::activation::PathModificationBehavior;
-use enum_dispatch::enum_dispatch;
-use itertools::Itertools;
-use rattler_conda_types::Platform;
-use std::collections::HashMap;
-use std::ffi::OsStr;
-use std::process::Command;
-use std::str::FromStr;
 use std::{
+    borrow::Cow,
+    collections::HashMap,
+    ffi::OsStr,
     fmt::Write,
     path::{Path, PathBuf},
+    process::Command,
+    str::FromStr,
 };
+
+use enum_dispatch::enum_dispatch;
+use itertools::Itertools;
+use rattler_conda_types::Platform;
 use thiserror::Error;
 
+use crate::activation::PathModificationBehavior;
+
 /// A trait for generating shell scripts.
 /// The trait is implemented for each shell individually.
 ///
@@ -32,7 +36,8 @@ use thiserror::Error;
 /// ```
 #[enum_dispatch(ShellEnum)]
 pub trait Shell {
-    /// Write a command to the script that forces the usage of UTF8-encoding for the shell script.
+    /// Write a command to the script that forces the usage of UTF8-encoding for
+    /// the shell script.
     fn force_utf8(&self, _f: &mut impl Write) -> std::fmt::Result {
         Ok(())
     }
@@ -46,7 +51,8 @@ pub trait Shell {
     /// Run a script in the current shell.
     fn run_script(&self, f: &mut impl Write, path: &Path) -> std::fmt::Result;
 
-    /// Test to see if the path can be executed by the shell, based on the extension of the path.
+    /// Test to see if the path can be executed by the shell, based on the
+    /// extension of the path.
     fn can_run_script(&self, path: &Path) -> bool {
         path.is_file()
             && path
@@ -55,8 +61,8 @@ pub trait Shell {
                 .map_or(false, |ext| ext == self.extension())
     }
 
-    /// Executes a command in the current shell. Use [`Self::run_script`] when you want to run
-    /// another shell script.
+    /// Executes a command in the current shell. Use [`Self::run_script`] when
+    /// you want to run another shell script.
     fn run_command<'a>(
         &self,
         f: &mut impl Write,
@@ -78,15 +84,16 @@ pub trait Shell {
             .map(|path| path.to_string_lossy().into_owned())
             .collect_vec();
         // Replace, Append, or Prepend the path variable to the paths.
+        let path_var = self.path_var(platform);
         match modification_behavior {
             PathModificationBehavior::Replace => (),
-            PathModificationBehavior::Append => paths_vec.insert(0, self.format_env_var("PATH")),
-            PathModificationBehavior::Prepend => paths_vec.push(self.format_env_var("PATH")),
+            PathModificationBehavior::Append => paths_vec.insert(0, self.format_env_var(path_var)),
+            PathModificationBehavior::Prepend => paths_vec.push(self.format_env_var(path_var)),
         }
         // Create the shell specific list of paths.
         let paths_string = paths_vec.join(self.path_seperator(platform));
 
-        self.set_env_var(f, "PATH", paths_string.as_str())
+        self.set_env_var(f, self.path_var(platform), paths_string.as_str())
     }
 
     /// The extension that shell scripts for this interpreter usually use.
@@ -95,7 +102,8 @@ pub trait Shell {
     /// The executable that can be called to start this shell.
     fn executable(&self) -> &str;
 
-    /// Constructs a [`Command`] that will execute the specified script by this shell.
+    /// Constructs a [`Command`] that will execute the specified script by this
+    /// shell.
     fn create_run_script_command(&self, path: &Path) -> Command;
 
     /// Path seperator
@@ -107,6 +115,17 @@ pub trait Shell {
         }
     }
 
+    /// Returns the name of the PATH variable for the given platform. On
+    /// Windows, path variables are case-insensitive but not all shells treat
+    /// them case-insensitive.
+    fn path_var(&self, platform: &Platform) -> &str {
+        if platform.is_windows() {
+            "Path"
+        } else {
+            "PATH"
+        }
+    }
+
     /// Format the environment variable for the shell.
     fn format_env_var(&self, var_name: &str) -> String {
         format!("${{{var_name}}}")
@@ -122,8 +141,8 @@ pub trait Shell {
         writeln!(f, "/usr/bin/env")
     }
 
-    /// Write the script to the writer and do some post-processing for line-endings.
-    /// Only really relevant for cmd.exe scripts.
+    /// Write the script to the writer and do some post-processing for
+    /// line-endings. Only really relevant for cmd.exe scripts.
     fn write_script(&self, f: &mut impl std::io::Write, script: &str) -> std::io::Result<()> {
         f.write_all(script.as_bytes())
     }
@@ -204,7 +223,8 @@ impl Shell for Bash {
         let mut paths_vec = paths
             .iter()
             .map(|path| {
-                // check if we are on Windows, and if yes, convert native path to unix for (Git) Bash
+                // check if we are on Windows, and if yes, convert native path to unix for (Git)
+                // Bash
                 if cfg!(windows) {
                     match native_path_to_unix(path.to_string_lossy().as_ref()) {
                         Ok(path) => path,
@@ -222,15 +242,16 @@ impl Shell for Bash {
             .collect_vec();
 
         // Replace, Append, or Prepend the path variable to the paths.
+        let path_var = self.path_var(platform);
         match modification_behavior {
             PathModificationBehavior::Replace => (),
-            PathModificationBehavior::Prepend => paths_vec.push(self.format_env_var("PATH")),
-            PathModificationBehavior::Append => paths_vec.insert(0, self.format_env_var("PATH")),
+            PathModificationBehavior::Prepend => paths_vec.push(self.format_env_var(path_var)),
+            PathModificationBehavior::Append => paths_vec.insert(0, self.format_env_var(path_var)),
         }
         // Create the shell specific list of paths.
         let paths_string = paths_vec.join(self.path_seperator(platform));
 
-        self.set_env_var(f, "PATH", paths_string.as_str())
+        self.set_env_var(f, self.path_var(platform), paths_string.as_str())
     }
 
     fn extension(&self) -> &str {
@@ -244,7 +265,8 @@ impl Shell for Bash {
     fn create_run_script_command(&self, path: &Path) -> Command {
         let mut cmd = Command::new(self.executable());
 
-        // check if we are on Windows, and if yes, convert native path to unix for (Git) Bash
+        // check if we are on Windows, and if yes, convert native path to unix for (Git)
+        // Bash
         if cfg!(windows) {
             cmd.arg(native_path_to_unix(path.to_str().unwrap()).unwrap());
         } else {
@@ -510,6 +532,13 @@ impl Shell for Fish {
 fn escape_backslashes(s: &str) -> String {
     s.replace('\\', "\\\\")
 }
+fn quote_if_required(s: &str) -> Cow<'_, str> {
+    if s.contains(|c: char| !c.is_ascii_alphanumeric() && c != '_' && c != '-') {
+        Cow::Owned(format!("\"{s}\""))
+    } else {
+        Cow::Borrowed(s)
+    }
+}
 
 /// A [`Shell`] implementation for the Bash shell.
 #[derive(Debug, Clone, Copy, Default)]
@@ -518,39 +547,45 @@ pub struct NuShell;
 impl Shell for NuShell {
     fn set_env_var(&self, f: &mut impl Write, env_var: &str, value: &str) -> std::fmt::Result {
         // escape backslashes for Windows (make them double backslashes)
-        writeln!(f, "$env.{} = \"{}\"", env_var, escape_backslashes(value))
+        writeln!(
+            f,
+            "$env.{} = \"{}\"",
+            quote_if_required(env_var),
+            escape_backslashes(value)
+        )
     }
 
     fn unset_env_var(&self, f: &mut impl Write, env_var: &str) -> std::fmt::Result {
-        writeln!(f, "hide-env {env_var}")
+        writeln!(f, "hide-env {}", quote_if_required(env_var))
     }
 
     fn run_script(&self, f: &mut impl Write, path: &Path) -> std::fmt::Result {
-        writeln!(f, "source \"{}\"", path.to_string_lossy())
+        writeln!(f, "source-env \"{}\"", path.to_string_lossy())
     }
 
     fn set_path(
         &self,
         f: &mut impl Write,
         paths: &[PathBuf],
         modification_behavior: PathModificationBehavior,
-        _platform: &Platform,
+        platform: &Platform,
     ) -> std::fmt::Result {
         let path = paths
             .iter()
             .map(|path| escape_backslashes(&format!("\"{}\"", path.to_string_lossy().into_owned())))
             .join(", ");
 
         // Replace, Append, or Prepend the path variable to the paths.
+        let path_var = self.path_var(platform);
         match modification_behavior {
             PathModificationBehavior::Replace => {
-                writeln!(f, "$env.PATH = [{path}]")
+                writeln!(f, "$env.{path_var} = [{path}]",)
             }
             PathModificationBehavior::Prepend => {
-                writeln!(f, "$env.PATH = ($env.PATH | prepend [{path}])")
+                writeln!(f, "$env.{path_var} = ($env.{path_var} | prepend [{path}])")
             }
             PathModificationBehavior::Append => {
-                writeln!(f, "$env.PATH = ($env.PATH | append [{path}])")
+                writeln!(f, "$env.{path_var} = ($env.{path_var} | append [{path}])")
             }
         }
     }
@@ -603,11 +638,11 @@ impl ShellEnum {
 
     /// Determine the user's current shell from the environment
     ///
-    /// This will read the SHELL environment variable and try to determine which shell is in use
-    /// from that.
+    /// This will read the SHELL environment variable and try to determine which
+    /// shell is in use from that.
     ///
-    /// If SHELL is set, but contains a value that doesn't correspond to one of the supported shell
-    /// types, then return `None`.
+    /// If SHELL is set, but contains a value that doesn't correspond to one of
+    /// the supported shell types, then return `None`.
     pub fn from_env() -> Option<Self> {
         if let Some(env_shell) = std::env::var_os("SHELL") {
             Self::from_shell_path(env_shell)
@@ -774,8 +809,8 @@ impl<T: Shell + 'static> ShellScript<T> {
         Ok(self)
     }
 
-    /// Add contents to the script. The contents will be added as is, so make sure to format it
-    /// correctly for the shell.
+    /// Add contents to the script. The contents will be added as is, so make
+    /// sure to format it correctly for the shell.
     pub fn append_script(&mut self, script: &Self) -> &mut Self {
         self.contents.push('\n');
         self.contents.push_str(&script.contents);