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

9 files changed, 107 insertions, 21 deletions

diff --git a/app/mailers/emails/service_desk.rb b/app/mailers/emails/service_desk.rb
index 9f3611df2cc..f6595a91bee 100644
--- a/app/mailers/emails/service_desk.rb
+++ b/app/mailers/emails/service_desk.rb
@@ -211,7 +211,11 @@ module Emails
     end
 
     def issue_description
-      @issue.description_html.to_s
+      return '' if @issue.description_html.blank?
+
+      # Remove references etc. from description HTML because external participants
+      # are no regular users and don't have permission to access them.
+      ::Banzai::Renderer.post_process(@issue.description_html, {})
     end
 
     def subject_base
diff --git a/lib/gitlab/import_export/command_line_util.rb b/lib/gitlab/import_export/command_line_util.rb
index dfe0815f0a0..ea91b01afdb 100644
--- a/lib/gitlab/import_export/command_line_util.rb
+++ b/lib/gitlab/import_export/command_line_util.rb
@@ -141,7 +141,7 @@ module Gitlab
 
           raise HardLinkError, 'File shares hard link' if Gitlab::Utils::FileInfo.shares_hard_link?(filepath)
 
-          FileUtils.rm(filepath) if Gitlab::Utils::FileInfo.linked?(filepath)
+          FileUtils.rm(filepath) if Gitlab::Utils::FileInfo.linked?(filepath) || File.pipe?(filepath)
         end
 
         true
diff --git a/lib/gitlab/import_export/project/relation_factory.rb b/lib/gitlab/import_export/project/relation_factory.rb
index 943c997a056..8e34a6d73ba 100644
--- a/lib/gitlab/import_export/project/relation_factory.rb
+++ b/lib/gitlab/import_export/project/relation_factory.rb
@@ -81,6 +81,8 @@ module Gitlab
 
         private
 
+        attr_reader :relation_hash, :user
+
         def invalid_relation?
           # Do not create relation if it is a legacy trigger
           legacy_trigger?
diff --git a/lib/gitlab/search/abuse_detection.rb b/lib/gitlab/search/abuse_detection.rb
index 1e4169f3fd7..1fd7c6cfe8d 100644
--- a/lib/gitlab/search/abuse_detection.rb
+++ b/lib/gitlab/search/abuse_detection.rb
@@ -6,6 +6,7 @@ module Gitlab
       include ActiveModel::Validations
       include AbuseValidators
 
+      MAX_PIPE_SYNTAX_FILTERS = 5
       ABUSIVE_TERM_SIZE = 100
       ALLOWED_CHARS_REGEX = %r{\A[[:alnum:]_\-\/\.!]+\z}
 
@@ -57,10 +58,18 @@ module Gitlab
 
       validates :query_string, :repository_ref, :project_ref, no_abusive_coercion_from_string: true
 
-      attr_reader(*READABLE_PARAMS)
+      validate :no_abusive_pipes, if: :detect_abusive_pipes
 
-      def initialize(params)
-        READABLE_PARAMS.each { |p| instance_variable_set("@#{p}", params[p]) }
+      attr_reader(*READABLE_PARAMS)
+      attr_reader :raw_params, :detect_abusive_pipes
+
+      def initialize(params, detect_abusive_pipes: true)
+        @raw_params = {}
+        READABLE_PARAMS.each do |p|
+          instance_variable_set("@#{p}", params[p])
+          @raw_params[p] = params[p]
+        end
+        @detect_abusive_pipes = detect_abusive_pipes
       end
 
       private
@@ -76,6 +85,23 @@ module Gitlab
       def stop_word_search?
         STOP_WORDS.include? query_string
       end
+
+      def no_abusive_pipes
+        pipes = query_string.to_s.split('|')
+        errors.add(:query_string, 'too many pipe syntax filters') if pipes.length > MAX_PIPE_SYNTAX_FILTERS
+
+        pipes.each do |q|
+          self.class.new(raw_params.merge(query_string: q), detect_abusive_pipes: false).tap do |p|
+            p.validate
+
+            p.errors.messages_for(:query_string).each do |msg|
+              next if errors.added?(:query_string, msg)
+
+              errors.add(:query_string, msg)
+            end
+          end
+        end
+      end
     end
   end
 end
diff --git a/lib/gitlab/search/params.rb b/lib/gitlab/search/params.rb
index 6eb24a92be6..a7896b7d80d 100644
--- a/lib/gitlab/search/params.rb
+++ b/lib/gitlab/search/params.rb
@@ -81,7 +81,7 @@ module Gitlab
       end
 
       def search_terms
-        @search_terms ||= query_string.split.select { |word| word.length >= MIN_TERM_LENGTH }
+        @search_terms ||= query_string.split
       end
 
       def not_too_many_terms
diff --git a/spec/lib/gitlab/import_export/command_line_util_spec.rb b/spec/lib/gitlab/import_export/command_line_util_spec.rb
index 76a35d07c7f..42c3b170e4d 100644
--- a/spec/lib/gitlab/import_export/command_line_util_spec.rb
+++ b/spec/lib/gitlab/import_export/command_line_util_spec.rb
@@ -84,6 +84,20 @@ RSpec.describe Gitlab::ImportExport::CommandLineUtil, feature_category: :importe
     end
   end
 
+  shared_examples 'deletes pipes' do |compression, decompression|
+    it 'deletes the pipes', :aggregate_failures do
+      FileUtils.touch("#{source_dir}/file.txt")
+      File.mkfifo("#{source_dir}/pipe")
+
+      archive_file = File.join(archive_dir, 'file_with_pipes.tar.gz')
+      subject.public_send(compression, archive: archive_file, dir: source_dir)
+      subject.public_send(decompression, archive: archive_file, dir: target_dir)
+
+      expect(File).to exist("#{target_dir}/file.txt")
+      expect(File).not_to exist("#{target_dir}/pipe")
+    end
+  end
+
   describe '#download_or_copy_upload' do
     let(:upload) { instance_double(Upload, local?: local) }
     let(:uploader) { instance_double(ImportExportUploader, path: :path, url: :url, upload: upload) }
@@ -302,6 +316,7 @@ RSpec.describe Gitlab::ImportExport::CommandLineUtil, feature_category: :importe
 
     it_behaves_like 'deletes symlinks', :tar_czf, :untar_zxf
     it_behaves_like 'handles shared hard links', :tar_czf, :untar_zxf
+    it_behaves_like 'deletes pipes', :tar_czf, :untar_zxf
 
     it 'has the right mask for project.json' do
       subject.untar_zxf(archive: tar_archive_fixture, dir: target_dir)
@@ -321,6 +336,7 @@ RSpec.describe Gitlab::ImportExport::CommandLineUtil, feature_category: :importe
 
     it_behaves_like 'deletes symlinks', :tar_cf, :untar_xf
     it_behaves_like 'handles shared hard links', :tar_cf, :untar_xf
+    it_behaves_like 'deletes pipes', :tar_czf, :untar_zxf
 
     it 'extracts archive without decompression' do
       filename = 'archive.tar.gz'
diff --git a/spec/lib/gitlab/search/abuse_detection_spec.rb b/spec/lib/gitlab/search/abuse_detection_spec.rb
index f9a1d0211b9..cbf20614ba5 100644
--- a/spec/lib/gitlab/search/abuse_detection_spec.rb
+++ b/spec/lib/gitlab/search/abuse_detection_spec.rb
@@ -10,12 +10,12 @@ RSpec.describe Gitlab::Search::AbuseDetection, feature_category: :global_search
   describe 'abusive scopes validation' do
     it 'allows only approved scopes' do
       described_class::ALLOWED_SCOPES.each do |scope|
-        expect(described_class.new(scope: scope)).to be_valid
+        expect(described_class.new({ scope: scope })).to be_valid
       end
     end
 
     it 'disallows anything not approved' do
-      expect(described_class.new(scope: 'nope')).not_to be_valid
+      expect(described_class.new({ scope: 'nope' })).not_to be_valid
     end
   end
 
@@ -55,14 +55,14 @@ RSpec.describe Gitlab::Search::AbuseDetection, feature_category: :global_search
     it 'considers non Integers to be invalid' do
       [:project_id, :group_id].each do |param|
         [[1, 2, 3], 'xyz', 3.14, { foo: :bar }].each do |dtype|
-          expect(described_class.new(param => dtype)).not_to be_valid
+          expect(described_class.new({ param => dtype })).not_to be_valid
         end
       end
     end
 
     it 'considers Integers to be valid' do
       [:project_id, :group_id].each do |param|
-        expect(described_class.new(param => 123)).to be_valid
+        expect(described_class.new({ param => 123 })).to be_valid
       end
     end
   end
@@ -70,7 +70,7 @@ RSpec.describe Gitlab::Search::AbuseDetection, feature_category: :global_search
   describe 'query_string validation' do
     using ::RSpec::Parameterized::TableSyntax
 
-    subject { described_class.new(query_string: search) }
+    subject { described_class.new({ query_string: search }) }
 
     let(:validation_errors) do
       subject.validate
@@ -82,11 +82,15 @@ RSpec.describe Gitlab::Search::AbuseDetection, feature_category: :global_search
         word | { query_string: ['stopword only abusive search detected'] }
       end
 
-      'x'                                        | { query_string: ['abusive tiny search detected'] }
-      ('x' * described_class::ABUSIVE_TERM_SIZE) | { query_string: ['abusive term length detected'] }
-      ''                                         | {}
-      '*'                                        | {}
-      'ruby'                                     | {}
+      (['apples'] * (described_class::MAX_PIPE_SYNTAX_FILTERS + 1)).join('|') | { query_string: ['too many pipe syntax filters'] } # rubocop:disable Layout/LineLength
+      (['apples'] * described_class::MAX_PIPE_SYNTAX_FILTERS).join('|')       | {}
+      'x'                                                   | { query_string: ['abusive tiny search detected'] }
+      'apples|x'                                            | { query_string: ['abusive tiny search detected'] }
+      ('x' * described_class::ABUSIVE_TERM_SIZE)            | { query_string: ['abusive term length detected'] }
+      "apples|#{'x' * described_class::ABUSIVE_TERM_SIZE}"  | { query_string: ['abusive term length detected'] }
+      ''                                                    | {}
+      '*'                                                   | {}
+      'ruby'                                                | {}
     end
 
     with_them do
@@ -100,14 +104,14 @@ RSpec.describe Gitlab::Search::AbuseDetection, feature_category: :global_search
     it 'considers anything not a String invalid' do
       [:query_string, :scope, :repository_ref, :project_ref].each do |param|
         [[1, 2, 3], 123, 3.14, { foo: :bar }].each do |dtype|
-          expect(described_class.new(param => dtype)).not_to be_valid
+          expect(described_class.new({ param => dtype })).not_to be_valid
         end
       end
     end
 
     it 'considers Strings to be valid' do
       [:query_string, :repository_ref, :project_ref].each do |param|
-        expect(described_class.new(param => "foo")).to be_valid
+        expect(described_class.new({ param => "foo" })).to be_valid
       end
     end
   end
diff --git a/spec/lib/gitlab/search/params_spec.rb b/spec/lib/gitlab/search/params_spec.rb
index 3235a0b2126..3c64082aeeb 100644
--- a/spec/lib/gitlab/search/params_spec.rb
+++ b/spec/lib/gitlab/search/params_spec.rb
@@ -17,7 +17,7 @@ RSpec.describe Gitlab::Search::Params, feature_category: :global_search do
     end
 
     it 'uses AbuseDetection by default' do
-      expect(Gitlab::Search::AbuseDetection).to receive(:new).and_call_original
+      expect(Gitlab::Search::AbuseDetection).to receive(:new).at_least(:once).and_call_original
       described_class.new(params)
     end
   end
@@ -73,9 +73,21 @@ RSpec.describe Gitlab::Search::Params, feature_category: :global_search do
     end
 
     it 'validates AbuseDetector on validation' do
-      expect(Gitlab::Search::AbuseDetection).to receive(:new).and_call_original
+      expect(Gitlab::Search::AbuseDetection).to receive(:new).at_least(:once).and_call_original
       subject.validate
     end
+
+    context 'when query has too many terms' do
+      let(:search) { Array.new((::Gitlab::Search::Params::SEARCH_TERM_LIMIT + 1), 'a').join(' ') }
+
+      it { is_expected.not_to be_valid }
+    end
+
+    context 'when query is too long' do
+      let(:search) { 'a' * (::Gitlab::Search::Params::SEARCH_CHAR_LIMIT + 1) }
+
+      it { is_expected.not_to be_valid }
+    end
   end
 
   describe '#valid?' do
@@ -89,7 +101,7 @@ RSpec.describe Gitlab::Search::Params, feature_category: :global_search do
     end
 
     it 'validates AbuseDetector on validation' do
-      expect(Gitlab::Search::AbuseDetection).to receive(:new).and_call_original
+      expect(Gitlab::Search::AbuseDetection).to receive(:new).at_least(:once).and_call_original
       subject.valid?
     end
   end
diff --git a/spec/mailers/emails/service_desk_spec.rb b/spec/mailers/emails/service_desk_spec.rb
index e3fe36237df..b700819ed2c 100644
--- a/spec/mailers/emails/service_desk_spec.rb
+++ b/spec/mailers/emails/service_desk_spec.rb
@@ -263,6 +263,28 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do
         let(:expected_template_html) { "<p dir=\"auto\">thank you, your new issue has been created. </p>#{issue.description_html}" }
 
         it_behaves_like 'a service desk notification email with template content', 'thank_you'
+
+        context 'when GitLab-specific-reference is in description' do
+          let(:full_issue_reference) { "#{issue.project.full_path}#{issue.to_reference}" }
+          let(:other_issue) { create(:issue, project: project, description: full_issue_reference) }
+
+          let(:template_content) { '%{ISSUE_DESCRIPTION}' }
+          let(:expected_template_html) { "<p data-sourcepos=\"1:1-1:22\" dir=\"auto\">#{full_issue_reference}</p>" }
+
+          subject { ServiceEmailClass.service_desk_thank_you_email(other_issue.id) }
+
+          before do
+            expect(Gitlab::Template::ServiceDeskTemplate).to receive(:find)
+              .with('thank_you', other_issue.project)
+              .and_return(template)
+
+            other_issue.issue_email_participants.create!(email: email)
+          end
+
+          it 'does not render GitLab-specific-reference links with title attribute' do
+            is_expected.to have_body_text(expected_template_html)
+          end
+        end
       end
 
       context 'when issue url placeholder is used' do