The vast majority of repos should be able to run CI on pull requests with no privileges at all. GitHub can manage any resource utilization issues on their end.
Is the issue here that a self-hosted runner was needed for some hardware tests?
Is the issue here that a self-hosted runner was needed for some hardware tests?
* pre-merge builds on PRs. These should not have privileges, but making the distinction between the two cases requires a lot of care.
* official builds of "master" or a feature branch from the main repo. These "need" privileges to upload the resulting artifacts somewhere. Of course, if all it did was wake up some daemon elsewhere, which could download straight from the CI in a verified way based on the CI's notion of the job name, it would be secure without privileges, but most CI systems don't want to preserve huge artifacts, and maintaining the separate daemon is also annoying.
Without that there would be no need to have an action to do an upload. It could be comfortably and safely done offline.
Builds off of main get the secrets, pull requests from randos don’t.
And public repos don’t pay for CI on GitHub.
Not rocket science, people.
In several of our other operations, not just PyTorch, we leveraged workflow_dispatch to steal a PAT from another workflows. Developers tend to over-provision PATs so often. More often than not we'd end up with a PAT that has all scopes checked and org admin permissions. With that one could clean out all of the secrets from an organization in minutes using automated tools such as https://github.com/praetorian-inc/gato.
The key thing with a non-ephemeral runner is that (after obtaining persistence) you can grab the GITHUB_TOKEN from a subsequent non-fork PR build or a build on another trigger, which will have write permissions unless restricted by the repository maintainers.
But that is real work to setup, audit, and maintain. It'd be better if, like phone app capabilities, the default would be no privs, any privs are explicitly granted, and if they aren't being used, the system detects that and asks if you want to remove specific ones.
Here's a similar mistake on an OSS repo a company I worked for made: https://goteleport.com/blog/hack-via-pull-request/
The smugness and overconfidence of someone who's about to be pwned?
Only because pypi removed support for gpg and their only "signature" now basically is "was uploaded from a gh action".
Otherwise a maintainer could do it offline without needing to give microsoft total access to uploading to pypi.
> without needing to give microsoft total access to uploading to pypi
I assume you're referring to Trusted Publishers here? It's a per-project configuration using the industry standard of OIDC that you don't have to opt in to, so "total access" is a silly characterization. Also if you're insinuating that MS is going to generate fraudulent OIDC tokens to compromise a PyPI package, then you might want to start weaning yourself off the kool-aid.
The vulnerability was exactly that the secrets of the trusted branch can get leaked :)
> you can still upload to PyPI manually with an API Token
I can, BUT if you want to be a Trusted Publisher™ the only way is to do it via github.
https://docs.pypi.org/trusted-publishers/
Most likely the plan is to make it compulsory for all projects eventually, just like they made 2fa compulsory.
So less secure is considered as MORE secure by pypi :) Which is consistent with the idea that no PGP signature is more secure than signed uploads. Or the idea that a global token in a clear text file is somehow safer than a password that gets typed every time.
When there are no side effects and no in-container secrets and the hosting is free or reasonably limited to prevent abusers, ideally yes.
Outside that, heck no, that'd be crazy. You're allowing randos to run arbitrary code on your budget. Locking it down until it's reviewed is like step 1, they can validate locally until then.
My prior is that GitHub is pretty competent in understanding malicious PRs to open source repos, and wouldn’t penalize the repo owner without other evidence of wrongdoing.
The issue is that now the pull requests don't get tested at all. I have to manually, locally, get all the commits, make a branch on the main repository with them, and then the actions run.
Challenge there is that if the pr changes, it's a bit clunky to retrigger the CI (you have to remove, then re-add).
I guess you could also do this with comments - can you trigger a workflow based on a specific comment being added from a specific user?
Afaict, a huge portion of this attack came from persistence on the self-hosted runner.
Absent that, they would have needed a container jailbreak as well, which substantially ups the difficulty.
And if a repo is running <100 builds a day, spin up + kill container seems a small per-build price to pay for the additional security isolation.
https://docs.github.com/en/actions/hosting-your-own-runners/...
1. ephemeral + zero implicit trust (2) https://blog.openziti.io/my-intern-assignment-call-a-dark-we...
2. zero implicit trust: https://github.com/openziti/ziti-webhook-action
(1) disclosure, maintainer (2) zero implicit trust in this case = no open inbound ports on underlay; need to access via app-specific overlay which requires strong identity, authN, authZ
[1]: https://runs-on.com