Merge branch '36805-confidential-issue' into 'master'

Prevent an endless checking loop for two merge requests targeting each other

See merge request gitlab-org/security/gitlab!85

changelogs/unreleased/36805-confidential-issue.yml

+---
+title: Prevent an endless checking loop for two merge requests targeting each other
+merge_request:
+author:
+type: security

lib/gitlab/user_access.rb

@@ -67,7 +67,13 @@ module Gitlab
       return false unless can_access_git?
       return false unless project
 
-      return false if !user.can?(:push_code, project) && !project.branch_allows_collaboration?(user, ref)
+      # Checking for an internal project to prevent an infinite loop:
+      # https://gitlab.com/gitlab-org/gitlab/issues/36805
+      if project.internal?
+        return false unless user.can?(:push_code, project)
+      else
+        return false if !user.can?(:push_code, project) && !project.branch_allows_collaboration?(user, ref)
+      end
 
       if protected?(ProtectedBranch, project, ref)
         protected_branch_accessible_to?(ref, action: :push)

spec/lib/gitlab/user_access_spec.rb

@@ -30,6 +30,17 @@ describe Gitlab::UserAccess do
       end
     end
 
+    describe 'push to branch in an internal project' do
+      it 'will not infinitely loop when a project is internal' do
+        project.visibility_level = Gitlab::VisibilityLevel::INTERNAL
+        project.save!
+
+        expect(project).not_to receive(:branch_allows_collaboration?)
+
+        access.can_push_to_branch?('master')
+      end
+    end
+
     describe 'push to empty project' do
       let(:empty_project) { create(:project_empty_repo) }
       let(:project_access) { described_class.new(user, project: empty_project) }

spec/models/project_spec.rb

@@ -4792,6 +4792,38 @@ describe Project do
     end
   end
 
+  context 'with cross internal project merge requests' do
+    let(:project) { create(:project, :repository, :internal) }
+    let(:forked_project) { fork_project(project, nil, repository: true) }
+    let(:user) { double(:user) }
+
+    it "does not endlessly loop for internal projects with MRs to each other", :sidekiq_inline do
+      allow(user).to receive(:can?).and_return(true, false, true)
+      allow(user).to receive(:id).and_return(1)
+
+      create(
+        :merge_request,
+        target_project: project,
+        target_branch: 'merge-test',
+        source_project: forked_project,
+        source_branch: 'merge-test',
+        allow_collaboration: true
+      )
+
+      create(
+        :merge_request,
+        target_project: forked_project,
+        target_branch: 'merge-test',
+        source_project: project,
+        source_branch: 'merge-test',
+        allow_collaboration: true
+      )
+
+      expect(user).to receive(:can?).at_most(5).times
+      project.branch_allows_collaboration?(user, "merge-test")
+    end
+  end
+
   context 'with cross project merge requests' do
     let(:user) { create(:user) }
     let(:target_project) { create(:project, :repository) }