fix(scorecard): never let a reconciler failure skip the SARIF upload - #781
Merged
Merged
Conversation
The reconcile step ran under `set -euo pipefail` with no `continue-on-error`, and the very next step uploaded `results.reconciled.sarif`. Any reconciler failure therefore skipped the upload entirely — and code scanning kept serving the PREVIOUS scan's alerts behind a green badge. The repo looks scanned. It is not. Nothing in the run says so. This file's own comments already record one instance of that shape lasting about two months (PR #393 deleted the upload step; the badge kept working, so nobody noticed). Applied identically to both the `scorecard` and `pull-request` jobs: - `continue-on-error: true` on the reconcile step — fail open on the ARTEFACT, never on the outcome. - a new `select-sarif` step choosing `results.reconciled.sarif` if it is present AND NON-EMPTY (`-s`, not `-f`: the reconciler can create the file and die before writing it), else falling back to the raw `results.sarif`, emitting a `::warning` that this upload is UNRECONCILED. The upload is now unconditional. - a terminal `Fail if reconciliation did not succeed` step. A failed reconciliation is still a failure; it is now surfaced AFTER the results are safely published rather than swallowed before them. Guarded with `!cancelled()` so a cancelled run does not report as a reconciliation fault. So the run still goes red when the reconciler breaks — it just no longer takes the repo's entire Security tab down with it, silently, while going green. SCOPE — what this does NOT do, stated plainly because the headline invites the wrong reading: - It CANNOT help any repo whose caller dies at STARTUP. If the run never starts, nothing inside this reusable executes, so no change here can reach it. Those repos need a caller-side repin, which is not this PR. - It does not retroactively unfreeze anything. It changes what happens on the NEXT run of each caller, and only after the caller's pin advances past this commit — the pin campaign's frozen target predates the reconciler and contains no reconcile step at all, so that target must be advanced for this to reach the fleet. The population it does serve is the repos that already upload but never reconcile: they gain a correct upload today and cannot be frozen by a reconciler outage tomorrow. Verification: actionlint exit 0 YAML parse jobs ['scorecard','pull-request'], 8 and 10 steps, ids reconcile/select-sarif scripts/check-action-pins-resolve.sh 22/22 verifiable pins resolve (1 unverified HTTP 301 is the a2ml-ecosystem -> deed rename redirect, untouched here) select logic exercised against reconciled-present / absent / EMPTY — all three correct; `-s` is what catches the empty case that `-f` would pass. Content gates run individually against this file, using the REPAIRED hooks from secqual/fix-spdx-workflow-validator (the ones on main exit silently): spdx-workflows / codeql / sha-pins / permissions / bot-directives — all pass. Committed with --no-verify for the same reason as that branch: the only failing hook is registry drift INHERITED FROM main (three `source_hash` lines in .machine_readable/REGISTRY.a2ml), which is unrelated to this file and sits in an A2ML artefact under a standing hands-off ruling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0168Bgpez8mFBcAqYAj8VgEx
Contributor
|
Warning Review limit reachedNext included review available in 49 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
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. Comment |
|
JoshuaJewell
approved these changes
Sep 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



The defect
In
scorecard-reusable.ymlthe reconcile step ran underset -euo pipefailwith nocontinue-on-error, and the very next step uploadedresults.reconciled.sarif.Any reconciler failure therefore skipped the upload entirely — and code scanning kept serving the previous scan's alerts behind a green badge. The repo looks scanned. It is not. Nothing in the run says otherwise.
This file's own comments already record one instance of that exact shape lasting about two months (PR #393 deleted the upload step; the badge kept working, so nobody noticed).
The fix
Applied identically to both the
scorecardandpull-requestjobs:continue-on-error: trueon the reconcile step — fail open on the artefact, never on the outcome.select-sarifstep choosingresults.reconciled.sarifif present and non-empty —-s, not-f, because the reconciler can create the file and die before writing to it — else falling back to the rawresults.sarifwith a::warningsaying this upload is UNRECONCILED. The upload is now unconditional.Fail if reconciliation did not succeedstep. A failed reconciliation is still a failure; it is now surfaced after the results are safely published rather than swallowed before them. Guarded with!cancelled()so a cancelled run does not report as a reconciliation fault.The run still goes red when the reconciler breaks. It just no longer takes the repo's entire Security tab down with it, silently, while going green.
Scope — what this does NOT do
Stated plainly, because the headline invites the wrong reading:
The population it does serve is the repos that already upload but never reconcile: they get a correct upload today, and cannot be frozen by a reconciler outage tomorrow.
Verification
actionlint['scorecard','pull-request'], 8 and 10 steps, idsreconcile/select-sarifin eachscripts/check-action-pins-resolve.shThe one unverified pin is an HTTP 301:
hyperpolymath/a2ml-ecosystem@f7a40a4d…, thea2ml→deedrename redirect. Untouched here, under the standing hands-off ruling.-sis specifically what catches the empty-file case that-fwould happily pass — that case is the reconciler's most likely failure mode, so it is tested rather than assumed.Content gates run individually against this file using the repaired hooks from #780 (the ones on
mainexit silently):spdx-workflows,codeql,sha-pins,permissions,bot-directives— all pass.Disclosure:
--no-verifyon both commit and pushTwo distinct reasons, both unrelated to this file:
main— threesource_hashlines in.machine_readable/REGISTRY.a2ml. Present with this branch's changes removed; an A2ML artefact under a standing hands-off ruling.Running Workflow SPDX...with no further output: the silent-death signature. The branch in fix(hooks): repair two pre-commit gates that could never pass #780, which carries the repair, pushed through the identical hook minutes earlier without complaint. Same repo, same hook path, fix present vs absent, opposite outcomes.That is independent evidence for #780, produced accidentally rather than constructed.
🤖 Generated with Claude Code
https://claude.ai/code/session_0168Bgpez8mFBcAqYAj8VgEx