Skip to content

feat(#10559): add zizmor static analysis for GitHub Actions CI security - #10837

Merged
mrjones-plip merged 19 commits into
medic:masterfrom
vikrantwiz02:feat/10559-zizmor-ci-security
Apr 24, 2026
Merged

mrjones-plip merged 19 commits into
medic:masterfrom
vikrantwiz02:feat/10559-zizmor-ci-security

Conversation

@vikrantwiz02

@vikrantwiz02 vikrantwiz02 commented Apr 10, 2026 •

Copy link
Copy Markdown
Contributor

Description

Integrates zizmor — a GitHub Actions static analysis tool — into CHT Core CI, and remediates all findings it surfaces across our 9 existing workflows.

Closes #10559

What zizmor found

Running zizmor --offline .github/workflows/ against our repo surfaced:

  • 45+ unpinned action references (@v4 tags instead of commit SHAs) — supply-chain risk if a third-party action's tag is silently moved to a malicious commit
  • 7 of 9 workflows with no explicit permissions: block, giving GITHUB_TOKEN broad write access by default
  • 1 script injection vulnerability in release-notes.yml — the free-text workflow_dispatch input milestone was interpolated directly in a run: shell command: node index.js ${{ github.event.inputs.milestone }}
  • 1 over-privileged workflow — stale-prs.yml had actions: write which is not needed for marking PRs stale
  • @v6 typos in release-notes.yml (v6 does not exist for actions/checkout or actions/setup-node)

What this PR does

New files:

  • .github/workflows/zizmor.yml — zizmor runs on every PR (offline to avoid rate limits), every push to master, and weekly on Sundays. Results upload to the GitHub Security tab as SARIF (Code Scanning).
  • .github/zizmor.yml — suppression config. One finding is accepted rather than fixed(see suppression rationale below).

Security fixes across all existing workflows:

  • release-notes.yml — fixed script injection: milestone input now goes through a quoted env var ("$MILESTONE") instead of raw template interpolation in the shell command
  • stale-prs.yml — removed actions: write (only pull-requests: write needed)
  • All 9 workflows — added explicit permissions: blocks scoped to minimum required
  • All 9 workflows — pinned every external action reference to a full 40-char commit SHA
    with the version tag preserved as a comment (e.g. @abc123... # v4)

Dependabot:

  • Added github-actions ecosystem to dependabot.yml — will open weekly PRs when
    pinned action versions are superseded, keeping SHAs current without manual effort

Suppression rationale

release-notes.yml — template-injection on skip_commit_checks is suppressed in .github/zizmor.yml because:

  • Input type is choice, constrained to ['', '--skip-commit-validation'] — no shell metacharacters possible in either value
  • workflow_dispatch is only triggerable by org members with Actions write permission — not accessible to external actors
  • The milestone input (free-text string) was fixed, not suppressed

Code review checklist

  • UI/UX backwards compatible — no UI changes
  • Readable: Concise, well named, follows the style guide
  • Documented: Configuration guidelines added to cht-docs (see parallel PR: docs(#10559): add guidelines for zizmor static analysis cht-docs#2185)
  • Tested: Validated locally (see test plan below)
  • Internationalised — no user facing text
  • Backwards compatible: Existing workflows unchanged in behaviour; only permissions scoped and actions pinned
  • AI disclosure: Please disclose use of AI per the guidelines.

Test plan

Verified locally (pre-merge)

  • Ran zizmor --offline .github/workflows/ locally — all findings remediated or documented in .github/zizmor.yml
  • Validated YAML syntax for all 12 modified/new files — no parser errors
  • Confirmed all 19 unique action SHAs are exactly 40 hex characters — none truncated
  • Confirmed no unpinned external action references remain across any workflow file

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

@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.

Comment thread .github/workflows/release-helm-charts.yml Fixed
Comment thread .github/dependabot.yml Fixed
Comment thread .github/workflows/build.yml Fixed
Comment thread .github/workflows/build.yml Fixed
Comment thread .github/workflows/build.yml Fixed
Comment thread .github/workflows/build.yml Fixed
Comment thread .github/workflows/build.yml Fixed
Comment thread .github/workflows/build.yml Fixed
Comment thread .github/workflows/build.yml Fixed
Comment thread .github/workflows/release-helm-charts.yml Fixed
Comment thread .github/workflows/release-helm-charts.yml Fixed

@mrjones-plip mrjones-plip left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This PR is looking pretty close to being ready! Some minor suggestions on cooldown for new section and as well, please respond to all the security call outs on the PR.

Finally, please open a parallel PR in docs repo to document how this works and how the hashes get updated via the cron job. Maybe in the static analysis section? don't feel strongly!

Comment thread .github/dependabot.yml
Comment thread .github/workflows/release-helm-charts.yml Fixed
@vikrantwiz02

Copy link
Copy Markdown
Contributor Author

Hi @mrjones-plip, done! Opened PR medic/cht-docs#2185 with the zizmor documentation in the static analysis section. It covers:

  • What zizmor catches and why
  • The CI workflow (offline PR mode vs online push/schedule with SARIF)
  • How action SHAs stay current via Dependabot (including the cooldown config)
  • How to run locally
  • How to handle findings — both inline # zizmor: ignore[rule-name] and config-file suppression

Ready for review again.

@mrjones-plip mrjones-plip left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey @vikrantwiz02 - sorry for the delay in getting back to you with review feedback.

I think this is good to go, but there's a small conflict on two of the files. Can you resolve the conflict we should be able me merge the change this week.

Thanks!

@vikrantwiz02

Copy link
Copy Markdown
Contributor Author

Hii @mrjones-plip, thanks for the review. I resolved the conflict.

@mrjones-plip mrjones-plip left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sorry- we were so close to merging!! Josh beat is in with some changes that I think we should be more gentle with on how we merge them in our branch.

see below

Comment thread .github/workflows/build.yml Outdated
Comment thread .github/workflows/build.yml Fixed
@vikrantwiz02

Copy link
Copy Markdown
Contributor Author

Hi @mrjones-plip

Fixed in 8483628, removed build-generated-docs entirely, updated publish's needs array, and rewrote publish-generated-docs to match @jkuester's new structure (checkout → gen-docs → deploy directly). Only added SHA pins and persist-credentials: false on top.

Ready for review again.

@mrjones-plip mrjones-plip left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for iterating on this @vikrantwiz02

Please check build.yml again though - both the build-generated-docs section and Restore generated docs section feel too invasively refactored for this PR which is mainly supposed to harden variable use, reduce permission scope and pin to hashes.

… steps

Removes the standalone publish-generated-docs job introduced during
the merge conflict resolution. Restores the original build-generated-docs
job and the inline Restore/Publish docs steps inside publish, matching
master's structure exactly. Only adds SHA pins and permissions: contents: read.
@vikrantwiz02

Copy link
Copy Markdown
Contributor Author

Hi @mrjones-plip, fixed in ef47b71. Here's exactly what changed vs master now:

  • build-generated-docs job restored as-is, only added permissions: contents: read and pinned actions/checkout, actions/setup-node, and actions/cache/save to their commit SHAs
  • publish.needs restored to [tests, config-tests, test-cht-form, build-generated-docs]
  • "Restore generated docs" and "Publish docs to GH pages" steps restored inline inside publish, only SHA-pinned actions/cache/restore and peaceiris/actions-gh-pages
  • The standalone publish-generated-docs job from the previous merge is gone

No structural changes, the only delta from master is SHA pins and permissions: contents: read on build-generated-docs.

@mrjones-plip mrjones-plip left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for working through my feedback and making CHT CI more secure \o/

@mrjones-plip
mrjones-plip merged commit 58547e4 into medic:master Apr 24, 2026
52 of 53 checks passed
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.

Implement static analysis of github CI to check for security vulnerabilities

3 participants