zizmor is all the rage these days, probably worth adding it

[graingert]

No description provided.

[A5rocks]

I was going to do this when I read woodruffw's blog post about ultralytics, but got distracted by updating action versions... oops...

Thanks for doing this!

[A5rocks]

It might be worth adding it as a GitHub action because I think it can catch more issues if it is passed a GITHUB_TOKEN.

[jakkdl]

(source: https://woodruffw.github.io/zizmor/usage/#operating-modes if others are curious)

it's sort-of possible to pass ENV variables to pre-commit hooks (pre-commit/pre-commit#758 (comment)) though it's somewhat hacky. If we expect the hook to trigger somewhat frequently we can have it as a pre-commit hook that's disabled in CI (idk where that's documented, but https://github.com/python-trio/flake8-async/blob/f82074ad631e46daa63fa4a2b016e54dc6e762f0/.pre-commit-config.yaml#L7) and a github action - but perhaps we only need to bother with it as a github action.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00000%. Comparing base (a670d60) to head (f6cc842).

Additional details and impacted files

@@               Coverage Diff               @@
##                 main        #3154   +/-   ##
===============================================
  Coverage   100.00000%   100.00000%           
===============================================
  Files             124          124           
  Lines           18427        18427           
  Branches         1215         1215           
===============================================
  Hits            18427        18427           

[webknjaz]

Most changes in this file need to be dropped since there's less checkout invocations now.

[graingert]

not sure what to do about this failure:

trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...............................................................Passed
check toml...............................................................Passed
check for merge conflicts................................................Passed
mixed line ending........................................................Passed
check for case conflicts.................................................Passed
sort simple yaml files...................................................Passed
black....................................................................Passed
ruff.....................................................................Passed
codespell................................................................Passed
Sphinx Lint..............................................................Passed
zizmor...................................................................Failed
- hook id: zizmor
- exit code: 11

2024-12-22T09:40:49.443244Z  WARN zizmor: skipping impostor-commit: can't run without a GitHub API token
2024-12-22T09:40:49.443272Z  WARN zizmor: skipping ref-confusion: can't run without a GitHub API token
2024-12-22T09:40:49.443275Z  WARN zizmor: skipping known-vulnerable-actions: can't run without a GitHub API token
2024-12-22T09:40:49.443458Z  INFO audit: zizmor: 🌈 completed /home/graingert/projects/trio/.github/workflows/autodeps.yml
2024-12-22T09:40:49.443528Z  INFO audit: zizmor: 🌈 completed /home/graingert/projects/trio/.github/workflows/check-newsfragment.yml
2024-12-22T09:40:49.444248Z  INFO audit: zizmor: 🌈 completed /home/graingert/projects/trio/.github/workflows/ci.yml
2024-12-22T09:40:49.444383Z  INFO audit: zizmor: 🌈 completed /home/graingert/projects/trio/.github/workflows/release.yml
info[template-injection]: code injection via template expansion
  --> /home/graingert/projects/trio/.github/workflows/ci.yml:65:9
   |
65 |         - name: Verify that the artifacts with expected names got created
   |           --------------------------------------------------------------- info: this step
66 | /         run: >-
67 | |           ls -1
68 | |           dist/${{ steps.artifact-name.outputs.sdist }}
69 | |           dist/${{ steps.artifact-name.outputs.wheel }}
   | |_______________________________________________________- info: steps.artifact-name.outputs.sdist may expand into attacker-controllable code
   |
   = note: audit confidence → Low

info[template-injection]: code injection via template expansion
  --> /home/graingert/projects/trio/.github/workflows/ci.yml:65:9
   |
65 |         - name: Verify that the artifacts with expected names got created
   |           --------------------------------------------------------------- info: this step
66 | /         run: >-
67 | |           ls -1
68 | |           dist/${{ steps.artifact-name.outputs.sdist }}
69 | |           dist/${{ steps.artifact-name.outputs.wheel }}
   | |_______________________________________________________- info: steps.artifact-name.outputs.wheel may expand into attacker-controllable code
   |
   = note: audit confidence → Low

32 findings (30 suppressed): 0 unknown, 2 informational, 0 low, 0 medium, 0 high

regenerate generated files...............................................Passed

[webknjaz]

@graingert can it be ignored? The values are constructed in the same job by our code. So this templating is safe in that regard. Though, the generic approach is to use step env vars as a trampoline..

[graingert]

How do you do the step env trampoline?

[A5rocks]

How do you do the step env trampoline?

Just do env: VAR: value for the step; GitHub actions automatically escapes the value

[webknjaz]

@graingert check out this https://woodruffw.github.io/zizmor/audits/#template-injection — if you scroll a little, there's a Remediation title. Under it, there's a concrete example with the Before and After tabs. Hope this helps!