Member check Push Rule prevents push repository mirroring when signed commits are enabled

Summary

When repository mirroring is enabled with signed commits and the "Confirm committer is a GitLab member" push rule is active, mirror synchronization fails with the error:

remote: GitLab: Committer '*****@example.gitlab.com' is not a member of team

Enabling signed commits causes noreply@example.gitlab.com instance email to be set as committer email, when push rule looks confirm email matches a legitimate user, the check fails:

File: ee/lib/ee/gitlab/checks/push_rules/commit_check.rb (lines 78-91)

def check_member(commit)
  return if signed_by_gitlab?(commit)
  return unless push_rule.member_check

  unless ::User.find_by_any_email(commit.author_email).present?
    return "Author '#{commit.author_email}' is not a member of team"
  end

  if commit.author_email.casecmp(commit.committer_email) != 0
    unless ::User.find_by_any_email(commit.committer_email).present?
      "Committer '#{commit.committer_email}' is not a member of team"
    end
  end
end

Steps to reproduce

  1. Enable signed commits
  2. Set up a push mirror between project A and B
  3. Tick "Ensure committer is a gitlab member" push rule in project B
  4. Observe mirroring now fails as noreply email associated with push commit is not attached to a gitlab instance user

Example Project

What is the current bug behavior?

Push commits by the gitaly RPC call for mirroring fail with:

13:push to mirror: git push: exit status 1, stderr: "remote: GitLab: Committer *****@gitlab.example.com' is not a member of team \nTo <example-repository>.git \n ! [remote rejected] main -> main (pre-receive hook declined) \n ! [remote rejected] 1.0.525 -> 1.0.525 (pre-receive hook declined) \nerror: failed to push some refs to '<example-repository>.git'\n".

What is the expected correct behavior?

Push mirroring should still be possible when signed commits are enabled for the instance and projects are configured with push rule to ensure committer is legitimate. Commits made by the gitaly RPC call should be considered as a member of team, or ignored for push mirroring check_member rule.

Relevant logs and/or screenshots

Output of checks

This bug happens on GitLab Self-Managed when Signed Commits feature is enabled.

Results of GitLab environment info

Expand for output related to GitLab environment info 

 (For installations with omnibus-gitlab package run and paste the output of: \`sudo gitlab-rake gitlab:env:info\`)  (For installations from source run and paste the output of: \`sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production\`)

Results of GitLab application Check

Expand for output related to the GitLab application check 

(For installations with omnibus-gitlab package run and paste the output of: `sudo gitlab-rake gitlab:check SANITIZE=true`)

(For installations from source run and paste the output of: `sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production SANITIZE=true`)

(we will only investigate if the tests are passing)

Possible fixes

The signed_by_gitlab?() method could check for :SIGNER_SYSTEM signer status without requiring updated_from_web? to be true (I don't think push mirroring is done by web, but by RPC call).

Option 1: Remove the updated_from_web? requirement

def signed_by_gitlab?(commit)
  return false unless commit.has_signature?
  commit_signatures[commit.id][:signer] == :SIGNER_SYSTEM
end

Patch release information for backports

If the bug fix needs to be backported in a patch release to a version under the maintenance policy, please follow the steps on the patch release runbook for GitLab engineers.

Refer to the internal "Release Information" dashboard for information about the next patch release, including the targeted versions, expected release date, and current status.

High-severity bug remediation

To remediate high-severity issues requiring an internal release for single-tenant SaaS instances, refer to the internal release process for engineers.

Edited by Danny Bailey