Harden CI security and fix the required-checks merge gate#100
Merged
Conversation
## Summary ### Why? The `required-checks` aggregator had the same latent bug that let a failing PR merge in submitqueue (PR #151): it had no `if: always()` and no failure detection. When a needed job fails, GitHub *skips* the dependent `required-checks` job, and a skipped required status check is treated as "not failed" — so a PR with failing build/test/lint could merge. The CI also ran with floating action tags, ambient write-scoped credentials, and no automated guard against workflow-security regressions. This ports submitqueue's CI security and required-checks hardening to tango. ### What? - `required-checks`: add `if: always()` and a step that fails on any `failure`/`cancelled`/`skipped` result among its `needs`. Add `workflow-security` to the gated set. - New `workflow-security` job: actionlint + zizmor (SHA-smell, dangerous triggers, credential persistence, template injection) plus a guard that fails if any non-GITHUB_TOKEN secret is referenced on the untrusted-PR-code path. - SHA-pin all actions (checkout, setup-go, actionlint, zizmor) with version comments instead of floating tags. - `persist-credentials: false` on every checkout that runs untrusted PR code (build-and-test, dependencies, lint, workflow-security). - Top-level `permissions: contents: read` (least privilege) and a `concurrency` group that cancels superseded PR runs only. - `merge_group` trigger so the merge queue is gated by required-checks; draft PRs skip CI and run on `ready_for_review`. - `.github/dependabot.yml` (gomod, security-updates only), `.github/zizmor.yml` config, and a CODEOWNERS `/.github/` admin override so workflow changes require admin review. ## Test Plan - ✅ `actionlint .github/workflows/ci.yml` — clean - ✅ `zizmor --offline .github/workflows/ci.yml` — no findings - ✅ YAML parses for ci.yml, dependabot.yml, zizmor.yml - ✅ Secrets guard: fires on `secrets.MY_PAT`, allows `secrets.GITHUB_TOKEN` - Full online zizmor audit + the gate behavior run in CI on this PR.
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.
Summary
Why?
The
required-checksaggregator had the same latent bug that let afailing PR merge in submitqueue (PR #151): it had no
if: always()andno failure detection. When a needed job fails, GitHub skips the
dependent
required-checksjob, and a skipped required status check istreated as "not failed" — so a PR with failing build/test/lint could
merge. The CI also ran with floating action tags, ambient write-scoped
credentials, and no automated guard against workflow-security
regressions. This ports submitqueue's CI security and required-checks
hardening to tango.
What?
required-checks: addif: always()and a step that fails on anyfailure/cancelled/skippedresult among itsneeds. Addworkflow-securityto the gated set.workflow-securityjob: actionlint + zizmor (SHA-smell, dangeroustriggers, credential persistence, template injection) plus a guard
that fails if any non-GITHUB_TOKEN secret is referenced on the
untrusted-PR-code path.
version comments instead of floating tags.
persist-credentials: falseon every checkout that runs untrusted PRcode (build-and-test, dependencies, lint, workflow-security).
permissions: contents: read(least privilege) and aconcurrencygroup that cancels superseded PR runs only.merge_grouptrigger so the merge queue is gated by required-checks;draft PRs skip CI and run on
ready_for_review..github/dependabot.yml(gomod, security-updates only),.github/zizmor.ymlconfig, and a CODEOWNERS/.github/adminoverride so workflow changes require admin review.
Test Plan
actionlint .github/workflows/ci.yml— cleanzizmor --offline .github/workflows/ci.yml— no findingssecrets.MY_PAT, allowssecrets.GITHUB_TOKEN