Preserve quotes in es run

Closes #72

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## Unreleased - [ReleaseDate]
+
+- Escape single quotes in variable names/values ([#65](https://github.com/LucasPickering/env-select/issues/65))
+- Preserve quotes in command arguments passed to `es run` ([#72](https://github.com/LucasPickering/env-select/issues/72))
+
 ## [1.1.2] - [2024-02-11]
 
 ### Changed

docs/src/SUMMARY.md

@@ -14,6 +14,7 @@
   - [Load Values from Kubernetes](./user_guide/env/kubernetes.md)
 - [Inheritance & Cascading Configs](./user_guide/inheritance.md)
 - [Side Effects](./user_guide/side_effects.md)
+- [`es run` and Shell Interactions](./user_guide/run_advanced.md)
 
 # API Reference
 

docs/src/user_guide/run_advanced.md

@@ -0,0 +1,51 @@
+# `es run` and Shell Interactions
+
+You may want to use `es run` in combination with shell features such as quotes, variable expansion, and command substitution. The general rule with `es run` is:
+
+> **The passed command will be executed exactly as the same as if `es run` weren't there, except with a different set of environment variables.**
+
+Here's a concrete example:
+
+```toml
+# We'll use this profile for all examples
+[applications.server.profiles.dev]
+variables.SERVICE1 = "dev"
+variables.SERVICE2 = "also-dev"
+```
+
+```sh
+# These two invocations of `echo` will look exactly the same to the shell
+es run server dev -- echo 'hi'
+echo 'hi'
+```
+
+These two commands will behave exactly the same. In other words, if you're not sure how your command will be executed, ignore everything up to and including the `--`, and that's what the shell will see.
+
+This allows you to add `es run` to any existing shell command without having to mess around with quotes and backslashes. Here's a more complex example where it's handy:
+
+```sh
+# Note: this is how you escape single quotes in fish. This command may look
+# slightly different in other shells.
+es run server dev -- psql -c 'select id from products where cost = \'$30\';'
+```
+
+This will pass the exact string `select id from products where cost = '$30';` to `psql`, without any variable expansion or other shell tomfoolery.
+
+> If you encounter any scenarios where the executed command does _not_ behave the same as passing it directly to the shell, please [file a bug](https://github.com/LucasPickering/env-select/issues/new).
+
+## Intentional Variable Expansion
+
+By default, there are two ways to handle variable expansion with `es run`: before invoking `es`, and not at all:
+
+```sh
+es run server dev -- echo $SERVICE1 # prints ""
+es run server dev -- echo '$SERVICE1' # prints "$SERVICE1"
+```
+
+In the first example, `$SERVICE1` is expanded by the shell _before_ `es` is executed. In the second example, the single quotes prevent the shell from ever expanding `$SERVICE1`.
+
+If you _want_ a variable to be expanded after exporting the profile before executing the command, e.g. if you want to use a variable defined by your profile, you'll need to manually invoke a subshell:
+
+```sh
+es run server dev -- fish -c 'echo $SERVICE1' # prints "dev"
+```

src/commands/run.rs

@@ -6,14 +6,19 @@ use crate::{
 };
 use clap::Parser;
 
-/// Run a shell command in an augmented environment, via a configured
-/// variable/application
+/// Run a shell command in an augmented environment
+///
+/// The passed command is run through your shell, meaning you can use aliases
+/// and other shell features. See
+/// https://env-select.lucaspickering.me/book/user_guide/run_advanced.html for
+/// more details of shell interactions.
 #[derive(Clone, Debug, Parser)]
 pub struct RunCommand {
     #[command(flatten)]
     selection: Selection,
 
-    /// Shell command to execute
+    /// Shell command to execute. Can include multiple space-separated tokens.
+    /// Will be executed as if passed directly to your shell.
     #[arg(required = true, last = true)]
     command: Vec<String>,
 }
@@ -25,7 +30,7 @@ impl SubcommandTrait for RunCommand {
 
         // Undo clap's tokenization
         let mut executable: Executable =
-            context.shell.executable(&self.command.join(" ").into());
+            context.shell.executable_from_slice(&self.command);
 
         // Execute the command
         let status =

src/commands/set.rs

@@ -8,7 +8,7 @@ use std::fs;
 
 const WEBSITE: &str = "https://env-select.lucaspickering.me";
 
-/// Modify shell environment via a configured variable/application
+/// Modify current shell environment
 #[derive(Clone, Debug, Parser)]
 pub struct SetCommand {
     #[command(flatten)]

src/execute.rs

@@ -79,6 +79,16 @@ impl Executable {
         executable
     }
 
+    #[cfg(test)]
+    pub fn program(&self) -> &str {
+        &self.program
+    }
+
+    #[cfg(test)]
+    pub fn arguments(&self) -> &[String] {
+        &self.arguments
+    }
+
     /// Set the current working directory of the command to be executed
     pub fn current_dir(&mut self, dir: &Path) -> &mut Self {
         debug!("Setting cwd for {self}: {dir:?}");

src/main.rs

@@ -60,7 +60,7 @@ fn main() -> ExitCode {
             0 => LevelFilter::Warn,
             1 => LevelFilter::Info,
             2 => LevelFilter::Debug,
-            _ => LevelFilter::Trace,
+            3.. => LevelFilter::Trace,
         })
         .init();
     let verbose = args.global.verbose > 0;

src/shell.rs

@@ -112,32 +112,78 @@ impl Shell {
         let mut output = String::new();
         for (variable, value) in environment.iter_unmasked() {
             // Escape single quotes to prevent injection vulnerabilities
-            let variable = escape(variable);
-            let value = escape(value);
+            let variable = self.escape(variable);
+            let value = self.escape(value);
 
             // Generate a shell command to export the variable
             match self.kind {
                 ShellKind::Bash | ShellKind::Zsh => {
-                    writeln!(output, "export '{variable}'='{value}'")
+                    writeln!(output, "export {variable}={value}")
                         .expect("string writing is infallible");
                 }
                 ShellKind::Fish => {
-                    writeln!(output, "set -gx '{variable}' '{value}'")
+                    writeln!(output, "set -gx {variable} {value}")
                         .expect("string writing is infallible");
                 }
             }
         }
         output
     }
 
-    /// Get an [Executable] command to run in this shell
+    /// Get an [Executable] command to run in this shell, from a shell command
     pub fn executable(&self, command: &ShellCommand) -> Executable {
         // Use the full shell path if we have it. Otherwise, just pass
         // the shell name and hope it's in PATH
         let shell_program =
             self.path.clone().unwrap_or_else(|| self.kind.to_string());
         (shell_program, ["-c", command]).executable()
     }
+
+    /// Get an [Executable] command to run in this shell, from a program name +
+    /// args. This will wrap each element in double quotes to preserve inner
+    /// quotes/parentheses/etc.
+    ///
+    /// This powers the arg de-parsing for `es run`. The goal is to execute the
+    /// command exactly as the shell would if `es run` weren't involved. E.g.:
+    ///
+    /// ```
+    /// es run app prof -- echo '$TEST'
+    /// ```
+    ///
+    /// should execute the same exact shell command as
+    ///
+    /// ```
+    /// echo '$TEST'
+    /// ```
+    ///
+    /// This means `$TEST` should *not* be expanded, because it's in single
+    /// quotes. The quotes will be dropped by the shell before `es run` is
+    /// invoked, so we need to add quotes back.
+    pub fn executable_from_slice(&self, command_args: &[String]) -> Executable {
+        // All supported shells currently treat single/double quotes the same so
+        // we can use unified logic
+        let command = command_args
+            .iter()
+            .map(|s| self.escape(s))
+            .collect::<Vec<_>>()
+            .join(" ");
+        self.executable(&command.into())
+    }
+
+    /// Wrap the given string in single quotes, escaping inner single quotes.
+    /// This makes it safe to pass as a shell argument without possibility of
+    /// injection.
+    fn escape(&self, value: &str) -> String {
+        let escaped = match self.kind {
+            // Bash and zsh don't support escaping ' with just a backslash.
+            // You have to terminate the current string, add a single quote,
+            // then open a new string
+            // https://stackoverflow.com/a/1250279/1907353
+            ShellKind::Bash | ShellKind::Zsh => value.replace('\'', "'\\''"),
+            ShellKind::Fish => value.replace('\'', "\\'"),
+        };
+        format!("'{escaped}'")
+    }
 }
 
 impl Display for Shell {
@@ -155,19 +201,16 @@ impl From<ShellKind> for Shell {
     }
 }
 
-/// Escape single quotes in the given string, replacing them with \'
-fn escape(value: &str) -> String {
-    value.replace('\'', "\\'")
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
     use crate::{
         config::Profile,
-        test_util::{literal, map},
+        test_util::{all_shells, literal, map},
     };
+    use assert_cmd::Command;
     use rstest::rstest;
+    use rstest_reuse::apply;
 
     /// Bash and Zsh use the same export format so we can test them together
     #[rstest]
@@ -180,7 +223,7 @@ mod tests {
             shell.export(&environment).as_str(),
             "\
 export 'SIMPLE'='simple'
-export 'ESCAPED\\'oops\\''='\\'; echo bobby tables \\''
+export 'ESCAPED'\\''oops'\\'''=''\\''; echo bobby tables '\\'''
 "
         );
     }
@@ -212,4 +255,28 @@ set -gx 'ESCAPED\\'oops\\'' '\\'; echo bobby tables \\''
         )
         .unwrap()
     }
+
+    /// A pseudo-integration test to make sure quotes and other scary characters
+    /// are escaped properly, so they *aren't* handled by the shell.
+    #[apply(all_shells)]
+    #[case(&[], "")]
+    #[case(&["echo", "-n", "$TEST"], "$TEST")]
+    #[case(&["echo", "-n", "double \"", "single '"], "double \" single '")]
+    #[case(&["echo", "-n", "$(echo scary!)"], "$(echo scary!)")]
+    fn test_executable_from_args(
+        shell_kind: ShellKind,
+        #[case] input: &[&str],
+        #[case] expected: &'static str,
+    ) {
+        let shell = Shell::from_kind(shell_kind);
+        let command: Vec<String> =
+            input.iter().copied().map(str::to_owned).collect();
+        let executable = shell.executable_from_slice(&command);
+        Command::new(executable.program())
+            .args(executable.arguments())
+            .assert()
+            .success()
+            .stdout(expected)
+            .stderr("");
+    }
 }

tests/.env-select.toml

@@ -1,15 +1,16 @@
-[applications.integration-tests.profiles.p1]
+[applications.test.profiles.p1]
 # These *shouldn't* have access to env vars
 pre_export = [
-  {setup = "echo pre setup 1 $VARIABLE1", teardown = "echo pre teardown 1 $VARIABLE1"},
-  {setup = "echo pre setup 2 $VARIABLE1", teardown = "echo pre teardown 2 $VARIABLE1"},
+  {setup = "echo pre setup 1 $VAR1", teardown = "echo pre teardown 1 $VAR1"},
+  {setup = "echo pre setup 2 $VAR1", teardown = "echo pre teardown 2 $VAR1"},
 ]
 # These *should* have access to env vars
 post_export = [
-  {setup = "echo post setup 1 $VARIABLE1", teardown = "echo post teardown 1 $VARIABLE1"},
-  {setup = "echo post setup 2 $VARIABLE1", teardown = "echo post teardown 2 $VARIABLE1"},
+  {setup = "echo post setup 1 $VAR1", teardown = "echo post teardown 1 $VAR1"},
+  {setup = "echo post setup 2 $VAR1", teardown = "echo post teardown 2 $VAR1"},
 ]
-variables.VARIABLE1 = "abc"
-variables.VARIABLE2 = "def"
-variables.VARIABLE3 = {type = "command", command = "echo ghi | cat -"}
-variables.file = {type = "file", path = "vars.env", multiple = ["FILE_VARIABLE1"]}
+variables.VAR1 = "abc"
+variables.VAR2 = {type = "command", command = "echo def | cat -"}
+variables.file = {type = "file", path = "vars.env", multiple = ["FILE_VAR1"]}
+
+[applications.test.profiles.empty]

tests/common/mod.rs

@@ -47,12 +47,7 @@ pub fn execute_script(
     // Inject the function source into the script
     let function_source = String::from_utf8(assert.get_output().stdout.clone())
         .expect("Function output is not valid UTF-8");
-    let script = format!(
-        "
-        {function_source}
-        {script}
-        "
-    );
+    let script = format!("{function_source} {script}");
 
     let shell = shell_path(shell_kind);
     let mut command = Command::new(&shell);

tests/test_run.rs

@@ -9,16 +9,12 @@ use rstest_reuse::{self, *};
 /// Test `es run` executes the command within a subshell, and the variables
 /// don't leak outside that subprocess
 #[apply(all_shells)]
-fn test_subcommand_run(shell_kind: &str) {
-    let echo_command = "echo '$VARIABLE1' '$VARIABLE2' '$VARIABLE3' \
-        '$VARIABLE4' '$FILE_VARIABLE1' '$FILE_VARIABLE2'";
+fn test_run_subcommand(shell_kind: &str) {
     execute_script(
-        &format!(
-            "
-            es run integration-tests p1 -- {echo_command}
-            echo Empty: $VARIABLE1
-            "
-        ),
+        "
+        es run test p1 -- printenv VAR1 VAR2 VAR3 FILE_VAR1 FILE_VAR2
+        echo Empty: $VAR1
+        ",
         shell_kind,
         true,
     )
@@ -29,7 +25,9 @@ fn test_subcommand_run(shell_kind: &str) {
 pre setup 2
 post setup 1 abc
 post setup 2 abc
-abc def ghi 123
+abc
+def
+123
 post teardown 2 abc
 post teardown 1 abc
 pre teardown 2
@@ -39,3 +37,18 @@ Empty:
     )
     .stderr("");
 }
+
+/// Test `es run` forwards quotes and other shell features in the command
+/// properly
+#[apply(all_shells)]
+fn test_run_escaping(shell_kind: &str) {
+    execute_script(
+        "es run test empty -- echo -n '$NOT_EXPANDED' '\"$(hello!!)\"'",
+        shell_kind,
+        true,
+    )
+    .assert()
+    .success()
+    .stdout("$NOT_EXPANDED \"$(hello!!)\"")
+    .stderr("");
+}