Skip to content

Conversation

@hugovk
Copy link
Member

@hugovk hugovk commented Dec 29, 2024

Follow on from #8526.

Add zizmor to pre-commit, run the new version 0.10.0, and fix the one new thing it finds:

error[cache-poisoning]: runtime artifacts potentially vulnerable to a cache poisoning attack
   --> /Users/hugo/github/Pillow/.github/workflows/wheels.yml:3:1
    |
  3 | / on:
  4 | |   schedule:
...   |
 29 | |       - "winbuild/fribidi.cmake"
 30 | |   workflow_dispatch:
    | |____________________^ generally used when publishing artifacts generated at runtime
 31 |
...
263 |         uses: actions/setup-python@v5
264 | /       with:
265 | |         python-version: "3.x"
266 | |         cache: pip
267 | |         cache-dependency-path: "Makefile"
    | |_________________________________________^ opt-in for caching here
    |
    = note: audit confidence → Low

54 findings (53 suppressed): 0 unknown, 0 informational, 0 low, 0 medium, 1 high

More info: https://woodruffw.github.io/zizmor/audits/#cache-poisoning

In short, the idea is not to use caches in workflows that produce release artifacts.

This featured in the recent Ultralytics supply chain attack:

We don't run the wheels workflow that often, and the main time bottleneck is building and testing all the wheels, so it's not a big loss to download things from PyPI each time.

@radarhere
Copy link
Member

The change is only for the sdist job, which is miles faster than any other wheel job, so I actually wouldn't consider it a speed loss at all.

@radarhere radarhere merged commit a4f976c into python-pillow:main Dec 29, 2024
68 checks passed
@hugovk hugovk deleted the pre-commit-zizmor branch December 29, 2024 23:00
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.

2 participants