Address zizmor issues

[pquentin]

There are still 6 remaining findings. I don't know how to fix them while keeping the functionality.

$ uvx zizmor --gh-token $(gh auth token) .github/workflows/*.yml
2024-12-13T07:07:50.665504Z  INFO audit: zizmor: 🌈 completed /.../urllib3/.github/workflows/changelog.yml
2024-12-13T07:08:17.811994Z  INFO audit: zizmor: 🌈 completed /.../urllib3/.github/workflows/ci.yml
2024-12-13T07:08:26.465969Z  INFO audit: zizmor: 🌈 completed /.../urllib3/.github/workflows/codeql.yml
2024-12-13T07:08:27.303472Z  INFO audit: zizmor: 🌈 completed /.../urllib3/.github/workflows/downstream.yml
2024-12-13T07:08:31.059292Z  INFO audit: zizmor: 🌈 completed /.../urllib3/.github/workflows/lint.yml
2024-12-13T07:08:38.847244Z  INFO audit: zizmor: 🌈 completed /.../urllib3/.github/workflows/publish.yml
2024-12-13T07:08:42.062256Z  INFO audit: zizmor: 🌈 completed /.../urllib3/.github/workflows/scorecards.yml
info[template-injection]: code injection via template expansion
  --> /Users/quentin/pub/urllib3/.github/workflows/changelog.yml:25:9
   |
25 |         - name: "Check changelog"
   |           ----------------------- info: this step
26 |           if: "!contains(github.event.pull_request.labels.*.name, 'Skip Changelog')"
27 | /         run: |
28 | |           if ! pipx run towncrier check --compare-with origin/${{ github.base_ref }}; then
29 | |           echo "Please see https://github.com/urllib3/urllib3/blob/main/changelog/README.rst for guidance."
30 | |             false
31 | |           fi
   | |_____________- info: github.base_ref may expand into attacker-controllable code
   |
   = note: audit confidence → Low

info[template-injection]: code injection via template expansion
   --> /Users/quentin/pub/urllib3/.github/workflows/ci.yml:122:9
    |
122 |         - name: Force override system chrome
    |           ---------------------------------- info: this step
123 | /         run: |
124 | |           sudo rm -f /usr/bin/google-chrome
...   |
127 | |           sudo ln -s ${{ steps.setup-chrome.outputs.chrome-path }} /usr/bin/chrome
128 | |           google-chrome --version
    | |_________________________________- info: steps.setup-chrome.outputs.chrome-path may expand into attacker-controllable code
    |
    = note: audit confidence → Low

info[template-injection]: code injection via template expansion
   --> /Users/quentin/pub/urllib3/.github/workflows/ci.yml:122:9
    |
122 |         - name: Force override system chrome
    |           ---------------------------------- info: this step
123 | /         run: |
124 | |           sudo rm -f /usr/bin/google-chrome
...   |
127 | |           sudo ln -s ${{ steps.setup-chrome.outputs.chrome-path }} /usr/bin/chrome
128 | |           google-chrome --version
    | |_________________________________- info: steps.setup-chrome.outputs.chrome-path may expand into attacker-controllable code
    |
    = note: audit confidence → Low

error[template-injection]: code injection via template expansion
  --> /Users/quentin/pub/urllib3/.github/workflows/publish.yml:86:7
   |
86 |       - name: "Upload dists to GitHub Release"
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
87 |         env:
88 |           GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
89 | /       run: |
90 | |         gh release upload ${{ github.ref_name }} dist/* --repo ${{ github.repository }}
   | |_______________________________________________________________________________________^ github.ref_name may expand into attacker-controllable code
   |
   = note: audit confidence → High

6 findings (2 suppressed): 0 unknown, 3 informational, 0 low, 0 medium, 1 high

[sethmlarson]

LGTM, thanks Quentin! :)

[illia-v]

Nice!

[sigmavirus24]

For

if ! pipx run towncrier check --compare-with origin/${{ github.base_ref }}

Can't that be fixed as

if ! pipx run towncrier check --compare-with "origin/${{ github.base_ref }}" 

[sethmlarson]

@sigmavirus24 ${{ }} is interpolated by GitHub Actions, not a part of the shell, so quoting can be defeated.

[sigmavirus24]

I guess no matter what it could be defeated because and attacker can theoretically inject whatever is necessary to evade why escaping necessary

[pquentin]

Attackers can't change the workflow itself, which also means we won't know if my pull request works until we merge it.

Actually the fix here is to hardcode main because this is the only base ref where we care about changelogs.