Merge pull request 'fix(931): a missing semgrep fails the security-scan release gate' (#933) from 931-scan-semgrep-fail into main

docs/ci-cd.md

@@ -9,7 +9,7 @@ server-management#481.
 
 | Host | IP | Role |
 |---|---|---|
-| **jazz** | 192.168.1.29 | **Docker host for the media transcoders** — prod `ersatztv` (8409), `ersatztv-test` (8410), Jellyfin. Release scans (`security-scan.sh`) and the prod-copy `migration-smoke.sh` run here. |
+| **jazz** | 192.168.1.29 | **Docker host for the media transcoders** — prod `ersatztv` (8409), `ersatztv-test` (8410), Jellyfin. The prod-copy `migration-smoke.sh` runs here. The release scan (`security-scan.sh`) boots its own throwaway target from the registry, so it runs on any docker host that meets its requirements (Security scanning → Where/when). |
 | **bumblebee** | 192.168.1.99 | CI runners (`bumblebee-runner`, `small-runner`), plus every other Docker stack. All the memory/lane measurements below were taken here. |
 | **ci-runner** | VM 127 (pve4) | The other `ubuntu-latest` CI runner. |
 
@@ -116,8 +116,12 @@ commit in the range (see the `decisions.md` header) — routine lifecycle metada
    > protection is never left off) — and record the outcome on `release.tag-protection-v-star`,
    > which is waiting for it.
 2. Wait for tag CI to build `:prod` + the immutable `:<version>` + `:<sha>` images. Run
-   `scripts/security-scan.sh` on jazz against **the immutable `:<version>` image**, not a
+   `scripts/security-scan.sh` against **the immutable `:<version>` image**, not a moving tag, from a
-   moving tag, and triage every ZAP/semgrep finding.
+   checkout of the tagged commit on a docker host whose non-interactive `PATH` has semgrep: check with
+   `ssh <host> command -v semgrep` first (bumblebee passed that check on 2026-10-27,
+   server-management#1072). A host without semgrep fails the gate with exit 2; the only alternative is
+   `ETV_SCAN_SKIP_SEMGREP=1` plus a separate semgrep run against the tagged commit, recorded as such.
+   Triage every ZAP/semgrep finding. Requirements: Security scanning → Where/when.
 3. Only after the candidate passes, manually `DeployStack jazz-media` and observe its pre-deploy
    output. Prod's compose deliberately follows floating `:prod` (Timothy's 2026-07-11 decision), so
    no CI push or pin bump is needed.
@@ -2660,15 +2664,21 @@ running product from outside our C#/review stack.
   `/api/v1` operation, injecting `X-Api-Key` on every request via a ZAP replacer rule so it reaches the
   `[RequiresAuthentication]` + `RequireKeyForReads` surface (not just the `/app` shell an unauthenticated
   spider sees). Then a **semgrep** SAST cross-check (`p/security-audit` + `p/secrets` + `p/csharp`). The
-  container is torn down on exit. **semgrep runs only if it is installed on that host**, and otherwise the
+  container is torn down on exit. **The SAST half is required** (ersatztv#931): with no semgrep on `PATH`
-  script prints a WARN and still exits 0. As of 2026-10-27 bumblebee has no semgrep, so run it separately
+  the script exits 2 naming the tool, before the image is pulled. The only skip is the explicit opt-out
-  against a `git archive` of the tagged commit (the Mac has it) and triage both halves.
+  `ETV_SCAN_SKIP_SEMGREP=1` (any other value, `0` included, does not skip). A run that sets it is DAST
-- **Where/when.** Runs on a **docker host** with registry access (bumblebee, as the script's own header names, or jazz), like
+  only and has not passed the SAST half: run semgrep separately and say so in the release record. A
-  `migration-smoke.sh`: `scripts/security-scan.sh [IMAGE] [PORT]` (defaults `…:latest` / `8411`). It is a
+  semgrep that is installed but exits non-zero (bad config, registry unreachable) also fails the script,
-  **manual release-gate**, deliberately **not** a per-PR CI job — it needs docker + a booted image, takes
+  since findings alone exit 0.
-  several minutes, and is noisy (expect to tune, not take raw). The continuous layer is the per-PR
+- **Where/when.** Runs on a **docker host** with registry access, from a checkout of the commit under test
-  white-box gates + the weekly `dependency-scan`; this is the per-release black-box pass. Re-run it each
+  (semgrep scans the checkout the script lives in), whose **non-interactive** `PATH` has semgrep: the
-  release and before any change to the exposure posture.
+  `PATH` that `ssh <host> command -v semgrep` sees, which for zsh comes from `~/.zshenv`, not `.zshrc`. A
+  host without it fails the gate by construction; the alternative is the opt-out plus a separate SAST
+  run. Like `migration-smoke.sh`: `scripts/security-scan.sh [IMAGE] [PORT]` (defaults `…:latest` /
+  `8411`). It is a **manual release-gate**, deliberately **not** a per-PR CI job — it needs docker + a
+  booted image, takes several minutes, and is noisy (expect to tune, not take raw). The continuous layer
+  is the per-PR white-box gates + the weekly `dependency-scan`; this is the per-release black-box pass.
+  Re-run it each release and before any change to the exposure posture.
 - **Exit-code contract (ersatztv#338).** `zap-api-scan.py`'s raw exit code is NOT a simple pass/fail — it
   conflates a clean run with a warnings-only run unless you know its wrapper contract: **0** clean (no
   FAIL or WARN alerts), **2** WARN-only (triage required, but **not** release-blocking), **1** FAIL (at
@@ -2676,12 +2686,19 @@ running product from outside our C#/review stack.
   hung post-scan cleanup (the report written before the hang is still usable — triage it), any other
   code means the scanner/tool itself errored (not a scan result at all). `scripts/security-scan.sh`
   encodes this in `classify_zap_exit()` and prints an unambiguous `==> ZAP result: <PASS|WARN|FAIL|
-  TIMEOUT|TOOL ERROR> ...` line; the script's own exit status reflects that classification (0 for
+  TIMEOUT|TOOL ERROR> ...` line. The script's **own** exit is not ZAP's raw code, so a warnings-only run
-  clean/WARN, 1 for FAIL/timeout/tool-error) rather than ZAP's raw code, so a warnings-only run no longer
+  no longer reads as a failed scan. Found when the v26.8.0 release scan (#335) returned raw exit 2 for a
-  reads as a failed scan. Found when the v26.8.0 release scan (#335) returned raw exit 2 for a report with
+  report with `FAIL-NEW: 0` and two known/expected warning classes — the shell result looked like a
-  `FAIL-NEW: 0` and two known/expected warning classes — the shell result looked like a failure though the
+  failure though the release gate had actually passed. Every non-zero script exit is release-blocking:
-  release gate had actually passed. Run `scripts/security-scan.sh --selftest` for a docker-free regression
+  **0** ZAP clean or WARN-only and the SAST half ran clean (or was opted out); **1** ZAP FAIL, timeout or
-  check of the classification logic.
+  tool error, or the throwaway target never came up or its key was unreadable; **2** — *not* ZAP's WARN —
+  a scanner half did not run: semgrep missing (before the scan) or erroring (after it), or docker
+  missing; if ZAP already classified non-zero, that status stands and the semgrep error prints beside
+  it. A setup command that fails under `set -e` (e.g. `docker pull`) exits with its own status. Run
+  `scripts/security-scan.sh --selftest` for a docker-free regression check: it pins every ZAP
+  classification, and runs the script itself with shimmed tools (ZAP reporting clean) to pin the
+  semgrep gate — missing → exit 2 before docker is checked, erroring → 2, succeeding → 0 with semgrep
+  run, opt-out → 0 with semgrep not run (`=0` still runs it). It exercises no real ZAP or semgrep.
 - **Triage.** Triage each WARN/FAIL finding false-positive vs real. Real, in-scope, go-live-blocking
   findings get fixed (e.g. the security headers from the #319 baseline; the Microsoft.OpenApi pin above);
   LAN-expected noise (Private-IP disclosure) is revisited only for genuine remote exposure. nuclei

scripts/security-scan.sh

@@ -10,7 +10,8 @@
 # injection reflections, known-CVE fingerprints. It is a #197 exit criterion — a HARD GATE before remote
 # exposure — and should be re-run each release (see docs/ci-cd.md → Security scanning).
 #
-# It runs on the docker host (bumblebee) against a THROWAWAY container booted from the image under test —
+# It runs on a docker host with registry access and semgrep on its non-interactive PATH (docs/ci-cd.md →
+# Security scanning → Where/when), against a THROWAWAY container booted from the image under test —
 # NEVER the deployed prod (or test) container: the authenticated active scan sends attack payloads to
 # write endpoints (creating/deleting rows, triggering playout builds), so it must target a disposable
 # instance with a fresh empty config volume. The container is torn down on exit.
@@ -21,7 +22,7 @@
 #     PORT   host port for the throwaway app  (default: 8411)
 #   Env:
 #     ETV_SCAN_OUT   host dir for reports      (default: /tmp/etv-scan-out)
-#     ETV_SCAN_SKIP_SEMGREP=1  skip the SAST pass (DAST only)
+#     ETV_SCAN_SKIP_SEMGREP=1  skip the SAST pass (DAST only) — the ONLY skip; a missing semgrep otherwise exits 2
 #
 # Reports land in $ETV_SCAN_OUT: zap-api-report.{html,md,json}. zap-api-scan.py's exit code is NOT a
 # simple pass/fail — its wrapper contract (ersatztv#338, found via the #335 exact-image release scan
@@ -31,8 +32,14 @@
 #   1     FAIL  — at least one FAIL-level alert — release-blocking
 #   124   our own `timeout` wrapper killed a hung wrapper (see the NOTE below) — report is still usable
 #   other the scanner/tool itself errored (bad args, crash) — not a scan result at all
-# classify_zap_exit() (below) is the single place this contract is encoded; the script's own final exit
+# classify_zap_exit() (below) is the single place this contract is encoded. The SCRIPT's own exit is not
-# code reflects the CLASSIFICATION (0 for clean/WARN, 1 for FAIL/timeout/tool-error), not ZAP's raw code.
+# ZAP's raw code; every non-zero exit is release-blocking:
+#   0     ZAP clean or WARN-only, AND the SAST half ran clean (or was opted out: ETV_SCAN_SKIP_SEMGREP=1)
+#   1     ZAP FAIL / timeout / tool error, or the throwaway target never came up or its key was unreadable
+#   2     NOT ZAP's WARN: a scanner half did not run — semgrep missing (checked before the scan) or erroring
+#         (after it), or docker missing. If ZAP already classified non-zero, that status stands and the
+#         semgrep error is printed alongside it.
+# A setup command that fails under `set -e` (e.g. `docker pull`) exits with that command's own status.
 
 set -euo pipefail
 
@@ -50,6 +57,24 @@ classify_zap_exit() {
   esac
 }
 
+# require_semgrep — the SAST half is part of the gate: a missing semgrep is an ERROR naming the tool, never a
+# warning that still exits 0 (ersatztv#931 — a v26.16.0 release gate could pass with its SAST half never run).
+# The only skip is the explicit opt-out ETV_SCAN_SKIP_SEMGREP=1. Standalone so `--selftest` can exercise it.
+require_semgrep() {
+  [ "${ETV_SCAN_SKIP_SEMGREP:-0}" = "1" ] && return 0
+  command -v semgrep >/dev/null && return 0
+  echo "error: semgrep not found on PATH — the SAST half of the release gate cannot run. Install semgrep, or set ETV_SCAN_SKIP_SEMGREP=1 to skip it explicitly (DAST only)." >&2
+  return 2
+}
+
+# run_semgrep ROOT — the SAST pass. Without --error semgrep exits 0 on findings (they print above), so ANY
+# non-zero exit means semgrep itself failed (bad config, registry unreachable, crash): the SAST half did not
+# run, which must fail the gate exactly like a missing semgrep (ersatztv#931). Returns semgrep's own status.
+run_semgrep() {
+  semgrep scan --config p/security-audit --config p/secrets --config p/csharp \
+    --exclude web/node_modules --exclude design-system --metrics off --quiet "$1"
+}
+
 if [ "${1:-}" = "--selftest" ]; then
   # Lightweight regression check for the classification contract above (ersatztv#338 Done-when) — no
   # docker/ZAP required, so it can run anywhere (incl. off bumblebee, incl. in per-PR CI if ever added).
@@ -70,8 +95,71 @@ if [ "${1:-}" = "--selftest" ]; then
   selftest_check 124 1 "TIMEOUT"
   selftest_check 3 1 "TOOL ERROR"
   selftest_check 77 1 "TOOL ERROR"
+  # semgrep gate (ersatztv#931): absent -> non-zero naming the tool; opt-out -> 0; present -> 0.
+  semgrep_check() {
+    local label="$1" want_status="$2" want_substr="$3" got_status err
+    err="$(require_semgrep 2>&1 >/dev/null)" && got_status=0 || got_status=$?
+    if [ "$got_status" != "$want_status" ] || [[ "$err" != *"$want_substr"* ]]; then
+      echo "SELFTEST FAIL: semgrep gate ($label) -> status=$got_status msg='$err' (want status=$want_status, msg containing '$want_substr')" >&2
+      return 1
+    else
+      echo "selftest ok: semgrep gate ($label) -> status=$got_status"
+    fi
+  }
+  sg_dir="$(mktemp -d)"; sg_empty="$(mktemp -d)"
+  printf '#!/bin/sh\n' > "$sg_dir/semgrep"; chmod +x "$sg_dir/semgrep"
+  (PATH="$sg_empty"; unset ETV_SCAN_SKIP_SEMGREP; semgrep_check "absent" 2 "semgrep") || selftest_failed=1
+  (PATH="$sg_empty"; ETV_SCAN_SKIP_SEMGREP=0; semgrep_check "absent, skip=0" 2 "semgrep") || selftest_failed=1
+  (PATH="$sg_empty"; ETV_SCAN_SKIP_SEMGREP=1; semgrep_check "absent, explicit opt-out" 0 "") || selftest_failed=1
+  (PATH="$sg_dir"; unset ETV_SCAN_SKIP_SEMGREP; semgrep_check "present" 0 "") || selftest_failed=1
+  # End-to-end through the real main path (pins the call sites, not just the helper). Absent semgrep + no
+  # docker on PATH: must exit 2 with the semgrep message BEFORE the docker check (which also exits 2).
+  out="$(env -u ETV_SCAN_SKIP_SEMGREP PATH="$sg_empty" "$BASH" "${BASH_SOURCE[0]}" 2>&1)" && rc=0 || rc=$?
+  if [ "$rc" = 2 ] && [[ "$out" == *"error: semgrep not found"* ]]; then
+    echo "selftest ok: script, semgrep absent -> exit 2 naming semgrep"
+  else
+    echo "SELFTEST FAIL: script with semgrep absent -> rc=$rc out='$out' (want rc=2 naming semgrep)" >&2; selftest_failed=1
+  fi
+  out="$(ETV_SCAN_SKIP_SEMGREP=1 PATH="$sg_empty" "$BASH" "${BASH_SOURCE[0]}" 2>&1)" && rc=0 || rc=$?
+  if [ "$rc" = 2 ] && [[ "$out" == *"docker not found"* ]] && [[ "$out" != *"semgrep not found"* ]]; then
+    echo "selftest ok: script, explicit opt-out -> gate skipped (stops at docker check)"
+  else
+    echo "SELFTEST FAIL: script with opt-out -> rc=$rc out='$out' (want to pass the semgrep gate)" >&2; selftest_failed=1
+  fi
+  # Full runs through the MAIN-path SAST block, with shimmed docker/curl/timeout (ZAP reports clean, so the
+  # script's exit is decided by the semgrep half alone). The semgrep shim records each call in
+  # $ETV_SCAN_OUT/semgrep.called and exits $SG_SHIM_RC, so a case can assert both the exit and whether
+  # SAST actually ran. These pin the main-path opt-out test and the sg_rc check, not just the helper.
+  sg_shim="$(mktemp -d)"; sg_out="$(mktemp -d)"
+  printf '#!/bin/sh\n[ "$1" = exec ] && echo shimkey\nexit 0\n' > "$sg_shim/docker"
+  printf '#!/bin/sh\necho 200\n' > "$sg_shim/curl"
+  printf '#!/bin/sh\nshift\nexec "$@"\n' > "$sg_shim/timeout"
+  printf '#!/bin/sh\necho called >> "$ETV_SCAN_OUT/semgrep.called"\n[ "${SG_SHIM_RC:-0}" = 0 ] || echo "fake semgrep: registry unreachable" >&2\nexit "${SG_SHIM_RC:-0}"\n' > "$sg_shim/semgrep"
+  chmod +x "$sg_shim"/*
+  # full_check LABEL SG_SHIM_RC SKIP WANT_RC WANT_CALLED WANT_SUBSTR — SKIP is "unset" or the value to export.
+  full_check() {
+    local label="$1" sg_shim_rc="$2" skip="$3" want_rc="$4" want_called="$5" want_substr="$6" out rc called
+    rm -f "$sg_out/semgrep.called"
+    if [ "$skip" = unset ]; then
+      out="$(env -u ETV_SCAN_SKIP_SEMGREP SG_SHIM_RC="$sg_shim_rc" PATH="$sg_shim:$PATH" ETV_SCAN_OUT="$sg_out" "$BASH" "${BASH_SOURCE[0]}" 2>&1)" && rc=0 || rc=$?
+    else
+      out="$(ETV_SCAN_SKIP_SEMGREP="$skip" SG_SHIM_RC="$sg_shim_rc" PATH="$sg_shim:$PATH" ETV_SCAN_OUT="$sg_out" "$BASH" "${BASH_SOURCE[0]}" 2>&1)" && rc=0 || rc=$?
+    fi
+    if [ -e "$sg_out/semgrep.called" ]; then called=1; else called=0; fi
+    if [ "$rc" = "$want_rc" ] && [ "$called" = "$want_called" ] && [[ "$out" == *"$want_substr"* ]]; then
+      echo "selftest ok: script, $label -> exit $rc, semgrep called=$called"
+    else
+      echo "SELFTEST FAIL: script, $label -> rc=$rc called=$called (want rc=$want_rc called=$want_called, output containing '$want_substr')" >&2
+      selftest_failed=1
+    fi
+  }
+  full_check "semgrep succeeds" 0 unset 0 1 "==> semgrep SAST cross-check"
+  full_check "semgrep succeeds, skip=0" 0 0 0 1 "==> semgrep SAST cross-check"
+  full_check "semgrep fails" 2 unset 2 1 "error: semgrep failed (exit 2)"
+  full_check "explicit opt-out, semgrep present" 0 1 0 0 "semgrep SAST skipped (ETV_SCAN_SKIP_SEMGREP=1)"
+  rm -rf "$sg_dir" "$sg_empty" "$sg_shim" "$sg_out"
   if [ "$selftest_failed" = 0 ]; then
-    echo "selftest: all ZAP exit classifications OK"; exit 0
+    echo "selftest: all ZAP exit classifications and semgrep-gate cases OK"; exit 0
   else
     echo "selftest: FAILURES ABOVE" >&2; exit 1
   fi
@@ -83,6 +171,9 @@ OUT="${ETV_SCAN_OUT:-/tmp/etv-scan-out}"
 TARGET_NAME="etv-secscan-target"
 ZAP_IMAGE="ghcr.io/zaproxy/zaproxy:stable"
 
+# Fail fast, before the image pull and the multi-minute ZAP run, if the SAST half cannot run.
+require_semgrep || exit $?
+
 command -v docker >/dev/null || { echo "error: docker not found — run this on the docker host (bumblebee)" >&2; exit 2; }
 
 CFG="$(mktemp -d /tmp/etv-secscan-cfg-XXXXXX)"
@@ -145,14 +236,15 @@ echo "==> ZAP result: $ZAP_CLASS_MSG (raw exit $zap_rc)"
 
 if [ "${ETV_SCAN_SKIP_SEMGREP:-0}" = "1" ]; then
   echo "==> semgrep SAST skipped (ETV_SCAN_SKIP_SEMGREP=1)"
-elif ! command -v semgrep >/dev/null; then
-  echo "WARN: semgrep not installed — SAST cross-check SKIPPED (install semgrep for full coverage)" >&2
 else
   echo "==> semgrep SAST cross-check (repo root)"
   REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
-  semgrep scan --config p/security-audit --config p/secrets --config p/csharp \
+  sg_rc=0
-    --exclude web/node_modules --exclude design-system --metrics off --quiet \
+  run_semgrep "$REPO_ROOT" || sg_rc=$?
-    "$REPO_ROOT" || echo "(semgrep reported findings — triage in the report above)"
+  if [ "$sg_rc" != 0 ]; then
+    echo "error: semgrep failed (exit $sg_rc) — the SAST half did not run; this is a gate failure, not a finding" >&2
+    [ "$zap_class_status" != 0 ] || zap_class_status=2
+  fi
 fi
 
 exit "$zap_class_status"