Improve security of GitHub Actions workflows

[caxu-rh]

This change addresses several attack vectors and weaknesses which have been seen in the wild over the past few years:

• Template injection. Instead of directly referencing GitHub Actions variables ${{ ... }} in scripts, where they are added literally, assign the value to an environment variable and use the environment variable in the script. The shell will expand the environment variable and escape any problematic inputs.
• Pin actions. Third-party actions are often referenced by tags, which are floating and can be re-assigned to a different ref. Instead, refer to third-party actions by a full Git SHA-1 commit hash. The intended tag is still left as a comment, which Dependabot can use for automatic updates. In March 2025, a supply chain attack conducted on the tj-actions/changed-files action resulted in the potential compromise of secrets for any workflows which ran the action without pinning it to a known good version.
• Excessive token permissions. The GitHub token used inside the workflow run has write access to most things in the repo, unless permissions are explicitly specified. Tools are also available to audit workflows and identify the minimal required set of permissions. We'll tackle this in a follow-up PR.
• Don't persist credentials in Git checkouts. By default, when using the actions/checkout action, a copy of the workflow credential (e.g. GITHUB_TOKEN) will be persisted in the checked-out repository's .git/config. This makes it easier to perform some further operations in the repo, e.g. pushing a commit or tag later. This can pose an issue if the checked-out repository is then uploaded as an artifact, which can be publicly downloaded.

This change also introduces scanning using zizmor, a static analyzer for GitHub Actions workflows, to run on pushes to main and all pull requests.

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[coveralls]

coverage: 83.812%. remained the same
when pulling fb1d072 on caxu-rh:github-actions-hardening
into bb06e5a on redhat-openshift-ecosystem:main.

[acornett21]

It will probably take a couple of passes to double check everything on this. Since this is such a sweeping change, I think once we feel this is no longer WIP, we setup your fork to cut releases to your own quay and test those workflows as well.

[caxu-rh]

I've reduced the scope of this PR to skip explicitly adding permissions on jobs/workflows for now, instead just leaving TODOs.

In follow-up PRs, we should audit the necessary scope of the tokens for each job using a tool like https://github.com/GitHubSecurityLab/actions-permissions, and then assign the necessary permissions based on findings.

[dcibot]

from change #1298:

• SUCCESS https://www.distributed-ci.io/jobs/1b8f42e9-df91-4f00-a120-1cee547b20a1/jobStates

[github-actions]

Code Review by Gemini

This pull request significantly improves the security posture of your GitHub Actions workflows, addressing several critical attack vectors. The commit message is exceptionally clear, detailing the vulnerabilities addressed and even noting what will be tackled in a follow-up PR.

Here's a detailed review:

General Observations & Strengths

1. Comprehensive Security Improvements: The PR successfully implements fixes for:

   • Template Injection: By assigning GitHub Actions expressions to environment variables before using them in run scripts, you effectively prevent shell injection vulnerabilities. This is a crucial improvement.
   • Action Pinning: All third-party actions are now pinned to specific Git SHA-1 hashes, eliminating the risk of supply chain attacks through mutable tags. This is a gold standard for GitHub Actions security.
   • Credential Persistence: The actions/checkout action now consistently uses persist-credentials: false, preventing the GITHUB_TOKEN from being written to the .git/config file, which could be leaked if artifacts are uploaded.
   • Static Analysis: The introduction of zizmor for GitHub Actions security scanning is an excellent proactive measure.

2. Clear Documentation: The commit message is a fantastic example of how to document security changes, explaining the "why" behind each modification and referencing external resources. The TODO comments and zizmor: ignore annotations in the code are also very helpful for tracking future work.

3. Idiomatic Go: This review is for GitHub Actions workflow files (YAML), not Go code. Therefore, the "Idiomatic Go" aspect is not applicable here.

Specific Feedback and Suggestions

add-release-info-to-pyxis.yml

• Template Injection Fix: The change to use DATA_PAYLOAD and PYXIS_ENDPOINT environment variables for the curl command is a perfect example of preventing template injection. This is well done.
• Permissions: As noted by the TODO comment, explicitly setting permissions for this job would be a good follow-up. Given it interacts with external secrets and an API, ensuring it only has the minimum necessary permissions is important.

build-main.yml, build-release.yml, go.yml, release-artifacts.yml

• Action Pinning: All actions are correctly pinned to SHAs. Excellent.
• persist-credentials: false: Correctly applied to actions/checkout.
• Template Injection Fixes: The use of environment variables for echo commands (e.g., REGISTRY_PATHS) is a good practice to prevent potential issues, even if the risk is lower for simple echo statements.
• Permissions: The TODO comments regarding explicit permissions are appropriate. This is the main remaining area for hardening these workflows.

build-multiarch.yml

• Template Injection Fixes: The changes to use INPUT_NAME and INPUT_TAG environment variables for buildah manifest and podman manifest commands are well-executed and prevent potential injection.
• Action Pinning: All actions are correctly pinned to SHAs. Excellent.

check-actions.yml (New File)

• Purpose: This is a fantastic addition. Running zizmor on push to main and all pull_request events provides continuous security scanning.
• Permissions: permissions: {} for this workflow is a best practice for security scanning tools that don't need to modify the repository. This is excellent.
• Action Pinning: Both actions/checkout and zizmorcore/zizmor-action are correctly pinned.

code-review.yml

• pull_request_target Trigger: The zizmor: ignore[dangerous-triggers] comment correctly identifies the inherent risk of pull_request_target. While necessary for actions that need to write back to the PR (like adding comments/labels), it means the workflow runs with the base repository's permissions, even for untrusted fork PRs. This makes the trustworthiness of the gemini-code-review-action paramount.
• Workflow-level Permissions: The TODO comment about assigning permissions at the job level rather than the workflow level is very important. Currently, pull-requests: write is granted to the entire workflow. If gemini-code-review only needs read access to the PR content, it should explicitly request pull-requests: read (or contents: read) to follow the principle of least privilege. This is a high-impact follow-up item.
• Template Injection Fix: The use of LABEL_JSON and EVENT_ACTION environment variables for jq processing is a good improvement.
• Action Pinning: Both actions/checkout and sshnaidm/gemini-code-review-action are correctly pinned.

copy-to-rhisv.yml

• Template Injection Fixes: The use of environment variables for registry and image names in the skopeo copy commands is a good security enhancement.
• Action Pinning: redhat-actions/podman-login is correctly pinned.

Performance Optimizations

• The changes are primarily security-focused and do not introduce or address significant performance bottlenecks. The impact on performance should be minimal.

Conclusion

This pull request is a strong positive change for the project's security. The implementation of action pinning, template injection prevention, and credential persistence control are critical and well-executed. The introduction of zizmor is also a valuable addition.

The main area for future improvement, as already identified in the TODO comments, is to refine the GitHub Actions token permissions to follow the principle of least privilege more strictly, especially for jobs that don't require write access.

Overall Recommendation: Approve with high confidence.

[github-actions]

Code Review by Gemini

This pull request significantly improves the security posture of your GitHub Actions workflows, addressing several common attack vectors. The changes are well-aligned with the detailed commit message and demonstrate a strong commitment to security best practices.

Here's a breakdown of the review:

Overall Assessment

The changes are excellent and directly address the stated goals:

• Template Injection Prevention: Successfully implemented by assigning GitHub Actions expressions to environment variables before use in shell scripts. This is a crucial improvement.
• Action Pinning: All third-party actions are now pinned to specific Git SHAs, which is a critical security best practice to prevent supply chain attacks. The comments indicating the original tag are helpful for Dependabot.
• Credential Persistence: persist-credentials: false has been added to all actions/checkout steps, preventing the GITHUB_TOKEN from being written to the .git/config file.
• zizmor Integration: The new check-actions.yml workflow introduces static analysis for GitHub Actions, which is a proactive security measure. It's also implemented with minimal permissions (permissions: {}), which is excellent.
• Excessive Token Permissions: The commit message explicitly states this is a follow-up, and the addition of TODO comments and zizmor: ignore[excessive-permissions] comments in the code acknowledges this. This is a good approach for incremental security improvements.

Detailed Feedback

Security Vulnerabilities & Best Practices

1. Template Injection Fixes (add-release-info-to-pyxis.yml, build-main.yml, build-multiarch.yml, copy-to-rhisv.yml, code-review.yml):

   • Positive: The refactoring to assign complex expressions or sensitive inputs to environment variables (e.g., DATA_PAYLOAD, PYXIS_ENDPOINT, RELEASE_TAG, INPUT_NAME, DIGEST, SOURCE_IMAGE_REGISTRY) before using them in run steps is a robust defense against template injection. This ensures the shell correctly interprets the values rather than potentially executing malicious code embedded in the input.
   • Example: The change in add-release-info-to-pyxis.yml from curl ... -d '{"commit":"${{ inputs.commit }}"...}' to using a DATA_PAYLOAD environment variable is a perfect example of this mitigation.

2. Action Pinning (All Workflows):

   • Positive: Every third-party action has been updated to use a full Git SHA-1 hash instead of a floating tag (e.g., actions/checkout@v4 to actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2). This is a critical security improvement that prevents an attacker from re-tagging an action to a malicious version. The comments indicating the original tag are very helpful for maintainability and Dependabot.

3. Credential Persistence (actions/checkout in build-main.yml, build-release.yml, check-actions.yml, code-review.yml, go.yml, release-artifacts.yml):

   • Positive: The addition of with: persist-credentials: false to all actions/checkout steps is an excellent security hardening measure. It prevents the GITHUB_TOKEN from being stored in the .git/config file within the checked-out repository, reducing the risk of credential leakage if the repository is later uploaded as an artifact or compromised.

4. Explicit Permissions (check-actions.yml):

   • Positive: The new check-actions.yml workflow correctly sets permissions: {} at the workflow level. This is a best practice for new workflows, ensuring that the GITHUB_TOKEN has no default permissions and jobs must explicitly request only what they need.

5. Acknowledged Security Debt (All Workflows):

   • Positive: The TODO: Set explicit permissions for this job. comments and zizmor: ignore[excessive-permissions] annotations are well-placed. They clearly indicate that the issue of excessive token permissions is recognized and planned for future work, aligning with the commit message. This transparency is valuable.
   • Positive: The code-review.yml workflow correctly uses zizmor: ignore[dangerous-triggers] for pull_request_target. This trigger is inherently risky as it runs with base repository permissions on untrusted PRs, but it's often necessary for certain automation tasks. Acknowledging this risk and mitigating it with other measures (like persist-credentials: false and careful input handling) is good.

Performance Optimizations

• The changes are primarily security-focused and do not introduce any significant performance optimizations or regressions. Pinning actions might slightly improve cache hit rates for action downloads, but the impact is likely negligible.

Idiomatic Go

• This review focuses on GitHub Actions workflows (YAML), not Go code. Therefore, "Idiomatic Go" is not applicable here.

Minor Suggestions

1. Consistency in Permission TODOs: While many jobs now have TODO: Set explicit permissions for this job. and zizmor: ignore[excessive-permissions], some jobs in add-release-info-to-pyxis.yml, build-multiarch.yml, copy-to-rhisv.yml, and release-artifacts.yml do not have these comments, even if their default permissions might be excessive. For consistency and to ensure these are tracked for future work, consider adding these comments to all jobs where explicit permissions are not yet defined.

Conclusion

This pull request represents a substantial and well-executed security improvement for your GitHub Actions workflows. The changes are thorough, correctly implemented, and demonstrate a proactive approach to supply chain security.

Approved.

[caxu-rh]

Edited to add scanning with actionlint, which also runs shellcheck on inline scripts, to enforce best practices such as quoting environment variables, etc.

[dcibot]

from change #1298:

• FAILURE https://www.distributed-ci.io/jobs/fcafdd2b-11d0-40c2-b939-71bd92e8ffa8/jobStates

[dcibot]

from change #1298:

• SUCCESS https://www.distributed-ci.io/jobs/78a0ea3c-4a35-4fe0-945d-2aee00b5e3ac/jobStates

[acornett21]

/lgtm

[dcibot]

from change #1298:

• SUCCESS https://www.distributed-ci.io/jobs/0191e8c5-93a2-4d0d-82a3-310000e41c0c/jobStates

[dcibot]

from change #1298:

• SUCCESS https://www.distributed-ci.io/jobs/95b62d93-31c7-4ea0-9a08-842241fea5e2/jobStates

[acornett21]

/lgtm

[bcrochet]

Would like to see a final pass that all of the release tags for actions used are up-to-date. Otherwise, lgtm

[caxu-rh]

Updated. Since the last time I visited this, Zizmor also added an audit on cooldown blocks in Dependabot config. This feature basically tells Dependabot to ignore new versions that are too new, which is a rough way of avoiding supply chain attacks. Zizmor audits the value and looks for at least 4 days or more in the config. I figure this is fine for us since:

• We cut releases far less often than we merge Dependabot PRs, so waiting an extra few days/week for the next Dependabot run is probably not that big of deal
• For high priority, urgent updates, we'd probably want to pick the update manually anyways (rather than waiting up to a week for the next Dependabot run)

More about the cooldown config in Dependabot, and more about the Zizmor audit.

Alternatively, we can also disable/ignore this particular audit if this seems unreasonable.

[dcibot]

from change #1298:

• SUCCESS https://www.distributed-ci.io/jobs/a9e590a8-6490-4ad0-a4ca-a4b01b238bf2/jobStates

[caxu-rh]

/retest

[bcrochet]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: acornett21, bcrochet, caxu-rh

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [acornett21,bcrochet]

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