feat(ci): Zizmor CI#1596
Conversation
There was a problem hiding this comment.
Pull request overview
Adds Zizmor static analysis to the CI pipeline and applies a set of GitHub Actions hardening changes (least-privilege permissions, non-persisted checkout credentials, and reduced expression interpolation in shell) to address Zizmor findings.
Changes:
- Add a
make zizmortarget (with optional local install) and a dedicated.github/workflows/zizmor.ymlworkflow to run Zizmor on workflow changes. - Harden multiple workflows by setting minimal
permissionsandactions/checkoutwithpersist-credentials: false. - Refactor some workflow
run:scripts to use environment variables (andjq) instead of direct${{ ... }}interpolation where applicable.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Makefile | Adds ZIZMOR_VERSION and a zizmor target to run the linter locally. |
| .github/workflows/zizmor.yml | New workflow to run Zizmor on .github/** workflow changes with minimal permissions. |
| .github/workflows/update-dependencies.yaml | Adds least-privilege permissions, disables persisted checkout credentials, and shifts to GitHub App token usage for Updatecli. |
| .github/workflows/security-audit-reactive.yml | Adds least-privilege permissions and disables persisted checkout credentials. |
| .github/workflows/security-audit-cron.yml | Adds least-privilege permissions and disables persisted checkout credentials. |
| .github/workflows/release.yml | Hardens permissions/checkout and refactors some scripting (env vars + jq) in release/sync steps. |
| .github/workflows/open-release-pr.yml | Adds minimal permissions, disables persisted checkout credentials, and keeps GitHub App token usage. |
| .github/workflows/fossa.yml | Switches to default permissions: {} + job contents: read, disables persisted checkout credentials. |
| .github/workflows/e2e-tests.yml | Adds job permissions, disables persisted checkout credentials, pins an action ref, and moves some expressions into env vars. |
| .github/workflows/ci.yml | Switches default permissions away from read-all, disables persisted checkout credentials across jobs, and moves expressions into env vars for the change-detection script. |
| .github/workflows/build-kwctl.yml | Adds job permissions and disables persisted checkout credentials in relevant jobs. |
| .github/workflows/build-containers.yml | Disables persisted checkout credentials and refactors tag propagation using env vars. |
| .github/workflows/attestation.yml | Refactors to use env vars for inputs/context (to reduce expression interpolation) in attestation retrieval/signing logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| DIGEST=$(cat $COMPONENT-$ARCH.txt) | ||
| echo "DIGEST=${DIGEST}" >> "$GITHUB_ENV" | ||
| - name: Find attestation digest | ||
| run: | | ||
| set -e | ||
| DIGEST=$(crane manifest ghcr.io/${{ github.repository_owner }}/${{ inputs.component }}@${{ env.DIGEST }} \ | ||
| DIGEST=$(crane manifest ghcr.io/$REPOSITORY_OWNER/$COMPONENT@$DIGEST \ | ||
| | jq -r '.manifests[] | ||
| | select(.annotations["vnd.docker.reference.type"] == "attestation-manifest") | ||
| | .digest') | ||
| if [[ -z "${DIGEST}" ]]; then |
There was a problem hiding this comment.
This workflow reuses the shell variable name DIGEST for multiple different values (image digest, attestation manifest digest, provenance digest, SBOM digest). It’s easy to accidentally reference the wrong one in future edits. Consider using distinct variable names per step (e.g., IMAGE_DIGEST, ATTESTATION_DIGEST, etc.) and/or quoting variable expansions when building image references to improve readability and reduce foot-guns.
There was a problem hiding this comment.
I think this is a good thing to have. But I do not see the need to do that in this PR
There was a problem hiding this comment.
This needs a commit with a couple of renames. I remember not understanding this well when we were reviewing the digest attestation change for v1.33.1.
| GIT_REF_NAME: ${{ github.ref_name }} | ||
| run: | | ||
| echo TAG_NAME=$(echo ${{ github.ref_name }}) >> $GITHUB_ENV | ||
| echo TAG_NAME=$(echo "$GIT_REF_NAME") >> $GITHUB_ENV |
There was a problem hiding this comment.
The Retrieve tag name step uses echo TAG_NAME=$(echo "$GIT_REF_NAME") >> $GITHUB_ENV. The nested echo is redundant, and unquoted writes to $GITHUB_ENV can behave unexpectedly if the value ever contains characters that need escaping. Prefer writing the env var with a single, properly quoted/printf’d line.
| echo TAG_NAME=$(echo "$GIT_REF_NAME") >> $GITHUB_ENV | |
| printf 'TAG_NAME=%s\n' "$GIT_REF_NAME" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
Ok, it's a good to have. But I think we can keep this for a future
61515d6 to
4fc023f
Compare
viccuad
left a comment
There was a problem hiding this comment.
LGTM, I really like the fixes.
| persist-credentials: false | ||
| - uses: zizmorcore/zizmor-action@71321a20a9ded102f6e9ce5718a2fcec2c4f70d8 # v0.5.2 | ||
| with: | ||
| advanced-security: false |
There was a problem hiding this comment.
If we prefer advanced-security instead of in-pr annotations, here's an example on how it looks: https://github.com/kubewarden/sbomscanner/security/code-scanning
| @@ -20,7 +20,8 @@ on: | |||
| default: false | |||
|
|
|||
| # Declare default permissions as read only. | |||
There was a problem hiding this comment.
Quibble: this comment does not match now, it should be "allow read contents such as GHCR packages" (I suppose default permissions are always read-only)
Adds a new CI to run Zizmor linter. Signed-off-by: José Guilherme Vanz <jguilhermevanz@suse.com> Assisted-by: Github Copilot
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Fix all the warnings reported by Zizmor. Signed-off-by: José Guilherme Vanz <jguilhermevanz@suse.com> Assisted-by: Github copilot
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1596 +/- ##
==========================================
- Coverage 80.42% 75.50% -4.92%
==========================================
Files 127 170 +43
Lines 16398 20889 +4491
==========================================
+ Hits 13188 15773 +2585
- Misses 3210 4904 +1694
- Partials 0 212 +212
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Adds a new CI to run Zizmor linter and fix all the warning reported by it
Fix #1534