From a5ba8ea73297f8cc3bccbb1fe63bafece8af75e6 Mon Sep 17 00:00:00 2001 From: Felix Geelhaar Date: Sun, 26 Jul 2026 00:08:43 +0200 Subject: [PATCH] fix(ci): stop the nox security gate skipping itself into a green check The gate step was guarded by `if: hashFiles('nox-results/findings.json') != ''`. A skipped step is a green step, so whenever nox wrote no JSON the gate did not fail, did not warn, and did not appear -- the job passed having evaluated nothing. That is not hypothetical. A repo-level `.nox.yaml` with `output.format: sarif` silently overrides the workflow's own `-format json,sarif`, so no findings.json is produced at all. nexa (20 findings) and lexora (63) are both in that state today, both with committed baselines, and both have been passing this job with the gate absent. Absence is now handled in the script rather than by the step guard: fatal where the repo has committed a baseline or sets nox-strict, a warning otherwise, which preserves the documented property that adopting the shared workflow never breaks a red build on day one. go-ci also looked for `nox-results/findings.sarif`, which nox has never written -- the file is `results.sarif`. Both the hashFiles() guard and the upload therefore never matched, so no Go repo in the fleet has ever uploaded SARIF to code scanning. js-ci was fixed for this; go-ci was not. Verified by executing the new gate script over all six cases: missing file with and without a baseline and under strict, present file with a net-new high, baselined-only, and no-baseline-with-findings. Claude-Session: https://claude.ai/code/session_01LMVPJFeTCEtUk2CSNBBBdK --- .github/workflows/go-ci.yml | 24 +++++++++++++++++++++--- .github/workflows/js-ci.yml | 20 +++++++++++++++++++- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/.github/workflows/go-ci.yml b/.github/workflows/go-ci.yml index 78c2ff5..60ad452 100644 --- a/.github/workflows/go-ci.yml +++ b/.github/workflows/go-ci.yml @@ -195,10 +195,10 @@ jobs: # gate step below decides what is actually fatal (net-new crit/high). run: nox scan ${{ inputs.working-directory }} -format json,sarif -output nox-results || true - name: Upload SARIF to code scanning - if: always() && hashFiles('nox-results/findings.sarif') != '' + if: always() && hashFiles('nox-results/results.sarif') != '' uses: github/codeql-action/upload-sarif@7211b7c8077ea37d8641b6271f6a365a22a5fbfa # v4 with: - sarif_file: nox-results/findings.sarif + sarif_file: nox-results/results.sarif - name: Annotate PR if: github.event_name == 'pull_request' && hashFiles('nox-results/findings.json') != '' env: @@ -210,11 +210,29 @@ jobs: # set). Un-baselined repos are report-only so adopting the shared # workflow never breaks a red build — run `nox baseline add` once to # turn enforcement on. Baselined repos gate on net-new crit/high. - if: always() && hashFiles('nox-results/findings.json') != '' + # + # Deliberately NOT guarded by hashFiles(findings.json). A missing file + # used to skip this step, and a skipped step is green -- so the gate + # quietly ceased to exist for any repo where nox wrote no JSON. That is + # live today: a repo-level .nox.yaml with `output.format: sarif` + # overrides the `-format json,sarif` above, and nexa (20 findings) and + # lexora (63) have been passing this job without it ever running. + # Absence is now handled in-script and is fatal wherever the repo has + # committed a baseline. + if: always() env: NOX_STRICT: ${{ inputs.nox-strict }} + WORKDIR: ${{ inputs.working-directory }} run: | f=nox-results/findings.json + if [ ! -f "$f" ]; then + if [ -f "${WORKDIR}/.nox/baseline.json" ] || [ "$NOX_STRICT" = "true" ]; then + echo "::error::nox wrote no findings.json, so the security gate evaluated nothing. Most likely .nox.yaml sets output.format, which overrides this workflow's '-format json,sarif' -- drop that key, or include json in it." + exit 1 + fi + echo "::warning::nox wrote no findings.json, so the security gate evaluated nothing. Report-only (no committed baseline), so not failing the build." + exit 0 + fi baselined=$(jq '[.findings[]? | select(.Status=="baselined")] | length' "$f" 2>/dev/null || echo 0) new=$(jq '[.findings[]? | select((.Severity=="critical" or .Severity=="high") and (.Status!="baselined"))] | length' "$f" 2>/dev/null || echo 0) echo "net-new critical/high: $new (baselined findings: $baselined)" diff --git a/.github/workflows/js-ci.yml b/.github/workflows/js-ci.yml index 29174ed..7c11c27 100644 --- a/.github/workflows/js-ci.yml +++ b/.github/workflows/js-ci.yml @@ -176,11 +176,29 @@ jobs: # Self-adjusting: report-only until the repo commits a nox baseline # (or nox-strict is set), then gates net-new critical/high. Adopting # the workflow never breaks a red build on day one. - if: always() && hashFiles('nox-results/findings.json') != '' + # + # Deliberately NOT guarded by hashFiles(findings.json). A missing file + # used to skip this step, and a skipped step is green -- so the gate + # quietly ceased to exist for any repo where nox wrote no JSON. That is + # live today: a repo-level .nox.yaml with `output.format: sarif` + # overrides the `-format json,sarif` above, and nexa (20 findings) and + # lexora (63) have been passing this job without it ever running. + # Absence is now handled in-script and is fatal wherever the repo has + # committed a baseline. + if: always() env: NOX_STRICT: ${{ inputs.nox-strict }} + WORKDIR: ${{ inputs.working-directory }} run: | f=nox-results/findings.json + if [ ! -f "$f" ]; then + if [ -f "${WORKDIR}/.nox/baseline.json" ] || [ "$NOX_STRICT" = "true" ]; then + echo "::error::nox wrote no findings.json, so the security gate evaluated nothing. Most likely .nox.yaml sets output.format, which overrides this workflow's '-format json,sarif' -- drop that key, or include json in it." + exit 1 + fi + echo "::warning::nox wrote no findings.json, so the security gate evaluated nothing. Report-only (no committed baseline), so not failing the build." + exit 0 + fi baselined=$(jq '[.findings[]? | select(.Status=="baselined")] | length' "$f" 2>/dev/null || echo 0) new=$(jq '[.findings[]? | select((.Severity=="critical" or .Severity=="high") and (.Status!="baselined"))] | length' "$f" 2>/dev/null || echo 0) echo "net-new critical/high: $new (baselined findings: $baselined)"