Add zizmor security linter to pre-commit

[tkoyama010]

Overview

Recently, there was an attack on the Ultralytics package. For more information, please see Supply-chain attack analysis: Ultralytics. In this article, it is suggested that checking actions with zizmor. Let's consider using zizmor checks in pre-commit.

Details

• None

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.77%. Comparing base (fbc9bdf) to head (8eb384c).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7006   +/-   ##
=======================================
  Coverage   94.77%   94.77%           
=======================================
  Files         145      145           
  Lines       29047    29047           
  Branches     3931     3931           
=======================================
  Hits        27529    27529           
  Misses        714      714           
  Partials      804      804           

[pyvista-bot]

🚀 Deployed on https://679b226185b6442cb00fdfdb--pyvista-dev.netlify.app

[tkoyama010]

We will need to manually add labels to pull requests from external repositories. However, this should be acceptable for safety reasons.

[user27182]

How does this tool work for enforcing policies? E.g. does it error if there are critical flaws, and only warn if mid-risk flaws are found?

e.g. if this change is reverted, will pre-commit fail?

[tkoyama010]

If this change is reverted, the check will fail. Currently, errors occur at all stages of risk assessment. If needed, the configuration can be reviewed for potential adjustments.

[user27182]

+1 for security, that ultralytics case sounds like a nightmare which we want to avoid.

[user27182]

How does this tool work for enforcing policies? E.g. does it error if there are critical flaws, and only warn if mid-risk flaws are found?

e.g. if this change is reverted, will pre-commit fail?

[user27182]

I guess there are new errors after updating to v1.0.0 but in principal this LGTM otherwise

[banesullivan]

Thanks for adding zizmor!

[user27182]

For the new cache-poisoning errors, I think ignoring the errors for any testing workflows is mostly ok. The main remediation seems to be to remove any caching from workflows that publish build artifacts, which I would think only applies to actions that check if startsWith(github.ref, 'refs/tags/v'). So maybe we just need to double check the caching logic to make sure that nothing is cached for releases, and ignore everything else.