Is the vulnerability in the code change glaringly obvious to you?
Looks like two security engineers actually took a look at it: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#...
Looks like two security engineers actually took a look at it: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#...
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.