ci: fix Zizmor-identified issues in workflows by HastD · Pull Request #27642 · podman-container-tools/podman

HastD · 2025-11-30T19:14:12Z

Fix various potentially security-relevant issues identified by Zizmor:

Pin third-party action dependencies by commit hashes: https://docs.zizmor.sh/audits/#unpinned-uses
Specify workflow permissions per job: https://docs.zizmor.sh/audits/#excessive-permissions
Specify persist-credentials for actions/checkout: https://docs.zizmor.sh/audits/#artipacked
Use environment variables to avoid template expansion in code contexts, which can lead to code injection vulnerabilities: https://docs.zizmor.sh/audits/#template-injection
Use cache: false with actions/setup-go to prevent possible cache poisoning: https://docs.zizmor.sh/audits/#cache-poisoning
Pass secrets explicitly to reusable workflow: https://docs.zizmor.sh/audits/#secrets-inherit

Also, a couple shell script issues fixed:

Double-quote variable expansions to avoid unintended shell splitting and globbing: https://github.com/koalaman/shellcheck/wiki/SC2086
Use --sandbox for sed commands with dynamically generated scripts: https://www.gnu.org/software/sed/manual/sed.html#Command_002dLine-Options ("Sandbox mode ensures sed operates only on the input files designated on the command line, and cannot run external programs.")

Checklist

Certify you wrote the patch or otherwise have the right to pass it on as an open-source patch by signing all
commits. (git commit -s). (If needed, use git commit -s --amend). The author email must match
the sign-off email address. See CONTRIBUTING.md
for more information.
Referenced issues using Fixes: #00000 in commit message (if applicable)
Tests have been added/updated (or no tests are needed)
Documentation has been updated (or no documentation changes are needed)
All commits pass make validatepr (format/lint checks)
Release note entered in the section below (or None if no user-facing changes)

Does this PR introduce a user-facing change?

None

Honny1 · 2025-12-01T09:52:52Z

PTAL @ashley-cui

Luap99

Thanks, this is a large change which is doing many things at once, please split them into small commits to fix one issue at a time to make reviewing easier and clearly document in each commit what problem you are solving and why this fixes it.

Overall I think most of the changes seem good but it is really hard to follow the logic here so I really like to see the commits split so I can review this better

HastD · 2025-12-01T12:20:39Z

Marking as draft while I split the PR into multiple commits as requested.

Signed-off-by: Daniel Hast <hast.daniel@protonmail.com>

Also remove some unnecessary permissions: * The notification job in release-artifacts.yml only needs to read repo contents, not write contents and actions. * All jobs in release.yml except "Create release" and "Update podman.io" only need to read repo contents. "Update podman.io" only needs to write repo contents and pull requests. * Likewise, permissions for update-podmanio.yml can be restricted to only writing repo contents and pull requests. Signed-off-by: Daniel Hast <hast.daniel@protonmail.com>

Explicitly set `persist-credentials: true` for uses of `actions/checkout` where it's needed (when the job does git operations using the stored credentials) and `persist-credentials: false` where the stored credentials are not later used. This reduces the risk of cached credentials accidentally being leaked via artifacts. Signed-off-by: Daniel Hast <hast.daniel@protonmail.com>

This mitigates a potential cache-poisoning attack. For details, see: https://docs.zizmor.sh/audits/#cache-poisoning Signed-off-by: Daniel Hast <hast.daniel@protonmail.com>

Using `secrets: inherit` forwards all secrets to the workflow and makes it harder to determine which secrets the workflow was actually executed with. See: https://docs.zizmor.sh/audits/#secrets-inherit Signed-off-by: Daniel Hast <hast.daniel@protonmail.com>

sed scripts are capable of doing file I/O and executing arbitrary commands. The `--sandbox` option prevents this by rejecting sed commands with such capabilities; it's good practice to use this whenever the sed script is dynamically generated (e.g. if it involves a variable expansion). Also fixed an error in one sed script where `.*` had been placed outside of the quoted string (and would therefore be subject to shell globbing), presumably due to single-quotes having been changed to double-quotes at some point in the past. Signed-off-by: Daniel Hast <hast.daniel@protonmail.com>

Template expansions are not aware of shell script syntax, and therefore can potentially result in code injection vulnerabilities when used in code contexts: https://docs.zizmor.sh/audits/#template-injection To avoid this, instead use environment variables to safely store the values of the template expansions. Also (in the process of doing the above) added double-quotes around a some instances of variable expansions in shell scripts, which is necessary to avoid unintended shell splitting and globbing. (I didn't see any instances where this was actually likely to result in erroneous behavior, but it's good practice and makes shell scripts more robust.) Signed-off-by: Daniel Hast <hast.daniel@protonmail.com>

packit-as-a-service · 2025-12-01T13:44:17Z

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

packit-as-a-service · 2025-12-01T13:44:18Z

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

packit-as-a-service · 2025-12-01T13:44:18Z

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

packit-as-a-service · 2025-12-01T13:44:18Z

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

HastD · 2025-12-01T13:46:08Z

PR has been split into multiple commits, each addressing just one issue. (Let me know if it'd be better to also make this multiple PRs—that'd be easy enough to do now that I've split the commits.)

baude · 2025-12-01T16:15:48Z

@timcoding1988 ptal

ashley-cui

LGTM

timcoding1988 · 2025-12-01T16:50:05Z

LGTM

lsm5 · 2025-12-02T13:43:11Z

Thanks for the PR @HastD . I just checked out your branch locally and tried zizmor v1.18.0 installed via pip on Fedora 43 and got this:

$ zizmor .github/workflows/*
🌈 zizmor v1.18.0
 INFO audit: zizmor: 🌈 completed .github/workflows/check_cirrus_cron.yml
 INFO audit: zizmor: 🌈 completed .github/workflows/dev-bump.yml
 INFO audit: zizmor: 🌈 completed .github/workflows/first_contrib_cert_generator.yml
 INFO audit: zizmor: 🌈 completed .github/workflows/issue-labeler.yml
 INFO audit: zizmor: 🌈 completed .github/workflows/issue_pr_lock.yml
 INFO audit: zizmor: 🌈 completed .github/workflows/labeler.yml
 INFO audit: zizmor: 🌈 completed .github/workflows/mac-pkg.yml
 INFO audit: zizmor: 🌈 completed .github/workflows/machine-os-pr.yml
 INFO audit: zizmor: 🌈 completed .github/workflows/needs-info-labeler.yaml
 INFO audit: zizmor: 🌈 completed .github/workflows/release-artifacts.yml
 INFO audit: zizmor: 🌈 completed .github/workflows/release.yml
 INFO audit: zizmor: 🌈 completed .github/workflows/stale.yml
 INFO audit: zizmor: 🌈 completed .github/workflows/update-podmanio.yml
 INFO audit: zizmor: 🌈 completed .github/workflows/upload-win-installer.yml
error[dangerous-triggers]: use of fundamentally insecure workflow trigger
  --> .github/workflows/first_contrib_cert_generator.yml:5:1
   |
 5 | / on:
 6 | |   pull_request_target:
 7 | |     types: [closed]
 8 | |     branches:
...  |
16 | |         description: 'The pull request number'
17 | |         required: true
   | |______________________^ pull_request_target is almost always used insecurely
   |
   = note: audit confidence → Medium

error[dangerous-triggers]: use of fundamentally insecure workflow trigger
 --> .github/workflows/labeler.yml:3:1
  |
3 | / on:
4 | | - pull_request_target
  | |_____________________^ pull_request_target is almost always used insecurely
  |
  = note: audit confidence → Medium

error[dangerous-triggers]: use of fundamentally insecure workflow trigger
 --> .github/workflows/machine-os-pr.yml:3:1
  |
3 | / on:
4 | |   pull_request_target:
5 | |     paths:
6 | |       - 'version/rawversion/version.go'
  | |_______________________________________^ pull_request_target is almost always used insecurely
  |
  = note: audit confidence → Medium

45 findings (42 suppressed): 0 informational, 0 low, 0 medium, 3 high

Is that expected?

Also, @containers/podman-maintainers , I think it'd be nice to have zizmor run as a separate CI job or part of validate.

HastD · 2025-12-02T13:51:31Z

@lsm5 Yes, this PR doesn't fix that issue—replacing pull_request_target workflow triggers sometimes requires more significant refactoring and I was trying to limit the scope of this PR to addressing the issues that could be fixed more straightforwardly with minimal risk of breaking anything in CI. I'll go ahead and open an issue about the use of insecure workflow triggers so we have a place to discuss it.

I also agree it'd be a good idea to add a Zizmor workflow to flag workflow issues if future PRs reintroduce them. It's easy to set up, there's a GitHub action: https://docs.zizmor.sh/integrations/#via-zizmorcorezizmor-action

lsm5 · 2025-12-02T13:59:06Z

@lsm5 Yes, this PR doesn't fix that issue—replacing pull_request_target workflow triggers sometimes requires more significant refactoring and I was trying to limit the scope of this PR to addressing the issues that could be fixed more straightforwardly with minimal risk of breaking anything in CI. I'll go ahead and open an issue about the use of insecure workflow triggers so we have a place to discuss it.

SGTM

I also agree it'd be a good idea to add a Zizmor workflow to flag workflow issues if future PRs reintroduce them. It's easy to set up, there's a GitHub action: https://docs.zizmor.sh/integrations/#via-zizmorcorezizmor-action

Great! Would be nice to get in once the insecure workflow trigger is fixed. Thanks @HastD

Luap99 · 2025-12-02T14:11:08Z

I am not sure how we could replace pull_request_target for machine-os-pr or the issue-labler but sure create an issue if you have ideas and we can discus there.

Luap99

/lgtm

Thank you

lsm5

LGTM with #27658 identified as a future fix

openshift-ci · 2025-12-02T14:13:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: HastD, lsm5, Luap99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [Luap99,lsm5]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Zizmor (https://docs.zizmor.sh/) is a static analysis tool for GitHub Actions. Most of the issues identified by Zizmor were fixed in podman-container-tools#27642. This Zizmor action integrates with GitHub Advanced Security and scans workflows for potential security issues, which should help ensure that such issues aren't reintroduced in the future. Signed-off-by: Daniel Hast <hast.daniel@protonmail.com>

openshift-ci Bot added the do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None label Nov 30, 2025

Luap99 reviewed Dec 1, 2025

View reviewed changes

Comment thread .github/workflows/check_cirrus_cron.yml Outdated

HastD marked this pull request as draft December 1, 2025 12:20

openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 1, 2025

HastD force-pushed the zizmor-issues branch from 3360e15 to 4ae8c06 Compare December 1, 2025 13:40

HastD added 7 commits December 1, 2025 08:41

ci: pin 3rd-party action dependencies by commit hashes

248d8f6

Signed-off-by: Daniel Hast <hast.daniel@protonmail.com>

ci: disable caching for actions/setup-go

64ddbfe

This mitigates a potential cache-poisoning attack. For details, see: https://docs.zizmor.sh/audits/#cache-poisoning Signed-off-by: Daniel Hast <hast.daniel@protonmail.com>

HastD force-pushed the zizmor-issues branch from 4ae8c06 to 67c050b Compare December 1, 2025 13:43

HastD marked this pull request as ready for review December 1, 2025 13:44

openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 1, 2025

openshift-ci Bot added release-note-none and removed do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels Dec 1, 2025

ashley-cui reviewed Dec 1, 2025

View reviewed changes

HastD mentioned this pull request Dec 2, 2025

Use of potentially insecure workflow triggers in CI #27658

Closed

Luap99 approved these changes Dec 2, 2025

View reviewed changes

openshift-ci Bot assigned Luap99 Dec 2, 2025

openshift-ci Bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Dec 2, 2025

lsm5 approved these changes Dec 2, 2025

View reviewed changes

openshift-merge-bot Bot merged commit 9a811bf into podman-container-tools:main Dec 2, 2025
40 checks passed

HastD deleted the zizmor-issues branch December 2, 2025 14:27

HastD mentioned this pull request Dec 2, 2025

ci: add Zizmor workflow #27663

Merged

6 tasks done

stale-locking-app Bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Mar 3, 2026

stale-locking-app Bot locked as resolved and limited conversation to collaborators Mar 3, 2026

Conversation

HastD commented Nov 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Does this PR introduce a user-facing change?

Uh oh!

Honny1 commented Dec 1, 2025

Uh oh!

Luap99 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

HastD commented Dec 1, 2025

Uh oh!

packit-as-a-service Bot commented Dec 1, 2025

Uh oh!

packit-as-a-service Bot commented Dec 1, 2025

Uh oh!

packit-as-a-service Bot commented Dec 1, 2025

Uh oh!

packit-as-a-service Bot commented Dec 1, 2025

Uh oh!

HastD commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

baude commented Dec 1, 2025

Uh oh!

ashley-cui left a comment

Choose a reason for hiding this comment

Uh oh!

timcoding1988 commented Dec 1, 2025

Uh oh!

lsm5 commented Dec 2, 2025

Uh oh!

HastD commented Dec 2, 2025

Uh oh!

lsm5 commented Dec 2, 2025

Uh oh!

Luap99 commented Dec 2, 2025

Uh oh!

Luap99 left a comment

Choose a reason for hiding this comment

Uh oh!

lsm5 left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Dec 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

HastD commented Nov 30, 2025 •

edited

Loading

HastD commented Dec 1, 2025 •

edited

Loading