Skip to content

Harden Github actions using zizmor#925

Merged
tjlaboss merged 11 commits intodevelopfrom
pin_all
Mar 9, 2026
Merged

Harden Github actions using zizmor#925
tjlaboss merged 11 commits intodevelopfrom
pin_all

Conversation

@MicahGale
Copy link
Collaborator

@MicahGale MicahGale commented Mar 9, 2026

Pull Request Checklist for MontePy

Description

This was inspired by a recent incident of an OpenClaw bot exploiting GHA. From this I was introduced to zizmor, which performs a security audit on your github actions configuration.

Things changed:

  1. Pinned all actions to a commit sha, irrespective of source, to mitigate supplier-side attacks.
  2. Explicitly set permissions for all jobs
  3. Set a cooldown for dependabot to avoid using brand new release
  4. set persist-credentials: false for all checkout runs. This avoid malicious code accessing the runner's secrets and being able to exploit them outside this one job.
  5. Removed a third-party GHA that's always sketched me out in favor for using the gh cli to create a release.

The current audit issues are:

🌈 zizmor v1.23.1
 INFO audit: zizmor: 🌈 completed ./.github/dependabot.yml
 INFO audit: zizmor: 🌈 completed ./.github/workflows/deploy-alpha.yml
 INFO audit: zizmor: 🌈 completed ./.github/workflows/deploy.yml
 INFO audit: zizmor: 🌈 completed ./.github/workflows/main.yml
 INFO audit: zizmor: 🌈 completed ./.github/workflows/rtd_link.yml
info[template-injection]: code injection via template expansion
  --> ./.github/workflows/deploy-alpha.yml:65:17
   |
63 |       - run: >-
   |         --- this run block
64 |           gh release upload
65 |           'v${{ steps.get_version.outputs.version }}' dist/**
   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ may expand into attacker-controllable code
   |
   = note: audit confidence → Low
   = note: this finding has an auto-fix

info[template-injection]: code injection via template expansion
  --> ./.github/workflows/deploy.yml:66:17
   |
64 |       - run: >-
   |         --- this run block
65 |           gh release upload
66 |           'v${{ steps.get_version.outputs.version }}' dist/**
   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ may expand into attacker-controllable code
   |
   = note: audit confidence → Low
   = note: this finding has an auto-fix

error[dangerous-triggers]: use of fundamentally insecure workflow trigger
 --> ./.github/workflows/rtd_link.yml:2:1
  |
2 | / on:
3 | |   pull_request_target:
4 | |     types: [opened, reopened, edited]
  | |_____________________________________^ pull_request_target is almost always used insecurely
  |
  = note: audit confidence → Medium

So for use of steps.get_version.outputs.version. This is inside of the release process which only can run on protected branches, so we should catch malicious code prior to this exploit being used. For security I added a new environment that requires manual workflow approval prior to running this job (via environment). This should provide a last line of defense.

On the pull_request_target issue. This issue seems to be that the job is ran using the PR's code, but in the context of the parent repo. So this could give an unknown user instant access to secrets. However this is the job in question:

name: add RTD links
on:
  pull_request_target:
    types: [opened, reopened, edited]
jobs:
  add_rtd_link:
    permissions:
      pull-requests: write
    runs-on: ubuntu-latest
    steps:
      - uses: readthedocs/actions/preview@31a30360a2d7530806bff6855aa209167f06a89c
        with:
          project-slug: "montepy"

I don't this is an issue for a few reasons:

  1. the permissions are limited
  2. No secrets are given to this job
  3. No user code is checked out.
  4. Rather RTD code is ran to update the pull request message.

Therefore I think this usage doesn't create a significant risk currently.

Note: this may break the deploy CI. I figure if it is broken it will stop the deployment, and we can deal with that issue when it comes up.


General Checklist

  • I have performed a self-review of my own code.
  • The code follows the standards outlined in the development documentation.
  • I have formatted my code with black version 25 or 26.
  • I have added tests that prove my fix is effective or that my feature works (if applicable).

LLM Disclosure

  1. Are you?

    • A human user
    • A large language model (LLM), including ones acting on behalf of a human
  2. Were any large language models (LLM or "AI") used in to generate any of this code?

  • Yes
    • Model(s) used:
  • No

Documentation Checklist

  • I have documented all added classes and methods.
  • For infrastructure updates, I have updated the developer's guide.
  • For significant new features, I have added a section to the getting started guide.

First-Time Contributor Checklist

  • If this is your first contribution, add yourself to pyproject.toml if you wish to do so.

Additional Notes for Reviewers

Ensure that:

  • This PR fully addresses and resolves the referenced issue(s).
  • The submitted code is consistent with the merge checklist outlined here.
  • The PR covers all relevant aspects according to the development guidelines.
  • 100% coverage of the patch is achieved, or justification for a variance is given.

📚 Documentation preview 📚: https://montepy--925.org.readthedocs.build/en/925/

@MicahGale MicahGale self-assigned this Mar 9, 2026
@MicahGale MicahGale requested a review from tjlaboss March 9, 2026 15:24
@MicahGale MicahGale added critical An issue that seriously limits user adoption or hampers current use. github_actions Pull requests that update GitHub Actions code labels Mar 9, 2026
@MicahGale MicahGale marked this pull request as ready for review March 9, 2026 15:35
@MicahGale
Copy link
Collaborator Author

I have also update permissions to require CI workflow approval for all external contributors.

@tjlaboss tjlaboss merged commit 1770771 into develop Mar 9, 2026
18 checks passed
@tjlaboss tjlaboss deleted the pin_all branch March 9, 2026 17:34
@MicahGale
Copy link
Collaborator Author

Thanks @tjlaboss

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

critical An issue that seriously limits user adoption or hampers current use. github_actions Pull requests that update GitHub Actions code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants