Push rule on author emails does not take commit email into account for web-based merges
Summary
EE has push rules that can reject a push if certain conditions are not matched, including for commit author.
When merging something in the web UI, these push rules are run manually in https://gitlab.com/gitlab-org/gitlab-ee/blob/master/ee/app/services/ee/merge_requests/merge_service.rb
There is an assumption that User#email
will be the email address of the merge commit's author email, but when a user has a different commit email set, that assumption will be broken, leading to the push rule failing to operate correctly.
Commit emails are being introduced in %11.4: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/21598 . There have been a few other regressions noted in CE: https://gitlab.com/gitlab-org/gitlab-ce/issues/51564
Steps to reproduce
- Set up a user with
email:
foo@example.com, commit_email: bar@invalid.invalid` - Set up a project with a push rule like
@example.com$
(so a push will only be accepted if the email domain is example.com) - As that user, merge an MR
What is the current bug behavior?
The merge succeeds, even though the merge commit is authored by bar@invalid.invalid
What is the expected correct behavior?
The merge should be rejected. A manual merge followed by a git push
over HTTP or SSH that contains the wrong address is correctly rejected - this applies only to merges in the web UI.
Possible fixes
A quick fix would be to user current_user.commit_email
here: https://gitlab.com/gitlab-org/gitlab-ee/blob/master/ee/app/services/ee/merge_requests/merge_service.rb#L34
It may be more sensible to read the email address from git if at all possible, so we avoid the (small) chance of a race between the checks running and the commit email being changed.
/cc @jramsay @DouweM @jcamp0x2a
/label ~bug ~Create authorization regression regression:11.4