Add zizmor pre-commit hook and pin GitHub Actions to SHAs

[thrix]

Add zizmor to pre-commit hooks for auditing GitHub Actions workflows for security issues.

https://docs.zizmor.sh/

Fix all findings reported by zizmor:

• Pin all action references to commit SHAs (unpinned-uses)
• Add persist-credentials: false to checkout in publish-images.yml (artipacked)
• Add environment: quay.io to the publish-images job to protect secrets access (secrets-outside-env)

Assisted-by: Claude Code

Pull Request Checklist:

• implement the feature

[gemini-code-assist]

Code Review

This pull request adds the zizmor-pre-commit hook to the .pre-commit-config.yaml file. I have no feedback to provide.

[LecrisUT]

• Pin all action references to commit SHAs (unpinned-uses)

This is a maintenance nightmare 👎. There are equal security concerns if we do not keep up to date with the updates and track security advisories. Unpinned releases is safer for us particularly since we do not use any sources that would be problematic.

• Add persist-credentials: false to checkout in publish-images.yml (artipacked)

Not an issue for us, we do not have private/sensitive repos in this org that would be exposed.

• Add environment: quay.io to the publish-images job to protect secrets access (secrets-outside-env)

Requires additional setup. Slightly in favor of this.

---

Overall I am not enthusiastic about this tool, and would like for us to dig deep in any changes that it suggests and make sure we understand why it is recommending and that we audit the audit. For us, at most it would make sense for the release actions, but even there I am not sure if it is worth it. Feel free to decompose the value of this tool so that I can change my opinion.

[thrix]

• Pin all action references to commit SHAs (unpinned-uses)

This is a maintenance nightmare 👎. There are equal security concerns if we do not keep up to date with the updates and track security advisories. Unpinned releases is safer for us particularly since we do not use any sources that would be problematic.

No, it is a security best practice to prevent tampering:

https://astral.sh/blog/open-source-security-at-astral#cicd-security

Dependabot should be able to deal with commit SHAs:

https://docs.github.com/en/code-security/reference/supply-chain-security/supported-ecosystems-and-repositories#github-actions

• Add persist-credentials: false to checkout in publish-images.yml (artipacked)

Not an issue for us, we do not have private/sensitive repos in this org that would be exposed.

Sure, so we can add a ignore, but it also can stay here, if somebody decides to add a secret sometime, he does not need to care!

• Add environment: quay.io to the publish-images job to protect secrets access (secrets-outside-env)

Requires additional setup. Slightly in favor of this.

Sure, it is there

Overall I am not enthusiastic about this tool, and would like for us to dig deep in any changes that it suggests and make sure we understand why it is recommending and that we audit the audit. For us, at most it would make sense for the release actions, but even there I am not sure if it is worth it. Feel free to decompose the value of this tool so that I can change my opinion.

I am very enthusiastic, but let's see what others think. It is another part to improve the possible issues in the face of raising supply chain security attacks.

[LecrisUT]

No, it is a security best practice to prevent tampering:

https://astral.sh/blog/open-source-security-at-astral#cicd-security

I know about the "best practice", but I am questioning its importance and effect for our use-case:

• We do not heavily use the github actions and there are no significant secrets that we need to protect (only 1 for quay.io)
• It creates significant maintenance burden because we are inherently not trusting the action providers: we would need to audit their commits, monitor the security tab; Otherwise this is just security theater with maintenance burden
• Why do we consider the current action providers as untrustworthy? They are one of Github, redhat-actions, pypa and hynek
• Even if we do the commit pinning it is meaningless if we do not dig deeply in the underlying actions. E.g. hynek/build-and-inspect-python-package works by uv install-ing from pypi effectively making the pinning mostly meaningless
• We are inherently making the github actions more flaky as we have to dig into the actions instead of having their bug fixes automatically picked up

Dependabot should be able to deal with commit SHAs:

https://docs.github.com/en/code-security/reference/supply-chain-security/supported-ecosystems-and-repositories#github-actions

Double 👎 to this. Dependabot closes a PR instead of rebasing which would mean that we would reach double this amount of PRs within a couple of months. This is the whole point of pushing for the use of renovate instead as it works on our pace, not on its pace.

• Add persist-credentials: false to checkout in publish-images.yml (artipacked)

Not an issue for us, we do not have private/sensitive repos in this org that would be exposed.

Sure, so we can add a ignore, but it also can stay here, if somebody decides to add a secret sometime, he does not need to care!

That is not what it does. The only thing that it protects is git clone commands. That's it. And we do not even give it permission.repository: read so it doesn't even have access to any hypothetical private repos to begin with.

It is another part to improve the possible issues in the face of raising supply chain security attacks.

Well then, why don't we address this explicitly, since the only relevant thing we would need to protect are the releases for which the artifacts are completely reproducible:

• Write a tmt test that does an sdist build on downstream and compare the sources
• Rebuild the container and diff the inner results

These seem like much more responsible ways to address supply chain attacks as we would be actively defending ourselves and others by catching the sources of such vulnerabilities.

[thrix]

just noting that this requires the secrets to be moved

[thrix]

No, it is a security best practice to prevent tampering:
https://astral.sh/blog/open-source-security-at-astral#cicd-security

I know about the "best practice", but I am questioning its importance and effect for our use-case:

• We do not heavily use the github actions and there are no significant secrets that we need to protect (only 1 for quay.io)

I would not call it not significant. It is a secret.

• It creates significant maintenance burden because we are inherently not trusting the action providers: we would need to audit their commits, monitor the security tab; Otherwise this is just security theater with maintenance burden
• Why do we consider the current action providers as untrustworthy? They are one of Github, redhat-actions, pypa and hynek

It is not about considering them untrustworthy, it is just a small piece to make sure we do not pick up the the new versions right away. Anybody can be attacked, and this is what lately happened to trivy scanner. If that happens we will just silently pick up a new hacked version and have attackers code running on our project. Sure, this does not prevent that possibility completely, but it helps.

• Even if we do the commit pinning it is meaningless if we do not dig deeply in the underlying actions. E.g. hynek/build-and-inspect-python-package works by uv install-ing from pypi effectively making the pinning mostly meaningless

No, it is not meaningless, it prevents tempering of the pinned versions. I understand that some actions might be doing things that we do not prevent with this simple change, but again, it is a small step in the right direction from my PoW.

• We are inherently making the github actions more flaky as we have to dig into the actions instead of having their bug fixes automatically picked up

I do not see how it would introduce more flakiness, all I want to achieve is to pin the deps by hashes, and let some automated bot to update the hashes from time to time (after a cooldown period), that is all.

Dependabot should be able to deal with commit SHAs:
https://docs.github.com/en/code-security/reference/supply-chain-security/supported-ecosystems-and-repositories#github-actions

Double 👎 to this. Dependabot closes a PR instead of rebasing which would mean that we would reach double this amount of PRs within a couple of months. This is the whole point of pushing for the use of renovate instead as it works on our pace, not on its pace.

Understood, sure, pushing for renovate looks like a good idea and it supports pinning to SHA just well:

https://docs.renovatebot.com/modules/manager/github-actions/#digest-pinning-and-updating

I am fine blocking this on the move to renovatebot.

• Add persist-credentials: false to checkout in publish-images.yml (artipacked)

Not an issue for us, we do not have private/sensitive repos in this org that would be exposed.

Sure, so we can add a ignore, but it also can stay here, if somebody decides to add a secret sometime, he does not need to care!

That is not what it does. The only thing that it protects is git clone commands. That's it. And we do not even give it permission.repository: read so it doesn't even have access to any hypothetical private repos to begin with.

I read about it more actions/checkout#485 and linked PRs/issues.
While I would just add it as a best practice that does not hurt even if meaningless in our context, seems with v6 anyway the concern is lower, even if we would be using cloning secrets. I am happy to add an ignore flag for this.

It is another part to improve the possible issues in the face of raising supply chain security attacks.

Well then, why don't we address this explicitly, since the only relevant thing we would need to protect are the releases for which the artifacts are completely reproducible:

• Write a tmt test that does an sdist build on downstream and compare the sources
• Rebuild the container and diff the inner results

These seem like much more responsible ways to address supply chain attacks as we would be actively defending ourselves and others by catching the sources of such vulnerabilities.

I am addressing here the concern of having another layer of protection (static analysis) for github actions. Sure, we do not use them a lot, but what if in 5 years somebody will add something and be not careful enough. Anyway, the main motivation was the SHA pinning. I am still not convinced it is not worth doing. Of course, I am working with the assumption we have a resonable system that is able to update the actions on the pace we want and not to create too much maintanance burden.

As I said, I am fine blocking this until renovate is in place, seems the experience with dependabot would just become worse.

[happz]

Three cons I consider not as significant as suggested:

• Why do we consider the current action providers as untrustworthy? They are one of Github, redhat-actions, pypa and hynek

Trust but verify. RH signs RPMs for users to be able to verify their content, despite being fairly trustworthy entity. Even trustworthy entities like Red Hat can be fooled into doing something nasty, as demonstrated quite clearly by the recent xz utils backdoor example. I see no problem in making it harder for even these entities to harm us.

• Even if we do the commit pinning it is meaningless if we do not dig deeply in the underlying actions. E.g. hynek/build-and-inspect-python-package works by uv install-ing from pypi effectively making the pinning mostly meaningless

There are more ways an action can act maliciously, installing compromised package from PyPI is certainly one way, but not the only. And this has never been about a silver bullet removing each and every attack vector, it's a game of layers and obstacles. This kind of change does not try to prevent uv install from installing bad packages, it tries to prevent malicious actor subverting code of the action itself. Therefore I see no problem in making it harder for Github action to harm us in this particular way.

• It creates significant maintenance burden because we are inherently not trusting the action providers: we would need to audit their commits, monitor the security tab; Otherwise this is just security theater with maintenance burden
• We are inherently making the github actions more flaky as we have to dig into the actions instead of having their bug fixes automatically picked up

But we do not automatically pick them up. We get a PR with proposed change, hynek/build-and-inspect-python-package@v2 -> hynek/build-and-inspect-python-package@v3. Then, before we hit the "Squash and merge" button, we should check what changes are we letting in - the sense of "having their bug fixes automatically picked up" comes from us not doing this deeply enough, often just merging the PR. Same for pre-commit checks. This is not a new demand brought on our heads by commit ID, it already exists. I see no problem here either, we should be doing this anyway, at least to some degree, and switching to commit ID doesn't add new obligation.

Whether their changes are pinned in our files by a version or commit ID seems irrelevant in this regard. Commit ID changes something else, it's not whether we should start inspecting changes in actions and pre-commit checks, but whether this inspection could be invalidated by a malicious actor, e.g. by poisoning v2 tag, without us knowing. One but not the only real-life example could be https://www.wiz.io/blog/github-action-tj-actions-changed-files-supply-chain-attack-cve-2025-30066

[thrix]

Btw, that is a place where AI could be useful, next to the renovatebot update analyze the changes, look for suspicious changes in the 3rd party actions code.

[LecrisUT]

But we do not automatically pick them up. We get a PR with proposed change, hynek/build-and-inspect-python-package@v2 -> hynek/build-and-inspect-python-package@v3.

I believe you are missing a context on the Github actions design. The vX tags in github actions are intentionally moving tags which move from v2.16.0 tag to v2.17.0 etc. after each release. The workflow you are thinking of is if we change to use vX.Y static tags and add the second layer of commit check on top of it, for which I am not opposed to. Using it to vX moving tags is where the design breaks down, because the whole purpose of those tags is to reduce maintenance overhead explicitly saying that "go ahead and give me all bug fixes as soon as they are available, I trust you".

Trust but verify. RH signs RPMs for users to be able to verify their content, despite being fairly trustworthy entity. Even trustworthy entities like Red Hat can be fooled into doing something nasty, as demonstrated quite clearly by the recent xz utils backdoor example. I see no problem in making it harder for even these entities to harm us.

Not opposed to trust but verify, but this does not do that. For a project with such a low github actions footprint, it is edging closer to security-theater. We have fairly simple release artifacts so we can do proper verification of the end-point artifacts as I've mentioned before

Well then, why don't we address this explicitly, since the only relevant thing we would need to protect are the releases for which the artifacts are completely reproducible:
* Write a tmt test that does an sdist build on downstream and compare the sources
* Rebuild the container and diff the inner results

The only tricky one is the container, but that's a different topic which I do not believe is solvable. The build-attestation are an interesting new tool that we could use though.

[happz]

But we do not automatically pick them up. We get a PR with proposed change, hynek/build-and-inspect-python-package@v2 -> hynek/build-and-inspect-python-package@v3.

I believe you are missing a context on the Github actions design. The vX tags in github actions are intentionally moving tags which move from v2.16.0 tag to v2.17.0 etc. after each release. The workflow you are thinking of is if we change to use vX.Y static tags and add the second layer of commit check on top of it, for which I am not opposed to. Using it to vX moving tags is where the design breaks down, because the whole purpose of those tags is to reduce maintenance overhead explicitly saying that "go ahead and give me all bug fixes as soon as they are available, I trust you".

Once vX release and tag are published, instead of saving vX tag as git ref on our side we would save the commit ID vX tag points at. If a malicious user two months from that moment manages sneak an exploit into the project, and manages to change what vX tag points at, we would be protected by not pointing at vX tag, but the at the original commit ID instead. I don't understand what breaks in your view, and I admit I don't understand the context you refer to. Seems like a fairly trivial and reasonable improvement to me. We do not stop being interested in releases, we would still include changes and bugfixes they ship, we would just not rely on the assumption that code vX refers to today is the same code vX refers to six months from now. The tags not being immutable is the issue the change is trying to address.

[LecrisUT]

The tags not being immutable is the issue the change is trying to address.

vX are by design mutable tags (see how https://github.com/hynek/build-and-inspect-python-package/tags have multiple tags that point to the same commit. If you follow the history, v2 was previously pointing to 35116b2 and this re-tagging is an intentional design, not a good one, but an intentional one), and those by design will not be picked up by any renovatebot/dependabot because that would defeat the purpose. The workflow and protection you have in mind is if we first move all of the github actions to the immutable tags vX.Y.Z, and then apply this change. That would be more acceptable, but again it requires more rapid babysitting of renovate. It will introduce a source of flakiness that we need to monitor.

[happz]

The tags not being immutable is the issue the change is trying to address.

vX are by design mutable tags (see how https://github.com/hynek/build-and-inspect-python-package/tags have multiple tags that point to the same commit. If you follow the history, v2 was previously pointing to 35116b2 and this re-tagging is an intentional design, not a good one, but an intentional one), and those by design will not be picked up by any renovatebot/dependabot because that would defeat the purpose. The workflow and protection you have in mind is if we first move all of the github actions to the immutable tags vX.Y.Z, and then apply this change. That would be more acceptable, but again it requires more rapid babysitting of renovate. It will introduce a source of flakiness that we need to monitor.

But vX.Y.Z tags are also mutable, aren't they? They are not supposed to be mutated, sure, the owner of the repository may not intend to ever change v2.17.0, but they are as mutable as any other tag. If the owner becomes compromised, the tag can be changed and suddenly include commits not added by the owner. The idea is us saving not the v2.17.0 tag as git ref, but the commit ID the tag pointed at when the it was created.

[LecrisUT]

But vX.Y.Z tags are also mutable, aren't they?

They are, but I am referring to the context of Github actions design. vX.Y.Z are meant to be immutable, and protecting with commit hash for these does make sense to me, and 👍 form me for that. vX are by design meant to be mutable and protecting the commit hash on these goes against the whole purpose of those tags.

There are 2 orthogonal actions here:

• Moving from vX to vX.Y.Z tags. Please let's make a separate PR for this.
• Locking the tags by the commit hash. Only makes sense if we do the above.

[LecrisUT]

Moving from vX to vX.Y.Z tags. Please let's make a separate PR for this.

From hacking session we are in agreement to start trying to do this (with the commit hash locking coming hand-in-hand)

• When renovate creates a new PR:
  • We are expected to at least check the changelogs
  • Checking the specific code changes if one can, but not feasible
• We would need to monitor if any of the actions are compromised after we have updated the commit, since we would not pick up the fix
  • We cannot use dependabot because it fails at uv sync, but maybe there is another way around it. More reserach is needed