fix(ci): resolve zizmor and actionlint findings in workflows - #470
Conversation
zizmor flags template-injection where `${{ ... }}` is interpolated
straight into a shell script. Bind them to env vars instead so the
values never become shell code.
Also fixes the shellcheck findings in the same blocks: unquoted
$GITHUB_OUTPUT / $GITHUB_ENV, an unquoted glob in `ls`, and SC2193 on
the `[[ "${{ github.ref }}" == refs/tags/* ]]` comparison, which now
reads $GITHUB_REF directly.
`security-events: write` sat at workflow level, so every job in the file inherited it. Move it plus `actions: read` down to the two scan jobs, workflow level keeps `contents: read`.
SC2086, no behavior change. Guards against word splitting if a version or image tag ever contains whitespace.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe pull request updates six GitHub Actions workflows. The changes move workflow expressions into environment variables, quote shell expansions and output paths, and scope OSV Scanner permissions to its jobs. ChangesWorkflow hardening
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This workflow-only change is merge-ready after normal checks and review; no actionable merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (6 skipped: 6 unsupported.) Full details: Description checkExplanation The description clearly explains what changed, why the changes were needed, and how the workflows were tested. The checklist is complete. The issue reference and reviewer notes are omitted, but neither omission prevents the description from being mostly complete. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2 participants
What
Fixes everything
zizmorandactionlintflag in.github/workflows/:docker.yaml,docs-release.yaml,helm-publish.yaml-> expressions moved intoenv:security-events: writeinosv-scanner.ymlmoved from workflow level down to the two scan jobsdocker.yaml,helm-publish.yaml,release.yaml,test-e2e.yamlWhy
The interesting one is
docs-release.yaml, it interpolated${{ github.event.release.tag_name }}right into amike deployline, and that job runs withcontents: writeandpersist-credentials: true. A tag name is attacker-shaped input, so it stays a value inenv:now, not shell code.helm-publish.yamlhad a real bug next to the lint noise:[[ "${{ github.ref }}" == refs/tags/* ]](SC2193). The expression is substituted before bash sees it, so the comparison was not doing what it looks like. Reads$GITHUB_REFnow.The osv-scanner change is just scoping, both jobs still get the same permissions, they are only granted per job instead of file-wide.
Two zizmor findings are intentionally left:
cache-poisoningondocker/setup-buildx-action(low confidence) -> gha cache is wanted here, dropping it costs real build timesuperfluous-actionsonsoftprops/action-gh-release-> swapping a pinned action for a hand-rolledgh releasescript in the signing job is a bigger change than a lint PR should carry. Happy to do it separately if we want it 🤷Testing
Both linters clean except the two above:
Before:
61 findings (51 suppressed, 7 unsafe fixes): 5 informational, 0 low, 1 medium, 4 highplus 11 actionlint/shellcheck findings.No behavior change intended, every edit is value-identical. Publish paths (docker sign, helm push, docs deploy, release) only run on tag/release, so they get their real test on the next release.
Checklist
Summary by CodeRabbit