Skip to content

.github: least-privilege permissions for docker/hive/kurtosis workflows (#21132) - #22687

Merged
AskAlexSharov merged 1 commit into
mainfrom
feature/lystopad/zizmor-excessive-perms-packages-21132
Jul 23, 2026
Merged

AskAlexSharov merged 1 commit into
mainfrom
feature/lystopad/zizmor-excessive-perms-packages-21132

Conversation

@lystopad

Copy link
Copy Markdown
Member

Part of the zizmor excessive-permissions cleanup (#21132). Follow-up to #22662 and #22677.

What

Adds explicit least-privilege permissions: blocks to 5 workflows that were running on the broad default token:

Workflow Scope Why
ci-cd-main-branch-docker-images contents: read DockerHub only — auth via DockerHub secrets, no GitHub token used
qa-txpool-performance-test contents: read DockerHub pull only
test-hive contents: read + actions: write merge-queue self-cancel via gh run cancel
test-hive-eest contents: read + actions: write merge-queue self-cancel via gh run cancel
test-kurtosis-assertoor top-level contents: read; actions: write on assertoor_test only only that job runs gh run cancel

Notes

  • DockerHub login/push uses DockerHub secrets, not the GitHub token, so it needs no GitHub permission.
  • test-kurtosis-assertoor builds the erigon image with load: true (no registry push), so no packages: write is required; actions: write is scoped to the single job that self-cancels rather than the workflow level (which would trip "overly broad").

Verification

  • zizmor excessive-permissions: 12 → 3 for these files (the remaining 3 are ci-gate/test-fuzz, handled next).
  • actionlint clean w.r.t. this change (only pre-existing shellcheck notes remain); make lint = 0 issues.

Rollout note

The audit stays disable: true in .github/zizmor.yml until the series finishes; the final PR flips it on so the gate enforces it.

Note: docker-tags.yml — originally slated for this batch — is being removed in a separate PR instead of scoped, as it is a dead 2023 experiment (0 runs ever, references a non-existent make release-dry-run target, triggers only on y*.*.* test tags).

…kflows

Add explicit permissions blocks instead of the broad default token:

- ci-cd-main-branch-docker-images, qa-txpool-performance-test: contents: read
  (DockerHub only — auth via DockerHub secrets, no GitHub token used).
- test-hive, test-hive-eest: contents: read + actions: write (merge-queue
  self-cancel via gh run cancel; DockerHub pull needs no GitHub permission).
- test-kurtosis-assertoor: top-level contents: read, with actions: write
  scoped to the assertoor_test job that runs gh run cancel. The image build
  uses load: true (no registry push) so no packages permission is needed.

Part of the #21132 excessive-permissions cleanup.
@lystopad
lystopad requested a review from mriccobene as a code owner July 23, 2026 09:07
@AskAlexSharov
AskAlexSharov added this pull request to the merge queue Jul 23, 2026
Merged via the queue into main with commit 619f1af Jul 23, 2026
94 checks passed
@AskAlexSharov
AskAlexSharov deleted the feature/lystopad/zizmor-excessive-perms-packages-21132 branch July 23, 2026 11:38
pull Bot pushed a commit to Dustin4444/erigon that referenced this pull request Jul 23, 2026
…cleanup (erigontech#22694)

**Final PR of the zizmor `excessive-permissions` cleanup — closes
erigontech#21132** (with erigontech#22688).

Follows erigontech#22662, erigontech#22677, erigontech#22687 (and the dead-workflow removal erigontech#22688).

## What
1. **`test-fuzz.yml`** — move `issues: write` from the workflow level to
the `notify` job (the only job that opens/updates the `nightly-fuzz`
tracking issue). The fuzz matrix job keeps `contents: read`.
2. **`.github/zizmor.yml`** — replace the blanket
`excessive-permissions: disable: true` with a per-file `ignore` of just
`ci-gate.yml`, and **enable the audit** for every other workflow.

## Why `ci-gate` is ignored rather than scoped
`ci-gate` orchestrates the reusable test leaves. Per GitHub's rules, **a
called workflow's `GITHUB_TOKEN` permissions are capped by the caller**
— so ci-gate's workflow-level grants are not "for itself", they're the
ceiling every leaf inherits:
- **`actions: write`** is the cap that lets all ~12 self-cancelling
leaves (scoped in erigontech#22677/erigontech#22687) actually run `gh run cancel` on
merge-queue failure. Dropping it silently breaks fast-cancel and the
gate stalls.
- **`pull-requests: write`** drives the gate's own `dequeuePullRequest`
GraphQL mutation (GitHub doesn't auto-remove UNMERGEABLE queue entries),
and is also the cap for sonar's inherited `pull-requests: read`.

Neither can be scoped to a single job without adding per-job
`permissions` caps to every leaf-call job on the merge gate — high risk
for no real privilege gain. The in-file comment documents this, mirrored
into the ignore justification.

## Verification
- Full repo, real `.github/zizmor.yml`, **with `docker-tags.yml` removed
(as erigontech#22688 does)**: `zizmor` → **exit 0, "No findings to report"** (17
ignored, 299 suppressed).
- On this branch alone (docker-tags still present until erigontech#22688 merges):
exit 13 — one *medium* `docker-tags` finding that erigontech#22688 removes; the
gate passes regardless (< 14).
- `actionlint` clean; `make lint` = 0 issues.

## Merge order
Pairs with **erigontech#22688** (docker-tags removal). Either order is safe (the
gate passes both ways); once both land, `excessive-permissions` reports
zero findings and **erigontech#21132 can close**. The only remaining zizmor
exceptions repo-wide are the deliberate policy ones (`unpinned-uses`,
`concurrency-limits`) and the justified per-file ignores
(`cache-poisoning`, `secrets-inherit`, and now `excessive-permissions` →
ci-gate).
Sahil-4555 pushed a commit to Sahil-4555/erigon that referenced this pull request Jul 24, 2026
…usable-workflow perms) (erigontech#22714)

## Regression fix — `startup_failure` on main since erigontech#22677

The `excessive-permissions` cleanup (erigontech#22677, erigontech#22687) gave the reusable
test leaves explicit permissions (`actions: write` for their merge-queue
`gh run cancel`, `pull-requests: read` for sonar). GitHub validates a
**called** workflow's declared permissions against the **caller's**
grant *at startup* and rejects the whole run if the caller grants less.
Callers that only granted `contents: read` therefore began failing to
start:

| Caller | Calls | Broke at | Effect |
|---|---|---|---|
| `cache-warming` | lint, sonar, test-bench, test-all-erigon, … | erigontech#22677
| `startup_failure`, **33 checks** dropped |
| `sonar-branch-scan` | sonar | erigontech#22677 | `startup_failure`, **1 check**
dropped |
| `cache-warming-kurtosis-cl-images` | test-kurtosis-assertoor | erigontech#22687
| `startup_failure` |

Both cache-warming and sonar-branch-scan went `success` →
`startup_failure` exactly at commit `6f7557ebec` (erigontech#22677) and have
failed on **every main commit since** — silently dropping ~34 checks per
commit (this is the "134 → 99" drop visible in main's commit history).
`ci-gate` was unaffected because it already grants `actions: write` +
`pull-requests: write`; `cache-warming-kurtosis-gloas-images` is
unaffected because its leaf (`test-kurtosis-gloas`) was never modified
and still declares only `contents: read` — a clean control confirming
the mechanism.

## Fix
Grant each broken caller the union of its leaves' declared permissions:
- `cache-warming` → `contents: read`, `actions: write`, `pull-requests:
read`
- `sonar-branch-scan` → `contents: read`, `actions: write`,
`pull-requests: read`
- `cache-warming-kurtosis-cl-images` → `contents: read`, `actions:
write`

These callers are orchestrators that must grant the leaf permission cap
workflow-wide (exactly like `ci-gate`), so they're added to the
`excessive-permissions` ignore list with a generalized justification.

## Verification
- `zizmor` (real `.github/zizmor.yml`, audit enabled): **exit 0, "No
findings to report."**
- `actionlint` clean on the diff (the pre-existing `concurrency.queue`
note is unrelated).
- `make lint` = 0 issues.
- Post-merge confirmation: the push-triggered `Cache Warming` and
`SonarCloud Branch Scan` runs on main should return to `success` (from
`startup_failure`).

## Note / lesson
Root cause was mine: when adding the leaves' `permissions:` in
erigontech#22677/erigontech#22687 I didn't account for GitHub's caller-must-grant rule for
reusable workflows, so non-ci-gate callers broke. This restores them
without weakening the least-privilege scoping.

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.

2 participants