shell: add shellcheck CI guard and drive the tree to zero findings (#19)

Adds .forgejo/workflows/shell-lint.yml (pinned shellcheck v0.11.0 + sha256, -x,
default severity, fail on any finding, not path-filtered) and drives all 26
scripts to zero findings.

Two defects shellcheck cannot see:

render-secrets.sh left DECRYPTED vault contents in /tmp whenever a sops decrypt
failed -- the caller's set -e aborted the function before either cleanup ran.
Now removed on every exit path, with `|| return 1` on both sops calls so a
decrypt failure can never write a partial /etc/thermograph.env regardless of the
caller's shell options. Explicitly not a `trap ... RETURN`: such a trap set in a
sourced function persists into the caller's shell and re-fires when the caller's
next `.`/source completes, where the function-local tmp is unset -- fatal and
silent under deploy.sh's set -u. The file now records that reasoning.

autoscale.sh ran `set -eu` without pipefail while piping docker stats into awk,
so a failed left side was swallowed and the loop autoscaled on empty input.
Promoted to pipefail with a missed sample treated as a skip; verified busybox ash
in docker:27-cli supports it.

Also: capture-fixtures.sh's `jq . || cat` ran cat after jq had consumed stdin,
silently writing truncated fixtures; deploy.sh/deploy-stack.sh `# shellcheck
source=` paths corrected for the monorepo layout.

.forgejo/workflows/shell-lint.yml

@@ -0,0 +1,75 @@
+name: shell-lint
+
+# Shellcheck over every *.sh in the tree. These scripts run as root over SSH
+# against production hosts (deploy, secrets provisioning, host bootstrap) with
+# no test suite in front of them -- a quoting bug or an unset-variable typo is
+# found *on the host* or not at all. The tree was driven to zero findings in
+# one pass; this guard keeps it there.
+#
+# Deliberately NOT path-filtered (same call as secrets-guard): a backstop that
+# only runs when you expect it to isn't a backstop, and shellcheck over the
+# ~26 scripts here is seconds. Scripts are discovered with `find`, not listed,
+# so a new script is covered the moment it lands -- that is the entire point.
+#
+# Shellcheck is installed from the pinned official static-binary release, NOT
+# `apt-get install shellcheck` (the observability-validate precedent): the
+# distro version drifts with the job image, and a drifted shellcheck can grow
+# NEW warnings that fail CI on an unrelated push. The pin (v0.11.0, matching
+# the koalaman/shellcheck:stable image the zero-findings pass was run against)
+# plus the sha256 makes the check reproducible; bump both together, and expect
+# to fix any new findings in the same PR as the bump.
+
+on:
+  pull_request:
+  push:
+    branches: [dev, main, release]
+
+env:
+  SHELLCHECK_VERSION: v0.11.0
+  SHELLCHECK_SHA256: 8c3be12b05d5c177a04c29e3c78ce89ac86f1595681cab149b65b97c4e227198
+
+jobs:
+  shellcheck:
+    runs-on: docker
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install shellcheck ${{ env.SHELLCHECK_VERSION }} (pinned static binary)
+        run: |
+          set -euo pipefail
+          # node:20-bookworm (this runner's job image) ships curl, tar and xz,
+          # so the pinned tarball needs no apt-get at all -- faster than the
+          # distro install it replaces.
+          curl -fsSL -o /tmp/shellcheck.tar.xz \
+            "https://github.com/koalaman/shellcheck/releases/download/${SHELLCHECK_VERSION}/shellcheck-${SHELLCHECK_VERSION}.linux.x86_64.tar.xz"
+          echo "${SHELLCHECK_SHA256}  /tmp/shellcheck.tar.xz" | sha256sum -c -
+          tar -xJf /tmp/shellcheck.tar.xz -C /tmp
+          install -m 0755 "/tmp/shellcheck-${SHELLCHECK_VERSION}/shellcheck" /usr/local/bin/shellcheck
+          shellcheck --version
+
+      - name: Shellcheck every *.sh in the tree
+        run: |
+          set -euo pipefail
+          mapfile -t files < <(find . -name '*.sh' -not -path './.git/*' | sort)
+          if [ ${#files[@]} -eq 0 ]; then
+            echo "no *.sh files yet — nothing to lint"
+            exit 0
+          fi
+          echo "shellcheck over ${#files[@]} scripts"
+          # -x follows sourced files: several scripts carry
+          # `# shellcheck source=` directives that only resolve with it.
+          # Default severity, no excludes: the tree is at zero findings, so
+          # anything shellcheck reports is a regression, not noise.
+          if shellcheck -x -f gcc "${files[@]}" > /tmp/findings.txt; then
+            echo "ok: all scripts clean"
+            exit 0
+          fi
+          # gcc format is file:line:col: level: message — reshape each line
+          # into a ::error annotation so findings land on the diff in the PR
+          # view. `rest` soaks up any further colons inside the message.
+          while IFS=: read -r f l _ rest; do
+            [ -n "$l" ] || continue
+            echo "::error file=${f#./},line=${l}::${rest# }"
+          done < /tmp/findings.txt
+          echo "shellcheck found problems in the files above"
+          exit 1

backend/scripts/smoke.sh

@@ -29,7 +29,7 @@ echo "==> starting backend + db ($IMG)"
 "${COMPOSE[@]}" up -d
 
 echo "==> waiting for /healthz (up to 90s)"
-for i in $(seq 1 45); do
+for _ in $(seq 1 45); do
   if curl -fsS -o /dev/null "$PORT_URL/healthz"; then ok=1; break; fi
   sleep 2
 done

frontend/scripts/capture-fixtures.sh

@@ -18,7 +18,7 @@ trap 'scripts/backend-for-tests.sh down >/dev/null 2>&1 || true' EXIT
 
 fetch() {  # <endpoint-path> <fixture-name>
   echo "==> $2.json  <-  $1"
-  curl -fsS "$BASE/$1" --max-time 40 | { command -v jq >/dev/null && jq . || cat; } > "$OUT/$2.json"
+  curl -fsS "$BASE/$1" --max-time 40 | { if command -v jq >/dev/null; then jq .; else cat; fi; } > "$OUT/$2.json"
 }
 
 fetch "api/v2/content/home"                          home
@@ -28,4 +28,4 @@ fetch "api/v2/content/city/$SLUG"                    city
 fetch "api/v2/content/city/$SLUG/month/$MONTH"       month
 fetch "api/v2/content/city/$SLUG/records"            records
 
-echo "==> captured $(ls "$OUT"/*.json | wc -l) fixtures for $SLUG into $OUT/"
+echo "==> captured $(find "$OUT" -maxdepth 1 -name '*.json' | wc -l) fixtures for $SLUG into $OUT/"

infra/deploy/deploy.sh

@@ -70,11 +70,16 @@ fi
 # existing /etc/thermograph.env. Then source it so a by-hand run interpolates the
 # same as the systemd unit does. See deploy/render-secrets.sh + deploy/secrets/.
 if [ -f "$INFRA_DIR/deploy/render-secrets.sh" ]; then
-  # shellcheck source=deploy/render-secrets.sh
+  # shellcheck source=infra/deploy/render-secrets.sh
   . "$INFRA_DIR/deploy/render-secrets.sh"
   render_thermograph_secrets "$INFRA_DIR"
 fi
-set -a; . /etc/thermograph.env 2>/dev/null || true; set +a
+# /etc/thermograph.env is rendered at deploy time from the SOPS vault — it
+# cannot exist at lint time, so don't ask shellcheck to follow it.
+set -a
+# shellcheck source=/dev/null
+. /etc/thermograph.env 2>/dev/null || true
+set +a
 
 # Pre-warm the ~750 city-page archives so /climate pages serve from cache and a
 # search-engine crawl never bursts the archive API quota. Detached inside the
@@ -156,7 +161,10 @@ TAGS_FILE="$INFRA_DIR/deploy/.image-tags.env"
 _incoming_backend="${BACKEND_IMAGE_TAG:-}"
 _incoming_frontend="${FRONTEND_IMAGE_TAG:-}"
 if [ -f "$TAGS_FILE" ]; then
-  set -a; . "$TAGS_FILE"; set +a
+  set -a
+  # shellcheck source=/dev/null # untracked runtime artifact this script writes below
+  . "$TAGS_FILE"
+  set +a
 fi
 [ -n "$_incoming_backend" ] && BACKEND_IMAGE_TAG="$_incoming_backend"
 [ -n "$_incoming_frontend" ] && FRONTEND_IMAGE_TAG="$_incoming_frontend"

infra/deploy/render-secrets.sh

@@ -48,15 +48,26 @@ render_thermograph_secrets() {
     key_env=("SOPS_AGE_KEY=$keymat")
   fi
   local tmp; tmp=$(mktemp)
-  : > "$tmp"
-  # set -e in the caller makes a decrypt failure fatal here (no partial env). common
-  # first, host second, so a host value overrides a shared one (last-wins).
+  # The tmp file holds DECRYPTED secrets, so every exit path below removes it.
+  #
+  # Deliberately NOT a `trap ... RETURN`, which looks like the tidier way to make
+  # that unconditional: a RETURN trap set inside a SOURCED function persists in
+  # the CALLER's shell after the function returns, and a RETURN trap also fires
+  # when a `.`/source completes. deploy.sh sources /etc/thermograph.env a few
+  # lines after calling us, which would re-fire the trap at top level where `tmp`
+  # (function-local) is unset -- fatal under deploy.sh's `set -u`, and silent,
+  # 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"
+      "$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"
+    "$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
@@ -67,14 +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
-    rm -f "$tmp"
     echo "!! cannot write /etc/thermograph.env (need file write access or passwordless sudo)" >&2
-    return 1
+    rc=1
   fi
   rm -f "$tmp"
+  return "$rc"
 }

infra/deploy/secrets/dry-run-render.sh

@@ -19,6 +19,10 @@ tmp="$(mktemp)"; live="$(mktemp)"; trap 'rm -f "$tmp" "$live"' EXIT
 [ -f "$common" ] && SOPS_AGE_KEY="$keymat" sops -d --input-type yaml --output-type dotenv "$common" >> "$tmp"
 SOPS_AGE_KEY="$keymat" sops -d --input-type yaml --output-type dotenv "$host" >> "$tmp"
 
+# sudo is only for the READ of the root-owned live env; the redirect target is our
+# own mktemp file, so the redirect correctly runs as the invoking user (sudo tee
+# would wrongly write it as root).
+# shellcheck disable=SC2024
 sudo cat /etc/thermograph.env > "$live"
 python3 - "$live" "$tmp" <<'PY'
 import sys

infra/deploy/stack/autoscale.sh

@@ -18,7 +18,11 @@
 #   - Clamped to [MIN_REPLICAS, MAX_REPLICAS]; scaling waits for convergence
 #     (--detach=false), so a stuck rollout blocks further changes rather than
 #     stacking them.
-set -eu
+# pipefail: without it a `docker stats` failure on the left of the avg_cpu
+# pipe is silently swallowed by awk's exit 0 and the loop trusts the output.
+# (Runs under busybox ash in docker:*-cli, which supports pipefail.)
+# shellcheck disable=SC3040 # pipefail is not POSIX, but the busybox ash this runs under has it
+set -euo pipefail
 
 STACK_NAME="${STACK_NAME:-thermograph}"
 SERVICE="${STACK_NAME}_web"
@@ -64,7 +68,10 @@ while :; do
 
   cur=$(replicas)
   [ -n "$cur" ] || { log "service $SERVICE not found; waiting"; continue; }
-  cpu=$(avg_cpu)
+  # With pipefail, a transient `docker stats` failure makes avg_cpu return
+  # non-zero; treat that as a missed sample (don't trust partial output, and
+  # don't let set -e kill the autoscaler over a daemon hiccup).
+  cpu=$(avg_cpu) || cpu=""
   [ -n "$cpu" ] || continue   # no running tasks visible this sample
 
   if [ "$cpu" -gt "$UP_AT" ]; then

infra/deploy/stack/deploy-stack.sh

@@ -49,11 +49,16 @@ export STACK_NAME
 # install the uid-10001-readable copy the tasks' env-entrypoint shim sources.
 # 10001 = the app images' `thermograph` user; the file is 0400 to that uid.
 if [ -f "$APP_DIR/infra/deploy/render-secrets.sh" ]; then
-  # shellcheck source=deploy/render-secrets.sh
+  # shellcheck source=infra/deploy/render-secrets.sh
   . "$APP_DIR/infra/deploy/render-secrets.sh"
   render_thermograph_secrets "$APP_DIR/infra"
 fi
-set -a; . /etc/thermograph.env 2>/dev/null || true; set +a
+# /etc/thermograph.env is rendered at deploy time from the SOPS vault — it
+# cannot exist at lint time, so don't ask shellcheck to follow it.
+set -a
+# shellcheck source=/dev/null
+. /etc/thermograph.env 2>/dev/null || true
+set +a
 sudo install -o 10001 -g 0 -m 0400 /etc/thermograph.env /etc/thermograph/stack.env \
   || install -o 10001 -g 0 -m 0400 /etc/thermograph.env /etc/thermograph/stack.env
 
@@ -65,6 +70,7 @@ export REGISTRY_HOST
 TAGS_FILE="$APP_DIR/infra/deploy/.stack-image-tags.env"
 _incoming_backend="${BACKEND_IMAGE_TAG:-}"
 _incoming_frontend="${FRONTEND_IMAGE_TAG:-}"
+# shellcheck source=/dev/null # untracked runtime artifact this script writes below
 if [ -f "$TAGS_FILE" ]; then set -a; . "$TAGS_FILE"; set +a; fi
 [ -n "$_incoming_backend" ] && BACKEND_IMAGE_TAG="$_incoming_backend"
 [ -n "$_incoming_frontend" ] && FRONTEND_IMAGE_TAG="$_incoming_frontend"