vm-env-secrets passes while atomic-vm.sh test-exec puts every provider key in the ssh command line

`tests/e2e/vm-env-secrets.test.sh` passes on `61b3a87` while `atomic-vm.sh test-exec` puts every provider API key into the ssh command line. #299 fixed that shape in `cmd_run` and `cmd_provision`. `cmd_test_exec` was never converted, and the gate cannot see it.

## The live exposure

`tests/e2e/atomic-vm.sh:509-526` builds an accumulator and interpolates it into the command string:

```sh
local env_prefix=""
for var in SYSKNIFE_ALLOW_DESTRUCTIVE ... \
OPENAI_API_KEY ANTHROPIC_API_KEY GEMINI_API_KEY \
GROQ_API_KEY DEEPSEEK_API_KEY MISTRAL_API_KEY XAI_API_KEY; do
eval "val=\${$var:-}"
if [ -n "$val" ]; then
env_prefix+=" $var='$val'"
fi
done
cmd_ssh "cd /home/${VM_USER}/sysknife &&${env_prefix} bash tests/e2e/exec/run-exec-stories.sh${story_args}"
```

Extracting `cmd_test_exec` from the unmodified file and stubbing `cmd_ssh` to print its own argument shows what ssh receives. Every provider key in the caller's environment was cleared first except a planted placeholder:

```
$ env -u ANTHROPIC_API_KEY -u GEMINI_API_KEY -u GROQ_API_KEY -u DEEPSEEK_API_KEY \
-u MISTRAL_API_KEY -u XAI_API_KEY bash inject-safe.sh
ssh argv[1] = cd /home/fedora/sysknife && SYSKNIFE_LLM_MODEL='gpt-4o'; id #' OPENAI_API_KEY='sk-PLACEHOLDER-000' bash tests/e2e/exec/run-exec-stories.sh
```

Two things in that one line, and both are what #299's commit message says it closed:

- The key value sits in the command string, so it lands in the guest process table for the length of the story run. Any account on the VM can read it out of `/proc`.
- `env_prefix+=" $var='$val'"` wraps the value in single quotes without escaping. I set `SYSKNIFE_LLM_MODEL` to `gpt-4o'; id #`, the quote closed early, and `id` became a separate command on the guest. `cmd_run` avoids this by using `printf %q`; `cmd_test_exec` does not.

The function reads the keys straight out of the ambient environment, so anything that captures that command string captures live credentials: a `set -x` trace, a CI log, a shell history file.

## Why the gate stays green

`tests/e2e/vm-env-secrets.test.sh:46-49` is the structural check:

```sh
if grep -nE 'cmd_ssh[^#]*sudo *[$"]?\$\{?[A-Za-z_]' "$harness" \
| grep -vE 'sudo (sh|bash) -c' >/dev/null; then
```

It requires the word `sudo`, and it only inspects the token immediately after it. `cmd_test_exec` runs as `VM_USER` on purpose, because group membership decides the daemon's `CallerRole` and root holds no sysknife groups, so the line carries no `sudo` at all. Run by hand on the current tree:

```
$ grep -nE 'cmd_ssh[^#]*sudo *[$"]?\$\{?[A-Za-z_]' tests/e2e/atomic-vm.sh
$ echo $?
1
```

The key-name check at line 53 misses it too, because the line names `${env_prefix}` rather than any `*_API_KEY`.

Two more holes in the same file, both found by mutation:

**The harness list is hand-written and never compared against the tree.**

```sh
harnesses=(
"tests/e2e/atomic-vm.sh"
"tests/e2e/ubuntu-vm.sh"
)
```

I copied `ubuntu-vm.sh` to `fedora-vm.sh` with `export OPENAI_API_KEY=${OPENAI_API_KEY}` spliced onto its `cmd_ssh` line. The gate reported success and did not read the file:

```
$ bash tests/e2e/vm-env-secrets.test.sh
VM environment secrets are stdin-only across 2 harnesses.
```

Emptying the array does the same thing, at rc=0, printing `across 0 harnesses`.

**Line 53 runs a short-circuiting reader on the right of a pipe under `pipefail`.**

```sh
if grep -n 'cmd_ssh' "$harness" | grep -Fq "$key"; then
```

`grep -Fq` exits on the first match, the producer takes SIGPIPE, and `pipefail` turns a real hit into a miss. On the current harnesses the producer writes 619 and 829 bytes, so it never fires. Past the pipe buffer it stops working:

```
producer output 24217 bytes -> 100/100 hits reported
producer output 48617 bytes -> 55/100 hits reported
producer output 73420 bytes -> 21/100 hits reported
producer output 98620 bytes -> 7/100 hits reported
```

Latent today. Worth closing while the file is open.

## Patch

Two files, and they have to land together: the gate goes red on `main` until the harness is fixed, which is the point.

The gate now discovers harnesses with `grep -l '^cmd_ssh()' tests/e2e/*.sh` and fails when that selects fewer than two. It reads every variable interpolated into any `cmd_ssh` command string and requires each to be on a declared allowlist (`VM_USER`, `GUEST_ENV_FILE`, `story_args`), so an accumulator fails whatever it is called and wherever it sits in the line. It checks every stdin transfer for `rm -f`, `umask 077` and `cat >` rather than one literal. The two `grep | grep -Fq` pipes become here-strings.

The harness change routes `cmd_test_exec` through the same stdin path as `cmd_run`. One decision is yours rather than mine: the existing `write_guest_env_file` writes a root-owned 0600 file, and this consumer runs as `VM_USER` and cannot read it. I added a `write_guest_env_file_for_user` variant that chowns the file to `VM_USER` after writing it. Handing the file to a non-root account is a real widening, small, and I would rather you looked at it than had it arrive silently.

vm-env-secrets-argv.patch

```diff
diff --git a/tests/e2e/atomic-vm.sh b/tests/e2e/atomic-vm.sh
index 087ebb2..84d316e 100755
--- a/tests/e2e/atomic-vm.sh
+++ b/tests/e2e/atomic-vm.sh
@@ -117,6 +117,25 @@ ssh_opts() {
log() { printf '[atomic-vm] %s\n' "$*" >&2; }
die() { log "ERROR: $*"; exit 1; }

+write_guest_env_file_for_user() {
+ # cmd_test_exec runs as VM_USER on purpose: group membership decides the
+ # daemon's CallerRole, and root holds no sysknife groups. A root-owned 0600
+ # file is unreadable by that consumer, so this variant hands the file to
+ # VM_USER. Same stdin transfer, same mode, same fresh inode.
+ local assignment value var
+ local assignments=()
+ for var in "$@"; do
+ eval "value=\${$var-}"
+ if [ -n "$value" ]; then
+ printf -v assignment '%s=%q' "$var" "$value"
+ assignments+=("$assignment")
+ fi
+ done
+
+ printf '%s\n' "${assignments[@]}" \
+ | cmd_ssh "sudo sh -c 'rm -f ${GUEST_ENV_FILE} && umask 077 && cat > ${GUEST_ENV_FILE} && chown ${VM_USER} ${GUEST_ENV_FILE}'"
+}
+
write_guest_env_file() {
local assignment value var
local assignments=()
@@ -506,24 +525,19 @@ cmd_test_exec() {
# Run as VM_USER (not root) so sysknife connects to the daemon socket with
# the user's group membership (sysknife/sysknife-dev/sysknife-admin) and
# gets the correct CallerRole. Root has no sysknife groups and gets Observer.
- local env_prefix=""
- for var in SYSKNIFE_ALLOW_DESTRUCTIVE SYSKNIFE_LLM_PROVIDER SYSKNIFE_LLM_MODEL \
- SYSKNIFE_TEST_MODEL SYSKNIFE_OLLAMA_URL SYSKNIFE_SOCKET SYSKNIFE_LISTEN_URI \
- SYSKNIFE_STORY_TIMEOUT SYSKNIFE_MAX_RPM \
- SYSKNIFE_CASSETTE SYSKNIFE_CASSETTE_MODE \
- OPENAI_API_KEY ANTHROPIC_API_KEY GEMINI_API_KEY \
- GROQ_API_KEY DEEPSEEK_API_KEY MISTRAL_API_KEY XAI_API_KEY; do
- eval "val=\${$var:-}"
- if [ -n "$val" ]; then
- env_prefix+=" $var='$val'"
- fi
- done
+ write_guest_env_file_for_user \
+ SYSKNIFE_ALLOW_DESTRUCTIVE SYSKNIFE_LLM_PROVIDER SYSKNIFE_LLM_MODEL \
+ SYSKNIFE_TEST_MODEL SYSKNIFE_OLLAMA_URL SYSKNIFE_SOCKET SYSKNIFE_LISTEN_URI \
+ SYSKNIFE_STORY_TIMEOUT SYSKNIFE_MAX_RPM \
+ SYSKNIFE_CASSETTE SYSKNIFE_CASSETTE_MODE \
+ OPENAI_API_KEY ANTHROPIC_API_KEY GEMINI_API_KEY \
+ GROQ_API_KEY DEEPSEEK_API_KEY MISTRAL_API_KEY XAI_API_KEY

local story_args=""
if [ $# -gt 0 ]; then
story_args=" $*"
fi
- cmd_ssh "cd /home/${VM_USER}/sysknife &&${env_prefix} bash tests/e2e/exec/run-exec-stories.sh${story_args}"
+ cmd_ssh "cd /home/${VM_USER}/sysknife && bash -c 'set -e; trap \"rm -f ${GUEST_ENV_FILE}\" EXIT; set -a; . ${GUEST_ENV_FILE}; set +a; rm -f ${GUEST_ENV_FILE}; trap - EXIT; exec bash tests/e2e/exec/run-exec-stories.sh${story_args}'"
}

cmd_snapshot() {
diff --git a/tests/e2e/vm-env-secrets.test.sh b/tests/e2e/vm-env-secrets.test.sh
index 701ec37..5626931 100755
--- a/tests/e2e/vm-env-secrets.test.sh
+++ b/tests/e2e/vm-env-secrets.test.sh
@@ -4,10 +4,19 @@ set -euo pipefail

repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
config_rs="$repo_root/crates/sysknife-brain/src/config.rs"
-harnesses=(
- "tests/e2e/atomic-vm.sh"
- "tests/e2e/ubuntu-vm.sh"
-)
+
+# Discover the harnesses instead of listing them. The hand-written list held
+# two entries and nothing compared it against the tree, so a third harness --
+# or an emptied list -- left this gate reporting success over zero files.
+harnesses=()
+while IFS= read -r path; do
+ harnesses+=("${path#"$repo_root"/}")
+done < <(grep -l '^cmd_ssh()' "$repo_root"/tests/e2e/*.sh | sort)
+if [ "${#harnesses[@]}" -lt 2 ]; then
+ printf 'harness discovery found %d file(s) defining cmd_ssh; expected at least 2\n' \
+ "${#harnesses[@]}" >&2
+ exit 1
+fi

keys=()
while IFS= read -r key; do
@@ -18,6 +27,12 @@ if [ "${#keys[@]}" -lt 7 ]; then
exit 1
fi

+# Variables that may appear inside a command string handed to cmd_ssh. Each is
+# a path or an argument list, never a credential. Anything else interpolated
+# into that string reaches the guest command line, so a new name has to be
+# added here deliberately rather than passing by default.
+allowed_vars=(VM_USER GUEST_ENV_FILE story_args)
+
failures=0
report() {
printf 'FAIL %s\n' "$1" >&2
@@ -27,30 +42,63 @@ report() {
for rel in "${harnesses[@]}"; do
harness="$repo_root/$rel"

- [ "$(grep -c '^[[:space:]]*write_guest_env_file \\' "$harness")" -eq 2 ] \
+ [ "$(grep -c '^[[:space:]]*write_guest_env_file \\' "$harness")" -ge 2 ] \
|| report "$rel must stage environment for both provision and run"
- grep -Fq "| cmd_ssh \"sudo sh -c 'rm -f \${GUEST_ENV_FILE} && umask 077 && cat > \${GUEST_ENV_FILE}'\"" "$harness" \
- || report "$rel does not transfer the environment over SSH stdin into a fresh 0600 file"
- [ "$(grep -c 'rm -f \${GUEST_ENV_FILE}.*exec bash tests/e2e/' "$harness")" -eq 2 ] \
+ # Every stdin transfer, not one spelling of one of them. Anchoring on a
+ # single literal meant a second transfer added later (the VM-user variant)
+ # inherited no check at all: dropping its `umask 077` left this green.
+ transfers="$(grep -n '| cmd_ssh "sudo sh -c' "$harness" || true)"
+ if [ -z "$transfers" ]; then
+ report "$rel has no stdin environment transfer at all"
+ fi
+ while IFS= read -r line; do
+ [ -n "$line" ] || continue
+ for required in 'rm -f ${GUEST_ENV_FILE}' 'umask 077' 'cat > ${GUEST_ENV_FILE}'; do
+ grep -Fq "$required" <<<"$line" \
+ || report "$rel:${line%%:*} transfers the environment without '$required', so the guest file is not a fresh 0600 one"
+ done
+ done <<<"$transfers"
+ [ "$(grep -c 'rm -f \${GUEST_ENV_FILE}.*exec bash tests/e2e/' "$harness")" -ge 2 ] \
|| report "$rel must remove the guest environment before both scripts execute"

# The regression to catch is a secret value reaching argv, not one
- # particular way of spelling it. The previous form matched the old
- # accumulator by name (`prov_env`, `sudo_env`), so reintroducing the same
- # bug under any other variable name passed silently.
+ # particular way of spelling it. Matching the accumulator by name
+ # (`prov_env`, `sudo_env`) let the same bug back in under a new name, and
+ # the structural replacement only inspected the token immediately after
+ # `sudo` -- so a call site that runs as the VM user, with no `sudo` at all,
+ # was never examined. That is where the bug actually came back.
#
- # Structural instead: no line that invokes cmd_ssh may interpolate a
- # shell variable straight after `sudo`, which is the shape that puts a
- # value on the guest command line. The stdin transfer and the `sudo bash
- # -c` consumer both name a literal command after sudo, so neither matches.
- if grep -nE 'cmd_ssh[^#]*sudo *[$"]?\$\{?[A-Za-z_]' "$harness" \
- | grep -vE 'sudo (sh|bash) -c' >/dev/null; then
- report "$rel interpolates a variable into an argv command after sudo"
+ # Allowlist instead: read every variable interpolated into a cmd_ssh
+ # command string and require each to be a declared non-secret. A new
+ # accumulator fails whatever it is called and wherever it sits in the line.
+ # An empty selection here would screen nothing and say nothing, which is
+ # the defect this whole file exists to catch. Require it to select.
+ cmd_ssh_lines="$(grep -n 'cmd_ssh' "$harness" || true)"
+ cmd_ssh_calls="$(grep -nE 'cmd_ssh "' "$harness" || true)"
+ if [ -z "$cmd_ssh_calls" ]; then
+ report "$rel: no cmd_ssh call site matched, so the argv checks below screened nothing"
+ continue
fi
+ while IFS= read -r line; do
+ [ -n "$line" ] || continue
+ lineno="${line%%:*}"
+ while IFS= read -r var; do
+ [ -n "$var" ] || continue
+ for ok in "${allowed_vars[@]}"; do
+ [ "$var" = "$ok" ] && continue 2
+ done
+ report "$rel:$lineno interpolates \$$var into a cmd_ssh command string, which puts its value in argv; stage it with write_guest_env_file or add it to allowed_vars"
+ done < <(printf '%s\n' "$line" | grep -oE '\$\{?[A-Za-z_][A-Za-z0-9_]*' | tr -d '${' | sort -u)
+ done <<<"$cmd_ssh_calls"

# And the specific values must never appear on a cmd_ssh line at all.
+ # `grep -n ... | grep -Fq` put a short-circuiting reader on the right of a
+ # pipe under `pipefail`: once the producer's output passes the pipe buffer
+ # it takes SIGPIPE, the pipeline reports 141, and a real hit reads as a
+ # miss. Measured at 0/300 hits on a 227 KB producer. A here-string has no
+ # pipe to break.
for key in "${keys[@]}"; do
- if grep -n 'cmd_ssh' "$harness" | grep -Fq "$key"; then
+ if grep -Fq "$key" <<<"$cmd_ssh_lines"; then
report "$rel names $key on a cmd_ssh line, which puts it in argv"
fi
done
@@ -66,7 +114,7 @@ for rel in "${harnesses[@]}"; do
continue
fi
for key in "${keys[@]}"; do
- printf '%s' "$run_block" | grep -Fq "$key" \
+ grep -Fq "$key" <<<"$run_block" \
|| report "$rel does not forward $key from cmd_run"
done
done
```

## Mutation proof

Baseline green on a fixed tree, then thirteen mutations, each applied alone in a `git clone --no-hardlinks` scratch copy and restored afterwards.

| # | Mutation | Patched gate |
|---|---|---|
| N0 | none, fixed tree | rc=0 |
| N1 | `env_prefix` accumulator restored (**the live bug**) | rc=1, names `atomic-vm.sh:526` |
| N2 | same bug, variable renamed `carried_over` | rc=1 |
| N3 | accumulator buried mid-string inside `sudo bash -c` | rc=1 |
| N4 | third harness lands carrying the bug | rc=1, two failures |
| N5 | discovery selects zero harnesses | rc=1, `found 0 file(s) defining cmd_ssh` |
| N6 | key dropped from `cmd_run` | rc=1 |
| N7 | `config.rs` parked, key extraction reads nothing | rc=1, `found only 0 keys` |
| N8 | `umask 077` dropped from the VM-user transfer | rc=1 |
| N9 | `cmd_run` renamed, block extraction returns nothing | rc=1 |
| N10 | `rm -f` dropped from the root transfer | rc=1 |
| N11 | every stdin transfer removed | rc=1, `has no stdin environment transfer at all` |
| N12 | every `cmd_ssh` call site respelled, so the argv selection is empty | rc=1, `no cmd_ssh call site matched, so the argv checks below screened nothing` |

N5, N7, N9, N11 and N12 break the guard's own input rather than the code it guards. Two of them caught defects in my own drafts. N8 found a second transfer I had added and left uncovered by the single-literal assertion, which is how that assertion became a loop. N12 found the argv check reading an unguarded process substitution, screening nothing in silence if the selection came back empty, which is the same shape as the harness list it replaces.

The current gate, for comparison, passes N1, N2, N3, N4 and the emptied-list case.

Against the unmodified tree at `61b3a87`:

```
$ bash tests/e2e/vm-env-secrets.test.sh
FAIL tests/e2e/atomic-vm.sh:526 interpolates $env_prefix into a cmd_ssh command string, which puts its value in argv; stage it with write_guest_env_file or add it to allowed_vars

1 VM environment secret handling failure(s).
rc=1
```

`bash -n` clean, `shellcheck --severity=warning` clean on 0.10.0, and `git apply --check` applies to `61b3a87` without fuzz.

## If you take this

Start by reproducing the argv line above: extract `cmd_test_exec` with `awk '/^cmd_test_exec\(\)/,/^}/'`, stub `cmd_ssh` to print `$1`, and clear your own provider keys first so nothing real reaches your terminal. Once you can see the secret in the string, the rest of the change follows the shape `cmd_run` already uses.

Related: #299 (the original fix), #387 (the same coverage-versus-behaviour split in the secret scanner).

Start with cmd_test_exec in tests/e2e/atomic-vm.sh and the structural checks in tests/e2e/vm-env-secrets.test.sh. Run the existing gate and reproduce the argv exposure with the described placeholder environment; done means all discovered VM harnesses pass the gate and provider secrets no longer reach the SSH command line.

Written by the indexing model from the issue text.