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.

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

Proposal: warn the user on user/password pair use #187

Closed
woodruffw opened this issue Oct 24, 2023 · 6 comments · Fixed by #190
Closed

Proposal: warn the user on user/password pair use #187

woodruffw opened this issue Oct 24, 2023 · 6 comments · Fixed by #190

Comments

@woodruffw
Copy link
Member

Filing this as a proposal, since it'll probably need a bit of discussion/deprecation planning.

Rationale:

  • For the vast majority of users publishing to PyPI via GitHub Actions, trusted publishing is the appropriate authentication mechanism
  • For users who can't use trusted publishing for whatever reason, manually configured API tokens are a suitable (and encouraged) alternative

Proposal: when the target is PyPI or TestPyPI, fail (with an explanatory error message) any run of the workflow that:

  1. Isn't using trusted publishing, and
  2. Is using a manually-configured credential that doesn't look like an API token (i.e. is not a username of __token__ and a password of pypi-...).

Other considerations:

  • This may be slightly more involved than just checking the inputs, since some users may choose to use TWINE_USERNAME etc. directly (if the action exposes these?)
  • There should probably be a lengthy deprecation period for this change: while there's a functional migration path that's immediately available, there are undoubtedly a large number of workflows currently using user/pass combinations
  • We should take care to only perform these checks if the index target is PyPI or TestPyPI (since other indices may not support API tokens or trusted publishing)

CC @di @sethmlarson for thoughts as well (this came up during discussion between the three of us)

@woodruffw
Copy link
Member Author

I've been reminded that PyPI is enforcing 2FA beginning in 2024, so this issue is somewhat of a moot point -- password authentication for package uploads will stop working anyways 🙂

So I'll amend the focus here: rather than prematurely rejecting, we should probably start voluminously warning users of this workflow that they'll begin to experience breakage soon and should migrate either to an API token or trusted publishing.

@woodruffw woodruffw changed the title Proposal: reject user/password combinations that aren't API tokens Proposal: warn the user on user/password pair use Oct 25, 2023
@sethmlarson
Copy link

Agreed to start warning, since we know that Trusted Publishers is the solution for almost everyone let's point to that primarily. Off-handed I wonder how hard it is to detect whether we're being run in a reusable workflow (and therefore can recommend API keys).

@woodruffw
Copy link
Member Author

Off-handed I wonder how hard it is to detect whether we're being run in a reusable workflow (and therefore can recommend API keys).

This is definitely doable by decoding the JWT itself, but that's pretty hacky. I'll see if there's some other signal we can use.

@woodruffw
Copy link
Member Author

Looks like comparing GITHUB_REPOSITORY and GITHUB_ACTION_REPOSITORY would mostly do the trick here -- that would miss reusable workflows that appear in the same logical repo, but I think that's the one case that does currently work?

@woodruffw
Copy link
Member Author

Opened #190 for this.

@webknjaz
Copy link
Member

but I think that's the one case that does currently work?

In some cases, yes. My understanding is that it requires secrets: inherit on both sides (reusable and calling workflows), combined with being in the same repo. I still haven't gotten to properly test that…

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 a pull request may close this issue.

3 participants