Add latest changes from gitlab-org/security/gitlab@15-1-stable-ee

4 files changed, 49 insertions, 23 deletions

diff --git a/lib/banzai/filter/commit_trailers_filter.rb b/lib/banzai/filter/commit_trailers_filter.rb
index a615abc1989..817bea42757 100644
--- a/lib/banzai/filter/commit_trailers_filter.rb
+++ b/lib/banzai/filter/commit_trailers_filter.rb
@@ -17,21 +17,10 @@ module Banzai
       include ActionView::Helpers::TagHelper
       include AvatarsHelper
 
-      TRAILER_REGEXP = /(?<label>[[:alpha:]-]+-by:)/i.freeze
-      AUTHOR_REGEXP = /(?<author_name>.+)/.freeze
-      # Devise.email_regexp wouldn't work here since its designed to match
-      # against strings that only contains email addresses; the \A and \z
-      # around the expression will only match if the string being matched
-      # contains just the email nothing else.
-      MAIL_REGEXP = /&lt;(?<author_email>[^@\s]+@[^@\s]+)&gt;/.freeze
-      FILTER_REGEXP = /(?<trailer>^\s*#{TRAILER_REGEXP}\s*#{AUTHOR_REGEXP}\s+#{MAIL_REGEXP}$)/mi.freeze
-
       def call
         doc.xpath('descendant-or-self::text()').each do |node|
           content = node.to_html
 
-          next unless content.match(FILTER_REGEXP)
-
           html = trailer_filter(content)
 
           next if html == content
@@ -52,11 +41,24 @@ module Banzai
       # Returns a String with all trailer lines replaced with links to GitLab
       # users and mailto links to non GitLab users. All links have `data-trailer`
       # and `data-user` attributes attached.
+      #
+      # The code intentionally avoids using Regex for security and performance
+      # reasons: https://gitlab.com/gitlab-org/gitlab/-/issues/363734
       def trailer_filter(text)
-        text.gsub(FILTER_REGEXP) do |author_match|
-          label = $~[:label]
-          "#{label} #{parse_user($~[:author_name], $~[:author_email], label)}"
-        end
+        text.lines.map! do |line|
+          trailer, rest = line.split(':', 2)
+
+          next line unless trailer.downcase.end_with?('-by') && rest.present?
+
+          chunks = rest.split
+          author_email = chunks.pop.delete_prefix('&lt;').delete_suffix('&gt;')
+          next line unless Devise.email_regexp.match(author_email)
+
+          author_name = chunks.join(' ').strip
+          trailer = "#{trailer.strip}:"
+
+          "#{trailer} #{link_to_user_or_email(author_name, author_email, trailer)}\n"
+        end.join
       end
 
       # Find a GitLab user using the supplied email and generate
@@ -67,7 +69,7 @@ module Banzai
       # trailer - String trailer used in the commit message
       #
       # Returns a String with a link to the user.
-      def parse_user(name, email, trailer)
+      def link_to_user_or_email(name, email, trailer)
         link_to_user User.find_by_any_email(email),
           name: name,
           email: email,
diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb
index 9fb34f74c82..529d4354a1b 100644
--- a/lib/gitlab/gitaly_client/commit_service.rb
+++ b/lib/gitlab/gitaly_client/commit_service.rb
@@ -5,6 +5,8 @@ module Gitlab
     class CommitService
       include Gitlab::EncodingHelper
 
+      TREE_ENTRIES_DEFAULT_LIMIT = 1000
+
       def initialize(repository)
         @gitaly_repo = repository.gitaly_repository
         @repository = repository
@@ -112,6 +114,9 @@ module Gitlab
       end
 
       def tree_entries(repository, revision, path, recursive, pagination_params)
+        pagination_params ||= {}
+        pagination_params[:limit] ||= TREE_ENTRIES_DEFAULT_LIMIT
+
         request = Gitaly::GetTreeEntriesRequest.new(
           repository: @gitaly_repo,
           revision: encode_binary(revision),
diff --git a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb
index f7cb6b92b48..44738e5e771 100644
--- a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb
+++ b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb
@@ -18,10 +18,20 @@ RSpec.describe Banzai::Filter::CommitTrailersFilter do
   context 'detects' do
     let(:email) { FFaker::Internet.email }
 
-    it 'trailers in the form of *-by and replace users with links' do
-      doc = filter(commit_message_html)
+    context 'trailers in the form of *-by' do
+      where(:commit_trailer) do
+        ["#{FFaker::Lorem.word}-by:", "#{FFaker::Lorem.word}-BY:", "#{FFaker::Lorem.word}-By:"]
+      end
 
-      expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
+      with_them do
+        let(:trailer) { commit_trailer }
+
+        it 'replaces users with links' do
+          doc = filter(commit_message_html)
+
+          expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
+        end
+      end
     end
 
     it 'trailers prefixed with whitespaces' do
@@ -121,7 +131,14 @@ RSpec.describe Banzai::Filter::CommitTrailersFilter do
 
   context "ignores" do
     it 'commit messages without trailers' do
-      exp = message = commit_html(FFaker::Lorem.sentence)
+      exp = message = commit_html(Array.new(5) { FFaker::Lorem.sentence }.join("\n"))
+      doc = filter(message)
+
+      expect(doc.to_html).to match Regexp.escape(exp)
+    end
+
+    it 'trailers without emails' do
+      exp = message = commit_html(Array.new(5) { 'Merged-By:' }.join("\n"))
       doc = filter(message)
 
       expect(doc.to_html).to match Regexp.escape(exp)
diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb
index 3a34d39c722..eb7bbbf1646 100644
--- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb
@@ -155,10 +155,11 @@ RSpec.describe Gitlab::GitalyClient::CommitService do
     let(:recursive) { false }
     let(:pagination_params) { nil }
 
-    it 'sends a get_tree_entries message' do
+    it 'sends a get_tree_entries message with default limit' do
+      expected_pagination_params = Gitaly::PaginationParameter.new(limit: Gitlab::GitalyClient::CommitService::TREE_ENTRIES_DEFAULT_LIMIT)
       expect_any_instance_of(Gitaly::CommitService::Stub)
         .to receive(:get_tree_entries)
-        .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
+        .with(gitaly_request_with_params({ pagination_params: expected_pagination_params }), kind_of(Hash))
         .and_return([])
 
       is_expected.to eq([[], nil])
@@ -188,9 +189,10 @@ RSpec.describe Gitlab::GitalyClient::CommitService do
           pagination_cursor: pagination_cursor
         )
 
+        expected_pagination_params = Gitaly::PaginationParameter.new(limit: 3)
         expect_any_instance_of(Gitaly::CommitService::Stub)
           .to receive(:get_tree_entries)
-          .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
+          .with(gitaly_request_with_params({ pagination_params: expected_pagination_params }), kind_of(Hash))
           .and_return([response])
 
         is_expected.to eq([[], pagination_cursor])