Skip to content

[MNT] harden the PyPI release workflow against tag-name injection - #10894

Open
WAHIB-EL-KHADIRI wants to merge 1 commit into
sktime:mainfrom
WAHIB-EL-KHADIRI:mnt/harden-wheels-release-workflow
Open

[MNT] harden the PyPI release workflow against tag-name injection#10894
WAHIB-EL-KHADIRI wants to merge 1 commit into
sktime:mainfrom
WAHIB-EL-KHADIRI:mnt/harden-wheels-release-workflow

Conversation

@WAHIB-EL-KHADIRI

Copy link
Copy Markdown
Contributor

What

.github/workflows/wheels.yml expands the release tag directly into the shell body of the tag check:

run: |
  TAG="${{ github.event.release.tag_name }}"

A ${{ ... }} expansion is substituted as text before bash parses the line, so a tag containing shell metacharacters is executed rather than compared. Passing it through env keeps it data.

This also adds the top-level permissions: contents: read block the file had never declared. It matters here because upload_wheels carries id-token: write for Trusted Publishing to PyPI — the most valuable token in the repository — and until now every job in the file ran with the default token scope.

Severity, stated plainly

Publishing a release requires write access, so this is defense in depth rather than an externally reachable vulnerability. It is worth closing because it removes a step from "can cut a release" to "can run arbitrary code in the workflow that publishes to PyPI".

Note

This is the same class and the same fix as pytorch-forecasting#2385; I found it while reading wheels.yml for a separate reason and it looked worth reporting here too. Kept the diff to the change itself with no explanatory comments in the file, per the review feedback on that PR.

One thing I deliberately left out of scope: pypa/gh-action-pypi-publish@release/v1 is a mutable ref rather than a pinned commit SHA. Happy to open that as a separate PR if you want it.

Verification

The workflow parses cleanly, no ${{ }} expansion remains in the run: body, and upload_wheels keeps its id-token: write. No logic changed.

🤖 Generated with Claude Code

The release tag was expanded directly into the shell body of the tag
check. Passing it through env keeps it data. Also declares the
least-privilege permissions block the file had never had.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant