Skip to content

ci: Fix various warnings reported by zizmor - #15600

Merged
rgacogne merged 6 commits into
PowerDNS:masterfrom
rgacogne:zizmor
Jun 5, 2025
Merged

rgacogne merged 6 commits into
PowerDNS:masterfrom
rgacogne:zizmor

Conversation

@rgacogne

Copy link
Copy Markdown
Member

Short description

Zizmor1 reports a few warnings when looking at our workflows:

  • the credentials are still available in the environment after calling checkout, which is not nice. see Remove persist-credentials or change the default to false actions/checkout#485
  • we use several external actions without pinning to an exact version. I fixed this for coveralls, but not for the remaining ones because it's not clear to me how to pin the exact version of a container image, for example
  • very often we pass the result of a template expansion to a shell command, which in theory might be vulnerable to script injection attacks if the input is untrusted. I did not find any untrusted input (except the branch name, in some workflows, but GH has some limitations on the values you can use there) but to prevent any issue in the future I tried to get rid of this pattern as suggested by GH

This PR needs a very thorough review before being merged, as the changes to several workflows have not been tested!

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@rgacogne rgacogne added this to the common-soon milestone May 27, 2025
@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15276506433

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 43 unchanged lines in 12 files lost coverage.
  • Overall coverage decreased (-0.005%) to 63.691%

Files with Coverage Reduction New Missed Lines %
pdns/backends/gsql/gsqlbackend.hh 1 97.77%
pdns/dnsdistdist/dnsdist-crypto.cc 1 76.01%
pdns/recursordist/recursor_cache.cc 1 84.22%
pdns/validate.cc 1 68.47%
modules/lmdbbackend/lmdbbackend.cc 2 74.52%
pdns/rcpgenerator.cc 2 90.82%
pdns/sstuff.hh 2 57.5%
pdns/dnsdistdist/dnsdist-carbon.cc 3 61.8%
pdns/stubresolver.cc 3 77.02%
pdns/shuffle.cc 4 53.93%
Totals Coverage Status
Change from base Build 15271100803: -0.005%
Covered Lines: 130642
Relevant Lines: 170263

💛 - Coveralls

@rgacogne
rgacogne requested review from Habbie and romeroalx June 2, 2025 10:45
romeroalx and others added 2 commits June 3, 2025 12:26
CI: added digest (index when possible) for external images used in gh actions

@romeroalx romeroalx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Triggered some extra tests:

All tests were correct. A few errors appeared but they seem related to connectivity issues (apt/yum) rather than something that was changed.

@rgacogne
rgacogne merged commit 3474952 into PowerDNS:master Jun 5, 2025
@rgacogne
rgacogne deleted the zizmor branch June 5, 2025 08:15
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.

3 participants