Skip to content

ci: fix zizmor security findings across all GitHub Actions workflows - #4460

Merged
reinkrul merged 1 commit into
masterfrom
ci/fix-zizmor-findings
Aug 19, 2026
Merged

ci: fix zizmor security findings across all GitHub Actions workflows#4460
reinkrul merged 1 commit into
masterfrom
ci/fix-zizmor-findings

Conversation

@reinkrul

Copy link
Copy Markdown
Member

Summary

qlty runs zizmor (a GitHub Actions security linter) on every PR. Running it repo-wide against master surfaced 71 findings across every workflow file (7 informational, 9 low, 9 medium, 46 high). This fixes all of them.

  • unpinned-uses (40, high): every third-party/first-party action pinned to a commit SHA instead of a mutable version tag, so a compromised or force-moved tag can't silently swap in different code.
  • artipacked (8): persist-credentials: false added to every actions/checkout step that doesn't need to push, so the job's GITHUB_TOKEN isn't left in .git/config for later steps to read.
  • excessive-permissions (9, medium): least-privilege permissions: added to every job that was relying on broad default token permissions, plus a deny-all top-level permissions: {} on each workflow (same pattern already used in maintainer-pr-notify.yml) so a future job added without its own block gets nothing by default.
  • cache-poisoning (2, high): cache: false on the two actions/setup-go steps that run in workflows publishing artifacts on release/push (build-binaries.yaml, e2e-tests.yaml), so a poisoned Go module cache can't taint a published build.
  • template-injection (11): values interpolated from ${{ }} expressions inside run: shell blocks routed through env: vars instead, so a value containing shell metacharacters can't break out of the intended string.
  • dangerous-triggers (1, medium): maintainer-pr-notify.yml's pull_request_target trigger flagged generically. Reviewed it — it's already safe (no checkout, restricted to same-repo PRs from an allowlist of maintainers, all PR data passed through toJSON()/format() rather than shell-interpolated). Added a # zizmor: ignore[dangerous-triggers] comment documenting why, instead of changing the trigger.

Verified locally with zizmor .github/workflows/: 0 findings under the default persona, 0 medium/high findings even under --persona=pedantic. The remaining pedantic-only findings (missing workflow name: fields, missing concurrency: blocks, undocumented permission comments) are cosmetic code-smell suggestions, not security issues, and out of scope here.

Not included

This branch is off master directly and does not include the docs-only-CI-skip changes job pattern from #4459/#4455 — kept separate on purpose.

Test plan

  • zizmor re-run locally confirms 0 findings (default persona) / 0 medium+ findings (pedantic persona)
  • YAML syntax validated for all 10 files
  • Confirm qlty check shows 0 blocking issues on this PR
  • Confirm all workflows still run successfully on this PR (build, test, e2e, govulncheck, CodeQL, docker)

zizmor (run via qlty on every PR) flags several classes of workflow
hardening issues; this fixes every one it reports under its default
persona, repo-wide:

- pin every third-party/first-party action to a commit SHA instead of
  a mutable tag, so a compromised or force-moved tag can't silently
  swap in different code (unpinned-uses)
- set persist-credentials: false on every actions/checkout step that
  doesn't need to push, so the job's GITHUB_TOKEN isn't left on disk
  for later steps/processes to read (artipacked)
- add least-privilege permissions to every job that was relying on
  the broad default token permissions, plus a deny-all top-level
  permissions: {} on each workflow so a future job added without its
  own block gets nothing by default (excessive-permissions)
- disable Go module/build caching on the two workflows that publish
  artifacts on release/push, so a poisoned cache can't taint a
  published build (cache-poisoning)
- route template-expanded values used inside shell run: blocks
  through env: vars instead of interpolating them directly, so a
  value that happens to contain shell metacharacters can't break out
  of the intended string (template-injection)
- document why maintainer-pr-notify.yml's pull_request_target trigger
  is safe as-is (no checkout, allowlisted same-repo senders only, all
  PR data passed through toJSON()) with a zizmor ignore comment rather
  than changing its trigger semantics (dangerous-triggers)

Re-ran zizmor locally after: 0 findings under the default persona,
0 medium/high findings even under --persona=pedantic.

Assisted by AI
@qltysh

qltysh Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Qlty


Coverage Impact

This PR will not change total coverage.

🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

@reinkrul
reinkrul merged commit d2ad162 into master Aug 19, 2026
15 checks passed
@reinkrul
reinkrul deleted the ci/fix-zizmor-findings branch August 19, 2026 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants