5 ms·
You can't rely on people spotting the significance of such changes
by Twirrim 1mo ago
You can't rely on people spotting the significance of such changes
- fn-mote 1mo ago^^ Absolutely. Nothing in the PR jumps out as a red flag. Unless you know how the internals work, I suppose.
- chrisjj 1mo ago> Nothing in the PR jumps out as a red flag. Made by AI?
- larsonian 1mo agoAre you kidding? It's a very obvious case of quote injection. Not some subtle race condition or anything.
- joombaga 1mo agoI think it's obvious too. I'd call out any case of `${{ }}` interpolation in a `run` block, and it's something I watch for in PRs. I also know other people don't watch for this, as I've corrected it about a hundred times. Over the last 10 years my average colleague understands less and less about injection or to watch for it at layer boundaries.
- bigfishrunning 1mo agoShouldn't anyone reviewing such a PR know how the internals work?
- koiueo 1mo agoNot anymore, it seems
- eithed 1mo agoTests would have caught it = https://github.com/rhysd/actionlint https://github.com/rhysd/actionlint injection check
- thejosh 1mo agoalso been a huge fan of zizmor (https://github.com/zizmorcore/zizmor https://github.com/zizmorcore/zizmor) lately, basically: "am I going to footgun myself?"
- dv_dt 1mo agoI have been talking to people who want to autoreview and autoapprove "minor" AI prs. For security especially, I think if the models weren't enough to prevent the issues, they aren't enough to judge what is minor.