Skip to content
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.

Sign up for GitHub

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

[WIP] Add optional pypi release job inside the reusable tox workflow #243

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

[WIP] Add optional pypi release job inside the reusable tox workflow #243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 0 additions & 40 deletions .github/workflows/release.yml
View file
Open in desktop

This file was deleted.

48 changes: 47 additions & 1 deletion .github/workflows/tox.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,16 @@ on:
description: Command to run after test commands.
required: false
type: string
publish_pypi:
default: false
description: Whether to publish to PyPI
required: false
type: boolean
# keep permissions at top level because this is a composite workflow
permissions:
checks: read
contents: read
id-token: write
id-token: write # release
Copy link
Member

Choose a reason for hiding this comment

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

Setting the OIDC privilege is highly discouraged because giving such elevated privileges to random scripts like transitive build dependencies, testing automation and other things can lead to privilege elevation and impersonation attacks and who knows what else. For this reason, I emphasize this in all the docs for pypi-publish that are in my reach. We even managed to update GitHub's own PyPI+OIDC doc to show that it's only acceptable in the publishing job that only downloads the dists and calls pypi-publish but nothing else.
So whenever I see id-token: write anywhere outside of the narrow scope of a single job doing this single thing, I know immediately that it's a security vulnerability.

packages: write # some tox environments might produce containers
pull-requests: write # allow codenotify to comment on pull-request
env:
Expand Down Expand Up @@ -247,3 +252,44 @@ jobs:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
# https://github.com/sourcegraph/codenotify/issues/19
continue-on-error: true

pypi:
name: release ${{ github.event.ref }}
# if: github.ref_type == 'tag' || inputs.publish_pypi == 'true'
needs: check
environment: release # keep it here to allow users to prompt for release
Copy link
Member

Choose a reason for hiding this comment

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

I finally updated PyPI and its docs to suggest using pypi and testpypi recently. So that people would stop thinking of deployment targets (which what the environments are) as processes (which they are not). So I recommend those name in place of cryptic generic terms.

runs-on: ubuntu-24.04
steps:
- name: Switch to using Python 3.12 by default
uses: actions/setup-python@v5
with:
python-version: "3.12"

- name: Install tox
run: python3 -m pip install --user "tox>=4.0.0"

- name: Check out src from Git
uses: actions/checkout@v4
with:
fetch-depth: 0 # needed by setuptools-scm

- name: Build dists
run: python3 -m tox -e pkg
Copy link
Member

Choose a reason for hiding this comment

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

Following the previous comment, building within the publishing job with OIDC is a popular example of letting the possibility of being impersonated over OIDC into your life. Don't do this, it's not supported.


- name: Fail if secrets are not available
env:
PYPI_API_TOKEN: ${{ secrets.PYPI_API_TOKEN }}
run: |
if [ -z "${PYPI_API_TOKEN}" ]; then
echo "PYPI_API_TOKEN is not set, please add it to your repository environment named 'release'."
Copy link
Member

Choose a reason for hiding this comment

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

I think the current environment name is available in the context vars somewhere, it's usually better than hardcoding.

exit 1
fi

- name: Publish to pypi.org
uses: pypa/gh-action-pypi-publish@release/v1
if: inputs.publish_pypi
with:
# trusted publishing is not possible with shared workflows due to
# https://github.com/pypi/warehouse/issues/11096 so we need to use
# secrets instead.
password: ${{ secrets.PYPI_API_TOKEN }}
Loading