ADR-0157: Fail Open on the CodeQL SARIF Upload Only, and Treat CodeQL as Not a Control Until GHAS Is Enabled
Date: 2026-07-31 Status: Accepted Deciders: Adrian (Soft4U BV), Claude Opus (implementation + analysis), Codex (review finding P2 on PR #887)
Decision context:
-
Latency: no runtime impact — this is CI configuration only. CI wall-clock is unchanged for CodeQL (the same analysis runs; only the upload destination moves to a separate step) and improves for Trivy, whose three scans now run as three parallel jobs instead of three sequential steps.
-
Dependency surface:
github/codeql-action/analyze@v4andgithub/codeql-action/upload-sarif@v4are both already used in this workflow (upload-sarifby the Semgrep and Trivy-config jobs) — those are re-uses, not new dependencies.actions/upload-artifact@v4IS new, and an earlier draft of this ADR wrongly said it was not. The parent revision contains noupload-artifactanywhere in.github/workflows; this change introduces it in three jobs (CodeQL, Semgrep, Trivy-config). It is a first-party GitHub action, pinned to a major tag consistent with every other action here, and it executes in CI with access to the SARIF being uploaded. That is a real addition to the supply-chain surface and is recorded as one — an ADR that under-reports its own dependency delta is the same defect class this ADR exists to fix. -
Debuggability: strictly better. Today a red CodeQL job has exactly one possible cause presented (the whole action failed) and, with the blanket
continue-on-error, no cause at all — it is green regardless. After this change the failure modes are separated: a redAnalyzestep means the analysis genuinely broke, and a warning-annotatedUpload SARIF to code scanningstep means the 403. The SARIF is also retained as a build artifact, so for the first time there is something to open when asking "what did CodeQL actually find?". -
Reversibility: single-file, ~20 lines, minutes to undo. The exit condition is a deletion, not a migration: when GHAS is enabled, remove
upload: never/output:and the two follow-on steps, and the action returns to its default analyse-and-upload. -
Blast radius:
.github/workflows/security.ymlonly. No application code, no schema, no runtime flag. It cannot affect a case, a decision, or a risk score. The only consumer is CI. -
Alternative considered: keep the blanket
continue-on-error: trueonanalyzeand record it in an ADR (what the review literally asked for). Rejected because the ADR would then be recording a posture that converts a crashed analysis into a green check — the exact defect class this repository tracks. The narrower fix is available, so the ADR records the narrow decision instead of blessing the broad one.
Context
This repository is private, and GitHub code scanning for private repositories
requires GitHub Advanced Security (GHAS), which is not licensed here. Every
upload-sarif call in .github/workflows/security.yml therefore fails with:
##[error]Resource not accessible by integration
##[warning]… Code scanning is not enabled for this repository.
Three jobs in that workflow declared they "report to the Security tab". That destination does not exist. This is not a security finding — it is plumbing failing against a service that was never turned on — but it turned three jobs red, and a permanently red job for a non-security reason is how a gate that does mean something gets routed around. The workflow's own header comment has spent a milestone arguing exactly that.
PR #887 addressed this for Semgrep and Trivy-config by scoping continue-on-error to
the upload step only, leaving each scan untouched and blocking. For CodeQL it could
not do the same, because github/codeql-action/analyze appeared to be a single action
that both analyses and uploads. So continue-on-error: true was applied to the
analyze step as a whole, with a comment stating that step-scoping was impossible.
Review (Codex, P2 on PR #887) correctly identified the consequence. That blanket flag does not only tolerate the 403 — it converts every CodeQL failure into a successful job: a CodeQL CLI crash, a source-extraction failure, an out-of-memory kill during query evaluation, a malformed query pack. All of them go green. A SAST control that reports success when it did not run is the "claim vs check" defect class this codebase exists to eliminate, applied to one of the controls that is supposed to detect it. The review also noted that this posture is not a transient repair — it is intended to persist until an org-level licensing change — and therefore warrants a recorded decision.
Investigating the fix revealed that the premise in the code comment was false.
github/codeql-action/analyze@v4 declares an upload input accepting
always (default) | failure-only | never, and an output input naming the
directory the SARIF is written to. upload: never "avoids uploading the SARIF file to
Code Scanning, even if the code scanning run fails". Analysis and upload can be split,
using the same upload-sarif action the Semgrep job already uses. The workflow comment
and §5 of docs/security/dependency-exceptions.md both asserted the opposite; both were
wrong.
One further consequence, unaddressed until now: because analyze uploaded to a dead
destination and kept the SARIF nowhere else, the CodeQL job produced no retrievable
output whatsoever. It extracted, analysed, POSTed, 403'd, and discarded the results.
The Semgrep and Trivy-config jobs at least retain their SARIF as build artifacts.
CodeQL did not.
Decision
Fail open on the CodeQL SARIF upload, and only on the SARIF upload.
github/codeql-action/analyze@v4runs withupload: neverandoutput: codeql-sarif. This step carries nocontinue-on-error. Any analysis failure — extraction, query evaluation, OOM, malformed pack — fails the step and reddens the job, as it should.- The SARIF directory is uploaded as a build artifact (
codeql-sarif-<language>) withif: always(), so CodeQL's findings are retrievable today, without GHAS. This is added signal, not removed. - A separate
github/codeql-action/upload-sarif@v4step, withif: always()andcontinue-on-error: true, attempts the code-scanning upload. This step is the entire fail-open.
Scope of the fail-open: exactly one step, in one job, in one workflow — the code-scanning POST. It tolerates one evidenced condition: HTTP 403 "Code scanning is not enabled for this repository." It does not extend to analysis, extraction, query evaluation, or artifact retention. It matches the treatment already applied to the Semgrep and Trivy-config uploads, so all three SARIF producers now share one posture.
Ownership: the repository owner (Adrian / Soft4U BV), who is also the only party who can resolve it — enabling GHAS is a licensing decision, not an engineering one.
Exit condition: enabling GitHub Advanced Security on this repository. On that day,
delete upload: never and output: from the analyze step and delete the two
follow-on steps; the action returns to its default analyse-and-upload behaviour and the
upload becomes load-bearing again. The same removal applies to the Semgrep and
Trivy-config upload steps, each of which already carries a REMOVE-WHEN-ENABLED note.
Until then, docs/security/dependency-exceptions.md §7 item 1 tracks enabling GHAS as
the highest-priority action in the register.
Standing statement, to be repeated wherever CodeQL is cited as a control: with code scanning disabled, CodeQL produces no triaged output and no gate reads it. After this change it produces a build artifact, which is strictly more than the nothing it produced before — but an artifact nobody opens is evidence, not a control. CodeQL is not an active SAST control on this repository today, and must not be counted as one in any compliance artifact, readiness assessment, or conformity record until GHAS is enabled. The job is left running rather than deleted so that it begins working the instant that happens.
Companion decision: one scan per job
The same review round found the identical failure mode in the Trivy job, which is recorded here because it is the same reasoning applied to a different gate.
GitHub applies an implicit success() to any step with no if: condition. Trivy's
backend image scan, frontend image scan, and IaC misconfig scan were three steps of one
job, so the first scan to exit 1 skipped the frontend image scan that followed it.
The backend image exits 1 today (two pip-vendored CVEs, register §4.4), which means the
frontend image gate — documented as blocking — would never have run in CI.
Scoped precisely, because the wider claim would be wrong: the IaC misconfiguration step
carried if: always() and therefore DID run. Saying "the first failure skipped every
later scan" would record the configuration control as never having executed, which is not
what happened. One gate was silently skipped, not all of them — and one is enough to
justify the split. The evidence in PR #887 that
the frontend image went 6 fixable crit/high → 0 was obtained by running Trivy locally;
in CI that scan would have been skipped.
The scans are now three independent jobs (trivy-backend, trivy-frontend,
trivy-config) with no needs: edges between them. One scan failing can no longer skip
another, structurally rather than by convention. npm-audit was reordered for the same
reason: its blocking high/critical gate now runs before the non-blocking moderate
report, so no step that is permitted to fail precedes a gate anywhere in the workflow.
Consequences
Positive
- A crashed CodeQL analysis now fails the job. Before this, every CodeQL failure mode was green.
- CodeQL produces a retrievable artifact for the first time. Findings existed but were discarded on every run.
- The fail-open is bounded to one step and one evidenced HTTP status, with a named exit condition, rather than being an open-ended tolerance for "CodeQL problems".
- All three SARIF producers (Semgrep, CodeQL, Trivy-config) now use one identical pattern: scan blocks → artifact always → upload fails open. One pattern to remove when GHAS lands, instead of two shapes and an exception.
- The Trivy split means a backend finding can no longer hide a frontend finding. The frontend gate is now capable of running while the backend backlog exists.
- The false claim that CodeQL "cannot be step-scoped" is corrected in the workflow, in the exceptions register, and here — so it cannot be cited again as a reason to widen a fail-open.
Negative
- A fail-open is still a fail-open. If GHAS were enabled tomorrow and the upload began failing for a different reason — a token scope regression, a rate limit, a malformed SARIF that the API rejects — this step would swallow it and show a warning annotation that nobody is required to read. The narrowing reduces the blast radius; it does not eliminate the category. The only real fix is deleting the flag, which requires GHAS.
- Nothing enforces the exit condition. There is no check that fails when GHAS becomes available and these steps are still here. It rests on the register's review date (§7, monthly) and on human memory. A dated reminder or a probe step that asserts code scanning is still disabled would be stronger, and is not built here.
- CodeQL's artifact is not triaged. Retaining SARIF makes findings retrievable, not reviewed. Nobody currently downloads it, no issue is opened from it, and no gate reads it. Calling this an improvement is accurate; calling it a control would be exactly the claim-vs-check error this ADR exists to prevent.
- Trivy now builds two images in two jobs, so the two builds no longer share a runner or a Docker layer cache. This costs additional CI minutes (and, on a repo whose Actions billing has already failed once, minutes are not free). Independence was judged worth more than the cache.
upload: neveris a behavioural dependency on a documented action input. Ifgithub/codeql-actionchanged the semantics ofuploadin a future major, the failure would be that SARIF stops being produced where expected — noticeable, but only by someone looking for the artifact.- This was verified by reading and by parsing the workflow, not by running it. The
job graph was dumped and inspected, and the CodeQL action inputs were verified against
github/codeql-action@v4's ownaction.yml. No CI run was observed. PR #887 already carries one self-correction for asserting a CI outcome that was actually measured locally; this ADR does not add a second.
Neutral
- CodeQL's actual scanning behaviour is unchanged: same languages, same
security-and-qualitypack, samebuild-mode: none, same report-first posture with no query severity promoted to blocking. - The
security-events: writepermission is retained. It is now used only by theupload-sarifsteps and is inert while those 403. - The Trivy split changes the number of status checks the workflow reports from one
trivyto threetrivy-*. No branch protection rule references any of them —mastercurrently has no required checks — so nothing breaks, and nothing is automatically enforced either. - Semgrep and Trivy-config are untouched by this ADR; they already had the narrow posture. This decision documents the posture they were given and extends it to CodeQL.
Alternatives Considered
Alternative 1: Keep the blanket continue-on-error: true on analyze and just write the ADR
The literal remediation the review asked for: record the scope, owner, and exit condition of the existing posture without changing it.
- Why rejected: it would be an ADR recording that every CodeQL failure — including a crashed analysis — is converted into a green check. Documenting a control that reports success when it did not run does not make it honest; it makes the dishonesty load-bearing and citable. The narrower fix exists and costs ~20 lines, so there is no trade to justify. The ADR is still written, but it records the narrow decision.
Alternative 2: Delete the CodeQL job entirely until GHAS is enabled
If it produces nothing retrievable and gates nothing, remove it and stop pretending.
- Why rejected: it did produce nothing, but that was a fixable consequence of
discarding the SARIF, not an inherent property. With
upload: never+ artifact retention it produces real, downloadable results today. Deleting it would also guarantee it is never re-added — a disabled control that is absent from the workflow is far easier to forget than one that is running with aREMOVE-WHEN-ENABLEDnote on a single step.
Alternative 3: Let CodeQL stay red
Remove continue-on-error entirely and accept a permanently red job until GHAS is
enabled, on the "a red gate with a written register beats a green gate that checks
nothing" principle this workflow already applies to npm audit and gitleaks-full.
- Why rejected: that principle applies to a gate that is red because of real
findings —
npm auditis red over a genuine unfixed HIGH, and its redness carries information. This job would be red over an HTTP 403 from a service that was never enabled, which carries none. Permanently red for a non-security reason is precisely what trains reviewers to ignore a red Security workflow, and that habit is what makes thenpm auditred — the one that does mean something — get routed around. The distinction is deliberate: fail open on plumbing, stay red on findings.
Alternative 4: if: always() on the Trivy frontend scan instead of splitting the jobs
The review's own suggested alternative for the companion decision: keep one Trivy job
and add if: always() to the frontend scan step so a backend failure cannot skip it.
- Why rejected: it fixes the instance and leaves the coupling. Every future scan
added to that job would need to remember the same annotation, and forgetting it
reintroduces the defect silently — the failure mode being fixed is precisely that a
gate can stop running without anyone noticing. Separate jobs make it structural: with
no
needs:edges there is no mechanism by which one scan's result can reach another. It also costs a second image build, which is a real price paid for that guarantee.
Alternative 5: Enable GitHub Advanced Security now
The actual fix. Enabling GHAS makes code scanning real, the uploads succeed, the Security
tab exists, and every continue-on-error in this workflow can be deleted.
- Why rejected here: it is a paid, org-level licensing decision, not one an
engineering change can make. It is recorded as the exit condition of this ADR and as
the top item in
docs/security/dependency-exceptions.md§7. This ADR describes how the workflow behaves honestly until that decision is taken — it does not compete with it.