Merge pull request #170 from codex-bot-andresmgsl/build/169-exec-newlines

fix: preserve multiline box exec commands

CHANGELOG.md

@@ -5,6 +5,10 @@ which records not just what changed but what each drill run proved.
 
 ## Unreleased
 
+### Fixed
+
+- `box exec` preserves newlines and command argv across its login-user boundary (#169)
+
 ### Changed
 
 - Release and repository governance now use the shared ceremony pinned at `0.1.0` (heavy-duty/ceremony#14)

bin/box

@@ -1845,13 +1845,18 @@ box_user() {
 }
 
 cmd_shell() { incus exec "$inst" -- sudo -u "$(box_user "$inst")" -i; }
-cmd_exec()  { incus exec "$inst" -- sudo -u "$(box_user "$inst")" -i "${args[@]:1}"; }
+# sudo -i joins its command argv into one shell string. In that join, a
+# backslash-newline becomes a shell continuation and silently deletes the
+# newline from a multi-line `box exec` payload (#169). Keep the login
+# environment explicitly, but let the inner shell exec the original argv.
+cmd_exec()  { incus exec "$inst" -- sudo -u "$(box_user "$inst")" -H bash -lc 'cd ~ && exec "$@"' _ "${args[@]:1}"; }
 
 # A shell is a child of the exec connection: drop the terminal and everything
 # in it is SIGHUP'd — a long Claude run dies with it. tmux 'new-session -A'
 # attaches when the session exists and creates it when it doesn't, so starting
 # work and reattaching after a disconnect are the same command. 'shell' stays
-# bare on purpose — two verbs, two contracts.
+# bare on purpose — two verbs, two contracts. Unlike cmd_exec, tmux has no
+# caller-supplied command payload; its session name is validated below.
 cmd_tmux() {
   local session="${args[1]:-main}"
   case "$session" in

drill/drill.sh

@@ -71,8 +71,8 @@ wait_box() {   # poll until exec answers (the VM agent can take a while), ~4 min
 # Read from inside a box WITHOUT ever hanging the drill.
 #
 # Two traps, both hit for real:
-#   · 'box exec' becomes 'sudo -u <template user> -i' — a LOGIN zsh (oh-my-zsh and
-#     all). Fine for a person, needless machinery for a probe.
+#   · 'box exec' crosses a login-user shell boundary. Fine for a person,
+#     needless machinery for a probe.
 #   · $( ) waits for stdout to CLOSE, not for the command to exit. A grandchild
 #     inheriting the exec session's stdout keeps the substitution open forever,
 #     and 'timeout' does not save you: it kills the wrapper, not the holder of

drill/multiuser.sh

@@ -202,6 +202,31 @@ fi
 rm -f "$mintlog"
 as_u "$U1" box list 2>/dev/null | grep -q '^mine ' && ok "(b) box list shows mine" || no "(b) box list does not show mine"
 as_u "$U1" box exec mine -- true >/dev/null 2>&1 && ok "(b) box exec mine -- true" || no "(b) box exec failed"
+
+# #169: compare the new explicit login-user boundary with sudo -i while the
+# latter is still safe (one fixed `env` argv), then exercise the exact
+# silent-success multiline shape that sudo -i used to corrupt.
+target_user="$(as_u "$U1" incus config get mine user.box.user)"
+legacy_env="$(as_u "$U1" incus exec mine -- sudo -u "$target_user" -i env \
+  | grep -E '^(USER|LOGNAME|SHELL|HOME|PWD|PATH)=' | sort)"
+exec_env="$(as_u "$U1" box exec mine -- env \
+  | grep -E '^(USER|LOGNAME|SHELL|HOME|PWD|PATH)=' | sort)"
+[ "$exec_env" = "$legacy_env" ] \
+  && ok "(b) box exec preserves cross-user login environment (USER/LOGNAME/SHELL/HOME/PWD/PATH)" \
+  || {
+    no "(b) box exec changed the cross-user login environment (#169)"
+    diff -u <(printf '%s\n' "$legacy_env") <(printf '%s\n' "$exec_env") | sed 's/^/        /' || true
+  }
+as_u "$U1" box exec mine -- bash -lc '
+  set -o errexit -o nounset -o pipefail
+  touch /tmp/box-169-step-one
+  touch /tmp/box-169-step-two
+' >/dev/null 2>&1 \
+  && as_u "$U1" box exec mine -- test -f /tmp/box-169-step-one \
+  && as_u "$U1" box exec mine -- test -f /tmp/box-169-step-two \
+  && ok "(b) box exec preserves multiline commands, including the silent-success set shape" \
+  || no "(b) box exec corrupted a multiline command (#169)"
+
 as_u "$U1" box info mine 2>/dev/null | grep -qF "$(boxnet_pfx)" \
   && ok "(g) box info shows a boxnet ($(boxnet_pfx)x) address — placed on the hardened network" \
   || no "(g) mine has no boxnet address in box info"

test/cli.sh

@@ -68,6 +68,55 @@ check "unknown flag on list exits 2"           2 "unknown option"   "$BOX" list
 # A flag that needs a value and gets none.
 check "--name with no value exits 2"           2 "--name needs a value" "$BOX" new --name
 
+# ---------------------------------------------------------------------------
+# box exec — preserve command argv across the login-environment boundary
+# (#169). `sudo -i <command...>` joins argv into one shell string; its escaped
+# newline becomes a continuation, so a multi-line body can fuse into a
+# different valid command and still return 0. Drive cmd_exec through a fake
+# incus boundary that validates the wrapper shape and then executes it. This
+# test therefore fails against the old -i implementation before trusting the
+# marker files.
+# ---------------------------------------------------------------------------
+EXECFN="$(mktemp)"
+grep '^cmd_exec()' "$BOX" > "$EXECFN"
+check "box exec: cmd_exec extracted (guards the grep)" 0 "exec \"\$@\"" cat "$EXECFN"
+check "box exec: extracted function is valid bash" 0 "" bash -n "$EXECFN"
+check "box exec: command argv never rides sudo -i" 1 "" grep -q -- ' -i ' "$EXECFN"
+
+exec_fixture() { # exec_fixture <command> [arg...]
+  bash -c '
+    set -e
+    . "$0"
+    box_user() { printf "%s\n" fixture-user; }
+    incus() {
+      [ "$1" = exec ] && [ "$2" = fixture ] && [ "$3" = -- ]
+      shift 3
+      [ "$1" = sudo ] && [ "$2" = -u ] && [ "$3" = fixture-user ] &&
+        [ "$4" = -H ]
+      shift 4
+      "$@"
+    }
+    inst=fixture
+    args=(fixture "$@")
+    cmd_exec
+  ' "$EXECFN" "$@"
+}
+
+EXEC_STATE="$(mktemp -d)"
+exec_body="set -euo pipefail
+touch '$EXEC_STATE/step-one'
+touch '$EXEC_STATE/step-two'"
+check "box exec: silent-success multiline body executes each statement" 0 "" \
+  exec_fixture bash -lc "$exec_body"
+check "box exec: multiline step one was not fused into set argv" 0 "" \
+  test -f "$EXEC_STATE/step-one"
+check "box exec: multiline step two was not fused into set argv" 0 "" \
+  test -f "$EXEC_STATE/step-two"
+check "box exec: plain argv remains separate" 0 "one argument" \
+  exec_fixture printf '%s\n' "one argument"
+rm -rf "$EXEC_STATE"
+rm -f "$EXECFN"
+
 # ---------------------------------------------------------------------------
 # A shim `id` on PATH: lets us drive install.sh's DEST branch with a canned uid +
 # group output, exactly the way rig drives assert_runner_repo against fixtures.