fix: fix zizmor security findings in CI workflow files#1550
Conversation
mkulke
left a comment
There was a problem hiding this comment.
what is the rationale behind re-setting the env with the same value? is this a readabiity measure? if yes then it probably makes sense to remove the job-level envs?
RUST_TARGET: ${{ env.RUST_TARGET }}
IMAGE_NAME: ${{ env.IMAGE_NAME }}
86af87a to
fe9f89a
Compare
Thanks for the review, You're right - RUST_TARGET and IMAGE_NAME are already defined at the job-level env: block and are automatically available as shell variables in all run: steps. The step-level re-declarations were redundant. Fixed - removed all the unnecessary env: re-assignments. The template-injection fix is achieved simply by using $ RUST_TARGET / $ IMAGE_NAME (shell variables) instead of $ {{ env.RUST_TARGET }} / $ {{ env.IMAGE_NAME }} (template expressions) directly in the run: blocks. |
Ran zizmor v1.25.2 locally against .github/workflows/ and fixed all 39 findings: - Add persist-credentials: false to all actions/checkout steps (artipacked - 20 findings across 17 files) - Add explicit permissions: contents: read to all jobs missing it (excessive-permissions - 10 findings) - Replace archived actions-rs/toolchain with actions-rust-lang/setup-rust-toolchain and actions-rs/cargo with inline run: step in aa_sev_kbc.yml (archived-uses - 2 findings) - Fix template-injection in publish-artifacts.yml by assigning env.RUST_TARGET and env.IMAGE_NAME to step-level env vars and referencing via $VAR in shell (template-injection - 7 findings) Signed-off-by: siddaram-sonnagi <siddaram.sonnagi@ibm.com>
fe9f89a to
09c6476
Compare
Summary
Ran
zizmor v1.25.2locally against.github/workflows/and fixed all findings.Changes
artipacked— Low severityAdded
persist-credentials: falseto allactions/checkoutsteps across 18 workflowfiles. This prevents the GitHub token from being persisted in the local git config after
checkout, reducing the risk of credential exposure via artifacts.
excessive-permissions— Medium severityAdded explicit
permissions: contents: readto all jobs that were using broad defaultpermissions. Exception:
vendor_release.ymlusespermissions: contents: writesince ituploads release assets via
gh release upload.archived-uses— Medium severityReplaced the abandoned
actions-rs/toolchainandactions-rs/cargoactions inaa_sev_kbc.ymlwithactions-rust-lang/setup-rust-toolchain(already used consistentlyacross all other workflows in this repo) and an inline
run:step respectively.The
actions-rsorganisation is archived and no longer receives security updates.template-injection— Medium severityIn
publish-artifacts.yml, replaced${{ env.RUST_TARGET }}and${{ env.IMAGE_NAME }}template expressions inside
run:blocks with their shell variable equivalents$RUST_TARGET/$IMAGE_NAME. Both variables are already defined at the job-levelenv:block and are automatically available as shell variables — no step-level redeclaration needed.
cache-poisoning— High severityIn
coco-extension-image.yml(new upstream file), addedcache: falseto theactions-rust-lang/setup-rust-toolchainstep. This workflow triggers onpull_requestand publishes runtime artifacts, making it vulnerable to cache poisoning without this flag.
Verification
zizmor .github/workflows/ # No findings to report. Good job! (88 suppressed)