Skip to content

ci: Fix various warnings reported by zizmor #15600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 5, 2025
Merged

Conversation

rgacogne
Copy link
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

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 romeroalx and Habbie 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
Copy link
Member

@romeroalx romeroalx left a comment

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
87 checks passed
@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