feat(standards): catch the CI drift classes that fail silently - #42
Conversation
Today's org triage found 431 failed runs in 30 days. The largest clusters were
not one-off breakages — they were configuration drift that no one noticed
because each failure mode is silent.
Adds four static checks to repo-standards, so they surface on the PR that
introduces them rather than months later:
1. security.yml calling the org scan without `actions: read`. An explicit
permissions block sets unlisted scopes to none, so the reusable's jobs
request more than the caller grants and the run is rejected at creation.
That is a startup_failure: no jobs, no logs, and invisible to tooling that
only inspects failed runs. Four repos had never run their security scan
even once.
2. dependabot.yml missing the `github/gh-aw-actions*` ignore. The trailing
`*` is load-bearing — Dependabot names an action by its full path, so a
bare `github/gh-aw-actions` matches nothing. Two repos had the rule, and
it had silently protected nothing.
3. gh-aw lock files whose compiler_version disagrees with the setup action
they pin. That skew makes every agentic workflow die with MODULE_NOT_FOUND
before the agent starts.
4. Workflows using the self-hosted `zima` label with no actionlint.yaml
declaring it.
Warn-only, matching the existing rollout pattern; no new inputs, so every
caller stays compatible.
Verified against fixtures for both states (4/4 caught, 0 false positives) and
against all ten org repos: nine clean, and resQ correctly flags the one fix
still sitting unmerged in resQ#722.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 52 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pull request documents four CI configuration requirements and adds workflow checks for security scan permissions, Dependabot ignores, gh-aw lockfile consistency, and ChangesCI standards conformance
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/repo-standards.yml:
- Around line 125-130: Update the validation around the security.yml
reusable-workflow reference so it identifies the job containing the
security-scan.yml uses entry and checks that job’s effective actions permission,
rather than accepting actions: read from any unrelated job. Preserve the
existing startup_failure note and only pass when the reusable-workflow caller
grants the required actions: read permission.
- Around line 145-148: Update the Dependabot validation around the lockfiles
check so repositories with gh-aw lock files are reported when
.github/dependabot.yml is missing as well as when it lacks the
github/gh-aw-actions* ignore rule. Preserve the existing note message and rule
matching for repositories that do have the configuration.
- Around line 168-171: Update the zima validation in the workflow standards
check to inspect the contents of .github/actionlint.yaml or
.github/actionlint.yml, not merely their existence. When workflows detected by
the existing grep use zima, parse the available actionlint configuration and
require self-hosted-runner.labels to include zima; fail with an appropriate
message when the section or label is missing.
In `@docs/standards/01-baseline.md`:
- Around line 80-82: Update the custom runner-label guidance near “Declare
custom runner labels” to document both supported actionlint configuration
filenames, .github/actionlint.yaml and .github/actionlint.yml, matching
repo-standards.yml; alternatively, align enforcement to a single documented
filename.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b703494c-e90c-4715-8283-4e13abe23934
📒 Files selected for processing (2)
.github/workflows/repo-standards.ymldocs/standards/01-baseline.md
Review found three ways the checks could pass a repo that is actually broken. A conformance check that gives false assurance is worse than no check, so all three are fixed. The `actions: read` check grepped the whole file, so a permission on any unrelated job satisfied it. It now resolves the permission that actually applies to the job holding the reusable `uses:` — job-level `permissions:` replaces the top-level block rather than merging with it, so a caller job with its own narrower block must be judged on that block. The `zima` check only tested that an actionlint config exists. A config that omits the label still fails actionlint, so it now requires the label to be declared under self-hosted-runner.labels, and accepts either .yaml or .yml. Docs: state that the Dependabot rule applies to repos with gh-aw lock files *and* a Dependabot config (no dependabot.yml is safe by construction), and record both accepted actionlint config paths. Verified with fixtures for each case: - actions: read on an unrelated job -> flagged - actions: read at top level, no job block -> clean - caller job block overriding and omitting it-> flagged - actionlint config present but missing zima -> flagged Org-wide re-run unchanged: nine repos clean, resQ still correctly flagging the fix pending in resQ#722.
All four correct — fixed, and the first one was the important one
Verified with fixtures for each shape:
The middle row matters as much as the other two — the fix had to stop false negatives without introducing false positives on the shape all nine repos actually use.
Missing Doc/enforcement path mismatch. Fixed — baseline now documents both Org-wide re-run after the changes is unchanged: nine repos clean, |
Every one of these could report a repo as fine when it is not — the failure mode this sweep exists to prevent. Readability was tested by listing .github/workflows, but a repo without that directory returns 404, which is indistinguishable from an access failure by exit status alone. `scripts` was being reported UNREADABLE while being perfectly readable — it simply has no workflows. Readability is now established against the repo object, and a missing directory means "no workflows". That also removes the need for the skip-repos default added a commit ago: scripts now scans clean on its own. The input stays as an escape hatch but defaults to empty — a repo suppressed there is a repo nobody is checking. The actions: read check was the loose file-wide grep, i.e. the exact bug caught in repo-standards during #42 review and fixed there. I had copied the pre-fix version into the sweep. It now uses the same caller-scoped awk, so a workflow whose caller job overrides permissions without actions: read is caught. A failed or partial `gh repo list` silently produced a short sweep that read as all-clean. It now errors out, including when enumeration returns fewer than two repos, which is the signature of GITHUB_TOKEN being used without ORG_READ_TOKEN. Limit raised 200 -> 1000. Verified live with and without skip-repos: 21 repos scanned, 0 unreadable, 2 findings (resQ pending resQ#722, vcpkg pending vcpkg#34).
1 participant
Why here
Today's org-wide triage surfaced 431 failed runs in 30 days. I fixed them repo-by-repo, but that was treating symptoms — the large clusters were not one-off breakages, they were configuration drift. And the reason each survived for a month is that every one of them fails silently: the run either never starts (producing no logs at all) or the guard meant to prevent it quietly matches nothing.
This is the right place to catch that class of problem once instead of nine times.
The four checks
security.ymlcalls the org scan withoutactions: readstartup_failure— no jobs, no logs, invisible to anything scanning failed runsdependabot.ymlmissing thegithub/gh-aw-actions*ignore*makes it match nothingcompiler_version≠ pinned setup versionMODULE_NOT_FOUNDdeep in an agent logzimarunner label with noactionlint.yamlThe trailing
*in #2 is the one I'd most want a machine watching. Dependabot names an action by its full path, so the dependency isgithub/gh-aw-actions/setup— a baregithub/gh-aw-actionssilently protects nothing. Two repos had that rule and believed they were covered.Verification
Extracted the new section into a standalone harness and ran it against fixtures for both states:
cratesshape): 4/4 violations caught, each with an accurate messageThen against all ten org repos:
Nine clean, and resQ correctly flags the single fix still sitting unmerged in
resq-software/resQ#722— which is exactly the behaviour I wanted to see, rather than a uniformly green result that proves nothing.Also:
actionlintclean (it shellchecks therun:block), and theworkflow_callinputs are unchanged (strict,require-license,readme-min-bytes), so every existing caller stays compatible.Rollout
Warn-only, matching the repo's existing audit → enforce pattern. Nothing starts failing on merge; violations appear as annotations. Flip
strict: trueper repo once it is clean — nine already are.docs/standards/01-baseline.mdgains a matching CI configuration subsection so the written standard and the enforcement agree.Summary by CodeRabbit
New Features
Documentation