8000 Add zizmor as a pre-commit hook by AlexWaygood · Pull Request #626 · python/typing_extensions · GitHub
[go: up one dir, main page]

Skip to content

Add zizmor as a pre-commit hook #626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 7, 2025
Merged

Add zizmor as a pre-commit hook #626

merged 3 commits into from
Jul 7, 2025

Conversation

AlexWaygood
Copy link
Member
@AlexWaygood AlexWaygood commented Jul 7, 2025

Zizmor is a linter that checks GitHub workflow files for security vulnerabilities. We run it in CI for Ruff and uv over at Astral. Since typing_extensions is a top-10 PyPI package and take security pretty seriously in general, I think it makes sense for this repo to add it as a pre-commit hook too.

On main, zizmor has the following complaints regarding our GitHub workflows:

Zizmor complaints
warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> .github/workflows/ci.yml:59:9
   |
59 |       - uses: actions/checkout@v4
   |         ------------------------- does not set persist-credentials: false
   |
   = note: audit confidence → Low
   = note: this finding has an auto-fix

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> .github/workflows/publish.yml:26:9
   |
26 |       - uses: actions/checkout@v4
   |         ------------------------- does not set persist-credentials: false
   |
   = note: audit confidence → Low
   = note: this finding has an auto-fix

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> .github/workflows/publish.yml:54:9
   |
54 |       - uses: actions/checkout@v4
   |         ------------------------- does not set persist-credentials: false
   |
   = note: audit confidence → Low
   = note: this finding has an auto-fix

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> .github/workflows/publish.yml:80:9
   |
80 |       - uses: actions/checkout@v4
   |         ------------------------- does not set persist-credentials: false
   |
   = note: audit confidence → Low
   = note: this finding has an auto-fix

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> .github/workflows/publish.yml:105:9
    |
105 |       - uses: actions/checkout@v4
    |         ------------------------- does not set persist-credentials: false
    |
    = note: audit confidence → Low
    = note: this finding has an auto-fix

error[template-injection]: code injection via template expansion
  --> .github/workflows/publish.yml:32:50
   |
32 |         run: python scripts/check_package.py ${{ github.ref }}
   |         ^^^ this run block                       ^^^^^^^^^^ may expand into attacker-controllable code
   |
   = note: audit confidence → High
   = note: this finding has an auto-fix

error[unpinned-uses]: unpinned action reference
   --> .github/workflows/publish.yml:149:9
    |
149 |         uses: pypa/gh-action-pypi-publish@release/v1
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
    |
    = note: audit confidence → High

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> .github/workflows/third_party.yml:62:9
   |
62 |         - name: Checkout typing_extensions
   |  _________-
63 | |         uses: actions/checkout@v4
64 | |         with:
65 | |           path: typing-extensions-latest
   | |________________________________________- does not set persist-credentials: false
   |
   = note: audit confidence → Low
   = note: this finding has an auto-fix

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> .github/workflows/third_party.yml:93:9
   |
93 |         - name: Checkout typing_extensions
   |  _________-
94 | |         uses: actions/checkout@v4
95 | |         with:
96 | |           path: typing-extensions-latest
   | |________________________________________- does not set persist-credentials: false
   |
   = note: audit confidence → Low
   = note: this finding has an auto-fix

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> .github/workflows/third_party.yml:130:9
    |
130 |         - name: Checkout typing_extensions
    |  _________-
131 | |         uses: actions/checkout@v4
132 | |         with:
133 | |           path: typing-extensions-latest
    | |________________________________________- does not set persist-credentials: false
    |
    = note: audit confidence → Low
    = note: this finding has an auto-fix

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> .github/workflows/third_party.yml:167:9
    |
167 |         - name: Checkout typing_extensions
    |  _________-
168 | |         uses: actions/checkout@v4
169 | |         with:
170 | |           path: typing-extensions-latest
    | |________________________________________- does not set persist-credentials: false
    |
    = note: audit confidence → Low
    = note: this finding has an auto-fix

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> .github/workflows/third_party.yml:204:9
    |
204 |         - name: Checkout typing_extensions
    |  _________-
205 | |         uses: actions/checkout@v4
206 | |         with:
207 | |           path: typing-extensions-latest
    | |________________________________________- does not set persist-credentials: false
    |
    = note: audit confidence → Low
    = note: this finding has an auto-fix

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> .github/workflows/third_party.yml:248:9
    |
248 |         - name: Checkout typing_extensions
    |  _________-
249 | |         uses: actions/checkout@v4
250 | |         with:
251 | |           path: typing-extensions-latest
    | |________________________________________- does not set persist-credentials: false
    |
    = note: audit confidence → Low
    = note: this finding has an auto-fix

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> .github/workflows/third_party.yml:283:9
    |
283 |         - name: Checkout typing_extensions
    |  _________-
284 | |         uses: actions/checkout@v4
285 | |         with:
286 | |           path: typing-extensions-latest
    | |________________________________________- does not set persist-credentials: false
    |
    = note: audit confidence → Low
    = note: this finding has an auto-fix

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> .github/workflows/third_party.yml:325:9
    |
325 |         - name: Checkout typing_extensions
    |  _________-
326 | |         uses: actions/checkout@v4
327 | |         with:
328 | |           path: typing-extensions-latest
    | |________________________________________- does not set persist-credentials: false
    |
    = note: audit confidence → Low
    = note: this finding has an auto-fix

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> .github/workflows/third_party.yml:361:9
    |
361 |         - name: Checkout typing_extensions
    |  _________-
362 | |         uses: actions/checkout@v4
363 | |         with:
364 | |           path: typing-extensions-latest
    | |________________________________________- does not set persist-credentials: false
    |
    = note: audit confidence → Low
    = note: this finding has an auto-fix

The three rules that have violations are:

@AlexWaygood AlexWaygood closed this Jul 7, 2025
@AlexWaygood AlexWaygood reopened this Jul 7, 2025
@srittau
Copy link
Collaborator
srittau commented Jul 7, 2025

Not a blocker, but is it possible to add pypa/gh-action-pypi-publish to a whitelist for unpinned-uses? PyPA is certainly considered trusted by us (and could do a lot more damage in other ways), and not always using the latest version of the workflow is probably more of a potential security problem.

@AlexWaygood
Copy link
Member Author
AlexWaygood commented Jul 7, 2025

Not a blocker, but is it possible to add pypa/gh-action-pypi-publish to a whitelist for unpinned-uses? PyPA is certainly considered trusted by us (and could do a lot more damage in other ways), and not always using the latest version of the workflow is probably more of a potential security problem.

We could do, but I note that even the README of the gh-action-pypi-publish GitHub action recommends that you pin to a SHA (linking to https://julienrenaux.fr/2019/12/20/github-actions-security-risk/), and in their SchemaStore entry I read this under the description field:

We strongly recommend that you include the version of the action you are using by specifying a Git ref, SHA, or Docker tag number. If you don't specify a version, it could break your workflows or cause unexpected behavior when the action owner publishes an update. Using the commit SHA of a released action version is the safest for stability and security.

@srittau
Copy link
Collaborator
srittau commented Jul 7, 2025

Meh, I don't agree with the reasoning. As far as I know, conventional wisdom is that you should always update to the latest version of a dependency – unless you performed a security audit of the specific version of the dependency or monitor updates for the (lack of) security fixes religiously. Old and compromised versions in the dependency chain are probably one of the biggest security problem, which is why stuff like dependabot and renovate was developed after all.

That said, if they recommend that themselves, so be it.

@srittau srittau merged commit 3b9b86e into main Jul 7, 2025
86 of 104 checks passed
@srittau srittau deleted the zizmor branch July 7, 2025 13:45
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
0