ci: fix zizmor template-injection in remaining QA/CD workflows (#21132) - #22386
Merged
Merged
Conversation
Route untrusted/dynamic contexts (inputs.*, github.*, runner.*, steps.*, matrix.*, env.* holding secrets or derived values) through env vars or GitHub built-ins so their values are treated as data rather than substituted into shell text. Also move DockerHub push credentials in ci-cd-main-branch-docker-images.yml out of run-block text into step env. Leaves only release.yml in the template-injection ignore list in .github/zizmor.yml (deferred: release-pipeline change). Part of #21132.
lystopad
requested review from
AskAlexSharov,
mriccobene and
yperbasis
as code owners
July 10, 2026 16:04
AskAlexSharov
approved these changes
Jul 11, 2026
Sahil-4555
pushed a commit
to Sahil-4555/erigon
that referenced
this pull request
Jul 13, 2026
…rigontech#22386) (erigontech#22417) Two changes to `ci-cd-main-branch-docker-images.yml`, in one PR. ## 1. Fix: empty docker tag broke the image build The workflow was failing on `main` ([run 29223744087](https://github.com/erigontech/erigon/actions/runs/29223744087)): ``` DOCKER_PUBLISH_CONDITION: --tag : ERROR: failed to build: invalid tag ":": invalid reference format ``` **Root cause:** `DOCKER_PUBLISH_CONDITION` / `BUILD_VERSION_CONDITION` computed their tag via `format(…, env.DOCKER_URL, env.BUILD_VERSION)`, but those are **sibling entries in the same `env:` block**, which GitHub evaluates to empty. The vars were previously unused (the run blocks computed the value inline, which works because run-block expressions see the merged env); erigontech#22386 switched the run blocks to reference the env vars and exposed the latent bug → `--tag :`. **Fix:** reference the real sources instead of the siblings — top-level `env.DOCKERHUB_REPOSITORY` and the `steps.*` outputs (the same values `DOCKER_URL`/`BUILD_VERSION` derive from), both available at step-env eval time. ## 2. Discord failure notification Posts to the repo `DISCORD_WEBHOOK` secret when the build fails on an automated push (manual dispatch runs are watched by whoever triggered them). Implemented as a `if: failure()` step inside the `Build` job so the secret stays scoped to the job's existing `environment: dockerhub-publish`. No-op when the webhook is unset. Mirrors the nightly Fuzz workflow's alert. ## zizmor Runs clean at the **auditor persona** (strictest) after this PR: - `anonymous-definition` → named the `Build` job. - `secrets-outside-env` → the webhook is used inside the environment-scoped `Build` job. - `concurrency-limits` → intentionally ignored in `.github/zizmor.yml` (documented): every triggering commit must build and publish, and a concurrency group would cancel intermediate queued runs. Verification: `zizmor 1.24.1 --persona=auditor` → *No findings to report*; `actionlint` → 0 errors; default-persona full-repo gate unchanged (exit 12, passes).
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
2 participants
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.
Part of #21132 — template-injection cleanup, final batch (all remaining files except
release.yml).After this PR the
template-injectionignore list in.github/zizmor.ymlcontains onlyrelease.yml(deferred — it needs a release-pipeline change).Files fixed / dropped from the ignore list:
backups-dashboards.ymlci-cd-main-branch-docker-images.ymlci-gate.yml— already clean at the regular persona (itstoJSON(needs)is already routed through anenv:block); just removed the stale ignore entry.docker-image-remove.ymlqa-sync-from-scratch-full-node.ymltest-all-erigon-race.ymlWhat changed
${{ … }}expansions insiderun:blocks are substituted into the script text before the shell parses it, so an attacker-influenced value could inject shell code. Each flagged expansion now arrives as data, not code:github.ref_name→$GITHUB_REF_NAME;runner.name→$RUNNER_NAME;github.workspace→$GITHUB_WORKSPACE.env:for everything else:inputs.checkout_ref/github.refconditionals →BRANCH_REF/IMAGE_REF;steps.*.outputs.*→BINARIES/COMMIT_ID/SHORT_COMMIT_ID/BUILD_VERSION*/TAG_KEY/KEEP_IMAGES;github.actor→ACTOR;matrix.test-group→TEST_GROUP;github.repository_owner→REPO_OWNER; the already-definedenv.CHAIN/env.MODE/env.TAG_KEY/env.DOCKER_URL/… referenced as plain$VAR.Security hardening beyond the raw findings
In
ci-cd-main-branch-docker-images.ymlanddocker-image-remove.yml, DockerHub push credentials were interpolated directly intorun:text ("'"${{ secrets.… }}"'",JWT ${{ env.TOKEN }}). A secret containing a shell metacharacter could have broken out of the surrounding quoting. These now travel through stepenv:(DH_USERNAME/DH_TOKEN/TOKEN) and are referenced as shell variables, so the secret value never lands in script text.Notes
--tagpublish flag in the docker build is produced by${DOCKER_PUBLISH_CONDITION}, left intentionally unquoted so it word-splits into--tag <url>:<ver>(or expands to nothing) — exactly reproducing the previous template behavior. Quoting it would change the docker invocation.sedpattern is translated to double quotes so only${TAG_KEY}expands;\(,\{7\},\1remain literal.matrix.exec_mode == 'parallel' && … || …,needs.*.result) and trusted command substitutions ($(git rev-parse HEAD)) are left as-is — zizmor does not flag them.run-name:and stepname:expansions are display strings, not shell, and are not flagged.Verification
zizmor 1.24.1(regular persona = CI's) with the repo config: 0 template-injection findings across all six files; full-repo exit code unchanged at 12 (< 14, passes the CI gate).actionlint: 0 errors for every file. The only net-new SC2086 (info-level, CI-ignored) is the intentional${DOCKER_PUBLISH_CONDITION}word-split noted above; every other variable this PR introduces is quoted.