Răsfoiți Sursa

fix: shell-quote entrypoint script so 'sh -c SCRIPT' survives bash word-split

The test-sleep recipe's heartbeat loop worked when invoked directly via
ssh but failed when run through cargo run serve on the cluster: the
container would exit in <1s with no logs.

Root cause: the docker command is built as a Vec<String> then joined
with spaces and handed to ssh as a single bash string. The session on
the SSH server runs 'bash -c "docker run ... sh -c SCRIPT ..."' which
word-splits the script: bash passes 'sh -c i=0;' (and stops) to docker,
then docker passes that to the container's sh, which runs 'i=0;' and
exits immediately. The heartbeat loop body was never executed.

Fix: detect 'sh -c SCRIPT' in the entrypoint and shell-quote the
script with single quotes so bash treats the whole script as one argv
element. Other args ('-e K=V', '--tensor-parallel-size 1', etc.) are
fine to leave unquoted because each flag + value naturally becomes a
distinct argv element after bash word-splitting.

Verified end-to-end against the 4nodes-switch cluster:
  cargo run serve --recipe test-sleep --cluster 2nodes-dac --keep
    => containers on dgx1.lan and dgx2.lan both produce heartbeats
  cargo run logs --recipe test-sleep
    => shows last 100 lines per node with docker timestamps
  cargo run stop --recipe test-sleep
    => cleans up

Three new unit tests:
- shq_wraps_simple_strings
- shq_escapes_inner_single_quotes
- entrypoint_script_is_shell_quoted
Alexander Korolev 4 luni în urmă
părinte
comite
d92da29eb5
2 a modificat fișierele cu 68 adăugiri și 1 ștergeri
  1. 29 1
      src/docker/mod.rs
  2. 39 0
      src/docker/tests.rs

+ 29 - 1
src/docker/mod.rs

@@ -77,6 +77,25 @@ pub(crate) fn logs_follow_cmd(recipe_name: &str) -> String {
     )
 }
 
+/// Shell-quote `s` so it survives being embedded in a `bash -c` argument.
+/// Wraps the string in single quotes, escaping any embedded single quotes
+/// as `'\''`.  Used to make `build_docker_command` safe against args that
+/// contain whitespace or shell metacharacters (most importantly, the
+/// entrypoint script passed to `sh -c`).
+fn shq(s: &str) -> String {
+    let mut out = String::with_capacity(s.len() + 2);
+    out.push('\'');
+    for c in s.chars() {
+        if c == '\'' {
+            out.push_str(r"'\''");
+        } else {
+            out.push(c);
+        }
+    }
+    out.push('\'');
+    out
+}
+
 /// Shows Docker logs for a recipe across all nodes (or follows head node).
 pub async fn show_logs(
     config: &Config,
@@ -384,7 +403,16 @@ fn build_docker_command(
     }
     cmd.push(recipe.docker_image.clone());
     if !recipe.entrypoint.is_empty() {
-        cmd.extend(recipe.entrypoint.clone());
+        let mut ep = recipe.entrypoint.clone();
+        // If the entrypoint is `sh -c SCRIPT`, shell-quote the script so
+        // bash on the SSH server treats it as one argv element to docker.
+        // Without this, bash word-splits the script and sh -c only sees
+        // the first token, then the container exits immediately.
+        if ep.len() >= 3 && ep[ep.len() - 2] == "-c" {
+            let last = ep.len() - 1;
+            ep[last] = shq(&ep[last]);
+        }
+        cmd.extend(ep);
     }
 
     // Model argument - engine-specific

+ 39 - 0
src/docker/tests.rs

@@ -343,3 +343,42 @@ fn logs_follow_cmd_format() {
     // Follow should not include --tail (we want a live stream from now).
     assert!(!cmd.contains("--tail"));
 }
+
+// ── Shell quoting ─────────────────────────────────────────────
+
+#[test]
+fn shq_wraps_simple_strings() {
+    assert_eq!(shq("foo"), "'foo'");
+    assert_eq!(shq(""), "''");
+    assert_eq!(shq("foo bar"), "'foo bar'");
+}
+
+#[test]
+fn shq_escapes_inner_single_quotes() {
+    // "don't" → 'don'\''t'
+    assert_eq!(shq("don't"), "'don'\\''t'");
+}
+
+#[test]
+fn entrypoint_script_is_shell_quoted() {
+    // sh -c SCRIPT should wrap the script in single quotes so it survives
+    // bash word-splitting on the SSH server.
+    let mut r = trec();
+    r.entrypoint = vec!["sh".into(), "-c".into(), "echo $i; sleep 1".into()];
+    let cmd = run(&r, &tcls().nodes[0], true);
+    // The script element should appear wrapped in single quotes
+    assert!(
+        cmd.contains("'echo $i; sleep 1'"),
+        "script should be shell-quoted, got: {cmd}"
+    );
+}
+
+#[test]
+fn entrypoint_script_non_sh_passes_through() {
+    // For non `sh -c` entrypoints (e.g. ["sh", "-c", "...", "extra"]) or
+    // direct binaries, we leave the args alone — no false positive shq.
+    let mut r = trec();
+    r.entrypoint = vec!["sh".into(), "-c".into()];
+    let cmd = run(&r, &tcls().nodes[0], true);
+    assert!(cmd.contains("docker run"));
+}