From 9fde0074d2802a8845462a935c818fc279e5c990 Mon Sep 17 00:00:00 2001
From: "gfargo-horizon-agent[bot]"
 <294710345+gfargo-horizon-agent[bot]@users.noreply.github.com>
Date: Fri, 28 Aug 2026 21:01:46 +0000
Subject: [PATCH 1/3] test(keys): add coverage for keys/pull, keys/status,
 keys/discovery + recipe CMD-SHELL contract check
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- tests/test_keys_pull.bats: covers keys_pull dispatch (--from vps/containers/
  env-file/unknown), keys_pull_from_vps (dry-run masking, chmod 600, missing env
  file, VPS_HOST check, --force/confirm, --keys filter, connection failure,
  JSON output), keys_pull_from_containers (dry-run, chmod 600, error paths),
  keys_pull_from_env_file (copy, chmod 600, dry-run, missing source, fallback
  to stack-specific env file, --force), and keys_pull_help.
- tests/test_keys_status.bats: covers keys_status --json output shape
  (ssh_keys/api_keys/vps_status/env_vars fields and values from fixtures),
  keys_status text output, keys_recent --limit slicing and edge cases,
  discover_local_keys (env file detection, template_secrets count),
  and generate_recommendations (SSH rotation, GitHub review, large-secret flag).
- tests/test_recipes.bats: adds contract check (d) — any healthcheck.test that
  contains a command substitution ($(...) / $$(...)) inside an exec-form CMD
  array must use CMD-SHELL instead, so the shell actually evaluates it.
- templates/recipes/ghost/docker-compose.yml: fixes the mysql healthcheck from
  exec-form CMD (never evaluates $$(cat ...)) to CMD-SHELL, the form the
  check now enforces. The $$(cat ...) idiom reads the root password from a
  file — it was silently broken before this fix.

No production lib/ changes; no CI workflow edits needed (bats tests/ is already
auto-discovered). lib/migrate/phase-*.sh coverage (~2 100 lines) is tracked as
a separate follow-up; flagged in PR description.

Closes #404
---
 templates/recipes/ghost/docker-compose.yml |   2 +-
 tests/test_keys_pull.bats                  | 343 +++++++++++++++++++
 tests/test_keys_status.bats                | 377 +++++++++++++++++++++
 tests/test_recipes.bats                    |  28 ++
 4 files changed, 749 insertions(+), 1 deletion(-)
 create mode 100644 tests/test_keys_pull.bats
 create mode 100644 tests/test_keys_status.bats

diff --git a/templates/recipes/ghost/docker-compose.yml b/templates/recipes/ghost/docker-compose.yml
index 3ddccf7..e7f0e7d 100644
--- a/templates/recipes/ghost/docker-compose.yml
+++ b/templates/recipes/ghost/docker-compose.yml
@@ -30,7 +30,7 @@ services:
     volumes:
       - mysql_data:/var/lib/mysql
     healthcheck:
-      test: ["CMD", "mysqladmin", "ping", "-h", "localhost", "-uroot", "-p$$(cat /etc/mysql/conf.d/passwd 2>/dev/null || echo)"]
+      test: ["CMD-SHELL", "mysqladmin ping -h localhost -uroot -p$$(cat /etc/mysql/conf.d/passwd 2>/dev/null || echo)"]
       interval: 30s
 
 volumes:
diff --git a/tests/test_keys_pull.bats b/tests/test_keys_pull.bats
new file mode 100644
index 0000000..82b4735
--- /dev/null
+++ b/tests/test_keys_pull.bats
@@ -0,0 +1,343 @@
+#!/usr/bin/env bats
+# ==================================================
+# tests/test_keys_pull.bats — Tests for lib/keys/pull.sh
+# ==================================================
+# Run:  bats tests/test_keys_pull.bats
+# Covers: keys_pull dispatch (--from vps/containers/env-file/unknown),
+#         keys_pull_from_vps (dry-run masking, chmod 600, missing env file,
+#         missing VPS_HOST, --force/confirm, --keys filter),
+#         keys_pull_from_containers (dry-run masking, chmod 600),
+#         keys_pull_from_env_file (copy, chmod 600, dry-run, missing source),
+#         keys_pull_help output,
+#         security-critical: pulled file always gets mode 600.
+
+setup() {
+  export CLI_ROOT
+  CLI_ROOT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)"
+  TEST_TMP="$(mktemp -d)"
+  STACK="test-keys-pull-$$"
+  mkdir -p "$CLI_ROOT/stacks/$STACK"
+
+  source "$CLI_ROOT/lib/utils.sh"
+  fail()  { echo "$1" >&2; return 1; }
+  error() { echo "$1" >&2; }
+  warn()  { echo "$1" >&2; }
+  ok()    { echo "$1"; }
+  log()   { echo "$1"; }
+
+  # Source the full keys module (includes pull.sh, status.sh, discovery.sh, etc.)
+  source "$CLI_ROOT/lib/keys.sh"
+
+  # Stub out network-touching helpers so tests never reach a real host
+  validate_vps_connection() { return 0; }
+  export -f validate_vps_connection
+
+  # ssh stub: by default succeed and echo the last argument (the remote cmd)
+  SSH_CALL_LOG="$TEST_TMP/ssh_calls.log"
+  : > "$SSH_CALL_LOG"
+  export SSH_CALL_LOG
+  ssh() {
+    echo "${@: -1}" >> "$SSH_CALL_LOG"
+    echo "KEY_A=value_a"
+    echo "KEY_B=value_b"
+    return 0
+  }
+  export -f ssh
+
+  build_ssh_opts() { echo ""; }
+  export -f build_ssh_opts
+
+  resolve_deploy_dir() { echo "/opt/deploy"; }
+  export -f resolve_deploy_dir
+
+  confirm() { return 0; }
+  export -f confirm
+
+  # .prod.env with a VPS_HOST so most paths proceed
+  PROD_ENV="$CLI_ROOT/.prod.env"
+  printf 'VPS_HOST=fakehost\nVPS_USER=ubuntu\n' > "$PROD_ENV"
+}
+
+teardown() {
+  rm -rf "$CLI_ROOT/stacks/$STACK"
+  rm -f "$CLI_ROOT/.prod.env"
+  rm -f "$CLI_ROOT/."*"-pulled.env"
+  rm -rf "$TEST_TMP"
+}
+
+# ── keys_pull dispatch ────────────────────────────────────────────────────────
+
+@test "keys_pull: --from vps dispatches to keys_pull_from_vps" {
+  keys_pull_from_vps()       { echo "CALLED_VPS"; return 0; }
+  keys_pull_from_containers() { echo "CALLED_CONTAINERS"; return 0; }
+  keys_pull_from_env_file()  { echo "CALLED_ENV"; return 0; }
+
+  run keys_pull "$STACK" --from vps
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"CALLED_VPS"* ]]
+}
+
+@test "keys_pull: --from containers dispatches to keys_pull_from_containers" {
+  keys_pull_from_vps()       { echo "CALLED_VPS"; return 0; }
+  keys_pull_from_containers() { echo "CALLED_CONTAINERS"; return 0; }
+  keys_pull_from_env_file()  { echo "CALLED_ENV"; return 0; }
+
+  run keys_pull "$STACK" --from containers
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"CALLED_CONTAINERS"* ]]
+}
+
+@test "keys_pull: --from env-file dispatches to keys_pull_from_env_file" {
+  keys_pull_from_vps()       { echo "CALLED_VPS"; return 0; }
+  keys_pull_from_containers() { echo "CALLED_CONTAINERS"; return 0; }
+  keys_pull_from_env_file()  { echo "CALLED_ENV"; return 0; }
+
+  run keys_pull "$STACK" --from env-file
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"CALLED_ENV"* ]]
+}
+
+@test "keys_pull: unknown --from source fails" {
+  run keys_pull "$STACK" --from bogus-source
+  [ "$status" -ne 0 ]
+  [[ "$output" == *"Unknown source"* ]]
+}
+
+@test "keys_pull: default source is vps (no --from)" {
+  keys_pull_from_vps() { echo "VPS_DEFAULT"; return 0; }
+  export -f keys_pull_from_vps
+
+  run keys_pull "$STACK"
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"VPS_DEFAULT"* ]]
+}
+
+@test "keys_pull: fails for missing stack directory" {
+  run keys_pull "nonexistent-stack-$$"
+  [ "$status" -ne 0 ]
+  [[ "$output" == *"Stack not found"* ]]
+}
+
+# ── keys_pull_from_vps: core behavior ────────────────────────────────────────
+
+@test "keys_pull_from_vps: writes env file to default target path" {
+  local target="$CLI_ROOT/.${STACK}-pulled.env"
+  run keys_pull_from_vps "$STACK" "$target" "false" "false" "" "env"
+  [ "$status" -eq 0 ]
+  [ -f "$target" ]
+}
+
+@test "keys_pull_from_vps: output file gets mode 600" {
+  local target="$TEST_TMP/pulled.env"
+  run keys_pull_from_vps "$STACK" "$target" "false" "false" "" "env"
+  [ "$status" -eq 0 ]
+  local perms
+  perms=$(stat -c "%a" "$target" 2>/dev/null || stat -f "%OLp" "$target")
+  [ "$perms" = "600" ]
+}
+
+@test "keys_pull_from_vps: fails when .prod.env is missing" {
+  rm -f "$CLI_ROOT/.prod.env"
+  local target="$TEST_TMP/pulled.env"
+  run keys_pull_from_vps "$STACK" "$target" "false" "false" "" "env"
+  [ "$status" -ne 0 ]
+  [[ "$output" == *"not found"* ]]
+}
+
+@test "keys_pull_from_vps: fails when VPS_HOST is not set" {
+  printf 'VPS_USER=ubuntu\n' > "$CLI_ROOT/.prod.env"
+  local target="$TEST_TMP/pulled.env"
+  run keys_pull_from_vps "$STACK" "$target" "false" "false" "" "env"
+  [ "$status" -ne 0 ]
+  [[ "$output" == *"VPS_HOST"* ]]
+}
+
+@test "keys_pull_from_vps: dry-run masks values and writes nothing" {
+  local target="$TEST_TMP/pulled-dry.env"
+  run keys_pull_from_vps "$STACK" "$target" "true" "false" "" "env"
+  [ "$status" -eq 0 ]
+  [ ! -f "$target" ]
+  [[ "$output" == *"MASKED"* ]]
+}
+
+@test "keys_pull_from_vps: dry-run output shows key names" {
+  local target="$TEST_TMP/pulled-dry.env"
+  # ssh stub returns KEY_A and KEY_B lines
+  run keys_pull_from_vps "$STACK" "$target" "true" "false" "" "env"
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"KEY_A"* ]] || [[ "$output" == *"KEY_B"* ]]
+  [[ "$output" != *"value_a"* ]]
+  [[ "$output" != *"value_b"* ]]
+}
+
+@test "keys_pull_from_vps: --keys filter passes only matching lines" {
+  # ssh stub returns KEY_A and KEY_B; only pull KEY_A
+  local target="$TEST_TMP/filtered.env"
+  run keys_pull_from_vps "$STACK" "$target" "false" "false" "KEY_A" "env"
+  [ "$status" -eq 0 ]
+  [ -f "$target" ]
+  grep -q "KEY_A" "$target"
+  # KEY_B should be absent since it doesn't match the filter
+  ! grep -q "KEY_B" "$target"
+}
+
+@test "keys_pull_from_vps: existing target without --force calls confirm" {
+  local target="$TEST_TMP/existing.env"
+  echo "OLD=1" > "$target"
+  # confirm returns 0 (yes) by default in setup → should proceed
+  run keys_pull_from_vps "$STACK" "$target" "false" "false" "" "env"
+  [ "$status" -eq 0 ]
+}
+
+@test "keys_pull_from_vps: existing target with confirm-no aborts" {
+  local target="$TEST_TMP/existing.env"
+  echo "OLD=1" > "$target"
+  confirm() { return 1; }
+  export -f confirm
+
+  run keys_pull_from_vps "$STACK" "$target" "false" "false" "" "env"
+  [ "$status" -ne 0 ]
+  [[ "$output" == *"Cancelled"* ]]
+}
+
+@test "keys_pull_from_vps: --force skips confirm even when target exists" {
+  local target="$TEST_TMP/existing.env"
+  echo "OLD=1" > "$target"
+  confirm() { echo "confirm should not be called" >&2; return 1; }
+  export -f confirm
+
+  run keys_pull_from_vps "$STACK" "$target" "false" "true" "" "env"
+  [ "$status" -eq 0 ]
+  [ -f "$target" ]
+}
+
+@test "keys_pull_from_vps: fails when VPS connection check fails" {
+  validate_vps_connection() { return 1; }
+  export -f validate_vps_connection
+
+  local target="$TEST_TMP/pulled.env"
+  run keys_pull_from_vps "$STACK" "$target" "false" "false" "" "env"
+  [ "$status" -ne 0 ]
+  [[ "$output" == *"Cannot connect"* ]]
+}
+
+# ── keys_pull_from_vps: JSON output ──────────────────────────────────────────
+
+@test "keys_pull_from_vps: json format produces a file with .keys object (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  local target="$TEST_TMP/pulled.json"
+  run keys_pull_from_vps "$STACK" "$target" "false" "false" "" "json"
+  [ "$status" -eq 0 ]
+  [ -f "$target" ]
+  # validate it's well-formed JSON with a 'keys' top-level key
+  run jq -e '.keys' "$target"
+  [ "$status" -eq 0 ]
+}
+
+# ── keys_pull_from_containers ────────────────────────────────────────────────
+
+@test "keys_pull_from_containers: dry-run masks values and writes nothing" {
+  local target="$TEST_TMP/containers-dry.env"
+  run keys_pull_from_containers "$STACK" "mycontainer" "$target" "true" "false" "" "env"
+  [ "$status" -eq 0 ]
+  [ ! -f "$target" ]
+  [[ "$output" == *"MASKED"* ]]
+}
+
+@test "keys_pull_from_containers: output file gets mode 600" {
+  local target="$TEST_TMP/containers.env"
+  run keys_pull_from_containers "$STACK" "mycontainer" "$target" "false" "false" "" "env"
+  [ "$status" -eq 0 ]
+  [ -f "$target" ]
+  local perms
+  perms=$(stat -c "%a" "$target" 2>/dev/null || stat -f "%OLp" "$target")
+  [ "$perms" = "600" ]
+}
+
+@test "keys_pull_from_containers: fails when .prod.env is missing" {
+  rm -f "$CLI_ROOT/.prod.env"
+  local target="$TEST_TMP/containers.env"
+  run keys_pull_from_containers "$STACK" "mycontainer" "$target" "false" "false" "" "env"
+  [ "$status" -ne 0 ]
+  [[ "$output" == *"not found"* ]]
+}
+
+@test "keys_pull_from_containers: fails when VPS connection check fails" {
+  validate_vps_connection() { return 1; }
+  export -f validate_vps_connection
+
+  local target="$TEST_TMP/containers.env"
+  run keys_pull_from_containers "$STACK" "mycontainer" "$target" "false" "false" "" "env"
+  [ "$status" -ne 0 ]
+  [[ "$output" == *"Cannot connect"* ]]
+}
+
+# ── keys_pull_from_env_file ──────────────────────────────────────────────────
+
+@test "keys_pull_from_env_file: copies .prod.env to target with mode 600" {
+  local target="$TEST_TMP/env-file-pulled.env"
+  run keys_pull_from_env_file "$STACK" "$target" "false" "false"
+  [ "$status" -eq 0 ]
+  [ -f "$target" ]
+  local perms
+  perms=$(stat -c "%a" "$target" 2>/dev/null || stat -f "%OLp" "$target")
+  [ "$perms" = "600" ]
+}
+
+@test "keys_pull_from_env_file: copies content faithfully" {
+  printf 'SECRET_KEY=abc123\nDB_PASS=hunter2\n' > "$CLI_ROOT/.prod.env"
+  local target="$TEST_TMP/env-file-pulled.env"
+  run keys_pull_from_env_file "$STACK" "$target" "false" "false"
+  [ "$status" -eq 0 ]
+  grep -q "SECRET_KEY=abc123" "$target"
+  grep -q "DB_PASS=hunter2" "$target"
+}
+
+@test "keys_pull_from_env_file: dry-run masks values and writes nothing" {
+  printf 'SECRET=val123\n' > "$CLI_ROOT/.prod.env"
+  local target="$TEST_TMP/env-dry.env"
+  run keys_pull_from_env_file "$STACK" "$target" "true" "false"
+  [ "$status" -eq 0 ]
+  [ ! -f "$target" ]
+  [[ "$output" == *"MASKED"* ]]
+}
+
+@test "keys_pull_from_env_file: fails when no env file found" {
+  rm -f "$CLI_ROOT/.prod.env"
+  local target="$TEST_TMP/env-missing.env"
+  run keys_pull_from_env_file "$STACK" "$target" "false" "false"
+  [ "$status" -ne 0 ]
+  [[ "$output" == *"No env file found"* ]]
+}
+
+@test "keys_pull_from_env_file: falls back to stack-specific env file" {
+  rm -f "$CLI_ROOT/.prod.env"
+  printf 'STACK_SECRET=42\n' > "$CLI_ROOT/.${STACK}-prod.env"
+  local target="$TEST_TMP/env-fallback.env"
+  run keys_pull_from_env_file "$STACK" "$target" "false" "false"
+  [ "$status" -eq 0 ]
+  grep -q "STACK_SECRET=42" "$target"
+  rm -f "$CLI_ROOT/.${STACK}-prod.env"
+}
+
+@test "keys_pull_from_env_file: --force skips confirm on existing target" {
+  local target="$TEST_TMP/existing.env"
+  echo "OLD=x" > "$target"
+  confirm() { echo "confirm should not be called" >&2; return 1; }
+  export -f confirm
+
+  run keys_pull_from_env_file "$STACK" "$target" "false" "true"
+  [ "$status" -eq 0 ]
+}
+
+# ── keys_pull_help ────────────────────────────────────────────────────────────
+
+@test "keys_pull_help: prints usage information" {
+  run keys_pull_help
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"Pull Key Values"* ]]
+  [[ "$output" == *"--from"* ]]
+  [[ "$output" == *"--dry-run"* ]]
+}
diff --git a/tests/test_keys_status.bats b/tests/test_keys_status.bats
new file mode 100644
index 0000000..392939b
--- /dev/null
+++ b/tests/test_keys_status.bats
@@ -0,0 +1,377 @@
+#!/usr/bin/env bats
+# ==================================================
+# tests/test_keys_status.bats — Tests for lib/keys/status.sh and lib/keys/discovery.sh
+# ==================================================
+# Run:  bats tests/test_keys_status.bats
+# Covers:
+#   keys_status — --json output shape (ssh_keys/api_keys/vps_status keys),
+#                  text output, counts from fixture JSON files
+#   keys_recent — --limit slicing from a seeded audit log
+#   discover_local_keys — env file and SSH key enumeration from fixtures
+#   generate_recommendations — recommendation strings for known inputs
+
+setup() {
+  export CLI_ROOT
+  CLI_ROOT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)"
+  TEST_TMP="$(mktemp -d)"
+  STACK="test-keys-status-$$"
+  mkdir -p "$CLI_ROOT/stacks/$STACK"
+
+  source "$CLI_ROOT/lib/utils.sh"
+  fail()  { echo "$1" >&2; return 1; }
+  error() { echo "$1" >&2; }
+  warn()  { echo "$1" >&2; }
+  ok()    { echo "$1"; }
+  log()   { echo "$1"; }
+
+  # Source the full keys module (includes status.sh, discovery.sh, pull.sh, etc.)
+  source "$CLI_ROOT/lib/keys.sh"
+
+  # Stub network helpers — tests never touch a real host
+  validate_vps_connection() { return 1; }  # default: unreachable (overridden per test)
+  export -f validate_vps_connection
+
+  build_ssh_opts() { echo ""; }
+  export -f build_ssh_opts
+
+  resolve_deploy_dir() { echo "/opt/deploy"; }
+  export -f resolve_deploy_dir
+
+  # Keys metadata directory
+  KEYS_DIR="$CLI_ROOT/stacks/$STACK/keys"
+  ensure_keys_dir "$STACK"
+
+  # Seed minimal JSON fixtures
+  printf '{"ssh_keys":[{"username":"alice"},{"username":"bob"}]}\n' \
+    > "$KEYS_DIR/ssh-keys.json"
+  printf '{"api_keys":[{"name":"mykey"}]}\n' \
+    > "$KEYS_DIR/api-keys.json"
+}
+
+teardown() {
+  rm -rf "$CLI_ROOT/stacks/$STACK"
+  rm -f "$CLI_ROOT/.prod.env"
+  rm -rf "$TEST_TMP"
+}
+
+# ── keys_status --json: output shape ─────────────────────────────────────────
+
+@test "keys_status --json: emits valid JSON (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  printf 'VPS_HOST=\nVPS_USER=ubuntu\n' > "$CLI_ROOT/.prod.env"
+
+  run keys_status "$STACK" --json
+  [ "$status" -eq 0 ]
+  run jq -e '.' <<< "$output"
+  [ "$status" -eq 0 ]
+}
+
+@test "keys_status --json: contains required top-level keys (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  printf 'VPS_HOST=\nVPS_USER=ubuntu\n' > "$CLI_ROOT/.prod.env"
+
+  run keys_status "$STACK" --json
+  [ "$status" -eq 0 ]
+  run jq -e '.ssh_keys' <<< "$output"
+  [ "$status" -eq 0 ]
+  run jq -e '.api_keys' <<< "$output"
+  [ "$status" -eq 0 ]
+  run jq -e '.vps_status' <<< "$output"
+  [ "$status" -eq 0 ]
+}
+
+@test "keys_status --json: ssh_keys count reflects fixture (2 keys) (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  printf 'VPS_HOST=\n' > "$CLI_ROOT/.prod.env"
+
+  run keys_status "$STACK" --json
+  [ "$status" -eq 0 ]
+  local count
+  count=$(jq -r '.ssh_keys' <<< "$output")
+  [ "$count" -eq 2 ]
+}
+
+@test "keys_status --json: api_keys count reflects fixture (1 key) (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  printf 'VPS_HOST=\n' > "$CLI_ROOT/.prod.env"
+
+  run keys_status "$STACK" --json
+  [ "$status" -eq 0 ]
+  local count
+  count=$(jq -r '.api_keys' <<< "$output")
+  [ "$count" -eq 1 ]
+}
+
+@test "keys_status --json: vps_status is 'unknown' when VPS_HOST is empty (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  printf 'VPS_HOST=\nVPS_USER=ubuntu\n' > "$CLI_ROOT/.prod.env"
+
+  run keys_status "$STACK" --json
+  [ "$status" -eq 0 ]
+  local status_val
+  status_val=$(jq -r '.vps_status' <<< "$output")
+  [ "$status_val" = "unknown" ]
+}
+
+@test "keys_status --json: vps_status is 'unreachable' when connection fails (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  printf 'VPS_HOST=fakehost\nVPS_USER=ubuntu\n' > "$CLI_ROOT/.prod.env"
+  validate_vps_connection() { return 1; }
+  export -f validate_vps_connection
+
+  run keys_status "$STACK" --json
+  [ "$status" -eq 0 ]
+  local status_val
+  status_val=$(jq -r '.vps_status' <<< "$output")
+  [ "$status_val" = "unreachable" ]
+}
+
+@test "keys_status --json: vps_status is 'connected' when connection succeeds (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  printf 'VPS_HOST=fakehost\nVPS_USER=ubuntu\n' > "$CLI_ROOT/.prod.env"
+  validate_vps_connection() { return 0; }
+  export -f validate_vps_connection
+
+  run keys_status "$STACK" --json
+  [ "$status" -eq 0 ]
+  local status_val
+  status_val=$(jq -r '.vps_status' <<< "$output")
+  [ "$status_val" = "connected" ]
+}
+
+@test "keys_status --json: env_vars count reflects .prod.env contents (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  printf 'FOO=1\nBAR=2\nBAZ=3\n' > "$CLI_ROOT/.prod.env"
+
+  run keys_status "$STACK" --json
+  [ "$status" -eq 0 ]
+  local env_count
+  env_count=$(jq -r '.env_vars' <<< "$output")
+  [ "$env_count" -eq 3 ]
+}
+
+# ── keys_status text output ───────────────────────────────────────────────────
+
+@test "keys_status text: exits 0 and produces non-empty output" {
+  printf 'VPS_HOST=\n' > "$CLI_ROOT/.prod.env"
+
+  run keys_status "$STACK"
+  [ "$status" -eq 0 ]
+  [ -n "$output" ]
+}
+
+@test "keys_status text: mentions SSH key count" {
+  printf 'VPS_HOST=\n' > "$CLI_ROOT/.prod.env"
+
+  run keys_status "$STACK"
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"SSH Keys"* ]] || [[ "$output" == *"ssh_keys"* ]]
+}
+
+# ── keys_recent: audit log slicing ───────────────────────────────────────────
+
+@test "keys_recent: returns 1 when no audit log exists" {
+  run keys_recent "$STACK"
+  [ "$status" -ne 0 ]
+  [[ "$output" == *"No audit log"* ]]
+}
+
+@test "keys_recent: shows all entries when log has fewer than the limit" {
+  local audit_log="$KEYS_DIR/key-audit.log"
+  printf '[2026-01-01T10:00:00Z] alice: add - added deploy key\n' > "$audit_log"
+  printf '[2026-01-02T11:00:00Z] bob: rotate - rotated api key\n' >> "$audit_log"
+
+  run keys_recent "$STACK" --limit 10
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"alice"* ]]
+  [[ "$output" == *"bob"* ]]
+}
+
+@test "keys_recent: --limit 1 returns only the last entry" {
+  local audit_log="$KEYS_DIR/key-audit.log"
+  printf '[2026-01-01T10:00:00Z] alice: add - added deploy key\n' > "$audit_log"
+  printf '[2026-01-02T11:00:00Z] bob: rotate - rotated api key\n' >> "$audit_log"
+  printf '[2026-01-03T12:00:00Z] carol: revoke - revoked old key\n' >> "$audit_log"
+
+  run keys_recent "$STACK" --limit 1
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"carol"* ]]
+  [[ "$output" != *"alice"* ]]
+}
+
+@test "keys_recent: --limit=N form (equals sign) also works" {
+  local audit_log="$KEYS_DIR/key-audit.log"
+  printf '[2026-01-01T10:00:00Z] alice: add - added deploy key\n' > "$audit_log"
+  printf '[2026-01-02T11:00:00Z] bob: rotate - rotated api key\n' >> "$audit_log"
+
+  run keys_recent "$STACK" --limit=1
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"bob"* ]]
+  [[ "$output" != *"alice"* ]]
+}
+
+# ── discover_local_keys ───────────────────────────────────────────────────────
+
+@test "discover_local_keys: returns valid JSON (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  # Stack has no .env.template; function should still succeed
+  run discover_local_keys "$STACK"
+  [ "$status" -eq 0 ]
+  run jq -e '.' <<< "$output"
+  [ "$status" -eq 0 ]
+}
+
+@test "discover_local_keys: detects .prod.env presence (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  printf 'KEY=val\n' > "$CLI_ROOT/.prod.env"
+
+  run discover_local_keys "$STACK"
+  [ "$status" -eq 0 ]
+  # env_files array should contain .prod.env
+  run jq -e '.env_files | map(select(. == ".prod.env")) | length > 0' <<< "$output"
+  [ "$status" -eq 0 ]
+  [[ "$output" == "true" ]]
+}
+
+@test "discover_local_keys: template_secrets reflects .env.template line count (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  mkdir -p "$CLI_ROOT/stacks/$STACK"
+  printf 'SECRET_A=\nSECRET_B=\nSECRET_C=\n' > "$CLI_ROOT/stacks/$STACK/.env.template"
+
+  run discover_local_keys "$STACK"
+  [ "$status" -eq 0 ]
+  local count
+  count=$(jq -r '.template_secrets' <<< "$output")
+  [ "$count" -eq 3 ]
+}
+
+@test "discover_local_keys: template_secrets is 0 when no .env.template (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  rm -f "$CLI_ROOT/stacks/$STACK/.env.template"
+
+  run discover_local_keys "$STACK"
+  [ "$status" -eq 0 ]
+  local count
+  count=$(jq -r '.template_secrets' <<< "$output")
+  [ "$count" -eq 0 ]
+}
+
+# ── generate_recommendations ─────────────────────────────────────────────────
+
+@test "generate_recommendations: returns JSON array (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  local input
+  input=$(jq -n '{
+    sources: {
+      vps: { ssh_keys: 3 },
+      github: { repos_scanned: 2, secrets_found: {} },
+      local: { env_files: ["a","b"], template_secrets: 5 }
+    }
+  }')
+
+  run generate_recommendations "$input"
+  [ "$status" -eq 0 ]
+  run jq -e '. | type == "array"' <<< "$output"
+  [ "$status" -eq 0 ]
+  [[ "$output" == "true" ]]
+}
+
+@test "generate_recommendations: recommends SSH key rotation when VPS has keys (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  local input
+  input=$(jq -n '{
+    sources: {
+      vps: { ssh_keys: 4 },
+      github: { repos_scanned: 0, secrets_found: {} },
+      local: { env_files: [], template_secrets: 0 }
+    }
+  }')
+
+  run generate_recommendations "$input"
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"SSH"* ]] || [[ "$output" == *"VPS SSH"* ]]
+}
+
+@test "generate_recommendations: recommends GitHub review when repos scanned (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  local input
+  input=$(jq -n '{
+    sources: {
+      vps: { ssh_keys: 0 },
+      github: { repos_scanned: 3, secrets_found: {} },
+      local: { env_files: [], template_secrets: 0 }
+    }
+  }')
+
+  run generate_recommendations "$input"
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"GitHub"* ]] || [[ "$output" == *"github"* ]]
+}
+
+@test "generate_recommendations: flags large secret count in template (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  local input
+  input=$(jq -n '{
+    sources: {
+      vps: { ssh_keys: 0 },
+      github: { repos_scanned: 0, secrets_found: {} },
+      local: { env_files: [], template_secrets: 25 }
+    }
+  }')
+
+  run generate_recommendations "$input"
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"25"* ]] || [[ "$output" == *"secret"* ]] || [[ "$output" == *"large"* ]]
+}
+
+@test "generate_recommendations: returns empty array for bare minimum input (requires jq)" {
+  if ! command -v jq &>/dev/null; then
+    skip "jq not available"
+  fi
+  local input
+  input=$(jq -n '{
+    sources: {
+      vps: { ssh_keys: 0 },
+      github: { repos_scanned: 0, secrets_found: {} },
+      local: { env_files: [], template_secrets: 0 }
+    }
+  }')
+
+  run generate_recommendations "$input"
+  [ "$status" -eq 0 ]
+  local arr_len
+  arr_len=$(jq '. | length' <<< "$output")
+  [ "$arr_len" -eq 0 ]
+}
diff --git a/tests/test_recipes.bats b/tests/test_recipes.bats
index 095cc6e..efdeacb 100644
--- a/tests/test_recipes.bats
+++ b/tests/test_recipes.bats
@@ -328,3 +328,31 @@ EOF
     done < <(grep -E '^[A-Z0-9_]+_PORT=' "$dir/services.conf")
   done
 }
+
+@test "every official recipe: healthcheck command-substitution uses CMD-SHELL, not exec-form CMD (contract check d, issue #404)" {
+  # Docker's exec-form CMD array (["CMD", "..."]) does NOT run a shell, so any
+  # shell syntax inside — $(), $$(cat ...), pipes, etc. — is passed verbatim to
+  # the binary and silently never evaluates. The correct form for a healthcheck
+  # that needs shell expansion is CMD-SHELL (["CMD-SHELL", "..."]) which runs
+  # via /bin/sh -c and evaluates $() correctly.
+  #
+  # This test flags any docker-compose.yml whose healthcheck.test array
+  # starts with "CMD" (exec-form) while also containing a $( or $$( pattern —
+  # the exact combination that produces a broken healthcheck at runtime.
+  for dir in "$CLI_ROOT"/templates/recipes/*/; do
+    local name; name="$(basename "$dir")"
+    [ -f "$dir/docker-compose.yml" ] || continue
+
+    # A line matching CMD-SHELL is fine — the substitution will be evaluated.
+    # A line matching "CMD", (exec-form) with $( or $$(  is the bug.
+    while IFS= read -r line; do
+      # Skip CMD-SHELL lines — those are fine
+      echo "$line" | grep -qE '"CMD-SHELL"' && continue
+      # Flag: exec-form CMD that contains command substitution
+      if echo "$line" | grep -qE '"CMD".*(\$\(|\$\$\()'; then
+        echo "recipe $name: healthcheck uses exec-form CMD with command substitution (\$(...)) — use CMD-SHELL so the shell evaluates it (docker-compose.yml line: $line)" >&2
+        return 1
+      fi
+    done < "$dir/docker-compose.yml"
+  done
+}

From d9d6f1721390c490d5c3f84190e571a72a70edc1 Mon Sep 17 00:00:00 2001
From: "gfargo-horizon-agent[bot]"
 <294710345+gfargo-horizon-agent[bot]@users.noreply.github.com>
Date: Fri, 28 Aug 2026 21:22:41 +0000
Subject: [PATCH 2/3] fix(tests): resolve 4 failing bats assertions in keys
 test suite

- test_keys_pull.bats: make ssh stub command-aware so 'test -f' and
  other probe calls don't emit KEY=value lines; dry-run output no longer
  contains unmasked values (test 1462)
- test_keys_status.bats: save $output into local json_out before
  subsequent 'run jq' calls overwrite it; each run jq now receives the
  original keys_status JSON, not the previous jq result (test 1525)
- test_keys_status.bats: delete key-audit.log before testing the
  no-log-exists path; ensure_keys_dir in setup() creates it via touch
  so the test must remove it first (test 1534)
- lib/keys/discovery.sh: guard empty recommendations array in
  generate_recommendations; printf '%s\n' on an empty array emits a
  blank line which jq -s . converts to [""] instead of [] (test 1546)
---
 lib/keys/discovery.sh       |  7 +++++++
 tests/test_keys_pull.bats   | 30 +++++++++++++++++++++++++++---
 tests/test_keys_status.bats | 11 ++++++++---
 3 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/lib/keys/discovery.sh b/lib/keys/discovery.sh
index a5fee8d..5a82298 100644
--- a/lib/keys/discovery.sh
+++ b/lib/keys/discovery.sh
@@ -306,5 +306,12 @@ generate_recommendations() {
     recommendations+=("Large number of secrets in template ($template_secrets) - consider secret management solution")
   fi
 
+  # Return empty JSON array when nothing was added; avoid the blank-line
+  # that printf '%s\n' produces when the array has no elements, which would
+  # cause 'jq -s .' to emit [""] instead of [].
+  if [ ${#recommendations[@]} -eq 0 ]; then
+    echo '[]'
+    return 0
+  fi
   printf '%s\n' "${recommendations[@]}" | jq -R . | jq -s .
 }
diff --git a/tests/test_keys_pull.bats b/tests/test_keys_pull.bats
index 82b4735..c11874d 100644
--- a/tests/test_keys_pull.bats
+++ b/tests/test_keys_pull.bats
@@ -37,9 +37,33 @@ setup() {
   : > "$SSH_CALL_LOG"
   export SSH_CALL_LOG
   ssh() {
-    echo "${@: -1}" >> "$SSH_CALL_LOG"
-    echo "KEY_A=value_a"
-    echo "KEY_B=value_b"
+    local last_arg="${@: -1}"
+    echo "$last_arg" >> "$SSH_CALL_LOG"
+    # Only emit env-file content for cat/docker-exec commands; test-f / find
+    # / wc commands must not print key values or they will appear in dry-run output.
+    case "$last_arg" in
+      cat\ *)
+        echo "KEY_A=value_a"
+        echo "KEY_B=value_b"
+        ;;
+      "test -f "*)
+        : # silent – file-exists check, no output
+        ;;
+      find\ *)
+        echo "/opt/deploy/.prod.env"
+        ;;
+      wc\ *)
+        echo "2"
+        ;;
+      *"docker exec"*)
+        # simulate 'docker exec <container> env'
+        echo "KEY_A=value_a"
+        echo "KEY_B=value_b"
+        ;;
+      *)
+        : # no output for other commands
+        ;;
+    esac
     return 0
   }
   export -f ssh
diff --git a/tests/test_keys_status.bats b/tests/test_keys_status.bats
index 392939b..5713d4d 100644
--- a/tests/test_keys_status.bats
+++ b/tests/test_keys_status.bats
@@ -76,11 +76,13 @@ teardown() {
 
   run keys_status "$STACK" --json
   [ "$status" -eq 0 ]
-  run jq -e '.ssh_keys' <<< "$output"
+  # Save JSON output before subsequent `run` calls clobber $output.
+  local json_out="$output"
+  run jq -e '.ssh_keys' <<< "$json_out"
   [ "$status" -eq 0 ]
-  run jq -e '.api_keys' <<< "$output"
+  run jq -e '.api_keys' <<< "$json_out"
   [ "$status" -eq 0 ]
-  run jq -e '.vps_status' <<< "$output"
+  run jq -e '.vps_status' <<< "$json_out"
   [ "$status" -eq 0 ]
 }
 
@@ -187,6 +189,9 @@ teardown() {
 # ── keys_recent: audit log slicing ───────────────────────────────────────────
 
 @test "keys_recent: returns 1 when no audit log exists" {
+  # setup() calls ensure_keys_dir which creates key-audit.log via 'touch'.
+  # Remove it so we can test the missing-log code path.
+  rm -f "$KEYS_DIR/key-audit.log"
   run keys_recent "$STACK"
   [ "$status" -ne 0 ]
   [[ "$output" == *"No audit log"* ]]

From 83d9a3b9025d7106482b82bbe376fca9fb161979 Mon Sep 17 00:00:00 2001
From: "gfargo-horizon-agent[bot]"
 <294710345+gfargo-horizon-agent[bot]@users.noreply.github.com>
Date: Fri, 28 Aug 2026 22:35:14 +0000
Subject: [PATCH 3/3] fix(tests): isolate keys pull/status fixtures to temp dir

Point CLI_ROOT at TEST_TMP instead of the real repo tree so
stacks/ and .prod.env fixtures live in the temp fixture and
cannot leak gitignored artifacts on interrupted runs; source
lib/ from REPO_ROOT and simplify teardown to rm -rf TEST_TMP.
---
 tests/test_keys_pull.bats   | 11 ++++-------
 tests/test_keys_status.bats | 10 ++++------
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/tests/test_keys_pull.bats b/tests/test_keys_pull.bats
index c11874d..b01a13f 100644
--- a/tests/test_keys_pull.bats
+++ b/tests/test_keys_pull.bats
@@ -12,13 +12,13 @@
 #         security-critical: pulled file always gets mode 600.
 
 setup() {
-  export CLI_ROOT
-  CLI_ROOT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)"
+  REPO_ROOT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)"
   TEST_TMP="$(mktemp -d)"
+  export CLI_ROOT="$TEST_TMP"
   STACK="test-keys-pull-$$"
   mkdir -p "$CLI_ROOT/stacks/$STACK"
 
-  source "$CLI_ROOT/lib/utils.sh"
+  source "$REPO_ROOT/lib/utils.sh"
   fail()  { echo "$1" >&2; return 1; }
   error() { echo "$1" >&2; }
   warn()  { echo "$1" >&2; }
@@ -26,7 +26,7 @@ setup() {
   log()   { echo "$1"; }
 
   # Source the full keys module (includes pull.sh, status.sh, discovery.sh, etc.)
-  source "$CLI_ROOT/lib/keys.sh"
+  source "$REPO_ROOT/lib/keys.sh"
 
   # Stub out network-touching helpers so tests never reach a real host
   validate_vps_connection() { return 0; }
@@ -83,9 +83,6 @@ setup() {
 }
 
 teardown() {
-  rm -rf "$CLI_ROOT/stacks/$STACK"
-  rm -f "$CLI_ROOT/.prod.env"
-  rm -f "$CLI_ROOT/."*"-pulled.env"
   rm -rf "$TEST_TMP"
 }
 
diff --git a/tests/test_keys_status.bats b/tests/test_keys_status.bats
index 5713d4d..8fe5f72 100644
--- a/tests/test_keys_status.bats
+++ b/tests/test_keys_status.bats
@@ -11,13 +11,13 @@
 #   generate_recommendations — recommendation strings for known inputs
 
 setup() {
-  export CLI_ROOT
-  CLI_ROOT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)"
+  REPO_ROOT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)"
   TEST_TMP="$(mktemp -d)"
+  export CLI_ROOT="$TEST_TMP"
   STACK="test-keys-status-$$"
   mkdir -p "$CLI_ROOT/stacks/$STACK"
 
-  source "$CLI_ROOT/lib/utils.sh"
+  source "$REPO_ROOT/lib/utils.sh"
   fail()  { echo "$1" >&2; return 1; }
   error() { echo "$1" >&2; }
   warn()  { echo "$1" >&2; }
@@ -25,7 +25,7 @@ setup() {
   log()   { echo "$1"; }
 
   # Source the full keys module (includes status.sh, discovery.sh, pull.sh, etc.)
-  source "$CLI_ROOT/lib/keys.sh"
+  source "$REPO_ROOT/lib/keys.sh"
 
   # Stub network helpers — tests never touch a real host
   validate_vps_connection() { return 1; }  # default: unreachable (overridden per test)
@@ -49,8 +49,6 @@ setup() {
 }
 
 teardown() {
-  rm -rf "$CLI_ROOT/stacks/$STACK"
-  rm -f "$CLI_ROOT/.prod.env"
   rm -rf "$TEST_TMP"
 }