sdk run: quote argv elements instead of joining them raw

CHANGELOG.md

@@ -52,6 +52,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   is skipped with nothing to say rather than asserting a version it never read.
 
 ### Breaking
+- **`sdk run` passes its argv through literally; the container shell no longer
+  re-splits or expands it.** Anything passing a `$VAR`, a glob, or a `&&` chain
+  as a single argument and counting on the container shell to interpret it now
+  gets that string through as one command word: `avocado sdk run -- 'ls /foo &&
+  ls /bar'` no longer runs two commands, and `avocado sdk run -- echo '$HOME'`
+  prints a literal `$HOME`. Ask for a shell explicitly instead — `avocado sdk
+  run -- bash -lc 'ls /foo && ls /bar'` — which is the form the flags already
+  implied and which only works correctly after this change.
+
+  This one breaks silently at runtime: there is no error and no warning, so a
+  pinned-version bump in CI will not fail the build, it will just run a
+  different command. Audit any job that shells through `sdk run` before
+  upgrading.
+
+  **What it fixes.** The arguments after `--` were spliced into the container's
+  script with a bare `join(" ")`, so the shell inside re-parsed them. `avocado
+  sdk run -- bash -lc 'U=/opt/x; ls $U'` arrived as `bash -lc U=/opt/x; ls $U`,
+  which the shell read as two commands and whose `$U` the *outer* shell expanded
+  to nothing — printing plausible output for a different directory rather than
+  failing. Each element is quoted now, so argv is argv, matching `docker run`
+  and `kubectl exec`.
 - **`is_version_compatible` returns `Option<bool>` instead of `bool`.** Only
   affects consumers of the `avocado_cli` lib target. `None` means the remote
   version could not be read at all, which the old `bool` could not express -

src/commands/sdk/run.rs

@@ -11,9 +11,21 @@ use crate::utils::{
     config::{ComposedConfig, Config},
     container::{RunConfig, SdkContainer},
     output::{print_info, print_success, OutputLevel},
+    shell::shell_escape,
     target::validate_and_log_target,
 };
 
+/// Join a user-supplied argv into one shell-safe command string.
+///
+/// The result is spliced into a shell script inside the container, so each
+/// element must be quoted or the container shell re-splits it.
+fn join_argv(argv: &[String]) -> String {
+    argv.iter()
+        .map(|a| shell_escape(a))
+        .collect::<Vec<_>>()
+        .join(" ")
+}
+
 /// Implementation of the 'sdk run' command.
 pub struct SdkRunCommand {
     /// Path to configuration file
@@ -261,7 +273,7 @@ impl SdkRunCommand {
         // Require either a command or --interactive flag
         if !self.interactive && self.command.is_none() {
             return Err(anyhow::anyhow!(
-                "You must either provide a --command (-c) or use --interactive (-i)."
+                "You must either provide a command to run or use --interactive (-i)."
             ));
         }
 
@@ -310,9 +322,16 @@ impl SdkRunCommand {
             println!("Container name: {name}");
         }
 
-        // Build the command to execute
+        // Build the command to execute.
+        //
+        // The argv vector is spliced into a shell script inside the container, so
+        // each element has to be quoted or the container shell re-splits it. A bare
+        // join(" ") turns `-- bash -lc 'X=1; echo $X'` into
+        // `bash -lc X=1; echo $X`, which the shell reads as two commands and
+        // expands `$X` in the *outer* shell (to nothing). That silently produces
+        // wrong output instead of an error, which is worse than failing.
         let command = if let Some(ref cmd) = self.command {
-            let user_command = cmd.join(" ");
+            let user_command = join_argv(cmd);
             if self.env {
                 format!(". avocado-env && {user_command}")
             } else {
@@ -442,6 +461,24 @@ mod tests {
         assert_eq!(cmd.command, Some(vec!["ls".to_string(), "-la".to_string()]));
     }
 
+    #[test]
+    fn test_join_argv_preserves_argv_boundaries() {
+        // A bare join(" ") would emit `bash -lc X=1; echo $X`, which the
+        // container shell splits into two commands and expands `$X` itself.
+        let argv = vec![
+            "bash".to_string(),
+            "-lc".to_string(),
+            "X=1; echo $X".to_string(),
+        ];
+        assert_eq!(join_argv(&argv), "'bash' '-lc' 'X=1; echo $X'");
+    }
+
+    #[test]
+    fn test_join_argv_escapes_embedded_single_quotes() {
+        let argv = vec!["echo".to_string(), "it's".to_string()];
+        assert_eq!(join_argv(&argv), "'echo' 'it'\\''s'");
+    }
+
     #[tokio::test]
     async fn test_invalid_arguments() {
         let cmd = SdkRunCommand::new(
@@ -493,7 +530,7 @@ mod tests {
         assert!(result
             .unwrap_err()
             .to_string()
-            .contains("You must either provide a --command"));
+            .contains("You must either provide a command to run"));
     }
 
     #[test]

src/utils/mod.rs

@@ -30,6 +30,7 @@ pub mod runs_on;
 pub mod runtime;
 pub mod runtime_extension;
 pub mod scheduler;
+pub mod shell;
 pub mod signing_keys;
 #[cfg(unix)]
 pub mod signing_service;

src/utils/runs_on.rs

@@ -18,6 +18,7 @@ use crate::utils::remote::{
     get_local_ip_for_remote, version_check_notice, RemoteHost, RemoteVolumeManager, SshClient,
     SshControlMaster, VersionNotice,
 };
+use crate::utils::shell::shell_escape;
 
 #[cfg(unix)]
 use crate::utils::remote::SshTunnel;
@@ -691,36 +692,3 @@ impl RunsOnContext {
         Ok(())
     }
 }
-
-/// Shell escape a string for safe use in a shell command
-fn shell_escape(s: &str) -> String {
-    format!("'{}'", s.replace('\'', "'\\''"))
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn test_shell_escape_simple() {
-        assert_eq!(shell_escape("hello"), "'hello'");
-    }
-
-    #[test]
-    fn test_shell_escape_with_spaces() {
-        assert_eq!(shell_escape("hello world"), "'hello world'");
-    }
-
-    #[test]
-    fn test_shell_escape_with_quotes() {
-        assert_eq!(shell_escape("it's"), "'it'\\''s'");
-    }
-
-    #[test]
-    fn test_shell_escape_complex() {
-        assert_eq!(
-            shell_escape("echo 'hello' && rm -rf /"),
-            "'echo '\\''hello'\\'' && rm -rf /'"
-        );
-    }
-}

src/utils/shell.rs

@@ -0,0 +1,38 @@
+//! Shell quoting helpers.
+//!
+//! Several commands splice values into a shell script that runs inside the SDK
+//! container. Anything user-supplied has to be quoted on the way in or the
+//! container shell re-splits and expands it.
+
+/// Shell escape a string for safe use in a shell command
+pub(crate) fn shell_escape(s: &str) -> String {
+    format!("'{}'", s.replace('\'', "'\\''"))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_shell_escape_simple() {
+        assert_eq!(shell_escape("hello"), "'hello'");
+    }
+
+    #[test]
+    fn test_shell_escape_with_spaces() {
+        assert_eq!(shell_escape("hello world"), "'hello world'");
+    }
+
+    #[test]
+    fn test_shell_escape_with_quotes() {
+        assert_eq!(shell_escape("it's"), "'it'\\''s'");
+    }
+
+    #[test]
+    fn test_shell_escape_complex() {
+        assert_eq!(
+            shell_escape("echo 'hello' && rm -rf /"),
+            "'echo '\\''hello'\\'' && rm -rf /'"
+        );
+    }
+}