Skip to content

fix(ci): stop the nox security gate skipping itself into a green check - #47

Merged
felixgeelhaar merged 1 commit into
mainfrom
fix/security-gate-cannot-silently-skip
Jul 25, 2026
Merged

fix(ci): stop the nox security gate skipping itself into a green check#47
felixgeelhaar merged 1 commit into
mainfrom
fix/security-gate-cannot-silently-skip

Conversation

@felixgeelhaar

Copy link
Copy Markdown
Contributor

Two silent failures in the shared nox security job. Both are the same shape: a hashFiles() step guard turning a missing artifact into a green check.

1. The gate could skip itself

- name: Gate on net-new critical/high findings
  if: always() && hashFiles('nox-results/findings.json') != ''

A skipped step is a green step. If nox wrote no findings.json, the gate did not fail, did not warn, and did not appear in the log — the job simply passed having evaluated nothing.

This is live. A repo-level .nox.yaml containing

output:
  format: sarif

silently overrides the workflow's own -format json,sarif — the config wins over the explicit CLI flag (filed as Nox-HQ/nox#362). Reproduced in a neutral directory: same command, same nox, findings.json present without that config and absent with it.

Two repos are in exactly that state right now, both with committed baselines, both green:

Repo SARIF results findings.json Gate
nexa 20 never written never ran
lexora 63 never written never ran

Absence is now handled in-script instead of by the step guard — fatal where the repo has committed a baseline or sets nox-strict, a warning otherwise. That keeps the documented property that adopting this workflow never breaks a red build on day one.

2. No Go repo has ever uploaded SARIF to code scanning

go-ci referenced nox-results/findings.sarif. nox has never written that file — it writes results.sarif. So the hashFiles() guard never matched and the upload never ran, fleet-wide, silently. js-ci was already fixed for this and carries a comment explaining it; go-ci was missed.

Verification

I ran the new gate script directly over all six cases rather than reasoning about it:

Case Exit
no findings.json, no baseline 0 warns, report-only
no findings.json, has baseline 1 the bug, now fatal
no findings.json, nox-strict 1 fatal
findings.json, 1 net-new high 1 unchanged behaviour
findings.json, baselined only 0 unchanged behaviour
no baseline, net-new high 0 unchanged report-only

Both files still parse as YAML and retain the gate step.

Expected blast radius

nexa and lexora will go red on merge. That is the intended effect — they have 20 and 63 unevaluated findings between them and have been passing a gate that never executed. Their findings still need triage on the nox#361 evidence (many fleet findings are turning out to be false positives), but that triage should happen with the gate working, not instead of it.

No other repo changes behaviour: every other repo in the fleet already produces findings.json, so it takes the same path as before. SARIF uploads will start appearing for Go repos for the first time.

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
@felixgeelhaar
felixgeelhaar merged commit a4ae8d3 into main Jul 25, 2026
@felixgeelhaar
felixgeelhaar deleted the fix/security-gate-cannot-silently-skip branch July 25, 2026 22:12
felixgeelhaar added a commit that referenced this pull request Jul 25, 2026
Correcting the SARIF filename in #47 made this step reachable for the
first time -- and it immediately failed the Security job on lexora with
"Resource not accessible by integration". Code scanning ingest requires
GitHub Advanced Security on private repositories, and most of the fleet
is private.

That is a regression introduced by #47: while the step was looking for a
filename nox never writes, hashFiles() never matched, so the missing
permission could not surface. Fixing the lookup exposed it.

continue-on-error, because this upload is reporting, not enforcement. The
gate step below is what decides pass/fail, and it is unaffected -- on the
same lexora run it evaluated correctly and reported "net-new critical/
high: 0 (baselined findings: 57)". A repo without Advanced Security
should still get its security gate; it just does not also get the
findings mirrored into the code-scanning UI.

Same rationale already applied to the taint-plugin install in this file.

Claude-Session: https://claude.ai/code/session_01LMVPJFeTCEtUk2CSNBBBdK
@felixgeelhaar

Copy link
Copy Markdown
Contributor Author

Follow-up: #47 had a regression, fixed in #49 (merged).

Correcting the SARIF filename made that upload step reachable for the first time, and it immediately failed lexora's whole Security job with Resource not accessible by integration — code-scanning ingest needs GitHub Advanced Security on private repos, and most of the fleet is private. While hashFiles() was looking for a filename nox never writes, the missing permission could not surface.

That is the same failure mode as the original bug, one layer down: a step that skips silently conceals whatever would have gone wrong inside it, and the concealment reads as health.

#49 makes the upload continue-on-error: true — it is reporting, not enforcement.

The gate half worked correctly on that same run, which is the thing #47 was actually for:

net-new critical/high: 0 (baselined findings: 57)

First time lexora's gate has ever executed, and it matched the local scan exactly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant