ci(security-scan): let a CodeQL configuration error fail the job

[WomB0ComB0]

What

Removes continue-on-error: true from the Analyze step of the reusable
security-scan.yml CodeQL job, and replaces it with a comment recording why a
real failure must now be allowed to fail the job.

       - name: Analyze
         uses: github/codeql-action/analyze@1c5b675653bb5c22dbe9b12b556ec555138e09fd  # v4.38.1
         with:
           category: "/language:${{ matrix.language }}"
-        continue-on-error: true   # default-setup collision otherwise fails

Why the flag was there, and why it can go now

The comment was accurate. On every repository where GitHub's CodeQL default
setup is enabled, the advanced-setup upload from this reusable is rejected:

Analysis upload status is failed.
##[error]Code Scanning could not process the submitted SARIF file:
CodeQL analyses from advanced configurations cannot be processed when the default setup is enabled
CodeQL job status was configuration error.

Measured in a job log on an affected repository. Positive control, same grep on
a repository with default setup off:

Analysis upload status is complete.
CodeQL job status was success.

The error is emitted by the Analyze step itself, in its main phase — not by a
post-job step — which is precisely what continue-on-error: true on that step
swallows. Had it genuinely been post-job, a continue-on-error on the step
could not have masked it, so "post-job" never explained the green check it was
offered to explain.

The ordering is established from step timings against the log, not from prose:
in an affected job the ##[error] line falls inside the Analyze step's own
span (Jobs API steps[].started_at/completed_at), ahead of the first
Post job cleanup. line. Only the trailing CodeQL job status was ... line is
post-job, printed by init-post, and it is not the rejection — the positive
control prints the same line reading CodeQL job status was success.

The result was that a configuration error reported green, indefinitely, for
scanning that never landed.

This is not a coverage loss. Default setup runs the extended query suite,
which is broader than the default suite these advanced workflows use. The
discarded work was also the weaker work. Removing the declaration removes waste
and a false signal; it does not reduce scanning.

Ordering — this must not merge first

Important

Do not merge before the consumer PRs below. Removing the flag while the
collision still exists would convert a silent failure into a hard failure on
every affected consumer at once.

Consumers that stop declaring advanced CodeQL, leaving default setup to own it:

• ci(security): stop declaring advanced CodeQL; default setup owns it dev#95
• ci(security): stop declaring advanced CodeQL; default setup owns it docs#143
• ci(security): stop declaring advanced CodeQL; default setup owns it npm#341
• ci(security): stop declaring advanced CodeQL; default setup owns it pypi#132
• ci(security): stop declaring advanced CodeQL, which default setup rejects dotnet-sdk#119
• ci(security): stop declaring advanced CodeQL, which default setup rejects vcpkg#48
• ci(security): stop declaring advanced CodeQL, which default setup rejects viz#286
• ci: stop declaring advanced CodeQL, which default setup rejects dotnet#81

Three of those repositories (viz, vcpkg, dotnet-sdk) reach this reusable
by a second path — ci.yml -> required.yml -> security-scan.yml,
passing codeql-languages. Closing only the security.yml path on those three
would leave the collision live, so these are gating too:

• ci: stop dispatching a second, discarded CodeQL analysis viz#285
• ci: stop dispatching a second, discarded CodeQL analysis vcpkg#47
• ci: stop dispatching a second, discarded CodeQL analysis dotnet-sdk#118

Every PR in both lists needs to land before this one.

The mitigating fact, stated rather than relied on

Every consumer pins this reusable by full commit SHA, and this repository has no
tags (git ls-remote --tags returns nothing). A change to security-scan.yml
on main therefore reaches no consumer until that consumer re-pins. That
makes the ordering more forgiving than it looks. It is not a reason to skip it:
the next routine pin bump on an unfixed consumer is what would collect the
hard failure, at a moment nobody is expecting it.

What else this flag was masking

The point of removing it is that a genuine CodeQL failure — an extraction
fault, autobuild breakage, a rejected SARIF — should stop the job. So the
question that matters is whether any consumer would newly fail for a reason
unrelated to the collision.

The repositories where removal actually bites are the ones with default setup
off, whose advanced runs are the real coverage. Checked by reading the
Analyze step's own job log, because step conclusions cannot answer this —
a continue-on-error step reports conclusion: success in the Jobs API even
when it failed, and an affected repository's Analyze step reads success
there while its log carries the configuration error.

Repo	default setup	languages	Analyze log
crates	off	["actions"]	Analysis upload status is complete. / CodeQL job status was success. — no ##[error]
programs	off	["actions"]	Analysis upload status is complete. / CodeQL job status was success. — no ##[error]

Both are clean. The only diagnostics in either log are non-fatal deprecation
warnings (CodeQL Action v3 EOL, Node 20), which do not fail a step. Neither
repository newly fails.

One further consumer outside this PR series also has default setup off and real
advanced coverage; its Analyze logs were checked the same way and are equally
clean.

One consumer is not yet covered

One consumer outside this PR series has default setup enabled and still
declares languages. Its latest run reproduces the collision on both matrix
languages. It pins an older SHA of this reusable, so it is unaffected today,
but it needs the same treatment as the consumers listed above before it
re-pins. Flagging it here rather than fixing it in this PR, which is scoped
to the reusable.

Note also that crates and programs run Rust CodeQL from a repo-local
codeql.yml, not through this reusable, so this change does not touch that
path.

The same pattern elsewhere

continue-on-error remains elsewhere in this same file. These uses each carry
a comment explaining themselves, and are left alone:

Location	Comment	Verdict
vet step	SOFT-LAUNCH: warn-only until promoted to required	deliberate, documented
Upload zizmor SARIF	GHAS-only upload 403s on private repos	deliberate, documented
Run Snyk	(opt-in scanner)	deliberate
Upload Snyk SARIF	GHAS-only upload 403s on private repos	deliberate, documented

One use has no such comment. It is worth a separate look and is not changed here:

• Run zizmor (run: uvx zizmor --format sarif .github/workflows/ > zizmor.sarif)
  carries a bare continue-on-error: true with no comment. Because the
  redirect creates zizmor.sarif whether or not the tool succeeded, the
  hashFiles guard on the upload step still fires, and that upload is itself
  continue-on-error — so a zizmor tool failure would pass silently end to
  end. It is not masking anything today (zizmor is uploading real findings on
  the repos checked, and its step exits 0), but the failure mode is unguarded
  and undocumented. Out of scope for this PR.

Pins

No uses: line is modified. The one in the touched hunk was re-verified:
github/codeql-action/analyze@1c5b675653bb5c22dbe9b12b556ec555138e09fd
resolves to refs/tags/v4.38.1^{} via git ls-remote --tags, matching its
# v4.38.1 comment.

Conflicts

The only other open PR touching this file is #59 (Dependabot), which changes the
astral-sh/setup-uv pin in the zizmor job, well away from this hunk. No
overlap.

Test plan

• Edited YAML parses; the codeql job has no continue-on-error on any step, and every other job is untouched
• Collision reproduced in a job log on an affected repo; positive control on a repo with default setup off returns Analysis upload status is complete.
• crates and programs Analyze logs confirmed free of ##[error] and reporting CodeQL job status was success.
• Action pin in the touched hunk re-verified against git ls-remote --tags
• Every consumer PR listed above merged before this one
• After merge: a consumer with default setup off re-pins and its security run stays green
• Follow-up PR for the one uncovered consumer, before it re-pins

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 396a8852-31d1-4b7a-ae56-be523ac5d55f

📥 Commits

Reviewing files that changed from the base of the PR and between 314419f and 375d3b6.

📒 Files selected for processing (1)

• .github/workflows/security-scan.yml

 _____________________________________
< ICBM: Intercontinental Bug Missile. >
 -------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}