fix(orchestration): use base64 encoding to prevent quote stripping in docker exec

Author: jlapenna

src/sparkrun/orchestration/executor.py

@@ -10,11 +10,16 @@
 from __future__ import annotations
 
 import logging
+import shlex
 from abc import ABC, abstractmethod
 from dataclasses import dataclass
 
 from scitrera_app_framework.util import ext_parse_bool
 
+from sparkrun.scripts import read_script
+from sparkrun.utils import merge_env
+from sparkrun.utils.shell import b64_encode_cmd
+
 logger = logging.getLogger(__name__)
 
 # Default executor settings for DGX Spark GPU workloads.
@@ -78,7 +83,9 @@ def from_chain(cls, chain) -> ExecutorConfig:
         # still means "not set" and should fall back.
         def _get(key):
             v = chain.get(key)
-            return v if v is not None else EXECUTOR_DEFAULTS.get(key)
+            val = v if v is not None else EXECUTOR_DEFAULTS.get(key)
+            logger.debug("ExecutorConfig resolve: %s=%r (from chain: %r)", key, val, v)
+            return val
 
         return cls(
             auto_remove=ext_parse_bool(_get("auto_remove")),
@@ -209,8 +216,6 @@ def generate_launch_script(
 
         Absorbs ``scripts.py::generate_container_launch_script``.
         """
-        from sparkrun.utils import merge_env
-        from sparkrun.scripts import read_script
 
         all_env = merge_env(nccl_env, env)
         cleanup = self.stop_cmd(container_name)
@@ -243,21 +248,23 @@ def generate_exec_serve_script(
 
         Absorbs ``scripts.py::generate_exec_serve_script``.
         """
-        from sparkrun.scripts import read_script
 
         env_exports = ""
         if env:
             for key, value in sorted(env.items()):
-                env_exports += "export %s='%s'; " % (key, value)
+                env_exports += "export %s=%s; " % (key, shlex.quote(str(value)))
+
+        full_cmd = "%s%s" % (env_exports, serve_command)
 
-        escaped_cmd = serve_command.replace("'", "'\\''")
-        full_cmd = "%s%s" % (env_exports, escaped_cmd)
+        # Base64 encode the command to avoid all bash string-escaping/quoting bugs
+        # when passing it into `docker exec ... bash -c "..."`
+        b64_cmd = b64_encode_cmd(full_cmd)
 
         script_name = "exec_serve_detached.sh" if detached else "exec_serve_foreground.sh"
         template = read_script(script_name)
         return template.format(
-            container_name=container_name,
-            full_cmd=full_cmd,
+            container_name=shlex.quote(container_name),
+            b64_cmd=b64_cmd,
         )
 
     def generate_ray_head_script(
@@ -275,8 +282,6 @@ def generate_ray_head_script(
 
         Absorbs ``scripts.py::generate_ray_head_script``.
         """
-        from sparkrun.utils import merge_env
-        from sparkrun.scripts import read_script
 
         all_env = merge_env({"RAY_memory_monitor_refresh_ms": "0"}, nccl_env, env)
 
@@ -314,8 +319,6 @@ def generate_ray_worker_script(
 
         Absorbs ``scripts.py::generate_ray_worker_script``.
         """
-        from sparkrun.utils import merge_env
-        from sparkrun.scripts import read_script
 
         all_env = merge_env({"RAY_memory_monitor_refresh_ms": "0"}, nccl_env, env)
 
@@ -356,7 +359,6 @@ def generate_node_script(
 
         Absorbs ``base.py::_generate_node_script``.
         """
-        from sparkrun.utils import merge_env
 
         all_env = merge_env(nccl_env, env)
         cleanup = self.stop_cmd(container_name)
@@ -374,19 +376,24 @@ def generate_node_script(
             "#!/bin/bash\n"
             "set -uo pipefail\n"
             "\n"
-            "echo 'Cleaning up existing container: %(name)s'\n"
+            "printf 'Cleaning up existing container: %%s\\n' %(name)s\n"
             "%(cleanup)s\n"
             "\n"
-            "echo 'Launching %(label)s: %(name)s'\n"
+            "printf 'Launching %%s: %%s\\n' %(label)s %(name)s\n"
             "%(run_cmd)s\n"
             "\n"
             "# Verify container started\n"
             "sleep 1\n"
             "if docker ps --format '{{.Names}}' | grep -q '^%(name)s$'; then\n"
-            "    echo 'Container %(name)s launched successfully'\n"
+            "    printf 'Container %%s launched successfully\\n' %(name)s\n"
             "else\n"
-            "    echo 'ERROR: Container %(name)s failed to start' >&2\n"
+            "    printf 'ERROR: Container %%s failed to start\\n' %(name)s >&2\n"
             "    docker logs %(name)s 2>&1 | tail -20 || true\n"
             "    exit 1\n"
             "fi\n"
-        ) % {"name": container_name, "cleanup": cleanup, "run_cmd": run, "label": label}
+        ) % {
+            "name": shlex.quote(container_name),
+            "cleanup": cleanup,
+            "run_cmd": run,
+            "label": shlex.quote(label),
+        }

src/sparkrun/orchestration/executor_docker.py

@@ -11,6 +11,7 @@
 import shlex
 
 from sparkrun.orchestration.executor import Executor
+from sparkrun.utils.shell import b64_wrap_bash
 
 logger = logging.getLogger(__name__)
 
@@ -28,33 +29,34 @@ def _build_default_opts(self) -> list[str]:
         if cfg.gpus:
             opts.extend(["--gpus", cfg.gpus])
         if cfg.ipc:
-            opts.append("--ipc=%s" % cfg.ipc)
+            opts.append("--ipc=%s" % shlex.quote(cfg.ipc))
         if cfg.shm_size:
-            opts.append("--shm-size=%s" % cfg.shm_size)
+            opts.append("--shm-size=%s" % shlex.quote(cfg.shm_size))
         if cfg.network:
-            opts.append("--network %s" % cfg.network)
+            logger.debug("DockerExecutor using network: %s", cfg.network)
+            opts.append("--network=%s" % shlex.quote(cfg.network))
         if cfg.user:
             if cfg.user == "$SHELL_USER":
                 opts.extend(["--user", "$(id -u):$(id -g)"])
                 opts.extend(["-v", "/etc/passwd:/etc/passwd:ro"])
                 opts.extend(["-v", "/etc/group:/etc/group:ro"])
                 opts.extend(["-e", "HOME=/tmp"])
             else:
-                opts.extend(["--user", cfg.user])
+                opts.extend(["--user", shlex.quote(cfg.user)])
         if cfg.security_opt:
             for opt in cfg.security_opt:
-                opts.extend(["--security-opt", opt])
+                opts.extend(["--security-opt", shlex.quote(opt)])
         if cfg.cap_add:
             for cap in cfg.cap_add:
-                opts.extend(["--cap-add", cap])
+                opts.extend(["--cap-add", shlex.quote(cap)])
         if cfg.ulimit:
             for ul in cfg.ulimit:
-                opts.extend(["--ulimit", ul])
+                opts.extend(["--ulimit", shlex.quote(ul)])
         if cfg.devices:
             for dev in cfg.devices:
-                opts.extend(["--device", dev])
+                opts.extend(["--device", shlex.quote(dev)])
         if cfg.memory_limit:
-            opts.append("--memory=%s" % cfg.memory_limit)
+            opts.append("--memory=%s" % shlex.quote(cfg.memory_limit))
 
         return opts
 
@@ -84,23 +86,23 @@ def run_cmd(
             parts.extend(["--restart", cfg.restart_policy])
 
         if container_name:
-            parts.extend(["--name", container_name])
+            parts.extend(["--name", shlex.quote(container_name)])
 
         if env:
             for key, value in sorted(env.items()):
-                parts.extend(["-e", "%s=%s" % (key, value)])
+                parts.extend(["-e", shlex.quote("%s=%s" % (key, value))])
 
         if volumes:
             for host_path, container_path in sorted(volumes.items()):
-                parts.extend(["-v", "%s:%s" % (host_path, container_path)])
+                parts.extend(["-v", shlex.quote("%s:%s" % (host_path, container_path))])
 
         if extra_opts:
             parts.extend(extra_opts)
 
         parts.append(shlex.quote(image))
 
         if command:
-            parts.append(command)
+            parts.extend(["bash", "-c", shlex.quote(b64_wrap_bash(command))])
 
         result = " ".join(parts)
 
@@ -125,9 +127,8 @@ def exec_cmd(
             parts.append("-d")
         if env:
             for key, value in sorted(env.items()):
-                parts.extend(["-e", "%s=%s" % (key, value)])
-        escaped_cmd = command.replace("'", "'\\''")
-        parts.extend([shlex.quote(container_name), "bash", "-c", "'%s'" % escaped_cmd])
+                parts.extend(["-e", shlex.quote("%s=%s" % (key, value))])
+        parts.extend([shlex.quote(container_name), "bash", "-c", shlex.quote(b64_wrap_bash(command))])
         return " ".join(parts)
 
     def stop_cmd(self, container_name: str, force: bool = True) -> str:
@@ -149,7 +150,7 @@ def logs_cmd(
             parts.append("-f")
         if tail is not None:
             parts.extend(["--tail", str(tail)])
-        parts.append(container_name)
+        parts.append(shlex.quote(container_name))
         return " ".join(parts)
 
     def inspect_exists_cmd(self, image: str) -> str:

src/sparkrun/scripts/exec_serve_detached.sh

@@ -1,8 +1,12 @@
 #!/bin/bash
 set -uo pipefail
 
-echo "Executing serve command in container {container_name} (detached)..."
-docker exec {container_name} bash -c "nohup bash -c '{full_cmd}' > /tmp/sparkrun_serve.log 2>&1 & echo \$! > /tmp/sparkrun_serve.pid"
+printf "Executing serve command in container '%s' (detached)...\n" "{container_name}"
+echo "--- Command ---"
+printf '%s' '{b64_cmd}' | base64 -d --
+echo -e "\n---------------"
+
+docker exec {container_name} bash -c "printf '%s' '{b64_cmd}' | base64 -d -- > /tmp/sparkrun_serve.sh && nohup bash --noprofile --norc /tmp/sparkrun_serve.sh > /tmp/sparkrun_serve.log 2>&1 & echo \$! > /tmp/sparkrun_serve.pid"
 
 # Watchdog: when serve process exits, kill sleep infinity (PID 1) so container exits
 docker exec -d {container_name} bash -c 'SERVE_PID=$(cat /tmp/sparkrun_serve.pid); while kill -0 $SERVE_PID 2>/dev/null; do sleep 5; done; kill 1'

src/sparkrun/scripts/exec_serve_foreground.sh

@@ -1,5 +1,9 @@
 #!/bin/bash
 set -uo pipefail
 
-echo "Executing serve command in container {container_name}..."
-docker exec {container_name} bash -c '{full_cmd}'
+printf "Executing serve command in container '%s'...\n" "{container_name}"
+echo "--- Command ---"
+printf '%s' '{b64_cmd}' | base64 -d --
+echo -e "\n---------------"
+
+docker exec {container_name} bash -c "printf '%s' '{b64_cmd}' | base64 -d -- > /tmp/sparkrun_serve.sh && bash --noprofile --norc /tmp/sparkrun_serve.sh"

src/sparkrun/utils/shell.py

@@ -5,10 +5,32 @@
 
 from __future__ import annotations
 
+import base64
 import re
 import shlex
 
 
+def b64_encode_cmd(cmd: str) -> str:
+    """Base64 encode a command string to avoid shell escaping issues.
+
+    Useful when passing complex commands (e.g., with nested quotes or JSON)
+    across SSH boundaries or into ``docker exec``.
+    """
+    return base64.b64encode(cmd.encode("utf-8")).decode("utf-8")
+
+
+def b64_wrap_bash(cmd: str) -> str:
+    """Wrap a command in a base64 pipeline that decodes and executes via bash.
+
+    Produces a string like: ``printf '%s' <b64> | base64 -d -- | bash``
+    """
+    b64_cmd = b64_encode_cmd(cmd)
+    # Using printf instead of echo is safer against strings starting with dashes.
+    # Adding -- to base64 -d prevents interpretation of the b64 string as flags.
+    # Using --noprofile --norc with bash ensures a clean execution environment.
+    return f"printf '%s' '{b64_cmd}' | base64 -d -- | bash --noprofile --norc"
+
+
 def quote(value: str) -> str:
     """Return a shell-safe quoted version of *value*.