From a042623a53cfc5cf24469ddbcd7dda61e208fe84 Mon Sep 17 00:00:00 2001 From: Timothy Date: Mon, 13 Jul 2026 08:36:08 +0200 Subject: [PATCH] =?UTF-8?q?fix(ci):=20#314=20review=20=E2=80=94=20reap=20Z?= =?UTF-8?q?AP=20container=20in=20the=20cleanup()=20trap;=20warn=20on=20mis?= =?UTF-8?q?sing=20semgrep?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cold review of PR #331: - Move the ZAP-container reap from an inline post-scan line into cleanup() (the EXIT trap) so a SIGINT/timeout kill mid-scan can't leave it running. - semgrep absent now prints a stderr WARN instead of silently skipping the SAST pass (no false confidence for a release gate). refs #314 Co-Authored-By: Claude Opus 4.8 (1M context) --- scripts/security-scan.sh | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/scripts/security-scan.sh b/scripts/security-scan.sh index 0e71b6a6d..013534d38 100755 --- a/scripts/security-scan.sh +++ b/scripts/security-scan.sh @@ -39,6 +39,9 @@ command -v docker >/dev/null || { echo "error: docker not found — run this on CFG="$(mktemp -d /tmp/etv-secscan-cfg-XXXXXX)" cleanup() { + # Reap the ZAP scanner container too — in the trap (not just inline after the scan) so an interrupt + # (SIGINT/SIGTERM) or a `timeout` kill mid-scan doesn't leave it running. + docker ps --filter ancestor="$ZAP_IMAGE" -q | xargs -r docker kill >/dev/null 2>&1 || true docker rm -f "$TARGET_NAME" >/dev/null 2>&1 || true rm -rf "$CFG" 2>/dev/null || true } @@ -85,11 +88,15 @@ timeout "${ETV_SCAN_TIMEOUT:-45m}" docker run --rm --network host -v "$OUT":/zap zap_rc=$? [ "$zap_rc" = 124 ] && echo "NOTE: ZAP wrapper hit the ${ETV_SCAN_TIMEOUT:-45m} timeout (usually a post-scan cleanup hang) — the report written before the hang is still valid; triage it." set -e -# A timed-out `docker run` leaves the ZAP container; reap any stragglers so a re-run is clean. -docker ps --filter ancestor="$ZAP_IMAGE" -q | xargs -r docker kill >/dev/null 2>&1 || true +# (The ZAP container + target + temp dir are reaped by cleanup() on the EXIT trap — every path, incl. +# a `timeout` kill or Ctrl-C mid-scan.) echo "==> ZAP report: $OUT/zap-api-report.html (+ .md/.json). Exit code: $zap_rc" -if [ "${ETV_SCAN_SKIP_SEMGREP:-0}" != "1" ] && command -v semgrep >/dev/null; then +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 \