> The PR was opened, the workflow run, and the PR closed within the space of 1 minute (screenshots include timestamps in UTC+2, the author's timezone):
It's an unfortunately common problem with GitHub Actions, it's easy to set things up to where any PR that's opened against your repo runs the workflows as defined in the branch. So you fork, make a malicious change to an existing workflow, and open a PR, and your code gets executed automatically.
Frankly at this point PRs from non-contributors should never run workflows, but I don't think that's the default yet.
Problem is that you might want to have the tests run before even looking at it.
I think the mistake was to put secrets in there and allow publishing directly from github's CI.
Hilariously the people at pypi advise to use trusted publishers (publishing on pypi from github rather than local upload) as a way to avoid this issue.
> Problem is that you might want to have the tests run before even looking at it.
Why is this a problem? The default `pull_request` trigger isn't dangerous in GitHub Actions; the issue here is specifically with `pull_request_target`. If all you want to do is have PRs run tests, you can do that with `pull_request` without any sort of credential or identity risk.
> Hilariously the people at pypi advise to use trusted publishers (publishing on pypi from github rather than local upload) as a way to avoid this issue.
There are two separate things here:
1. When we designed Trusted Publishing, one of the key observations was that people do use CI to publish, and will continue to do so because it conveys tangible benefits (mostly notably, it doesn't tie release processes to an opaque phase on a developer's machine). Given that people do use CI to publish, giving them a scheme that provides self-expiring, self-scoping credentials instead of long-lived ones is the sensible thing to do.
2. Separately, publishing from CI is probably a good thing for the median developer: developer machines are significantly more privileged than the average CI runner (in terms of access to secrets/state that a release process simply doesn't need). One of the goals behind Trusted Publishing was to ensure that people could publish from an otherwise minimal CI environment, without even needing to configure a long-lived credential for authentication.
Like with every scheme, Trusted Publishing isn't a magic bullet. But I think the proscription to use it here is essentially correct: Shai-Hulud propagates through stored credentials, and a compromised credential from a TP flow is only useful for a short period of time. In other words, Trusted Publishing would make it harder for the parties behind Shai-Hulud to group and orchestrate the kinds of compromise waves we're seeing.
> the issue here is specifically with `pull_request_target`
I just went to github to search for references to that trigger-type, and I admit I was surprised at the sheer number of times it is visible in a code-search.
The kind of argument of "just don't make mistakes, how hard is it" (and we're talking about something very obscure and badly documented here) didn't work for C and in my opinion doesn't work for this either.
It does largely avoid the issue if you configure to allow only specific environments AND you require reviews before pushing/merging to branches in that environment.
Yes and anyone who knows anything about software dev knows that the first thing you should do with an important repo is set up branch protections to disallow that, and require reviews etc. Basic CI/CD.
This incident reflects extremely poorly on PostHog because it demonstrates a lack of thought to security beyond surface level. It tells us that any dev at PostHog has access at any time to publish packages, without review (because we know that the secret to do this is accessible from plain GHA secret which can be read from any GHA run which presumably run on any internal dev's PR). The most charitable interpretation of this is that it's consciously justified by them because it reduces friction, in which case I would say that demonstrates poor judgement, a bad balance.
A casual audit would have revealed this and suggested something like restricting the secret to a specific GHA environment and requiring reviews to push to that env. Or something like that.
It's an unfortunately common problem with GitHub Actions, it's easy to set things up to where any PR that's opened against your repo runs the workflows as defined in the branch. So you fork, make a malicious change to an existing workflow, and open a PR, and your code gets executed automatically.
Frankly at this point PRs from non-contributors should never run workflows, but I don't think that's the default yet.