Skip to content

Conversation

di
Copy link
Member

@di di commented Aug 10, 2023

Resolves pypa/gh-action-pypi-publish#173.

Sometimes we receive an OIDC token where the ref claim is an empty string and the job_workflow_ref claim takes the form {owner}/{repo}/.github/workflows/{filename}@{sha} rather than {owner}/{repo}/.github/workflows/{filename}@{ref}. It's currently unclear why/when this happens.

Since the job_workflow_ref claim can still be considered valid if it contains the SHA, we can expand our validation of it to check for the form containing the SHA.

This PR also makes the sha claim a required unverifiable claim, and removes the Sentry message introduced in #14333.

@di di requested a review from a team as a code owner August 10, 2023 21:49
if signed_claim not in expected:
raise InvalidPublisherError(
f"The claim does not match, expecting {expected!r}, got {signed_claim!r}"
f"The claim does not match, expecting one of {sorted(expected)!r}, got "
Copy link
Member

Choose a reason for hiding this comment

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

Small nitpick, noticed that "the claim" isn't super clear in the error messages we return:

Suggested change
f"The claim does not match, expecting one of {sorted(expected)!r}, got "
f"job_workflow_ref does not match, expecting one of {sorted(expected)!r}, got "

@di di merged commit 0cad89c into pypi:main Aug 10, 2023
@di di deleted the allow-sha-in-workflow_ref branch August 10, 2023 22:06
@di
Copy link
Member Author

di commented Aug 10, 2023

Huh, was not expecting to be able to accidentally push to main with this...

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.

invalid-publisher: valid token, but no corresponding publisher
2 participants