[theptip]

Thanks for doing the issue sleuthing. This is an excruciatingly bad look.

You'd have thought with all the code-owner functionality that GL has, they would lock down the `/lib/gitlab/auth/` files to require a security engineer to give additional signoff on top of a normal review. It looks like anyone at Gitlab can approve changes to the auth code (except LDAP): https://gitlab.com/gitlab-org/gitlab/-/blob/master/.gitlab/C... which is terrifying if true.

[mdaniel]

> to require a security engineer to give additional signoff on top of a normal review

Like this?

> cc @gitlab-com/gl-security/appsec

https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#...

I do so desperately hope it doesn't come across as throwing shade, because hindsight-2020-etc, but I do also think there was some kind of weird process breakdown here because this change somehow slipped past a "4 eyes" and an appsec review phase

[nightpool]

This was already required in this case, you can see in the comments that an additional AppSec review was done and the appsec team signed off on the MR above and beyond the normal code review process.

[bloomburger]

And two of those appsec reviewers are now out of office for the next two weeks…

[orojackson]

Heh, was this you [1]? Pretty much asking the same good questions you brought up in a different post.

Additionally, I see that the Senior Director of Engineering, Tim Zallmann, has left a bunch of GitLab project repos about 14 hours ago as of this writing. He was one of the folks who tried pinging [3] Mr. Coutable (he's one of the reviewers that's currently OOO). The ping is likely regarding the discovery of the security vulnerability.

[1] https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#...

[2] https://gitlab.com/users/timzallmann/activity

[3] https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76318#...