fix(ci): add zizmor scanner and fix workflow security findings#10618
fix(ci): add zizmor scanner and fix workflow security findings#10618jasonsaayman merged 3 commits intoaxios:v1.xfrom
Conversation
jasonsaayman
left a comment
There was a problem hiding this comment.
thanks looks good to me and thanks for adding zizmor
|
@shaanmajid could you please rebase |
|
@jasonsaayman Done. Also removed the |
|
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. |
|
Hello, after reading #10633, I found out about zizmor and this PR. |
|
Based off #10636 (comment), it looks like you've already been tinkering with zizmor, so maybe you've answered your own questions by now :-). But in case you haven't: zizmor isn't a runtime security gate; it's a static analysis linter for GitHub Actions workflow (and related) files, similar to what ESLint is for JavaScript. It scans your workflow YAML for insecure patterns during development (e.g.,unpinned action references, injection-prone expressions, etc.). It runs in CI as a check on PRs so reviwers can catch workflow security issues before they're merged. (zizmor-action, which is used here, also uploads results as SARIF to GitHub's Security tab (Code Scanning alerts), so findings are tracked alongside other security alerts for the repo.) It doesn't stop a compromised dev from triggering a workflow or pushing a tag; that's not it's job. Gating workflow execution is the job of e.g., branch protection rules and GitHub Environments (required reviewers, branch restrictions, etc.). (One thing to note if anyone does try to remove the zizmor workflow, it will have to be done publicly via git, so at the very least there's an audit trail.) And as discussed in #10636, none of these CI-level controls would have prevented this specific attack anyway, since the attacker published directly to the npm registry from the compromised machine without going through CI at all. |
|
@shaanmajid Thank you for your reply. |
Summary
Adds zizmor as a GitHub Actions security scanner and fixes the issues it flagged. All fixes are low-risk, zero-cost improvements to CI hygiene.
Template injection (
deprecate.yml,release-branch.yml)Routes
${{ github.event.inputs.* }}throughenv:blocks instead of interpolating directly inrun:shells. Both workflows are maintainer-only (workflow_dispatch), so the practical risk was minimal, but the fix is trivial and eliminates the code smell entirely. See zizmor's template-injection docs.Credential persistence (all workflows)
Sets
persist-credentials: falseon everyactions/checkoutstep. Risk here was also low, but there's no reason to leave credentials around when no workflow needs them for post-checkout git operations.update-sponsor-block.ymlusespeter-evans/create-pull-request@v8, which manages its own credentials and does not depend on checkout's. See zizmor's artipacked docs.Continuous scanning (
zizmor.yml)New workflow runs zizmor on pushes to
v1.xand on PRs, uploading SARIF results to GitHub code scanning.Exceptions
TwoOne zizmor rules areis disabled in.github/zizmor.ymlsince fixingthemit here would add noise:Removed — addressed by ci: pin versions of actions and review to be certain these are correct #10627.unpinned-uses: covered separately by chore(ci): pin GitHub Actions to commit SHAs #10615; exception can be removed once that mergesexcessive-permissions: narrowing permissions to job level requires auditing each job individually; planned as a follow-upSummary by cubic
Adds
zizmorGitHub Actions security scanning and hardens CI by removing template-injection vectors and disabling checkout credential persistence. No changes to build or release behavior.Description
Summary of changes
.github/workflows/zizmor.ymlto runzizmoron pushes tov1.xand on all PRs, uploading SARIF with job-levelsecurity-events: writeand workflowpermissions: {}..github/zizmor.ymlwithexcessive-permissionstemporarily disabled.${{ github.event.inputs.* }}throughenvinrelease-branch.ymlto prevent shell template injection.persist-credentials: falseon allactions/checkoutsteps across workflows.publish.yml, replacedcache: npmwithpackage-manager-cache: falseto avoid unused cache config.Reasoning
run:shells.Additional context
update-sponsor-block.ymlusespeter-evans/create-pull-request, which manages its own credentials.excessive-permissionsrule after a permissions audit.Docs
zizmorfindings appear under Security > Code scanning alerts.v1.x.Testing
workflow_dispatchinputs inrelease-branch.ymlnow flow throughenv.Written for commit 18d24a2. Summary will update on new commits.