fix(931): a missing semgrep fails the security-scan release gate #933

Merged

timothy merged 6 commits from 931-scan-semgrep-fail into main 2026-10-30 00:52:55 +01:00

Conversation 1 Commits 6 Files Changed 2

+137 -28

timothy commented 2026-10-28 00:35:32 +01:00

Owner

Copy Link

Copy Source

fixes #931

Root cause

scripts/security-scan.sh treated a missing semgrep as WARN: ... SKIPPED and still exited 0, so a release gate could pass with its SAST half never run (v26.16.0 cut, 2026-10-27). The scan host's non-interactive PATH lacked semgrep (server-management#1072).

Change

• A missing semgrep exits 2 naming semgrep; an erroring semgrep fails the gate; ETV_SCAN_SKIP_SEMGREP=1 is the only explicit opt-out.
• --selftest now runs the script itself against stub semgrep/docker (present, failing, absent, opted-out); deleting the call site reddens it.
• Header comment and docs Exit-code contract cover every exit the script produces (exit 2 = a scanner half did not run: semgrep missing/erroring, or docker missing).
• docs/ci-cd.md states the invariant (semgrep on the scan host's non-interactive PATH, or the explicit opt-out plus a separate SAST run) and the release step no longer names an unverified host. The Dependency management section is untouched.

Measured

• bash scripts/security-scan.sh --selftest: all ZAP exit classifications and semgrep-gate cases OK, rc=0
• PYTHONPATH=. python3 -m pytest scripts/tests -q: 1599 passed, 3 skipped, rc=0
• check-doc-narrative.py --diff origin/main: 0 warnings

Review

<<REVIEW HISTORY
round 1: 2 lens(es); 0 blocking, 0 should-fix, 4 nit; clean — loop ended
REVIEW HISTORY>>

No fix commit exists because round one was clean. Cross-family review: not required (routine risk class under process.independent-review-rubric).

Deliberately left

Nothing deferred. If the release scan must run on jazz, semgrep needs installing there (server-management issue to be filed by the orchestrator).

🤖 Generated with Claude Code

fixes #931 ## Root cause `scripts/security-scan.sh` treated a missing `semgrep` as `WARN: ... SKIPPED` and still exited 0, so a release gate could pass with its SAST half never run (v26.16.0 cut, 2026-10-27). The scan host's non-interactive PATH lacked semgrep (server-management#1072). ## Change - A missing semgrep exits 2 naming semgrep; an erroring semgrep fails the gate; `ETV_SCAN_SKIP_SEMGREP=1` is the only explicit opt-out. - `--selftest` now runs the script itself against stub semgrep/docker (present, failing, absent, opted-out); deleting the call site reddens it. - Header comment and docs Exit-code contract cover every exit the script produces (exit 2 = a scanner half did not run: semgrep missing/erroring, or docker missing). - `docs/ci-cd.md` states the invariant (semgrep on the scan host's non-interactive PATH, or the explicit opt-out plus a separate SAST run) and the release step no longer names an unverified host. The Dependency management section is untouched. ## Measured - `bash scripts/security-scan.sh --selftest`: all ZAP exit classifications and semgrep-gate cases OK, rc=0 - `PYTHONPATH=. python3 -m pytest scripts/tests -q`: 1599 passed, 3 skipped, rc=0 - `check-doc-narrative.py --diff origin/main`: 0 warnings ## Review <<REVIEW HISTORY round 1: 2 lens(es); 0 blocking, 0 should-fix, 4 nit; clean — loop ended REVIEW HISTORY>> No fix commit exists because round one was clean. Cross-family review: not required (routine risk class under process.independent-review-rubric). ## Deliberately left Nothing deferred. If the release scan must run on jazz, semgrep needs installing there (server-management issue to be filed by the orchestrator). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

timothy referenced this pull request 2026-10-29 21:15:27 +01:00

script-tests jq tripwire: runner fleet now ships jq 1.7; re-pin --expect and update the jq contract #935

timothy referenced this issue from a commit 2026-10-29 23:39:02 +01:00

fix(935): script-tests jq tripwire accepts exactly {1.6, 1.7} until server-management#1080 pins the fleet

timothy added 6 commits 2026-10-30 00:03:50 +01:00

fix(931): a missing semgrep fails the security-scan release gate instead of warning and exiting 0 ... f21dc121d3

The SAST half is now required: require_semgrep exits 2 naming the tool before the
image pull; ETV_SCAN_SKIP_SEMGREP=1 stays the only skip. --selftest pins absent,
opt-out and present. docs/ci-cd.md states the invariant, not the stale host claim.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: f9ce6bc6-5b36-491a-8d72-89c1f70dd3e1

fix(931): pin the script's own exit in --selftest and fail the gate on a semgrep that errors ... 3f7febf656

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: f9ce6bc6-5b36-491a-8d72-89c1f70dd3e1

docs(931): name the host that can pass the SAST gate and the script's own exit 2 ... e04f75e9ec

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: f9ce6bc6-5b36-491a-8d72-89c1f70dd3e1

fix(931): pin the main-path semgrep block in --selftest and state every exit the script produces ... 2e4de81309

Round-3 review found two clause mutants of the main path surviving the selftest:
the sg_rc check forced true (M9) and the main-path opt-out widened to any
non-empty value (M10). The present case was pinned only through require_semgrep,
and no full shimmed run used a semgrep that succeeds.

--selftest now runs the script end to end with shimmed docker/curl/timeout (ZAP
clean) and a semgrep shim that records each call: succeeding (unset and =0) ->
exit 0 with semgrep run; failing -> exit 2; explicit opt-out with semgrep
present -> exit 0 with semgrep not run. Witnessed red: M9 fails two cases, M10
fails the skip=0 case, and forcing the main-path skip off (`if false`) fails
the opt-out case; M1-M8 stay red.

The header's exit contract claimed exit 2 always meant the SAST half did not
run, but a missing docker also exits 2. It now lists 0/1/2/other, with 2 as
"a scanner half did not run", and the host line states the requirement instead
of naming bumblebee.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: f9ce6bc6-5b36-491a-8d72-89c1f70dd3e1

docs(931): state the scan host's requirement instead of naming hosts, and every script exit ... 0e64328ca2

Round-3 review: the Security scanning section named bumblebee (has semgrep) and
jazz (needs it) with no date or source, the Exit-code contract still said the
script exits 0 for clean/WARN and 1 for FAIL, the exit-2 sentence said only the
SAST half, and the --selftest sentence claimed more main-path coverage than it
pinned.

- Security scanning drops host names: the requirement is a docker host with
  registry access, a checkout of the commit under test (semgrep scans the
  checkout the script lives in), and semgrep on the non-interactive PATH, or
  the opt-out plus a separate SAST run.
- The Exit-code contract lists 0/1/2/other; 2 is "a scanner half did not run"
  (semgrep missing or erroring, or docker missing). The --selftest sentence
  names exactly the cases it pins, now including the main-path ones.
- The Hosts table and release step 2 sent the scan to jazz. That dates from
  d6aec6de7 (2026-07-20), which moved it there with the transcoders, but the
  scan boots its own throwaway target from the registry and needs nothing
  jazz-local. Step 2 now states the requirement, with the one measured host
  dated and sourced (bumblebee, 2026-10-27, server-management#1072). jazz had
  no semgrep when measured 2026-10-27, so following the old step would fail
  the gate with exit 2.
- Edited prose rewrapped to the section's ~105 columns.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: f9ce6bc6-5b36-491a-8d72-89c1f70dd3e1

docs(931): a failing setup command's exit is its own status, and the Hosts row names no excluded host ... f71e2fd0d0

The exit list gave "any other code" to a set -e setup failure, but `docker pull`
usually exits 1, which collides with the ZAP FAIL code; it now says such a
failure exits with that command's own status. The jazz Hosts row said the scan
runs "not [on] this one", which would stay wrong once jazz gets semgrep; it now
says any docker host that meets the requirements.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: f9ce6bc6-5b36-491a-8d72-89c1f70dd3e1

timothy force-pushed 931-scan-semgrep-fail from c645c9b96d to f71e2fd0d0 2026-10-30 00:03:50 +01:00 Compare

timothy commented 2026-10-30 00:52:31 +01:00

Author

Owner

Copy Link

Copy Source

Review-verdict: MERGEABLE @ f71e2fd

Orchestrator verdict at f71e2fd0d (a pure rebase of c645c9b96 onto main after #938; the patch is unchanged). Final review round clean. CI green, including script-tests under the new jq set. pytest 1599 passed at c645c9b96. Nits not fixed: opt-out predicate mutants M16/M17 survive; the docker-missing message names bumblebee.

Review-verdict: MERGEABLE @ f71e2fd Orchestrator verdict at f71e2fd0d (a pure rebase of c645c9b96 onto main after #938; the patch is unchanged). Final review round clean. CI green, including script-tests under the new jq set. pytest 1599 passed at c645c9b96. Nits not fixed: opt-out predicate mutants M16/M17 survive; the docker-missing message names bumblebee.

timothy merged commit 4ba56848f6 into main 2026-10-30 00:52:55 +01:00

timothy deleted branch 931-scan-semgrep-fail 2026-10-30 00:52:56 +01:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

ad-hoc

One-off / ad-hoc work not tracked by a dedicated issue

api

REST API / HTTP endpoints

bug

Something isn't working

ci-cd

Build, test, deploy pipeline

content

Channel content / schedules / playlists

dependencies

Dependency updates (Renovate)

enhancement

New feature or improvement

frontend

ChicoryTV React SPA frontend

in-progress

Claimed by an active session — do not pick up

jellyfin

Jellyfin tuner / IPTV integration

parked

Excluded from automatic queue pickup; work only when explicitly selected

priority: high

priority: low

priority: medium

review

Adversarial review finding

security

Security / vulnerability fix

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#933