| ▲ | ssanderson11235 an hour ago | |
The fundamental mistake here seems to have been not fully understanding the threat model of the pull_request_target action trigger. pull_request_target jobs run in response to various events related to a pull request opened against your repo from a fork (e.g, someone opens a new PR or updates an existing one). Unlike pull_request jobs, which are read-only by default, pull_request_target jobs have read/write permissions. The broader permissions of pull_request_target are supposed to be mitigated by the fact that pull_request_target jobs run in a checkout of your current default branch rather than on a checkout of the opened PR. For example, if someone opens a PR from some branch, pull_request_target runs on `main`, not on the new branch. The compromised action, however, checked out the source code of the PR to run a benchmark task, which resulted in running malicious attacker-controlled code in a context that had sensitive credentials. The GHA docs warn about this risk specifically: > Running untrusted code on the pull_request_target trigger may lead to security vulnerabilities. These vulnerabilities include cache poisoning and granting unintended access to write privileges or secrets. They also further link to a post from 2021 about this specific problem: https://securitylab.github.com/resources/github-actions-prev.... That post opens with: > TL;DR: Combining pull_request_target workflow trigger with an explicit checkout of an untrusted PR is a dangerous practice that may lead to repository compromise. The workflow authors presumably thought this was safe because they had a block setting permissions.contents: read, but that block only affects the permissions for GITHUB_TOKEN, which is not the token used to interact with the cache. This seems like the biggest oversight in the existing GHA documentation/api (beyond the general unsafety of having pull_request_target at all). Someone could (and presumably did!) see that block and think "this job runs with read-only permissions", which wasn't actually true here. | ||
| ▲ | consumer451 20 minutes ago | parent [-] | |
From a GitHub product owner POV, if the architecture is not to be changed, what is the solution? A big ugly warning in the UI? Or, push back on the architecture? Or, is threatening a big ugly warning in the UI actually pushing back on the architecture? | ||