Skip to content

Fix zizmor security issues in GitHub Actions workflows#361

Open
jezdez wants to merge 5 commits intomainfrom
fix/zizmor-security-issues
Open

Fix zizmor security issues in GitHub Actions workflows#361
jezdez wants to merge 5 commits intomainfrom
fix/zizmor-security-issues

Conversation

@jezdez
Copy link
Copy Markdown
Member

@jezdez jezdez commented Jan 6, 2026

Rebased onto current main and re-audited with zizmor v1.24.1. The previous iteration touched synced workflows that are managed by conda/infrastructure; this version leaves those untouched and suppresses their findings in zizmor.yml instead.

What changed

Dependabot cooldowns (dependabot.yml)

  • Added 7-day cooldown to all package ecosystem entries (auto-fixed by zizmor --fix).

Template injection fixes (action files)

  • Replaced direct ${{ inputs.* }} expansions in run: blocks with environment variables across canary-release, combine-durations, create-fork, read-file, template-files.
  • Replaced ${{ inputs.* }} in actions/github-script steps with process.env.* in read-yaml, set-commit-status, check-cla.
  • Replaced ${{ inputs.* }} in Python shell steps with os.environ[...] in check-cla.

Credential persistence (tests.yml, check-cla/action.yml)

  • Added persist-credentials: false to all actions/checkout steps.

Explicit permissions (tests.yml)

  • Added workflow-level permissions: contents: read.
  • Added job-level permissions for template-files and analyze jobs that need write access.
  • Fixed template injection in read-file test assertions.

Synced workflow suppressions (zizmor.yml)

  • Workflows synced from conda/infrastructure (cla.yml, issues.yml, labels.yml, project.yml, stale.yml, update.yml) cannot be modified here. Their findings are suppressed with documentation explaining why.

Pre-commit hooks (.pre-commit-config.yaml)

  • Added zizmor (v1.24.1) and actionlint (v1.7.12) hooks, as suggested by @dbast.

Audit result

zizmor v1.24.1: No findings to report. Good job! (12 ignored, 61 suppressed)

Notes for reviewers

  • The 61 suppressed findings are zizmor's own internal suppressions (e.g., ${{ inputs.* }} in with: blocks passed to known-safe actions).
  • The 12 ignored findings are from zizmor.yml for synced workflows.
  • Fixing the synced workflow findings should be done upstream in conda/infrastructure.

@jezdez jezdez requested a review from a team as a code owner January 6, 2026 12:18
@github-project-automation github-project-automation Bot moved this to 🆕 New in 🔎 Review Jan 6, 2026
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Jan 6, 2026
@conda-bot
Copy link
Copy Markdown
Contributor

conda-bot commented Jan 6, 2026

Template Success

Warning

This is what the audit looks like when the templating has no errors.

Templating Audit
  • 🔄 Fetching files from ./templates
    • file1.txt.github_cache/template-files/success/file1.txt
      • ✅️ (used) stub1.txt
      • ✅️ (used) stub2.txt
      • ❌️ (missing) stub3.txt
    • ⚠️ file2.txt.github_cache/template-files/success/file2.txt
      • ✅️ (used) stub2.txt
      • ❌️ (missing) stub3.txt
      • 📚️ (context) variable=value
      • ➕️ (optional) optional=
Stub State Count
stub1.txt ✅️ (used) 1
stub2.txt ✅️ (used) 2
stub3.txt ❌️ (missing) 2

Template Error

Warning

This is what the audit looks like when templating results in errors.

Templating Audit
  • 🔄 Fetching files from ./templates
    • file1.txt.github_cache/template-files/error/file1.txt
      • ✅️ (used) stub1.txt
      • ✅️ (used) stub2.txt
      • ❌️ (missing) stub3.txt
    • ❌ Context missing file2.txt.github_cache/template-files/error/file2.txt
      • ✅️ (used) stub2.txt
      • ❌️ (missing) stub3.txt
      • ❌️ (missing) variable=
      • ➕️ (optional) optional=
Stub State Count
stub1.txt ✅️ (used) 1
stub2.txt ✅️ (used) 2
stub3.txt ❌️ (missing) 2

Got 1 error(s)

@jezdez
Copy link
Copy Markdown
Member Author

jezdez commented Jan 6, 2026

The comment above is to be expected because of the change to the template-files action files.

@dbast
Copy link
Copy Markdown
Member

dbast commented Jan 8, 2026

@jezdez any reason not to add zizmor as pre-commit hook via?:

  - repo: https://github.com/zizmorcore/zizmor-pre-commit
    rev: v1.20.0
    hooks:
      - id: zizmor

@dbast
Copy link
Copy Markdown
Member

dbast commented Jan 8, 2026

as this is the central repo maybe adding actionlint is also of additional help to find things before they come to the consumers of the actions:

  - repo: https://github.com/rhysd/actionlint
    rev: v1.7.9
    hooks:
      - id: actionlint

@jezdez
Copy link
Copy Markdown
Member Author

jezdez commented Jan 9, 2026

No, these are good ideas, you're right! But adding the actions wouldn't have made these changes

@kenodegard
Copy link
Copy Markdown
Contributor

kenodegard commented Jan 16, 2026

Cannot make changes to the following here (as they're synced from conda/infrastructure):

  • cla.yml
  • issues.yml
  • labels.yml
  • project.yml
  • stale.yml
  • update.yml

edit: but I'm starting to wonder if some of the workflows should be synced from here instead, e.g., it makes more sense for the cla workflow to live here alongside the cla action?

@kenodegard
Copy link
Copy Markdown
Contributor

Can we combine cla-trigger with cla? Isn't it possible to define permissions per job instead of on the workflow level?

jezdez added 4 commits May 8, 2026 19:33
Adds 7-day cooldown period to all Dependabot package ecosystem
entries to prevent excessive update frequency.

Applied by: zizmor v1.24.1 --fix (dependabot-cooldown rule)
Replace direct template expansions (${{ inputs.* }}) in run blocks,
github-script steps, and python steps with environment variables.
Inputs are now passed via env vars which are properly escaped by
the shell/runtime.

Detected by: zizmor v1.24.1 (template-injection rule)
- Add persist-credentials: false to all checkout steps
- Add workflow-level permissions: contents: read
- Add job-level permissions for template-files and analyze jobs
  that need write access to issues/pull-requests
- Fix template injection in read-file test assertions by using
  environment variables instead of direct template expansion

Detected by: zizmor v1.24.1 (artipacked, excessive-permissions,
template-injection rules)
Workflows synced from conda/infrastructure (cla.yml, issues.yml,
labels.yml, project.yml, stale.yml) cannot be modified in this
repo. Suppress their findings here; fixes should be applied
upstream.
jezdez added a commit that referenced this pull request May 8, 2026
Catches GitHub Actions security and syntax issues before they
reach CI. Suggested by @dbast in #361.
@jezdez jezdez force-pushed the fix/zizmor-security-issues branch from 208d3c8 to f2ad403 Compare May 8, 2026 17:39
@jezdez
Copy link
Copy Markdown
Member Author

jezdez commented May 8, 2026

Rebased the entire branch onto current main and re-ran zizmor v1.24.1 from scratch. The previous iteration was 40 commits behind and had conflicts.

Addressing @kenodegard's feedback:

Synced workflows (cla.yml, issues.yml, labels.yml, project.yml, stale.yml, update.yml) are no longer touched. Their findings are suppressed in zizmor.yml with comments explaining that fixes should go upstream in conda/infrastructure.

The cla-trigger.yml split is dropped since cla.yml is synced. The question of whether the CLA workflow should live here alongside the CLA action (instead of being synced from infrastructure) is a separate conversation.

What's left:

  • Template injection fixes across all action files (env vars instead of direct ${{ inputs.* }})
  • persist-credentials: false on all checkout steps
  • Explicit permissions on tests.yml
  • Dependabot cooldowns
  • zizmor.yml config for synced workflow suppressions
  • zizmor + actionlint pre-commit hooks (per @dbast's suggestion)

Clean audit: No findings to report. Good job! (12 ignored, 61 suppressed)

Catches GitHub Actions security and syntax issues before they
reach CI. Suggested by @dbast in #361.
@jezdez jezdez force-pushed the fix/zizmor-security-issues branch from f2ad403 to 0825b40 Compare May 8, 2026 17:44
@jezdez jezdez requested review from dbast and kenodegard May 8, 2026 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed [bot] added once the contributor has signed the CLA

Projects

Status: 🆕 New

Development

Successfully merging this pull request may close these issues.

4 participants