Skip to main content

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@v4 and github/codeql-action/upload-sarif@v4 are both already used in this workflow (upload-sarif by the Semgrep and Trivy-config jobs) — those are re-uses, not new dependencies.

    actions/upload-artifact@v4 IS new, and an earlier draft of this ADR wrongly said it was not. The parent revision contains no upload-artifact anywhere 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 red Analyze step means the analysis genuinely broke, and a warning-annotated Upload SARIF to code scanning step 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.yml only. 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: true on analyze and 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.

  1. github/codeql-action/analyze@v4 runs with upload: never and output: codeql-sarif. This step carries no continue-on-error. Any analysis failure — extraction, query evaluation, OOM, malformed pack — fails the step and reddens the job, as it should.
  2. The SARIF directory is uploaded as a build artifact (codeql-sarif-<language>) with if: always(), so CodeQL's findings are retrievable today, without GHAS. This is added signal, not removed.
  3. A separate github/codeql-action/upload-sarif@v4 step, with if: always() and continue-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: never is a behavioural dependency on a documented action input. If github/codeql-action changed the semantics of upload in 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 own action.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-quality pack, same build-mode: none, same report-first posture with no query severity promoted to blocking.
  • The security-events: write permission is retained. It is now used only by the upload-sarif steps and is inert while those 403.
  • The Trivy split changes the number of status checks the workflow reports from one trivy to three trivy-*. No branch protection rule references any of them — master currently 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 a REMOVE-WHEN-ENABLED note 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 findingsnpm audit is 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 the npm audit red — 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.