gh-action-pypi-publish icon indicating copy to clipboard operation
gh-action-pypi-publish copied to clipboard

Show a warning in workflow runs suspect of building in the publish job

Open webknjaz opened this issue 10 months ago • 1 comments

Previously, I didn't know if it's possible to detect. However, I just occurred to me that this can be inferred from the environment state. I think that we should be able to inspect the presence of other actions being checked out on disk to see if there's anything beyond pypa/gh-action-pypi-publish and actions/download-artifact present in the actions cache directory.

webknjaz avatar Jan 20 '25 18:01 webknjaz

I just realized that it's as simple as detecting the presence of .git/ in CWD. Somebody downloading artifacts from another job wouldn't use actions/checkout and so there wouldn't be a Git repository in the first place. #363 is what inspired me with this idea.

webknjaz avatar May 30 '25 11:05 webknjaz

I just realized that it's as simple as detecting the presence of .git/ in CWD.

Another tell could be the presence of anything besides dist/ (or the packages-dir value).

webknjaz avatar Jul 08 '25 17:07 webknjaz

I think this is a bad idea. Publishing can contain several things like creating a GH release and adding a tag. Uploading to PyPI is only one part of the process and I wouldn't force the user to do this in several jobs.

Spacetown avatar Jul 09 '25 14:07 Spacetown

This is necessary to be secure by default. So I'm limiting footguns. It's already stated in the README that those configs are unsupported. This will just add a more prominent warning. People tend to abuse what's available in ways that compromise security and I don't want to encourage that. I might make it opt out with an explicit confirmation of the consequences, though. This project is security-sensitive, while the maintenance resources are limited, which is why I'm going to keep making such unpopular improvements.

webknjaz avatar Jul 13 '25 00:07 webknjaz

Some workflows need to be allowed, e.g. the download of the Artifacts. If the opt out would be by specifying the tasks with an explicit commit hash I'm fine.

Spacetown avatar Jul 13 '25 15:07 Spacetown

That's an action, not a workflow. We can't see the list of steps programmatically, which is why I'm looking at ways of infer possible insecure uses from the env. Having a download step, followed by an upload step is the only thing I declared supported. Anything else either has regular disadvantages or is straight up dangerous. In general, keeping everything else in separate jobs will work for anyone. So that's what I'm settling on. And the planned reusable workflow will be separate anyway.

webknjaz avatar Jul 13 '25 15:07 webknjaz

Using several jobs will work but can be error-prone. E.g. the condition needs to be the same in every job to not have an incomplete release. You can advise how to write the workflow but shouldn't dictate it.

Spacetown avatar Jul 13 '25 19:07 Spacetown

@webknjaz I hit the following errors while trying to work on issue #324.

  • Branch creation is blocked due to repository rules
  • Branch name restrictions apply (must match regex ***) due to repository rules

[!NOTE]

  • To learn about managing repository rules, head to the GitHub Docs.

After resolving these issues, you can ask me to try again by unassigning and then reassigning the issue to me again.

Copilot avatar Aug 07 '25 10:08 Copilot

@webknjaz I hit the following errors while trying to work on issue #324.

  • Branch name restrictions apply (must match regex ***) due to repository rules

[!NOTE]

  • To learn about managing repository rules, head to the GitHub Docs.

After resolving these issues, you can ask me to try again by unassigning and then reassigning the issue to me again.

Copilot avatar Aug 07 '25 12:08 Copilot

Hey @webknjaz !

It seems I also stumbled upon this issue in my repo setup.

Could you please help me (and others who may run into the same issue) understand:

  1. Why this is insecure?
  2. What change exactly would be needed to make it secure?

ekohilas avatar Aug 11 '25 13:08 ekohilas

@ekohilas giving OIDC privilege to the transitive build deps can lead to impersonation/privilege escalation in systems other than PyPI. Imagine that you have this in one workflow for years and then set up something else, like AWS, to trust your repository's identity w/o remembering there's some other seemingly unrelated job that also has access to OIDC.

It's easily fixable by building in a job with reduced privileges, saving the dists as a GHA artifact and only downloading them in the publishing job. So the publishing job would only have two steps — download+upload. This job can also have a dedicated conditional clause, letting you run the build job even if you're not releasing. As a smoke test in PRs or even integrated better, with the same dists used in the test matrix before being published (as opposed to rebuilding them after testing again and hoping they are about the same as what you tested). Besides, the publishing job can have extra protections through GitHub Environments, which the build job does not really need.

webknjaz avatar Aug 11 '25 13:08 webknjaz