diff --git a/.forgejo/workflows/shell-lint.yml b/.forgejo/workflows/shell-lint.yml new file mode 100644 index 0000000..6aa69eb --- /dev/null +++ b/.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 diff --git a/backend/scripts/smoke.sh b/backend/scripts/smoke.sh index 4854c3b..7165756 100755 --- a/backend/scripts/smoke.sh +++ b/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 diff --git a/frontend/scripts/capture-fixtures.sh b/frontend/scripts/capture-fixtures.sh index de7578b..7641883 100755 --- a/frontend/scripts/capture-fixtures.sh +++ b/frontend/scripts/capture-fixtures.sh @@ -18,7 +18,7 @@ trap 'scripts/backend-for-tests.sh down >/dev/null 2>&1 || true' EXIT fetch() { # 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/" diff --git a/infra/deploy/deploy.sh b/infra/deploy/deploy.sh index b4a56ce..4b8f8f3 100755 --- a/infra/deploy/deploy.sh +++ b/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" diff --git a/infra/deploy/render-secrets.sh b/infra/deploy/render-secrets.sh index b44a965..d4a434e 100755 --- a/infra/deploy/render-secrets.sh +++ b/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: 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" } diff --git a/infra/deploy/secrets/dry-run-render.sh b/infra/deploy/secrets/dry-run-render.sh index 03533f1..44b41a1 100755 --- a/infra/deploy/secrets/dry-run-render.sh +++ b/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 diff --git a/infra/deploy/stack/autoscale.sh b/infra/deploy/stack/autoscale.sh index 2a36ade..b737bab 100755 --- a/infra/deploy/stack/autoscale.sh +++ b/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 diff --git a/infra/deploy/stack/deploy-stack.sh b/infra/deploy/stack/deploy-stack.sh index 8833aec..8516a6b 100755 --- a/infra/deploy/stack/deploy-stack.sh +++ b/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"