You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
ci: ci-success does not fail on skipped jobs, so a skipped required check reads as green #1688
Noticed during the adversarial review of PR #1682. Generic CI-gating hole, not specific to that PR.
What
ci-success (.github/workflows/ci.yml) is the summary job every other job feeds into. It fails the build on failure and on cancelled:
if: always()steps:
- run: | if [[ "${{ contains(needs.*.result, 'failure') }}" == "true" ]]; then ... exit 1; fi if [[ "${{ contains(needs.*.result, 'cancelled') }}" == "true" ]]; then ... exit 1; fi echo "All CI checks passed!"
It does not test for skipped. A needed job that is skipped therefore reports as an overall pass.
Why it is latent rather than live
No job in needs currently carries an if: or a paths: filter, so nothing can be skipped today, and ci-success is accurate as things stand. The hole opens the first time any gating job gains a condition: from then on the gate silently stops covering it, and the failure mode is a green ci-success with a check that never ran. That is the same defect class as a guard asserting a property it does not inspect.
Fix direction
Add the third branch:
if [[ "${{ contains(needs.*.result, 'skipped') }}" == "true" ]]; thenecho "One or more required CI jobs were skipped"exit 1fi
If a job ever legitimately needs to be skippable, it should be excluded from needs deliberately, or use if: always() internally and report a neutral success, rather than relying on ci-success not looking.
Worth checking the other workflows for the same summary-job pattern while in there.
Verified resolved at 3c0f8ac: the ci-success gate now allowlists success instead of denylisting failure and cancelled, so any needed job whose result is not exactly 'success' -- skipped included -- fails the summary job and names itself in the log. Evidence: .github/workflows/ci.yml:1086-1103 (commit 6a2117c). Residual axes checked: the suggested sweep of other workflows for the same summary-job pattern -- no other workflow uses the contains(needs.*.result, ...) denylist form; the four deploy workflows' only needs-result usage is an allowlist needs.<job>.result == 'success' guard on a notify job. Closing as completed; reopen if the behaviour recurs.
Activity
sec(ci): make the Secret Manager guard cover iac/ and stop overclaiming
sec(ci): make the Secret Manager guard cover iac/ and stop overclaiming
sec(iac/gcp): scope cleanup-function SA to the one secret it reads (#…
cristim commentedon Sep 2, 2026
Verified resolved at 3c0f8ac: the ci-success gate now allowlists success instead of denylisting failure and cancelled, so any needed job whose result is not exactly 'success' -- skipped included -- fails the summary job and names itself in the log. Evidence: .github/workflows/ci.yml:1086-1103 (commit 6a2117c). Residual axes checked: the suggested sweep of other workflows for the same summary-job pattern -- no other workflow uses the contains(needs.*.result, ...) denylist form; the four deploy workflows' only needs-result usage is an allowlist
needs.<job>.result == 'success'guard on a notify job. Closing as completed; reopen if the behaviour recurs.sec(iac/gcp): scope cleanup-function SA to the one secret it reads (#…