Skip to content

Harden CI security and fix the required-checks merge gate#100

Merged
behinddwalls merged 1 commit into
mainfrom
preetam/ci-security-hardening
Jun 5, 2026
Merged

Harden CI security and fix the required-checks merge gate#100
behinddwalls merged 1 commit into
mainfrom
preetam/ci-security-hardening

Conversation

@behinddwalls
Copy link
Copy Markdown
Contributor

@behinddwalls behinddwalls commented Jun 5, 2026

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.

## 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.
@behinddwalls behinddwalls marked this pull request as ready for review June 5, 2026 04:59
@behinddwalls behinddwalls requested review from a team as code owners June 5, 2026 04:59
@behinddwalls behinddwalls merged commit 36f9a0e into main Jun 5, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant