It’s easy to spot in hindsight. I can see how it slipped through if they didn’t actually closely read the changes in each file, it’s easy to let your guard down with many-file changesets like this. (That’s a cognitive bias that should be kept in mind when doing security review, particularly if that’s your only control in place to prevent issues like this).

Fundamentally, “fix tests” changesets like this should not touch non-test code and anything touching /var/lib/gitlab would be a big code smell to me if I was reviewing.

The advantage of the code owners approach is it highlights that there were changes to the prod auth code (shows the rule that triggers in the MR I believe), which gives you an additional chance to spot issues to focus on. Seems they also have bots running checks and so they could have the bot ask for an extra review if …/auth/ was touched. Test helpers shouldn’t live next to prod code, for obvious reasons, so the “prod auth code owner” rule being hit would have been an alarm bell on this MR.

I do think it’s concerning that security review was given here, so I’m keen to see the post-mortem and see what other gaps they identify.