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

11 files changed, 132 insertions, 50 deletions

diff --git a/app/controllers/projects/refs_controller.rb b/app/controllers/projects/refs_controller.rb
index 4c2bd2a9d42..278d306301a 100644
--- a/app/controllers/projects/refs_controller.rb
+++ b/app/controllers/projects/refs_controller.rb
@@ -15,6 +15,8 @@ class Projects::RefsController < Projects::ApplicationController
   urgency :low, [:switch, :logs_tree]
 
   def switch
+    Gitlab::PathTraversal.check_path_traversal!(@id)
+
     respond_to do |format|
       format.html do
         new_path =
@@ -40,6 +42,8 @@ class Projects::RefsController < Projects::ApplicationController
         redirect_to new_path
       end
     end
+  rescue Gitlab::PathTraversal::PathTraversalAttackError
+    head :bad_request
   end
 
   def logs_tree
diff --git a/app/graphql/resolvers/group_issues_resolver.rb b/app/graphql/resolvers/group_issues_resolver.rb
index 43f01395896..7bbc662c6c8 100644
--- a/app/graphql/resolvers/group_issues_resolver.rb
+++ b/app/graphql/resolvers/group_issues_resolver.rb
@@ -9,6 +9,11 @@ module Resolvers
 
     include GroupIssuableResolver
 
+    before_connection_authorization do |nodes, _|
+      projects = nodes.map(&:project)
+      ActiveRecord::Associations::Preloader.new(records: projects, associations: :namespace).call
+    end
+
     def ready?(**args)
       if args.dig(:not, :release_tag).present?
         raise ::Gitlab::Graphql::Errors::ArgumentError, 'releaseTag filter is not allowed when parent is a group.'
diff --git a/app/graphql/resolvers/issues_resolver.rb b/app/graphql/resolvers/issues_resolver.rb
index 17e3e159a5b..589366ba26d 100644
--- a/app/graphql/resolvers/issues_resolver.rb
+++ b/app/graphql/resolvers/issues_resolver.rb
@@ -23,6 +23,7 @@ module Resolvers
       projects = nodes.map(&:project)
       ::Preloaders::UserMaxAccessLevelInProjectsPreloader.new(projects, current_user).execute
       ::Preloaders::GroupPolicyPreloader.new(projects.filter_map(&:group), current_user).execute
+      ActiveRecord::Associations::Preloader.new(records: projects, associations: :namespace).call
     end
 
     def ready?(**args)
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index f6a2ce0f829..6d13512aad6 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -691,6 +691,7 @@ module API
       desc 'Mark this project as forked from another' do
         success code: 201, model: Entities::Project
         failure [
+          { code: 401, message: 'Unauthorized' },
           { code: 403, message: 'Unauthenticated' },
           { code: 404, message: 'Not found' }
         ]
@@ -708,7 +709,11 @@ module API
 
         authorize! :fork_project, fork_from_project
 
-        result = ::Projects::ForkService.new(fork_from_project, current_user).execute(user_project)
+        service = ::Projects::ForkService.new(fork_from_project, current_user)
+
+        unauthorized!('Target Namespace') unless service.valid_fork_target?(user_project.namespace)
+
+        result = service.execute(user_project)
 
         if result
           present_project user_project.reset, with: Entities::Project, current_user: current_user
diff --git a/lib/gitlab/pagination/gitaly_keyset_pager.rb b/lib/gitlab/pagination/gitaly_keyset_pager.rb
index 6235874132f..82d6fc64d89 100644
--- a/lib/gitlab/pagination/gitaly_keyset_pager.rb
+++ b/lib/gitlab/pagination/gitaly_keyset_pager.rb
@@ -15,7 +15,7 @@ module Gitlab
       # It is expected that the given finder will respond to `execute` method with `gitaly_pagination:` option
       # and supports pagination via gitaly.
       def paginate(finder)
-        return finder.execute(gitaly_pagination: false) if no_pagination?
+        return finder.execute(gitaly_pagination: false) if no_pagination?(finder)
 
         return paginate_via_gitaly(finder) if keyset_pagination_enabled?(finder)
         return paginate_first_page_via_gitaly(finder) if paginate_first_page?(finder)
@@ -28,8 +28,8 @@ module Gitlab
 
       private
 
-      def no_pagination?
-        params[:pagination] == 'none'
+      def no_pagination?(finder)
+        params[:pagination] == 'none' && finder.is_a?(::Repositories::TreeFinder)
       end
 
       def keyset_pagination_enabled?(finder)
diff --git a/spec/controllers/projects/refs_controller_spec.rb b/spec/controllers/projects/refs_controller_spec.rb
index 0b1d0b75de7..7ea0e678a41 100644
--- a/spec/controllers/projects/refs_controller_spec.rb
+++ b/spec/controllers/projects/refs_controller_spec.rb
@@ -12,40 +12,70 @@ RSpec.describe Projects::RefsController, feature_category: :source_code_manageme
   end
 
   describe 'GET #switch' do
-    using RSpec::Parameterized::TableSyntax
+    context 'with normal parameters' do
+      using RSpec::Parameterized::TableSyntax
 
-    let(:id) { 'master' }
-    let(:params) do
-      { destination: destination, namespace_id: project.namespace.to_param, project_id: project, id: id,
-        ref_type: ref_type }
-    end
+      let(:id) { 'master' }
+      let(:id_and_path) { "#{id}/#{path}" }
+
+      let(:params) do
+        { destination: destination, namespace_id: project.namespace.to_param, project_id: project, id: id,
+          ref_type: ref_type, path: path }
+      end
+
+      subject { get :switch, params: params }
+
+      where(:destination, :ref_type, :path, :redirected_to) do
+        'tree'           | nil     | nil | lazy { project_tree_path(project, id) }
+        'tree'           | 'heads' | nil | lazy { project_tree_path(project, id) }
+        'tree'           | nil     | 'foo/bar' | lazy { project_tree_path(project, id_and_path) }
+        'blob'           | nil     | nil | lazy { project_blob_path(project, id) }
+        'blob'           | 'heads' | nil | lazy { project_blob_path(project, id) }
+        'blob'           | nil     | 'foo/bar' | lazy { project_blob_path(project, id_and_path) }
+        'graph'          | nil     | nil | lazy { project_network_path(project, id) }
+        'graph'          | 'heads' | nil | lazy { project_network_path(project, id, ref_type: 'heads') }
+        'graph'          | nil     | 'foo/bar' | lazy { project_network_path(project, id_and_path) }
+        'graphs'         | nil     | nil | lazy { project_graph_path(project, id) }
+        'graphs'         | 'heads' | nil | lazy { project_graph_path(project, id, ref_type: 'heads') }
+        'graphs'         | nil     | 'foo/bar' | lazy { project_graph_path(project, id_and_path) }
+        'find_file'      | nil     | nil | lazy { project_find_file_path(project, id) }
+        'find_file'      | 'heads' | nil | lazy { project_find_file_path(project, id) }
+        'find_file'      | nil     | 'foo/bar' | lazy { project_find_file_path(project, id_and_path) }
+        'graphs_commits' | nil     | nil | lazy { commits_project_graph_path(project, id) }
+        'graphs_commits' | 'heads' | nil | lazy { commits_project_graph_path(project, id) }
+        'graphs_commits' | nil     | 'foo/bar' | lazy { commits_project_graph_path(project, id_and_path) }
+        'badges'         | nil     | nil | lazy { project_settings_ci_cd_path(project, ref: id) }
+        'badges'         | 'heads' | nil | lazy { project_settings_ci_cd_path(project, ref: id) }
+        'badges'         | nil     | 'foo/bar' | lazy { project_settings_ci_cd_path(project, ref: id_and_path) }
+        'commits'        | nil     | nil | lazy { project_commits_path(project, id) }
+        'commits'        | 'heads' | nil | lazy { project_commits_path(project, id, ref_type: 'heads') }
+        'commits'        | nil     | 'foo/bar' | lazy { project_commits_path(project, id_and_path) }
+        nil              | nil     | nil | lazy { project_commits_path(project, id) }
+        nil              | 'heads' | nil | lazy { project_commits_path(project, id, ref_type: 'heads') }
+        nil              | nil     | 'foo/bar' | lazy { project_commits_path(project, id_and_path) }
+      end
 
-    subject { get :switch, params: params }
-
-    where(:destination, :ref_type, :redirected_to) do
-      'tree'           | nil     | lazy { project_tree_path(project, id) }
-      'tree'           | 'heads' | lazy { project_tree_path(project, id) }
-      'blob'           | nil     | lazy { project_blob_path(project, id) }
-      'blob'           | 'heads' | lazy { project_blob_path(project, id) }
-      'graph'          | nil     | lazy { project_network_path(project, id) }
-      'graph'          | 'heads' | lazy { project_network_path(project, id, ref_type: 'heads') }
-      'graphs'         | nil     | lazy { project_graph_path(project, id) }
-      'graphs'         | 'heads' | lazy { project_graph_path(project, id, ref_type: 'heads') }
-      'find_file'      | nil     | lazy { project_find_file_path(project, id) }
-      'find_file'      | 'heads' | lazy { project_find_file_path(project, id) }
-      'graphs_commits' | nil     | lazy { commits_project_graph_path(project, id) }
-      'graphs_commits' | 'heads' | lazy { commits_project_graph_path(project, id) }
-      'badges'         | nil     | lazy { project_settings_ci_cd_path(project, ref: id) }
-      'badges'         | 'heads' | lazy { project_settings_ci_cd_path(project, ref: id) }
-      'commits'        | nil     | lazy { project_commits_path(project, id) }
-      'commits'        | 'heads' | lazy { project_commits_path(project, id, ref_type: 'heads') }
-      nil              | nil     | lazy { project_commits_path(project, id) }
-      nil              | 'heads' | lazy { project_commits_path(project, id, ref_type: 'heads') }
+      with_them do
+        it 'redirects to destination' do
+          expect(subject).to redirect_to(redirected_to)
+        end
+      end
     end
 
-    with_them do
-      it 'redirects to destination' do
-        expect(subject).to redirect_to(redirected_to)
+    context 'with bad path parameter' do
+      it 'returns 400 bad request' do
+        params = {
+          destination: 'tree',
+          namespace_id: project.namespace.to_param,
+          project_id: project,
+          id: 'master',
+          ref_type: nil,
+          path: '../bad_path_redirect'
+        }
+
+        get :switch, params: params
+
+        expect(response).to have_gitlab_http_status(:bad_request)
       end
     end
   end
diff --git a/spec/helpers/nav/new_dropdown_helper_spec.rb b/spec/helpers/nav/new_dropdown_helper_spec.rb
index 26dadd3b4f1..4ec120d152b 100644
--- a/spec/helpers/nav/new_dropdown_helper_spec.rb
+++ b/spec/helpers/nav/new_dropdown_helper_spec.rb
@@ -22,6 +22,7 @@ RSpec.describe Nav::NewDropdownHelper, feature_category: :navigation do
       allow(helper).to receive(:can?).and_return(false)
       allow(user).to receive(:can_create_project?) { with_can_create_project }
       allow(user).to receive(:can_create_group?) { with_can_create_group }
+      allow(user).to receive(:can?).and_call_original
       allow(user).to receive(:can?).with(:create_snippet) { with_can_create_snippet }
     end
 
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index aa064a26ec4..e1537c7b287 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -1170,6 +1170,7 @@ RSpec.describe ProjectsHelper, feature_category: :source_code_management do
         current_user = user if has_user
 
         allow(helper).to receive(:current_user).and_return(current_user)
+        allow(user).to receive(:can?).and_call_original
         allow(user).to receive(:can?).with(:fork_project, project).and_return(true)
         allow(user).to receive(:can?).with(:create_fork).and_return(true)
         allow(user).to receive(:can?).with(:create_projects, anything).and_return(true)
diff --git a/spec/lib/gitlab/pagination/gitaly_keyset_pager_spec.rb b/spec/lib/gitlab/pagination/gitaly_keyset_pager_spec.rb
index 2e87c582040..cb3f1fe86dc 100644
--- a/spec/lib/gitlab/pagination/gitaly_keyset_pager_spec.rb
+++ b/spec/lib/gitlab/pagination/gitaly_keyset_pager_spec.rb
@@ -127,17 +127,27 @@ RSpec.describe Gitlab::Pagination::GitalyKeysetPager do
       end
     end
 
-    context 'with "none" pagination option' do
+    context "with 'none' pagination option" do
       let(:expected_result) { double(:result) }
       let(:query) { { pagination: 'none' } }
 
-      it 'uses offset pagination' do
-        expect(finder).to receive(:execute).with(gitaly_pagination: false).and_return(expected_result)
-        expect(Kaminari).not_to receive(:paginate_array)
-        expect(Gitlab::Pagination::OffsetPagination).not_to receive(:new)
+      context "with a finder that is not a TreeFinder" do
+        it_behaves_like 'offset pagination'
+      end
+
+      context "with a finder that is a TreeFinder" do
+        before do
+          allow(finder).to receive(:is_a?).with(::Repositories::TreeFinder).and_return(true)
+        end
 
-        actual_result = pager.paginate(finder)
-        expect(actual_result).to eq(expected_result)
+        it "doesn't uses offset pagination" do
+          expect(finder).to receive(:execute).with(gitaly_pagination: false).and_return(expected_result)
+          expect(Kaminari).not_to receive(:paginate_array)
+          expect(Gitlab::Pagination::OffsetPagination).not_to receive(:new)
+
+          actual_result = pager.paginate(finder)
+          expect(actual_result).to eq(expected_result)
+        end
       end
     end
   end
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index 26132215404..601e8cb3081 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -3311,16 +3311,41 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and
           project_fork_target.add_maintainer(user)
         end
 
-        it 'allows project to be forked from an existing project' do
-          expect(project_fork_target).not_to be_forked
+        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) }
 
-          post api(path, user)
-          project_fork_target.reload
+          before do
+            target_group.add_reporter(user)
+          end
 
-          expect(response).to have_gitlab_http_status(:created)
-          expect(project_fork_target.forked_from_project.id).to eq(project_fork_source.id)
-          expect(project_fork_target.fork_network_member).to be_present
-          expect(project_fork_target).to be_forked
+          it 'fails as target namespace is unauthorized' do
+            post api(path, user)
+
+            expect(response).to have_gitlab_http_status(:unauthorized)
+            expect(json_response['message']).to eq "401 Unauthorized - Target Namespace"
+          end
+        end
+
+        context 'and user is a developer 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) }
+
+          before do
+            target_group.add_developer(user)
+          end
+
+          it 'allows project to be forked from an existing project' do
+            expect(project_fork_target).not_to be_forked
+
+            post api(path, user)
+            project_fork_target.reload
+
+            expect(response).to have_gitlab_http_status(:created)
+            expect(project_fork_target.forked_from_project.id).to eq(project_fork_source.id)
+            expect(project_fork_target.fork_network_member).to be_present
+            expect(project_fork_target).to be_forked
+          end
         end
 
         it 'fails without permission from forked_from project' do
diff --git a/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb b/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb
index cbd0ffbab21..f2052f4202d 100644
--- a/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb
+++ b/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb
@@ -254,7 +254,7 @@ RSpec.shared_examples 'a redacted search results' do
     end
 
     context 'with :with_api_entity_associations' do
-      it_behaves_like "redaction limits N+1 queries", limit: 14
+      it_behaves_like "redaction limits N+1 queries", limit: 15
     end
   end