infra/deploy/render-secrets.sh

@@ -48,21 +48,26 @@ render_thermograph_secrets() {
     key_env=("SOPS_AGE_KEY=$keymat")
   fi
   local tmp; tmp=$(mktemp)
-  # The tmp file holds DECRYPTED secrets: a RETURN trap makes its removal
+  # The tmp file holds DECRYPTED secrets, so every exit path below removes it.
-  # unconditional (fires on every explicit return, success or failure, and is
+  #
-  # scoped to this function — it doesn't leak into or clobber the caller's traps).
+  # Deliberately NOT a `trap ... RETURN`, which looks like the tidier way to make
-  trap 'rm -f "$tmp"' RETURN
+  # that unconditional: a RETURN trap set inside a SOURCED function persists in
-  : > "$tmp"
+  # the CALLER's shell after the function returns, and a RETURN trap also fires
-  # Decrypt failures return 1 explicitly (no partial env, no reliance on the
+  # when a `.`/source completes. deploy.sh sources /etc/thermograph.env a few
-  # caller's shell options — a bare set -e abort would skip the RETURN trap and
+  # lines after calling us, which would re-fire the trap at top level where `tmp`
-  # strand plaintext in /tmp). common first, host second, so a host value
+  # (function-local) is unset -- fatal under deploy.sh's `set -u`, and silent,
-  # overrides a shared one (last-wins).
+  # because that line already redirects stderr to /dev/null. Explicit removal on
+  # each path is duller and correct.
+  : > "$tmp" || { rm -f "$tmp"; return 1; }
+  # Decrypt failures return 1 explicitly, so a partial env is never written and
+  # correctness doesn't depend on the caller's shell options. common first, host
+  # second, so a host value overrides a shared one (last-wins).
   if [ -f "$repo/deploy/secrets/common.yaml" ]; then
     env "${key_env[@]}" sops -d --input-type yaml --output-type dotenv \
-      "$repo/deploy/secrets/common.yaml" >> "$tmp" || return 1
+      "$repo/deploy/secrets/common.yaml" >> "$tmp" || { rm -f "$tmp"; return 1; }
   fi
   env "${key_env[@]}" sops -d --input-type yaml --output-type dotenv \
-    "$repo/deploy/secrets/${env_name}.yaml" >> "$tmp" || return 1
+    "$repo/deploy/secrets/${env_name}.yaml" >> "$tmp" || { rm -f "$tmp"; return 1; }
 
   # Write /etc/thermograph.env. Prefer an in-place write when the existing file is
   # writable by us (e.g. a group-writable 0660 root:<deploygroup> on a box whose CI
@@ -73,12 +78,19 @@ render_thermograph_secrets() {
   # next line of deploy.sh (`. /etc/thermograph.env` as that non-root user) can't read
   # it, so POSTGRES_PASSWORD never enters the env and `docker compose` dies on
   # interpolation. Fail loudly rather than deploy against stale secrets.
+  # rc + a single cleanup point: the plaintext tmp must go whichever branch runs,
+  # but the old trailing `rm` was also the function's last command, so it masked a
+  # failed in-place `cat` write to status 0. Capture the status instead, so a
+  # half-written /etc/thermograph.env fails loudly rather than deploying stale.
+  local rc=0
   if [ -f /etc/thermograph.env ] && [ -w /etc/thermograph.env ]; then
-    cat "$tmp" > /etc/thermograph.env
+    cat "$tmp" > /etc/thermograph.env || rc=1
   elif install -m 0640 "$tmp" /etc/thermograph.env 2>/dev/null; then :
   elif sudo install -m 0640 -o "$(id -un)" -g "$(id -gn)" "$tmp" /etc/thermograph.env 2>/dev/null; then :
   else
     echo "!! cannot write /etc/thermograph.env (need file write access or passwordless sudo)" >&2
-    return 1
+    rc=1
   fi
+  rm -f "$tmp"
+  return "$rc"
 }