From 9914bfc5941699a065b30f16a726f7faac2e02a8 Mon Sep 17 00:00:00 2001
From: Ben <ben@nousresearch.com>
Date: Sat, 23 May 2026 15:31:46 +1000
Subject: [PATCH] docker: drop sh -c wrappers from stage2-hook.sh
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

PR #30136 review caught: three `s6-setuidgid hermes sh -c "..."`
invocations in stage2-hook.sh interpolated $HERMES_HOME into a
nested shell context. Practically low-risk (a malicious HERMES_HOME
already requires container-launch privileges) but the cleaner
pattern is to invoke commands directly so the shell isn't a second
interpreter.

* `mkdir -p` of the data subdirs now runs directly via s6-setuidgid,
  one path per arg.
* The .install_method stamp is written via `printf | tee` — also no
  shell wrapper.
* The skills_sync invocation uses the venv's python by absolute path
  instead of sourcing activate inside a shell. skills_sync.py doesn't
  need anything from activate beyond sys.path, which the bin-stub
  python already provides.

No behavior change. Just a smaller attack surface and a script
that's easier to read.
---
 docker/stage2-hook.sh | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/docker/stage2-hook.sh b/docker/stage2-hook.sh
index 2989f27a032..6a5bedc9f6d 100755
--- a/docker/stage2-hook.sh
+++ b/docker/stage2-hook.sh
@@ -75,15 +75,29 @@ fi
 # --- Seed directory structure as hermes user ---
 # Run as hermes via s6-setuidgid so dirs end up owned correctly (matters
 # under rootless Podman where chown back to root would fail).
-s6-setuidgid hermes sh -c "mkdir -p \"$HERMES_HOME\"/cron \
-    \"$HERMES_HOME\"/sessions \"$HERMES_HOME\"/logs \"$HERMES_HOME\"/hooks \
-    \"$HERMES_HOME\"/memories \"$HERMES_HOME\"/skills \"$HERMES_HOME\"/skins \
-    \"$HERMES_HOME\"/plans \"$HERMES_HOME\"/workspace \"$HERMES_HOME\"/home"
+#
+# Use direct `mkdir -p` invocation (no `sh -c "..."` wrapper) so the
+# shell isn't a second interpreter — defends against $HERMES_HOME values
+# containing shell metacharacters. PR #30136 review item O2.
+s6-setuidgid hermes mkdir -p \
+    "$HERMES_HOME/cron" \
+    "$HERMES_HOME/sessions" \
+    "$HERMES_HOME/logs" \
+    "$HERMES_HOME/hooks" \
+    "$HERMES_HOME/memories" \
+    "$HERMES_HOME/skills" \
+    "$HERMES_HOME/skins" \
+    "$HERMES_HOME/plans" \
+    "$HERMES_HOME/workspace" \
+    "$HERMES_HOME/home"
 
 # --- Install-method stamp (read by detect_install_method() in hermes status) ---
 # Preserved from the tini-era entrypoint (PR #27843). Must be written as
 # the hermes user so ownership matches the file's documented owner.
-s6-setuidgid hermes sh -c "echo docker > \"$HERMES_HOME/.install_method\"" 2>/dev/null || true
+# tee is invoked directly via s6-setuidgid (no `sh -c` wrapper) for the
+# same shell-metacharacter safety described above.
+printf 'docker\n' | s6-setuidgid hermes tee "$HERMES_HOME/.install_method" >/dev/null \
+    || true
 
 # --- Seed config files (only on first boot) ---
 seed_one() {
@@ -107,9 +121,13 @@ if [ ! -f "$HERMES_HOME/auth.json" ] && [ -n "${HERMES_AUTH_JSON_BOOTSTRAP:-}" ]
 fi
 
 # --- Sync bundled skills ---
+# Invoke the venv's python by absolute path so we don't need a `sh -c`
+# wrapper to source the activate script. This is safe because
+# skills_sync.py doesn't depend on any environment exports beyond what
+# the python binary's own bin-stub already sets up (sys.path is rooted
+# at the venv's site-packages by virtue of running .venv/bin/python).
 if [ -d "$INSTALL_DIR/skills" ]; then
-    s6-setuidgid hermes sh -c \
-        ". $INSTALL_DIR/.venv/bin/activate && python3 $INSTALL_DIR/tools/skills_sync.py" \
+    s6-setuidgid hermes "$INSTALL_DIR/.venv/bin/python" "$INSTALL_DIR/tools/skills_sync.py" \
         || echo "[stage2] Warning: skills_sync.py failed; continuing"
 fi