Merge branch 'fix_validation_for_tags' into 'master'

Fix input parameter validation for tags controller

See merge request gitlab-org/gitlab!79464

app/controllers/projects/tags_controller.rb

@@ -16,13 +16,14 @@ class Projects::TagsController < Projects::ApplicationController
   # rubocop: disable CodeReuse/ActiveRecord
   def index
     begin
-      params[:sort] = params[:sort].presence || sort_value_recently_updated
+      tags_params[:sort] = tags_params[:sort].presence || sort_value_recently_updated
 
-      @sort = params[:sort]
+      @sort = tags_params[:sort]
+      @search = tags_params[:search]
 
-      @tags = TagsFinder.new(@repository, params).execute
+      @tags = TagsFinder.new(@repository, tags_params).execute
 
-      @tags = Kaminari.paginate_array(@tags).page(params[:page])
+      @tags = Kaminari.paginate_array(@tags).page(tags_params[:page])
       tag_names = @tags.map(&:name)
       @tags_pipelines = @project.ci_pipelines.latest_successful_for_refs(tag_names)
 
@@ -100,6 +101,10 @@ class Projects::TagsController < Projects::ApplicationController
 
   private
 
+  def tags_params
+    params.permit(:search, :sort, :per_page, :page_token, :page)
+  end
+
   # TODO: remove this with the release creation moved to it's own form https://gitlab.com/gitlab-org/gitlab/-/issues/214245
   def find_evidence_pipeline
     evidence_pipeline_sha = @project.repository.commit(params[:ref])&.sha

app/finders/git_refs_finder.rb

@@ -11,11 +11,11 @@ class GitRefsFinder
   attr_reader :repository, :params
 
   def search
-    @params[:search].presence
+    @params[:search].to_s.presence
   end
 
   def sort
-    @params[:sort].presence || 'name'
+    @params[:sort].to_s.presence || 'name'
   end
 
   def by_search(refs)

app/helpers/tags_helper.rb

@@ -6,12 +6,6 @@ module TagsHelper
   end
 
   def filter_tags_path(options = {})
-    exist_opts = {
-      search: params[:search],
-      sort: params[:sort]
-    }
-
-    options = exist_opts.merge(options)
     project_tags_path(@project, @id, options)
   end
 

app/views/projects/tags/index.html.haml

-- @sort ||= sort_value_recently_updated
 - page_title s_('TagsPage|Tags')
 = content_for :meta_tags do
   = auto_discovery_link_tag(:atom, project_tags_url(@project, rss_url_options), title: "#{@project.name} tags")
@@ -9,7 +8,7 @@
       = s_('TagsPage|Tags give the ability to mark specific points in history as being important')
 
     .nav-controls
-      #js-tags-sort-dropdown{ data: { filter_tags_path: filter_tags_path, sort_options: tags_sort_options_hash.to_json } }
+      #js-tags-sort-dropdown{ data: { filter_tags_path: filter_tags_path(search: @search, sort: @sort), sort_options: tags_sort_options_hash.to_json } }
       = link_to project_tags_path(@project, rss_url_options), title: _("Tags feed"), class: 'btn gl-button btn-default btn-icon has-tooltip gl-ml-auto' do
         = sprite_icon('rss', css_class: 'gl-icon qa-rss-icon')
       - if can?(current_user, :admin_tag, @project)

spec/finders/tags_finder_spec.rb

@@ -32,6 +32,14 @@ RSpec.describe TagsFinder do
 
         expect(load_tags(params).first.name).to eq('v1.0.0')
       end
+
+      context 'when sort is not a string' do
+        it 'ignores sort parameter' do
+          params = { sort: { 'invalid' => 'string' } }
+
+          expect(load_tags(params).first.name).to eq('v1.0.0')
+        end
+      end
     end
 
     context 'filter only' do
@@ -70,6 +78,13 @@ RSpec.describe TagsFinder do
         result = load_tags({ search: 'nope$' })
         expect(result.count).to eq(0)
       end
+
+      context 'when search is not a string' do
+        it 'returns no matches' do
+          result = load_tags({ search: { 'a' => 'b' } })
+          expect(result.count).to eq(0)
+        end
+      end
     end
 
     context 'filter and sort' do