Hacker News new | ask | show | jobs
by perlgeek 245 days ago
CI/CD actions for pull/merge requests are a nightmare. When a developer writes test/verification steps, they are mostly in the mindset "this is my code running in the context of my github/gitlab account", which is true for commits made by themselves and their team members.

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.

1 comments

I don't think the problem is CI/CD runs on pull requests, per se: it's that GitHub has two extremely similar triggers (`pull_request` and `pull_request_target`). One of these is almost entirely safe (you have to go out of your way to misuse it), while the other is almost entirely unsafe (it's almost impossible to use safely).

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.

> while the other is almost entirely unsafe (it's almost impossible to use safely).

I don't believe this is fair. "Don't run untrusted code" is what it comes down to. Don't trust test suites or scripts in the incoming branch, etc.

That pull_request_target workflows are (still) privileged by default is nuts and indeed a footgun but no need for "almost impossible" hysteria.

> I don't believe this is fair. "Don't run untrusted code" is what it comes down to. Don't trust test suites or scripts in the incoming branch, etc.

TFA is a great example of how this breaks down. The two examples in the post obtain code execution/credential exfiltration without running an attacker controlled test suite or script.

I never understood what it is about labeling/commenting that prevents in from working in the regular event. They could just add a permission that specifically allows those actions.