context: quote program, args, cwd and env in ssh, schroot and unshare drivers

All three non-local drivers assembled remote/chroot command strings by
raw concatenation: ssh pushed args verbatim (TODO: escape), schroot
interpolated env values raw so DEB_BUILD_OPTIONS='parallel=4 nocheck'
made sh treat 'nocheck' as the command, and unshare wrapped args in
unescaped double quotes letting quotes break out and $/backticks
expand. Add a shared POSIX shell_quote helper and use it for every
component interpolated into a shell string, including ssh copy_path
(which used Rust's {:?}, not shell quoting) and schroot write_file
(which also switches echo -ne to printf %s so backslash sequences in
content are no longer interpreted).

src/context/mod.rs

@@ -3,6 +3,7 @@ pub(crate) mod capture;
 mod local;
 mod manager;
 mod schroot;
+pub(crate) mod shell;
 mod ssh;
 mod unshare;
 

src/context/schroot.rs

@@ -1,6 +1,7 @@
 /// Schroot context: execute commands in a schroot session
 /// Not tested, will need more work!
-use super::api::{ContextDriver, LineSink};
+use super::api::{Context, ContextConfig, ContextDriver, LineSink};
+use super::shell::shell_quote;
 use std::io;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
@@ -11,8 +12,6 @@ pub struct SchrootDriver {
     pub parent: Option<Arc<super::api::Context>>,
 }
 
-use super::api::{Context, ContextConfig};
-
 impl SchrootDriver {
     fn parent(&self) -> Arc<Context> {
         self.parent
@@ -106,6 +105,11 @@ impl SchrootDriver {
 
     /// Wrap `(program, args)` in `sh -c` when a working directory or
     /// environment variables are needed.
+    ///
+    /// Everything interpolated into the resulting shell string — the `cd`
+    /// target, env keys and values, the program and each argument — is
+    /// POSIX-shell-quoted (see [`shell_quote`]), so metacharacters (spaces,
+    /// quotes, `$`, ...) can neither split words nor trigger expansion.
     fn wrap_command(
         program: &str,
         args: &[String],
@@ -119,17 +123,21 @@ impl SchrootDriver {
             let mut shell_cmd = String::new();
 
             if let Some(dir) = cwd {
-                shell_cmd.push_str(&format!("cd {} && ", dir));
+                shell_cmd.push_str(&format!("cd {} && ", shell_quote(dir)));
             }
 
             if !env.is_empty() {
                 shell_cmd.push_str("env ");
                 for (k, v) in env {
-                    shell_cmd.push_str(&format!("{}={} ", k, v));
+                    shell_cmd.push_str(&format!("{}={} ", shell_quote(k), shell_quote(v)));
                 }
             }
 
-            shell_cmd.push_str(&format!("{} {}", program, args.join(" ")));
+            shell_cmd.push_str(&shell_quote(program));
+            for arg in args {
+                shell_cmd.push(' ');
+                shell_cmd.push_str(&shell_quote(arg));
+            }
 
             actual_program = "sh".to_string();
             actual_args = vec!["-c".to_string(), shell_cmd];
@@ -260,9 +268,13 @@ impl ContextDriver for SchrootDriver {
             &[
                 "-c".to_string(),
                 format!(
-                    "echo -ne '{}' > '{}'",
+                    // `printf '%s'` writes the content verbatim (the previous
-                    content.replace("'", "'\\''"),
+                    // `echo -ne` mangled backslashes, and dash's echo prints
-                    path.to_string_lossy()
+                    // "-ne" literally). Content and path are shell-quoted so
+                    // metacharacters in either cannot break out.
+                    "printf '%s' {} > {}",
+                    shell_quote(content),
+                    shell_quote(&path.to_string_lossy())
                 ),
             ],
             &[],
@@ -284,3 +296,74 @@ impl ContextDriver for SchrootDriver {
         Ok(status.success())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::SchrootDriver;
+
+    /// Without cwd/env the program and args go to schroot as direct argv
+    /// (no shell involved), so they must pass through untouched.
+    #[test]
+    fn wrap_command_passthrough_without_env_or_cwd() {
+        let (prog, args) = SchrootDriver::wrap_command(
+            "make",
+            &["install".to_string(), "DEST=x y".to_string()],
+            &[],
+            None,
+        );
+        assert_eq!(prog, "make");
+        assert_eq!(args, vec!["install".to_string(), "DEST=x y".to_string()]);
+    }
+
+    /// A value with a space must stay a single env assignment: previously
+    /// DEB_BUILD_OPTIONS="parallel=4 nocheck" made sh treat `nocheck` as
+    /// the command to run.
+    #[test]
+    fn wrap_command_quotes_env_values_cwd_and_args() {
+        let (prog, args) = SchrootDriver::wrap_command(
+            "dpkg-buildpackage",
+            &["-us".to_string(), "-uc".to_string()],
+            &[(
+                "DEB_BUILD_OPTIONS".to_string(),
+                "parallel=4 nocheck".to_string(),
+            )],
+            Some("/build/pkg 1.0"),
+        );
+        assert_eq!(prog, "sh");
+        assert_eq!(args[0], "-c");
+        assert_eq!(
+            args[1],
+            "cd '/build/pkg 1.0' && env 'DEB_BUILD_OPTIONS'='parallel=4 nocheck' \
+             'dpkg-buildpackage' '-us' '-uc'"
+        );
+    }
+
+    /// The definitive check: a real shell must execute the wrapped command
+    /// exactly as intended — cwd applied, env set verbatim, the inner
+    /// program invoked with its argument — despite quotes in the values.
+    #[cfg(unix)]
+    #[test]
+    fn wrapped_command_survives_shell_parsing() {
+        let dir = tempfile::tempdir().unwrap();
+        let (prog, args) = SchrootDriver::wrap_command(
+            "printenv",
+            &["SOME_OPT".to_string()],
+            &[(
+                "SOME_OPT".to_string(),
+                "parallel=4 noch'eck \"x\"".to_string(),
+            )],
+            Some(dir.path().to_str().unwrap()),
+        );
+        let output = std::process::Command::new(&prog)
+            .arg(&args[0])
+            .arg(&args[1])
+            .output()
+            .unwrap();
+        assert!(output.status.success());
+        // printenv's output ends with a newline.
+        assert_eq!(
+            String::from_utf8_lossy(&output.stdout),
+            "parallel=4 noch'eck \"x\"\n"
+        );
+    }
+}

src/context/shell.rs

@@ -0,0 +1,109 @@
+//! POSIX-shell quoting for command strings assembled by the remote/chroot
+//! execution contexts.
+//!
+//! Unlike [`super::local`] — which spawns programs directly through
+//! `std::process::Command`, with no shell in between — the SSH, schroot and
+//! unshare drivers ultimately hand a *string* to a shell (`ssh
+//! channel.exec`, `sh -c`, `bash -c`). Every program name, argument,
+//! path or environment value interpolated into such a string must be
+//! quoted, or shell metacharacters (`;`, `|`, `&`, quotes, `$`, backticks,
+//! globs, whitespace, ...) are reinterpreted by the shell: at best the
+//! command breaks, at worst it executes injected input.
+
+/// Quote `s` for safe interpolation into a POSIX shell command line.
+///
+/// The result is `s` wrapped in single quotes, with every embedded single
+/// quote replaced by the standard `'\''` sequence (close the quoting, an
+/// escaped literal quote, reopen). Whatever the input contains — spaces,
+/// newlines, `"`, `'`, `$`, backticks, globs, `;` — the shell parses the
+/// result back into exactly `s` as a single word. The empty string becomes
+/// `''` (one empty argument, not zero arguments).
+///
+/// Use this for *every* value interpolated into a shell command string:
+/// programs, arguments, `cd` targets, `env` assignments (both key and
+/// value) and paths. It is safe (though redundant) to quote values that are
+/// known to need no quoting.
+pub(crate) fn shell_quote(s: &str) -> String {
+    let mut quoted = String::with_capacity(s.len() + 2);
+    quoted.push('\'');
+    for c in s.chars() {
+        if c == '\'' {
+            quoted.push_str("'\\''");
+        } else {
+            quoted.push(c);
+        }
+    }
+    quoted.push('\'');
+    quoted
+}
+
+#[cfg(test)]
+mod tests {
+    use super::shell_quote;
+
+    #[test]
+    fn plain_word() {
+        assert_eq!(shell_quote("plain"), "'plain'");
+    }
+
+    #[test]
+    fn spaces_stay_one_word() {
+        assert_eq!(shell_quote("parallel=4 nocheck"), "'parallel=4 nocheck'");
+        assert_eq!(
+            shell_quote(" leading and trailing "),
+            "' leading and trailing '"
+        );
+    }
+
+    #[test]
+    fn embedded_single_quotes() {
+        assert_eq!(shell_quote("it's"), "'it'\\''s'");
+        assert_eq!(shell_quote("''"), r"''\'''\'''");
+    }
+
+    #[test]
+    fn double_quotes_and_metacharacters() {
+        assert_eq!(
+            shell_quote("say \"hi\" $HOME `id` ; | & * ?"),
+            "'say \"hi\" $HOME `id` ; | & * ?'"
+        );
+    }
+
+    #[test]
+    fn dollar_and_backtick_do_not_expand() {
+        assert_eq!(shell_quote("$HOME"), "'$HOME'");
+        assert_eq!(shell_quote("$(rm -rf /)"), "'$(rm -rf /)'");
+        assert_eq!(shell_quote("`touch /tmp/pwned`"), "'`touch /tmp/pwned`'");
+    }
+
+    #[test]
+    fn empty_string() {
+        assert_eq!(shell_quote(""), "''");
+    }
+
+    #[test]
+    fn unicode_preserved() {
+        assert_eq!(shell_quote("héllo→wörld ✓"), "'héllo→wörld ✓'");
+    }
+
+    #[test]
+    fn newlines_preserved() {
+        assert_eq!(shell_quote("a\nb"), "'a\nb'");
+    }
+
+    /// The definitive check: a real shell must parse the quoted string back
+    /// into the original value as a single argument, without expanding or
+    /// executing anything inside it.
+    #[cfg(unix)]
+    #[test]
+    fn round_trips_through_sh() {
+        let tricky = "a'b\"c $HOME `echo pwned` ; | & \n x*y";
+        let output = std::process::Command::new("sh")
+            .arg("-c")
+            .arg(format!("printf '%s' {}", shell_quote(tricky)))
+            .output()
+            .unwrap();
+        assert!(output.status.success());
+        assert_eq!(String::from_utf8_lossy(&output.stdout), tricky);
+    }
+}

src/context/ssh.rs

@@ -2,6 +2,7 @@
 /// Context driver: Copies over SFTP with ssh2, executes commands over ssh2 channels
 use super::api::{ContextDriver, LineSink, Stream};
 use super::capture::pump;
+use super::shell::shell_quote;
 use log::debug;
 use ssh2;
 use std::fs;
@@ -53,6 +54,41 @@ pub struct SshDriver {
     pub port: Option<u16>,
 }
 
+impl SshDriver {
+    /// Build the remote shell command line: `export` assignments for `env`,
+    /// an optional `cd` to `cwd`, then `program` with its `args`.
+    ///
+    /// The line is executed verbatim by the remote login shell through
+    /// `channel.exec`, so every component is POSIX-shell-quoted (see
+    /// [`shell_quote`]): metacharacters in arguments, paths or environment
+    /// values can neither break out of their word nor be expanded by the
+    /// remote shell.
+    fn build_command_line(
+        env: &[(String, String)],
+        cwd: Option<&str>,
+        program: &str,
+        args: &[String],
+    ) -> String {
+        let mut cmd_line = String::new();
+        for (key, value) in env {
+            cmd_line.push_str(&format!(
+                "export {}={}; ",
+                shell_quote(key),
+                shell_quote(value)
+            ));
+        }
+        if let Some(dir) = cwd {
+            cmd_line.push_str(&format!("cd {} && ", shell_quote(dir)));
+        }
+        cmd_line.push_str(&shell_quote(program));
+        for arg in args {
+            cmd_line.push(' ');
+            cmd_line.push_str(&shell_quote(arg));
+        }
+        cmd_line
+    }
+}
+
 impl ContextDriver for SshDriver {
     fn ensure_available(&self, src: &Path, dest_root: &str) -> io::Result<PathBuf> {
         let sess = connect_ssh(&self.host, self.user.as_deref(), self.port)?;
@@ -106,22 +142,7 @@ impl ContextDriver for SshDriver {
 
         // Construct command line with env vars
         // TODO: No, use ssh2 channel.set_env
-        let mut cmd_line = String::new();
+        let cmd_line = Self::build_command_line(env, cwd, program, args);
-        for (key, value) in env {
-            cmd_line.push_str(&format!(
-                "export {}='{}'; ",
-                key,
-                value.replace("'", "'\\''")
-            ));
-        }
-        if let Some(dir) = cwd {
-            cmd_line.push_str(&format!("cd {} && ", dir));
-        }
-        cmd_line.push_str(program);
-        for arg in args {
-            cmd_line.push(' ');
-            cmd_line.push_str(arg); // TODO: escape
-        }
 
         debug!("Executing SSH command: {}", cmd_line);
 
@@ -152,23 +173,8 @@ impl ContextDriver for SshDriver {
         let sess = connect_ssh(&self.host, self.user.as_deref(), self.port)?;
         let mut channel = sess.channel_session().map_err(io::Error::other)?;
 
-        // Construct command line with env vars (same escaping as `run`)
+        // Construct command line with env vars (same quoting as `run`)
-        let mut cmd_line = String::new();
+        let cmd_line = Self::build_command_line(env, cwd, program, args);
-        for (key, value) in env {
-            cmd_line.push_str(&format!(
-                "export {}='{}'; ",
-                key,
-                value.replace("'", "'\\''")
-            ));
-        }
-        if let Some(dir) = cwd {
-            cmd_line.push_str(&format!("cd {} && ", dir));
-        }
-        cmd_line.push_str(program);
-        for arg in args {
-            cmd_line.push(' ');
-            cmd_line.push_str(arg); // TODO: escape
-        }
 
         debug!("Executing SSH command (captured): {}", cmd_line);
 
@@ -200,23 +206,8 @@ impl ContextDriver for SshDriver {
         let sess = connect_ssh(&self.host, self.user.as_deref(), self.port)?;
         let mut channel = sess.channel_session().map_err(io::Error::other)?;
 
-        // Construct command line with env vars
+        // Construct command line with env vars (same quoting as `run`)
-        let mut cmd_line = String::new();
+        let cmd_line = Self::build_command_line(env, cwd, program, args);
-        for (key, value) in env {
-            cmd_line.push_str(&format!(
-                "export {}='{}'; ",
-                key,
-                value.replace("'", "'\\''")
-            ));
-        }
-        if let Some(dir) = cwd {
-            cmd_line.push_str(&format!("cd {} && ", dir));
-        }
-        cmd_line.push_str(program);
-        for arg in args {
-            cmd_line.push(' ');
-            cmd_line.push_str(arg); // TODO: escape
-        }
 
         channel.exec(&cmd_line).map_err(io::Error::other)?;
 
@@ -266,7 +257,11 @@ impl ContextDriver for SshDriver {
         let sess = connect_ssh(&self.host, self.user.as_deref(), self.port)?;
         let mut channel = sess.channel_session().map_err(io::Error::other)?;
         // TODO: use sftp
-        let cmd = format!("cp -a {:?} {:?}", src, dest);
+        let cmd = format!(
+            "cp -a {} {}",
+            shell_quote(&src.to_string_lossy()),
+            shell_quote(&dest.to_string_lossy())
+        );
         debug!("Executing remote copy: {}", cmd);
         channel.exec(&cmd).map_err(io::Error::other)?;
         channel.wait_close().map_err(io::Error::other)?;
@@ -360,3 +355,65 @@ impl SshDriver {
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::SshDriver;
+
+    /// Program, arguments and the `cd` target must each be a single,
+    /// quoted word; `$` in the cwd must not be expanded.
+    #[test]
+    fn command_line_quotes_program_args_and_cwd() {
+        let line = SshDriver::build_command_line(
+            &[],
+            Some("/tmp/some dir/$HOST"),
+            "make",
+            &["install".to_string(), "PREFIX=/opt/my app".to_string()],
+        );
+        assert_eq!(
+            line,
+            "cd '/tmp/some dir/$HOST' && 'make' 'install' 'PREFIX=/opt/my app'"
+        );
+    }
+
+    /// Env keys and values are quoted too (values used to be escaped by
+    /// hand, keys and everything else not at all).
+    #[test]
+    fn command_line_quotes_env_keys_and_values() {
+        let line = SshDriver::build_command_line(
+            &[(
+                "DEB_BUILD_OPTIONS".to_string(),
+                "parallel=4 nocheck".to_string(),
+            )],
+            None,
+            "dpkg-buildpackage",
+            &["-us".to_string(), "-uc".to_string()],
+        );
+        assert_eq!(
+            line,
+            "export 'DEB_BUILD_OPTIONS'='parallel=4 nocheck'; 'dpkg-buildpackage' '-us' '-uc'"
+        );
+    }
+
+    /// The definitive check: a real shell must execute the assembled line
+    /// exactly as intended — one argument through, one env value verbatim —
+    /// even when both contain quotes, spaces and `$`.
+    #[cfg(unix)]
+    #[test]
+    fn command_line_survives_shell_parsing() {
+        let dir = tempfile::tempdir().unwrap();
+        let line = SshDriver::build_command_line(
+            &[("OPT".to_string(), "a b'c \"$d\"".to_string())],
+            Some(dir.path().to_str().unwrap()),
+            "printenv",
+            &["OPT".to_string()],
+        );
+        let output = std::process::Command::new("sh")
+            .arg("-c")
+            .arg(&line)
+            .output()
+            .unwrap();
+        assert!(output.status.success());
+        assert_eq!(String::from_utf8_lossy(&output.stdout), "a b'c \"$d\"\n");
+    }
+}

src/context/unshare.rs

@@ -1,4 +1,5 @@
 use super::api::{Context, ContextCommand, ContextDriver, LineSink};
+use super::shell::shell_quote;
 use log::debug;
 use std::fs;
 use std::io;
@@ -484,21 +485,85 @@ impl UnshareDriver {
 
         // Build the bash command: set up /dev/pts and run the program
         // /proc should already be bind-mounted from the host before entering the namespace
-        let program_args = args
-            .iter()
-            .map(|a| format!("\"{a}\""))
-            .collect::<Vec<_>>()
-            .join(" ");
-
         cmd.arg("--")
             .arg("bash")
             .arg("-c")
-            .arg(format!(
+            .arg(build_namespace_script(program, args));
-                "mkdir -p /dev/pts; mount -t devpts devpts /dev/pts 2>/dev/null || true; touch /dev/ptmx; mount --bind /dev/pts/ptmx /dev/ptmx 2>/dev/null || true; {} {}",
-                program,
-                program_args
-            ));
 
         cmd
     }
 }
+
+/// Build the shell script executed by `bash -c` inside the user namespace:
+/// bring up `/dev/pts`, then run `program` with `args`.
+///
+/// The script is parsed by bash, so the program and every argument are
+/// POSIX-shell-quoted (see [`shell_quote`]): quotes, `$`, backticks or
+/// whitespace inside them can neither split the command into different
+/// words nor trigger expansion. (Previously arguments were wrapped in
+/// unescaped double quotes, so a `"` in an argument broke out and
+/// `$`/backticks still expanded.)
+fn build_namespace_script(program: &str, args: &[String]) -> String {
+    let mut script = String::from(
+        "mkdir -p /dev/pts; mount -t devpts devpts /dev/pts 2>/dev/null || true; touch /dev/ptmx; mount --bind /dev/pts/ptmx /dev/ptmx 2>/dev/null || true; ",
+    );
+    script.push_str(&shell_quote(program));
+    for arg in args {
+        script.push(' ');
+        script.push_str(&shell_quote(arg));
+    }
+    script
+}
+
+#[cfg(test)]
+mod tests {
+    use super::build_namespace_script;
+
+    fn tail_after_devpts_setup(script: &str) -> &str {
+        script
+            .rsplit_once("|| true; ")
+            .map(|(_, rest)| rest)
+            .unwrap()
+            .trim_end()
+    }
+
+    /// Program and arguments must each be a single, quoted word at the end
+    /// of the `/dev/pts` setup script.
+    #[test]
+    fn script_quotes_program_and_args() {
+        let script = build_namespace_script(
+            "make",
+            &[
+                "install".to_string(),
+                "a b".to_string(),
+                "PREFIX=/opt/my app".to_string(),
+            ],
+        );
+        assert_eq!(
+            tail_after_devpts_setup(&script),
+            "'make' 'install' 'a b' 'PREFIX=/opt/my app'"
+        );
+    }
+
+    /// An argument containing a double quote must not break out of the
+    /// script (arguments used to be wrapped in unescaped `"`), and `$`/
+    /// backticks must stay literal for bash.
+    #[test]
+    fn script_neutralizes_quotes_and_expansions() {
+        let script = build_namespace_script(
+            "echo",
+            &["$(touch /tmp/pwned) `id` \"; rm -rf /\"".to_string()],
+        );
+        assert_eq!(
+            tail_after_devpts_setup(&script),
+            "'echo' '$(touch /tmp/pwned) `id` \"; rm -rf /\"'"
+        );
+    }
+
+    /// Empty arguments must survive as one empty word (`''`), not vanish.
+    #[test]
+    fn script_preserves_empty_args() {
+        let script = build_namespace_script("prog", &[String::new(), "x".to_string()]);
+        assert_eq!(tail_after_devpts_setup(&script), "'prog' '' 'x'");
+    }
+}