Our great sponsors
-
-
WorkOS
The modern identity platform for B2B SaaS. The APIs are flexible and easy-to-use, supporting authentication, user identity, and complex enterprise features like SSO and SCIM provisioning.
-
-
This is super interesting; we had a fairly long discussion about whether or not to add this action to cert-manager[1], and ended up rejecting it in part because it increased the risk of supply-chain attacks and that risk wasn't, in our opinion, outweighed by the potential benefit of catching more spelling mistakes.[2]
For me, I think there's a wider point here that GitHub Actions are pretty scary in terms of these kinds of attacks. Pre-packaged actions are easy to add to a project but come with risks, as this security advisory shows! There are a few aspects to Actions which made me a little uneasy in terms of my threat models when building software, and personally I've tended to avoid them.
[1] https://github.com/jetstack/cert-manager (Full disclosure, I'm part of the team paid by Jetstack to work on the cert-manager project)
You are wise to be wary.
There are some pretty subtle tigers waiting to maul you if you run workflows against untrusted PRs.
The way I currently do this is that our workflows run the CI build/test job in the repo of the user proposing the PR and uploads the logs/results as an "Artifact". Our repo waits for that job to complete and then downloads the artifacts and produces a pretty comment with the junit test results on the PR.
However, despite recommending a model like this, GitHub still makes it infuriatingly hard to actually do, and we ended up with all of this crud to be able to get one XML file: https://github.com/Hammerspoon/hammerspoon/blob/master/.gith...
> If my repo always runs all tests on a PR, could someone just add a PR with a new test that is then run? Thus running their arbitrary code.
Running arbitrary code is inevitable if an action is configured to run on all PRs. People have abused this to run crypto miners and stuff in the past, but this for the most part is merely an annoyance to maintainers, not a security problem. It does become a security problem when arbitrary code execution is allowed with your secrets, including your configured secrets and the read/write GITHUB_TOKEN.
Expanding on the topic of secrets, if you trigger your test from the usual pull_request event, the workflow won't have access to GITHUB_TOKEN or configured secrets, so it's the safe default you should almost always choose. That becomes a problem when you need write access to the repo, e.g. to assign labels or add comments to the PR from the workflow, in which case you have to use the privileged pull_request_target event to expose GITHUB_TOKEN and secrets. pull_request_target by default runs in the context of the base of the PR, so there's still no arbitrary code, but you can explicitly check out the PR in that context, and when you do, your secrets are potentially exposed to arbitrary code. If you execute that arbitrary code in any job, or like in this case, post the content of effectively any file on disk as directed by an attacker, boom, owned.
Therefore, you should
- Avoid pull_request_target unless white access to the repo and/or access to configured secrets is absolutely necessary;
- When using pull_request_target, avoid checking out untrusted code;
- If it's absolutely necessary to check out untrusted code, make absolutely sure that the untrusted code isn't executed in any way, and that your trusted handling code can't be tricked by untrusted content in any way, like an arbitrary symlink. This is of course difficult to verify.
In this specific case, the fix seems to be checking that the absolute path of the untrusted advice.txt is within GITHUB_WORKSPACE (https://github.com/check-spelling/check-spelling/commit/4363...). IMO that's a wrong fix only covering the symptom. The real cause is using untrusted configuration files at all; why not make a copy of the trusted version of configuration files and use those instead???
GitHub has an article about security considerations here: https://securitylab.github.com/research/github-actions-preve...