Add latest changes from gitlab-org/security/gitlab@16-4-stable-ee

22 files changed, 323 insertions, 65 deletions

diff --git a/app/assets/javascripts/behaviors/markdown/render_math.js b/app/assets/javascripts/behaviors/markdown/render_math.js
index b2348cf0bad..7525fc76d16 100644
--- a/app/assets/javascripts/behaviors/markdown/render_math.js
+++ b/app/assets/javascripts/behaviors/markdown/render_math.js
@@ -66,16 +66,12 @@ class SafeMathRenderer {
     el.removeAttribute('style');
     if (!forceRender && (this.totalMS >= MAX_RENDER_TIME_MS || text.length > MAX_MATH_CHARS)) {
       // Show unrendered math code
-      const wrapperElement = document.createElement('div');
       const codeElement = document.createElement('pre');
 
       codeElement.className = 'code';
       codeElement.textContent = el.textContent;
       codeElement.dataset.mathStyle = el.dataset.mathStyle;
 
-      const { parentNode } = el;
-      parentNode.replaceChild(wrapperElement, el);
-
       let message;
       if (text.length > MAX_MATH_CHARS) {
         message = sprintf(
@@ -103,11 +99,11 @@ class SafeMathRenderer {
           </div>
           `;
 
-      if (!wrapperElement.classList.contains('lazy-alert-shown')) {
+      if (!el.classList.contains('lazy-alert-shown')) {
         // eslint-disable-next-line no-unsanitized/property
-        wrapperElement.innerHTML = html;
-        wrapperElement.append(codeElement);
-        wrapperElement.classList.add('lazy-alert-shown');
+        el.innerHTML = html;
+        el.append(codeElement);
+        el.classList.add('lazy-alert-shown');
       }
 
       // Render the next math
@@ -125,6 +121,12 @@ class SafeMathRenderer {
       }
 
       try {
+        if (displayContainer.dataset.mathStyle === 'inline') {
+          displayContainer.classList.add('math-content-inline');
+        } else {
+          displayContainer.classList.add('math-content-display');
+        }
+
         // eslint-disable-next-line no-unsanitized/property
         displayContainer.innerHTML = this.katex.renderToString(text, {
           displayMode: el.dataset.mathStyle === 'display',
@@ -169,8 +171,7 @@ class SafeMathRenderer {
   render() {
     // Replace math blocks with a placeholder so they aren't rendered twice
     this.elements.forEach((el) => {
-      const placeholder = document.createElement('span');
-      placeholder.style.display = 'none';
+      const placeholder = document.createElement('div');
       placeholder.dataset.mathStyle = el.dataset.mathStyle;
       placeholder.textContent = el.textContent;
       el.parentNode.replaceChild(placeholder, el);
diff --git a/app/assets/stylesheets/framework/markdown_area.scss b/app/assets/stylesheets/framework/markdown_area.scss
index b87fd3e67d4..62782de5402 100644
--- a/app/assets/stylesheets/framework/markdown_area.scss
+++ b/app/assets/stylesheets/framework/markdown_area.scss
@@ -137,6 +137,16 @@
   border-radius: $border-radius-default $border-radius-default 0 0;
 }
 
+.math-content-inline {
+  overflow: auto;
+  display: inline-flex;
+}
+
+.math-content-display {
+  overflow: auto;
+  display: block;
+}
+
 @include media-breakpoint-down(xs) {
   .referenced-users {
     margin-right: 0;
diff --git a/app/controllers/projects/error_tracking/projects_controller.rb b/app/controllers/projects/error_tracking/projects_controller.rb
index 531bd327e43..372fbfdc183 100644
--- a/app/controllers/projects/error_tracking/projects_controller.rb
+++ b/app/controllers/projects/error_tracking/projects_controller.rb
@@ -5,7 +5,7 @@ module Projects
     class ProjectsController < Projects::ApplicationController
       respond_to :json
 
-      before_action :authorize_read_sentry_issue!
+      before_action :authorize_admin_sentry!
 
       feature_category :error_tracking
       urgency :low
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index a90a16e120c..06eb3fcc233 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -133,6 +133,8 @@ module MergeRequestsHelper
       _('Not available for private projects')
     elsif ProtectedBranch.protected?(merge_request.source_project, merge_request.source_branch)
       _('Not available for protected branches')
+    elsif !merge_request.author.can?(:push_code, merge_request.source_project)
+      _('Merge request author cannot push to target project')
     end
   end
 
diff --git a/app/models/project.rb b/app/models/project.rb
index 68196f0a757..5989584ce43 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -3456,6 +3456,7 @@ class Project < ApplicationRecord
       # Issue for N+1: https://gitlab.com/gitlab-org/gitlab-foss/issues/49322
       Gitlab::GitalyClient.allow_n_plus_1_calls do
         merge_requests_allowing_collaboration(branch_name).any? do |merge_request|
+          merge_request.author.can?(:push_code, self) &&
           merge_request.can_be_merged_by?(user, skip_collaboration_check: true)
         end
       end
diff --git a/app/models/project_team.rb b/app/models/project_team.rb
index 34754f4fc95..38521ae6090 100644
--- a/app/models/project_team.rb
+++ b/app/models/project_team.rb
@@ -121,7 +121,7 @@ class ProjectTeam
   def import(source_project, current_user)
     target_project = project
 
-    source_members = source_project.project_members.to_a
+    source_members = source_members_for_import(source_project)
     target_user_ids = target_project.project_members.pluck_user_ids
 
     importer_access_level = max_member_access(current_user.id)
@@ -242,6 +242,10 @@ class ProjectTeam
   def member_user_ids
     Member.on_project_and_ancestors(project).select(:user_id)
   end
+
+  def source_members_for_import(source_project)
+    source_project.project_members.to_a
+  end
 end
 
 ProjectTeam.prepend_mod_with('ProjectTeam')
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index 38e6360f81d..a57b6f8daf7 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -289,6 +289,7 @@ class ProjectPolicy < BasePolicy
     enable :change_visibility_level
     enable :remove_project
     enable :archive_project
+    enable :link_forked_project
     enable :remove_fork_project
     enable :destroy_merge_request
     enable :destroy_issue
@@ -545,6 +546,7 @@ class ProjectPolicy < BasePolicy
     enable :destroy_release
     enable :destroy_artifacts
     enable :admin_operations
+    enable :admin_sentry
     enable :read_deploy_token
     enable :create_deploy_token
     enable :destroy_deploy_token
diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb
index 1539e24df9d..67d690d64e7 100644
--- a/app/services/error_tracking/list_projects_service.rb
+++ b/app/services/error_tracking/list_projects_service.rb
@@ -5,6 +5,8 @@ module ErrorTracking
     private
 
     def perform
+      return error('Access denied', :unauthorized) unless can?(current_user, :admin_sentry, project)
+
       unless project_error_tracking_setting.valid?
         return error(project_error_tracking_setting.errors.full_messages.join(', '), :bad_request)
       end
diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml
index d14c20426df..05626a2b8b8 100644
--- a/config/sidekiq_queues.yml
+++ b/config/sidekiq_queues.yml
@@ -389,6 +389,8 @@
   - 2
 - - mailers
   - 2
+- - members_destroyer_clean_up_group_protected_branch_rules
+  - 1
 - - members_expiring_email_notification
   - 1
 - - merge
diff --git a/doc/operations/error_tracking.md b/doc/operations/error_tracking.md
index 82982a03016..1ebbb047c01 100644
--- a/doc/operations/error_tracking.md
+++ b/doc/operations/error_tracking.md
@@ -161,7 +161,7 @@ GitLab provides a way to connect Sentry to your project.
 
 Prerequisites:
 
-- You must have at least the Developer role for the project.
+- You must have at least the Maintainer role for the project.
 
 To enable the Sentry integration:
 
diff --git a/lib/api/commits.rb b/lib/api/commits.rb
index 069d117db17..c0222539c98 100644
--- a/lib/api/commits.rb
+++ b/lib/api/commits.rb
@@ -219,7 +219,7 @@ module API
         if params[:start_project]
           start_project = find_project!(params[:start_project])
 
-          unless user_project.forked_from?(start_project)
+          unless can?(current_user, :read_code, start_project) && user_project.forked_from?(start_project)
             forbidden!("Project is not included in the fork network for #{start_project.full_name}")
           end
         end
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index 6d13512aad6..ac28effea43 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -701,7 +701,7 @@ module API
         requires :forked_from_id, type: String, desc: 'The ID of the project it was forked from', documentation: { example: 'gitlab' }
       end
       post ":id/fork/:forked_from_id", feature_category: :source_code_management do
-        authorize! :admin_project, user_project
+        authorize! :link_forked_project, user_project
 
         fork_from_project = find_project!(params[:forked_from_id])
 
diff --git a/lib/banzai/filter/asset_proxy_filter.rb b/lib/banzai/filter/asset_proxy_filter.rb
index 00ffdd3d809..512c55381ec 100644
--- a/lib/banzai/filter/asset_proxy_filter.rb
+++ b/lib/banzai/filter/asset_proxy_filter.rb
@@ -22,13 +22,13 @@ module Banzai
 
           begin
             uri = URI.parse(original_src)
+
+            next if uri.host.nil? && !original_src.start_with?('///')
+            next if asset_host_allowed?(uri.host)
           rescue StandardError
-            next
+            # Ignored
           end
 
-          next if uri.host.nil? && !original_src.start_with?('///')
-          next if asset_host_allowed?(uri.host)
-
           element['src'] = asset_proxy_url(original_src)
           element['data-canonical-src'] = original_src
         end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index bf468a3b209..e5a755f12b5 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -29344,6 +29344,9 @@ msgstr ""
 msgid "Merge request approvals"
 msgstr ""
 
+msgid "Merge request author cannot push to target project"
+msgstr ""
+
 msgid "Merge request change summary"
 msgstr ""
 
@@ -57085,6 +57088,9 @@ msgstr[1] ""
 msgid "security Reports|There was an error creating the merge request"
 msgstr ""
 
+msgid "security policy bot users cannot be added to other projects"
+msgstr ""
+
 msgid "selective_code_owner_removals can only be enabled when retain_approvals_on_push is enabled"
 msgstr ""
 
diff --git a/spec/controllers/projects/error_tracking/projects_controller_spec.rb b/spec/controllers/projects/error_tracking/projects_controller_spec.rb
index 7529c701b2b..63346faccbf 100644
--- a/spec/controllers/projects/error_tracking/projects_controller_spec.rb
+++ b/spec/controllers/projects/error_tracking/projects_controller_spec.rb
@@ -19,7 +19,7 @@ RSpec.describe Projects::ErrorTracking::ProjectsController do
       let(:user) { create(:user) }
 
       it 'returns 404' do
-        project.add_guest(user)
+        project.add_developer(user)
 
         get :index, params: list_projects_params
 
diff --git a/spec/features/markdown/math_spec.rb b/spec/features/markdown/math_spec.rb
index 25459494a0c..0bc8f2146e9 100644
--- a/spec/features/markdown/math_spec.rb
+++ b/spec/features/markdown/math_spec.rb
@@ -24,9 +24,7 @@ RSpec.describe 'Math rendering', :js, feature_category: :team_planning do
       ```
     MATH
 
-    issue = create(:issue, project: project, description: description)
-
-    visit project_issue_path(project, issue)
+    create_and_visit_issue_with_description(description)
 
     expect(page).to have_selector('.katex .mord.mathnormal', text: 'b')
     expect(page).to have_selector('.katex-display .mord.mathnormal', text: 'b')
@@ -40,9 +38,7 @@ RSpec.describe 'Math rendering', :js, feature_category: :team_planning do
       This link is valid $`\\href{https://gitlab.com}{Gitlab}`$.
     MATH
 
-    issue = create(:issue, project: project, description: description)
-
-    visit project_issue_path(project, issue)
+    create_and_visit_issue_with_description(description)
 
     page.within '.description > .md' do
       # unfortunately there is no class selector for KaTeX's "unsupported command"
@@ -59,9 +55,7 @@ RSpec.describe 'Math rendering', :js, feature_category: :team_planning do
       ```
     MATH
 
-    issue = create(:issue, project: project, description: description)
-
-    visit project_issue_path(project, issue)
+    create_and_visit_issue_with_description(description)
 
     page.within '.description > .md' do
       expect(page).to have_selector('.js-lazy-render-math-container', text: /math block exceeds 1000 characters/)
@@ -75,11 +69,7 @@ RSpec.describe 'Math rendering', :js, feature_category: :team_planning do
       ```
     MATH
 
-    issue = create(:issue, project: project, description: description)
-
-    visit project_issue_path(project, issue)
-
-    wait_for_requests
+    create_and_visit_issue_with_description(description)
 
     page.within '.description > .md' do
       expect(page).not_to have_selector('.katex-error')
@@ -93,11 +83,7 @@ RSpec.describe 'Math rendering', :js, feature_category: :team_planning do
       ```
     MATH
 
-    issue = create(:issue, project: project, description: description)
-
-    visit project_issue_path(project, issue)
-
-    wait_for_requests
+    create_and_visit_issue_with_description(description)
 
     page.within '.description > .md' do
       click_button 'Display anyway'
@@ -113,11 +99,7 @@ RSpec.describe 'Math rendering', :js, feature_category: :team_planning do
       ```
     MATH
 
-    issue = create(:issue, project: project, description: description)
-
-    visit project_issue_path(project, issue)
-
-    wait_for_requests
+    create_and_visit_issue_with_description(description)
 
     page.within '.description > .md' do
       expect(page).to have_selector('.katex-error',
@@ -132,11 +114,7 @@ RSpec.describe 'Math rendering', :js, feature_category: :team_planning do
       ```
     MATH
 
-    issue = create(:issue, project: project, description: description)
-
-    visit project_issue_path(project, issue)
-
-    wait_for_requests
+    create_and_visit_issue_with_description(description)
 
     page.within '.description > .md' do
       expect(page).to have_selector('.katex-error', text: /&amp;lt;script&amp;gt;/)
@@ -162,4 +140,72 @@ RSpec.describe 'Math rendering', :js, feature_category: :team_planning do
       expect(page).not_to have_selector('.js-lazy-render-math')
     end
   end
+
+  it 'uses math-content-display for display math', :js do
+    description = <<~MATH
+      ```math
+        1 + 2
+      ```
+    MATH
+
+    create_and_visit_issue_with_description(description)
+
+    page.within '.description > .md' do
+      expect(page).to have_selector('.math-content-display')
+    end
+  end
+
+  it 'uses math-content-inline for inline math', :js do
+    description = 'one $`1 + 2`$ two'
+
+    create_and_visit_issue_with_description(description)
+
+    page.within '.description > .md' do
+      expect(page).to have_selector('.math-content-inline')
+    end
+  end
+
+  context 'when math tries to cover other elements on the page' do
+    it 'prevents hijacking for display math', :js do
+      description = <<~MATH
+        [test link](#)
+
+        ```math
+          \\hskip{-200pt}\\href{https://example.com}{\\smash{\\raisebox{20em}{$\\smash{\\raisebox{20em}{$\\phantom{\\underset{\\underset{\\underset{\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}}{\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}}}{\\underset{\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}}{\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}}}}{\\underset{\\underset{\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}}{\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}}}{}}}$}}$}}}
+        ```
+      MATH
+
+      issue = create_and_visit_issue_with_description(description)
+
+      page.within '.description > .md' do
+        click_link 'test link'
+
+        expect(page).to have_current_path(project_issue_path(project, issue))
+      end
+    end
+
+    it 'prevents hijacking for inline math', :js do
+      description = <<~MATH
+        [test link](#) $`\\hskip{-200pt}\\href{https://example.com}{\\smash{\\raisebox{20em}{$\\smash{\\raisebox{20em}{$\\phantom{\\underset{\\underset{\\underset{\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}}{\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}}}{\\underset{\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}}{\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}}}}{\\underset{\\underset{\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}}{\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}\\rule{20em}{20em}}}{}}}$}}$}}}`$
+      MATH
+
+      issue = create_and_visit_issue_with_description(description)
+
+      page.within '.description > .md' do
+        click_link 'test link'
+
+        expect(page).to have_current_path(project_issue_path(project, issue))
+      end
+    end
+  end
+
+  def create_and_visit_issue_with_description(description)
+    issue = create(:issue, project: project, description: description)
+
+    visit project_issue_path(project, issue)
+
+    wait_for_requests
+
+    issue
+  end
 end
diff --git a/spec/helpers/merge_requests_helper_spec.rb b/spec/helpers/merge_requests_helper_spec.rb
index b6c8653a563..3614cbe5011 100644
--- a/spec/helpers/merge_requests_helper_spec.rb
+++ b/spec/helpers/merge_requests_helper_spec.rb
@@ -234,4 +234,43 @@ RSpec.describe MergeRequestsHelper, feature_category: :code_review_workflow do
       it { expect(tab_count_display(merge_request, '10')).to eq('10') }
     end
   end
+
+  describe '#allow_collaboration_unavailable_reason' do
+    subject { allow_collaboration_unavailable_reason(merge_request) }
+
+    let(:merge_request) do
+      create(:merge_request, author: author, source_project: project, source_branch: generate(:branch))
+    end
+
+    let_it_be(:public_project) { create(:project, :small_repo, :public) }
+    let(:project) { public_project }
+    let(:forked_project) { fork_project(project) }
+    let(:author) { project.creator }
+
+    context 'when the merge request allows collaboration for the user' do
+      before do
+        allow(merge_request).to receive(:can_allow_collaboration?).with(current_user).and_return(true)
+      end
+
+      it { is_expected.to be_nil }
+    end
+
+    context 'when the project is private' do
+      let(:project) { create(:project, :empty_repo, :private) }
+
+      it { is_expected.to eq(_('Not available for private projects')) }
+    end
+
+    context 'when the source branch is protected' do
+      let!(:protected_branch) { create(:protected_branch, project: project, name: merge_request.source_branch) }
+
+      it { is_expected.to eq(_('Not available for protected branches')) }
+    end
+
+    context 'when the merge request author cannot push to the source project' do
+      let(:author) { create(:user) }
+
+      it { is_expected.to eq(_('Merge request author cannot push to target project')) }
+    end
+  end
 end
diff --git a/spec/lib/banzai/filter/asset_proxy_filter_spec.rb b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb
index dc6ac52a8c2..7a34bf13c8f 100644
--- a/spec/lib/banzai/filter/asset_proxy_filter_spec.rb
+++ b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb
@@ -89,6 +89,33 @@ RSpec.describe Banzai::Filter::AssetProxyFilter, feature_category: :team_plannin
       expect(doc.at_css('img')['data-canonical-src']).to eq src
     end
 
+    it 'replaces by default, even strings that do not look like URLs' do
+      src     = 'oigjsie8787%$**(#(%0'
+      new_src = 'https://assets.example.com/1b893f9a71d66c99437f27e19b9a061a6f5d9391/6f69676a7369653837383725242a2a2823282530'
+      doc     = filter(image(src), @context)
+
+      expect(doc.at_css('img')['src']).to eq new_src
+      expect(doc.at_css('img')['data-canonical-src']).to eq src
+    end
+
+    it 'replaces URL with non-ASCII characters' do
+      src     = 'https://example.com/x?¬'
+      new_src = 'https://assets.example.com/2f29a8c7f13f3ae14dc18c154dbbd657d703e75f/68747470733a2f2f6578616d706c652e636f6d2f783fc2ac'
+      doc     = filter(image(src), @context)
+
+      expect(doc.at_css('img')['src']).to eq new_src
+      expect(doc.at_css('img')['data-canonical-src']).to eq src
+    end
+
+    it 'replaces out-of-spec URL that would still be rendered in the browser' do
+      src     = 'https://example.com/##'
+      new_src = 'https://assets.example.com/d7d0c845cc553d9430804c07e9456545ef3e6fe6/68747470733a2f2f6578616d706c652e636f6d2f2323'
+      doc     = filter(image(src), @context)
+
+      expect(doc.at_css('img')['src']).to eq new_src
+      expect(doc.at_css('img')['data-canonical-src']).to eq src
+    end
+
     it 'skips internal images' do
       src      = "#{Gitlab.config.gitlab.url}/test.png"
       doc      = filter(image(src), @context)
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index aedfc7fca53..46bf80b1e8f 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -6438,6 +6438,8 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr
     let!(:merge_request) do
       create(
         :merge_request,
+        author: author,
+        state_id: state_id,
         target_project: target_project,
         target_branch: 'target-branch',
         source_project: project,
@@ -6446,6 +6448,9 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr
       )
     end
 
+    let(:author) { project.creator }
+    let(:state_id) { MergeRequest.available_states[:opened] }
+
     before do
       target_project.add_developer(user)
     end
@@ -6455,10 +6460,12 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr
         expect(project.merge_requests_allowing_push_to_user(user)).to include(merge_request)
       end
 
-      it 'does not include closed merge requests' do
-        merge_request.close
+      context 'when the merge requests are closed' do
+        let(:state_id) { MergeRequest.available_states[:closed] }
 
-        expect(project.merge_requests_allowing_push_to_user(user)).to be_empty
+        it 'does not include closed merge requests' do
+          expect(project.merge_requests_allowing_push_to_user(user)).to be_empty
+        end
       end
 
       it 'does not include merge requests for guest users' do
@@ -6480,16 +6487,38 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr
     end
 
     describe '#any_branch_allows_collaboration?' do
-      it 'allows access when there are merge requests open allowing collaboration', :sidekiq_might_not_need_inline do
-        expect(project.any_branch_allows_collaboration?(user))
-          .to be_truthy
-      end
+      context 'when there is an open merge request allowing collaboration' do
+        it 'allows access', :sidekiq_might_not_need_inline do
+          expect(project.any_branch_allows_collaboration?(user))
+            .to be_truthy
+        end
+
+        context 'when the merge request author is not allowed to push_code' do
+          let(:author) { create(:user) }
 
-      it 'does not allow access when there are no merge requests open allowing collaboration' do
-        merge_request.close!
+          it 'returns false' do
+            expect(project.any_branch_allows_collaboration?(user))
+              .to be_falsey
+          end
+        end
 
-        expect(project.any_branch_allows_collaboration?(user))
-          .to be_falsey
+        context 'when the merge request is closed' do
+          let(:state_id) { MergeRequest.available_states[:closed] }
+
+          it 'returns false' do
+            expect(project.any_branch_allows_collaboration?(user))
+              .to be_falsey
+          end
+        end
+
+        context 'when the merge request is merged' do
+          let(:state_id) { MergeRequest.available_states[:merged] }
+
+          it 'returns false' do
+            expect(project.any_branch_allows_collaboration?(user))
+              .to be_falsey
+          end
+        end
       end
     end
 
@@ -6537,6 +6566,15 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr
             .not_to exceed_query_limit(control).with_threshold(2)
         end
       end
+
+      context 'when the merge request author is not allowed to push_code' do
+        let(:author) { create(:user) }
+
+        it 'returns false' do
+          expect(project.branch_allows_collaboration?(user, 'awesome-feature-1'))
+            .to be_falsey
+        end
+      end
     end
   end
 
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index 8b9ac7cd588..90595c2d7f9 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -774,6 +774,62 @@ RSpec.describe API::Commits, feature_category: :source_code_management do
               end
             end
 
+            context 'when project repository access becomes restricted after being forked' do
+              let!(:fork_owner) { create(:user) }
+              let!(:forked_project) { fork_project(public_project, fork_owner, namespace: fork_owner.namespace, repository: true) }
+              let(:url) { "/projects/#{forked_project.id}/repository/commits" }
+
+              before do
+                # Restrict repository visibility of the public project
+                public_project.merge_requests_access_level = 'private'
+                public_project.builds_access_level = 'private'
+                public_project.repository_access_level = 'private'
+                public_project.save!
+
+                valid_c_params[:start_branch] = 'master'
+                valid_c_params[:branch] = 'patch'
+                valid_c_params[:start_project] = public_project.id
+              end
+
+              after do
+                # Reopen repository visibility of the public project
+                public_project.merge_requests_access_level = 'enabled'
+                public_project.repository_access_level = 'enabled'
+                public_project.builds_access_level = 'enabled'
+                public_project.save!
+              end
+
+              it 'returns a 403' do
+                post api(url, fork_owner), params: valid_c_params
+
+                expect(response).to have_gitlab_http_status(:forbidden)
+              end
+            end
+
+            context 'when fork owner has no more access to a private repository' do
+              let_it_be(:private_project) { create(:project, :private, :repository) }
+              let_it_be(:fork_owner) { create(:user) }
+              let_it_be(:fork_owner_membership) { private_project.add_developer(fork_owner) }
+              let_it_be(:forked_project) { fork_project(private_project, fork_owner, namespace: fork_owner.namespace, repository: true) }
+              let(:url) { "/projects/#{forked_project.id}/repository/commits" }
+
+              before do
+                # Restrict user from repository
+                Members::DestroyService.new(private_project.owner).execute(fork_owner_membership)
+                Sidekiq::Worker.drain_all
+
+                valid_c_params[:start_branch] = 'master'
+                valid_c_params[:branch] = 'patch'
+                valid_c_params[:start_project] = private_project.id
+              end
+
+              it 'returns a 402' do
+                post api(url, fork_owner), params: valid_c_params
+
+                expect(response).to have_gitlab_http_status(:not_found)
+              end
+            end
+
             context 'when the target project is not part of the fork network of start_project' do
               let(:unrelated_project) { create(:project, :public, :repository, creator: guest) }
               let(:url) { "/projects/#{unrelated_project.id}/repository/commits" }
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index e3e8df79a1d..12898060e22 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -3292,6 +3292,10 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and
         let(:failed_status_code) { :not_found }
       end
 
+      it 'refreshes the forks count cache' do
+        expect(project_fork_source.forks_count).to be_zero
+      end
+
       context 'user is a developer' do
         before do
           project_fork_target.add_developer(user)
@@ -3304,15 +3308,23 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and
         end
       end
 
-      it 'refreshes the forks count cache' do
-        expect(project_fork_source.forks_count).to be_zero
-      end
-
       context 'user is maintainer' do
         before do
           project_fork_target.add_maintainer(user)
         end
 
+        it 'denies project to be forked from an existing project' do
+          post api(path, user)
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
+      end
+
+      context 'user is owner' do
+        before do
+          project_fork_target.add_owner(user)
+        end
+
         context 'and user is a reporter of target group' do
           let_it_be_with_reload(:target_group) { create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) }
           let_it_be_with_reload(:project_fork_target) { create(:project, namespace: target_group) }
diff --git a/spec/services/error_tracking/list_projects_service_spec.rb b/spec/services/error_tracking/list_projects_service_spec.rb
index d91808edc8d..53dbbfd8c71 100644
--- a/spec/services/error_tracking/list_projects_service_spec.rb
+++ b/spec/services/error_tracking/list_projects_service_spec.rb
@@ -19,7 +19,7 @@ RSpec.describe ErrorTracking::ListProjectsService, feature_category: :integratio
   subject { described_class.new(project, user, params) }
 
   before do
-    project.add_reporter(user)
+    project.add_maintainer(user)
   end
 
   describe '#execute' do
@@ -137,6 +137,16 @@ RSpec.describe ErrorTracking::ListProjectsService, feature_category: :integratio
       end
     end
 
+    context 'with user with insufficient permissions' do
+      before do
+        project.add_developer(user)
+      end
+
+      it 'returns error' do
+        expect(result).to include(status: :error, message: 'Access denied', http_status: :unauthorized)
+      end
+    end
+
     context 'with error tracking disabled' do
       before do
         expect(project).to receive(:error_tracking_setting).at_least(:once)