Skip to content

Use environment variables instead of contexts #1765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 23, 2024

Conversation

tomkins
Copy link
Contributor

@tomkins tomkins commented Dec 22, 2024

Closes #1748

Updates the "Signing the distribution packages" section to use environment variables rather than GitHub Actions context variables. No additional setting of environment variables are needed - these are all set/available already.

I've got a repo where we're using trusted publishing, although it's a slight variation on this, but hopefully it's an example that the environment variables work just fine:

https://github.com/developersociety/django-findreplace/blob/7fcf87397590b984dacaca58719f2d8b737d4f77/.github/workflows/publish.yml#L81
https://github.com/developersociety/django-findreplace/actions/runs/12457101550


📚 Documentation preview 📚: https://python-packaging-user-guide--1765.org.readthedocs.build/en/1765/

@tomkins tomkins requested a review from webknjaz as a code owner December 22, 2024 20:45
@tomkins
Copy link
Contributor Author

tomkins commented Dec 22, 2024

I've also added a persist-credentials: false for checkout, which is another potential zizmor found issue.

This might be overkill for fixing, as in theory the directory should never be added into a package. However, if for some bizarre reason a mistake is made - it would be good to help prevent it.

After this, zizmor is happy with the file:

$ zizmor --gh-token "$(gh auth token)" ./source/guides/github-actions-ci-cd-sample/publish-to-test-pypi.yml
2024-12-22T20:51:21.772745Z  INFO audit: zizmor: 🌈 completed /Users/tomkins/Development/packaging.python.org/source/guides/github-actions-ci-cd-sample/publish-to-test-pypi.yml
No findings to report. Good job! (9 suppressed)

Copy link
Member

@ncoghlan ncoghlan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks.

I added a link to the relevant zizmor issue explanation to the original bug report.

@ncoghlan
Copy link
Member

ncoghlan commented Dec 23, 2024

PyPI still has its webscraping protections turned on (#1744), so I expect the merge request to fail (alas)

@ncoghlan ncoghlan disabled auto-merge December 23, 2024 04:04
@ncoghlan ncoghlan enabled auto-merge December 23, 2024 04:04
@ncoghlan ncoghlan disabled auto-merge December 23, 2024 04:04
@ncoghlan
Copy link
Member

Pending checks seem to be in a weird state. Closing/reopening to try to kick them.

@ncoghlan ncoghlan closed this Dec 23, 2024
@ncoghlan ncoghlan reopened this Dec 23, 2024
@ncoghlan
Copy link
Member

ncoghlan commented Dec 23, 2024

Huh, looks like it may have been a bug in the new check review widget (the workflows needed approval to run, but the review widget didn't tell me that, I had to go look at the Actions UI).

Edit: checking the feedback request post at https://github.com/orgs/community/discussions/143787, that's a known limitation of the new widget (and fixing it is already on their TODO list)

@ncoghlan ncoghlan added this pull request to the merge queue Dec 23, 2024
Merged via the queue into pypa:main with commit fe434c3 Dec 23, 2024
5 checks passed
@webknjaz
Copy link
Member

@ncoghlan I've working around that by manually going to the checks/actions tab and clicking the button in the workflow..

@tomkins tomkins deleted the zizmor-actions-fixes branch December 23, 2024 10:22
@di di mentioned this pull request Jan 2, 2025
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.

zizmor reveals "github.ref_name may expand into attacker-controllable code", should something else be used?
3 participants