At the moment, the only legitimate uses of `pull_request_target` are for things like labeling and auto-commenting on third-party PRs. But there's no reason for these actions to have default write access to the repository; GitHub can and should be able to grant fine-grained or (even better) single-use tokens that enable those exact operations.
(This is why zizmor blanket-flags all use of `pull_request_target` and other dangerous triggers[1]).
> This event runs in the context of the base of the pull request, rather than in the context of the merge commit, as the pull_request event does. This prevents execution of unsafe code from the head of the pull request that could alter your repository or steal any secrets you use in your workflow.
Which is comical given how easily secrets were exilfiltrated.
GitHub has written a series of blog posts[1] over the years about "pwn requests," which do a great job of explaining the problem. But the misleading documentation persists, and has led to a lot of user confusion where maintainers mistakenly believe that any use of `pull_request_target` is somehow more secure than `pull_request`, when the exact opposite is true.
[1]: https://securitylab.github.com/resources/github-actions-prev...
The best move would be for github to have a setting for allowing the automation to run on PRs that don't have clean merges, off by default and intended for use with linters only really. Until that happens though pull_request_target is the only game in town to get around that limitation. Much to my and other SecDevOps engineers sadness.
NOTE: with these external tools you absolutely cannot do the merge manually in github unless you want to break the entire thing. It's a whole heap of not fun.
However, it's worth noting that you don't (necessarily) need `pull_request_target` for the OIDC credential in a private repo: all first-party PRs will get it with the `pull_request` event. You can configure the subject for that credential with whatever components you want to make it deterministic.
But then in a pull request, the CI/CD pipeline actually runs untrusted code.
Getting this distinction correct 100% of the time in your mental model is pretty hard.
For the base case, where you maybe run a test suite and a linter, it's not too bad. But then you run into edge cases where you have to integrate with your own infrastructure (either for end2end tests, or for checking if contributors have CLAs submitted, or anything else that requires a bit more privs), and then it's very easy byte you.
To make things worse, GitHub has made certain operations on PRs (like auto-labeling and leaving automatic comments) completely impossible unless the extremely dangerous version (`pull_request_target`) is used. So this is a case of incentive-driven insecurity: people want to perform reasonable operations on third-party PRs, but the only mechanism GitHub Actions offers is a foot-cannon.
> but it gets worse. since the workflow was checking out our PR code, we could replace the OWNERS file with a symbolic link to ANY file on the runner. like, say, the github actions credentials file
So git allows committing soft links. So the issue above could affect almost any workflow.
Eh... That is taken out of context quite a bit, that sentence does continue. Just do `cat "$HOME/changed_files" | xargs -r editorconfig-checker --` and this specific problem is fixed.
(This is traditionally a non-issue, since the whole point is to execute code. So this isn't xargs' fault so much as it's the undying problem of tools being reused across privilege contexts.)
For sure though, this can get tricky, but I am not really aware of an alternative. :/ Since the calling convention is just an array of strings, there is no generic way to handle this without knowing what program you are calling and how it handles command line. This is not specific to xargs...
Well, I guess FFI would be a way, but it seems like a major PITA to have to figure out how to call a golang function from bash shell just to "call" a program.
Right, it's just that xargs surfaces it easily. I suspect most people don't realize that they're fanning arbitrary arguments into programs when they use xargs to fan input files.
If you have to opt in to safe usage at every turn, then it's an unsafe way of doing things.
Bearer tokens should be replaced with schemes based on signing and the private keys should never be directly exposed (if they are there's no difference between them and a bearer token). Signing agents do just that. Github's API is based on HTTP but mutual TLS authentication with a signing agent should be sufficient.
>> It is not possible for xargs to be used securely
However, the security issue this warning relates to is not the one that's applicable here. The one here is possible to avoid by using -- at the end of the command.
jmclnx•5h ago
But I have been seeing docs indication those projects are looking to go to git, will see if it really happens. In OpenBSD's case seems it will be based upon got(1).
seanhunter•5h ago
graemep•5h ago
Mic92•5h ago
edoceo•4h ago
udev4096•4h ago