Add latest changes from gitlab-org/gitlab@master

CHANGELOG.md

@@ -2,6 +2,19 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 16.10.1 (2024-03-27)
+
+### Fixed (2 changes)
+
+- [Update redis-client to v0.21.1](gitlab-org/security/gitlab@c9d6f434dbc8d5ca244d0c00d8c5cf0d9092df39)
+- [Fix new project group templates pagination](gitlab-org/security/gitlab@956b01c404e55bc92276ab7d21c63a09bc3edfb5) **GitLab Enterprise Edition**
+
+### Security (3 changes)
+
+- [Merge branch 'dchevalier2-master-patch-88770' into 'master'](gitlab-org/security/gitlab@9e621975bf405f2e66541faebf11b06a31360b5d) ([merge request](gitlab-org/security/gitlab!3936))
+- [Limit the number of emojis we will transform](gitlab-org/security/gitlab@e935e1cc26a06990832781b30827d5afa53d0194) ([merge request](gitlab-org/security/gitlab!3927))
+- [Fix stored xss in wikis using the abstract_reference_filter](gitlab-org/security/gitlab@d1bad1a4847917d5f10c883d0d2f627088a00ca5) ([merge request](gitlab-org/security/gitlab!3929))
+
 ## 16.10.0 (2024-03-20)
 
 ### Added (115 changes)
@@ -687,6 +700,17 @@ entry.
 - [Add sharding keys for system_access](gitlab-org/gitlab@62c2fd4788e62e46f1469e2f18d178840e8e3df2) ([merge request](gitlab-org/gitlab!142501))
 - [Add sharding keys for purchase](gitlab-org/gitlab@9c3843da74714c72483c17489d5d3d68ceffd2c8) ([merge request](gitlab-org/gitlab!142505))
 
+## 16.9.3 (2024-03-27)
+
+### Fixed (1 change)
+
+- [Fix new project group templates pagination](gitlab-org/security/gitlab@93a68da5a3ddc7f2f5f44658a163198a8c5da240) **GitLab Enterprise Edition**
+
+### Security (2 changes)
+
+- [Limit the number of emojis we will transform](gitlab-org/security/gitlab@41ec64318e92b428edf9796b2777dc1d8b9b3bc2) ([merge request](gitlab-org/security/gitlab!3926))
+- [Fix stored xss in wikis using the abstract_reference_filter](gitlab-org/security/gitlab@a39b0ea96cf309dfc2d8a3a73ea4a047567bd0a1) ([merge request](gitlab-org/security/gitlab!3921))
+
 ## 16.9.2 (2024-03-06)
 
 ### Fixed (2 changes)
@@ -1456,6 +1480,13 @@ entry.
 
 - [Add remediation badge to vulnerability report](gitlab-org/gitlab@e6236197509eae1bb27edf8fb2c63ccf769c2642) ([merge request](gitlab-org/gitlab!142455))
 
+## 16.8.5 (2024-03-27)
+
+### Security (2 changes)
+
+- [Limit the number of emojis we will transform](gitlab-org/security/gitlab@8d949c60d508b6cf3d558fc4f906c82b03e06748) ([merge request](gitlab-org/security/gitlab!3925))
+- [Fix stored xss in wikis using the abstract_reference_filter](gitlab-org/security/gitlab@39a9847874a56baabacfba4d832b6d30ca388baf) ([merge request](gitlab-org/security/gitlab!3922))
+
 ## 16.8.4 (2024-03-06)
 
 ### Fixed (3 changes)

GITALY_SERVER_VERSION

@@ -1 +1 @@
-ea2d6a70ad44444a5b9cb61fbb363c85f7585024
+ad9825ba4a0dc46d1b2fc3d555cce300d8f12bb2

app/assets/javascripts/main.js

@@ -227,7 +227,7 @@ requestIdleCallback(deferredInitialisation);
 
 // initialize hiding of tooltip after clicking on dropdown's links and buttons
 document
-  .querySelectorAll('a[data-toggle="dropdown"], button[data-toggle="dropdown"]')
+  .querySelectorAll('a[data-toggle="dropdown"], button[data-toggle="dropdown"], a.has-tooltip')
   .forEach((element) => {
     element.addEventListener('click', () => tooltips.hide(element));
   });

app/models/ci/catalog/resources/component.rb

@@ -26,7 +26,7 @@ module Ci
         validates :version, :catalog_resource, :project, :name, presence: true
 
         def include_path
-          "#{Settings.gitlab_ci['component_fqdn']}/#{project.full_path}/#{name}@#{version.version}"
+          "#{Gitlab.config.gitlab_ci.server_fqdn}/#{project.full_path}/#{name}@#{version.version}"
         end
       end
     end

app/models/container_repository.rb

@@ -431,11 +431,15 @@ class ContainerRepository < ApplicationRecord
     File.join(registry.path, path)
   end
 
+  # If the container registry GitLab API is available, the API
+  # does a search of tags containing the name and we filter them
+  # to find the exact match. Otherwise, we instantiate a tag.
   def tag(tag)
     if can_access_the_gitlab_api?
-      page = tags_page(name: tag, page_size: 1)
+      page = tags_page(name: tag)
+      return if page[:tags].blank?
 
-      page[:tags].first
+      page[:tags].find { |result_tag| result_tag.name == tag }
     else
       ContainerRegistry::Tag.new(self, tag)
     end

app/services/merge_requests/base_service.rb

@@ -278,9 +278,9 @@ module MergeRequests
       # Implemented in EE
     end
 
-    def remove_approval(merge_request)
-      MergeRequests::RemoveApprovalService.new(project: project, current_user: current_user)
-        .execute(merge_request)
+    def remove_approval(merge_request, user)
+      MergeRequests::RemoveApprovalService.new(project: project, current_user: user)
+        .execute(merge_request, skip_system_note: true, skip_notification: true, skip_updating_state: true)
     end
 
     def update_reviewer_state(merge_request, user, state)

app/services/merge_requests/remove_approval_service.rb

@@ -3,7 +3,7 @@
 module MergeRequests
   class RemoveApprovalService < MergeRequests::BaseService
     # rubocop: disable CodeReuse/ActiveRecord
-    def execute(merge_request)
+    def execute(merge_request, skip_updating_state: false, skip_system_note: false, skip_notification: false)
       return unless merge_request.approved_by?(current_user)
       return if merge_request.merged?
 
@@ -12,13 +12,17 @@ module MergeRequests
 
       approval = merge_request.approvals.where(user: current_user)
 
-      trigger_approval_hooks(merge_request) do
+      trigger_approval_hooks(merge_request, skip_notification) do
         next unless approval.destroy_all # rubocop: disable Cop/DestroyAll
 
-        update_reviewer_state(merge_request, current_user, :unapproved)
+        update_reviewer_state(merge_request, current_user, :unapproved) unless skip_updating_state
         reset_approvals_cache(merge_request)
-        create_note(merge_request)
-        merge_request_activity_counter.track_unapprove_mr_action(user: current_user)
+
+        unless skip_system_note
+          create_note(merge_request)
+          merge_request_activity_counter.track_unapprove_mr_action(user: current_user)
+        end
+
         trigger_merge_request_merge_status_updated(merge_request)
         trigger_merge_request_approval_state_updated(merge_request)
       end
@@ -33,9 +37,11 @@ module MergeRequests
       merge_request.approvals.reset
     end
 
-    def trigger_approval_hooks(merge_request)
+    def trigger_approval_hooks(merge_request, skip_notification)
       yield
 
+      return if skip_notification
+
       notification_service.async.unapprove_mr(merge_request, current_user)
       execute_hooks(merge_request, 'unapproved')
     end

app/services/merge_requests/request_review_service.rb

@@ -8,13 +8,15 @@ module MergeRequests
       reviewer = merge_request.find_reviewer(user)
 
       if reviewer
+        has_unapproved = remove_approval(merge_request, user).present?
+
         return error("Failed to update reviewer") unless reviewer.update(state: :unreviewed)
 
         notify_reviewer(merge_request, user)
+        trigger_merge_request_merge_status_updated(merge_request)
         trigger_merge_request_reviewers_updated(merge_request)
-        create_system_note(merge_request, user)
-
-        remove_approval(merge_request)
+        trigger_merge_request_approval_state_updated(merge_request)
+        create_system_note(merge_request, user, has_unapproved)
 
         success
       else
@@ -29,8 +31,8 @@ module MergeRequests
       todo_service.create_request_review_todo(merge_request, current_user, reviewer)
     end
 
-    def create_system_note(merge_request, user)
-      ::SystemNoteService.request_review(merge_request, merge_request.project, current_user, user)
+    def create_system_note(merge_request, user, has_unapproved)
+      ::SystemNoteService.request_review(merge_request, merge_request.project, current_user, user, has_unapproved)
     end
   end
 end

app/services/merge_requests/update_reviewer_state_service.rb

@@ -14,7 +14,7 @@ module MergeRequests
 
         return success if state != 'requested_changes'
 
-        if merge_request.approved_by?(current_user) && !remove_approval(merge_request)
+        if merge_request.approved_by?(current_user) && !remove_approval(merge_request, current_user)
           return error("Failed to remove approval")
         end
 

app/services/system_note_service.rb

@@ -45,8 +45,8 @@ module SystemNoteService
     ::SystemNotes::IssuablesService.new(noteable: issuable, project: project, author: author).change_issuable_reviewers(old_reviewers)
   end
 
-  def request_review(issuable, project, author, user)
-    ::SystemNotes::IssuablesService.new(noteable: issuable, project: project, author: author).request_review(user)
+  def request_review(issuable, project, author, user, has_unapproved)
+    ::SystemNotes::IssuablesService.new(noteable: issuable, project: project, author: author).request_review(user, has_unapproved)
   end
 
   def change_issuable_contacts(issuable, project, author, added_count, removed_count)

app/services/system_notes/issuables_service.rb

@@ -133,10 +133,12 @@ module SystemNotes
       create_note(NoteSummary.new(noteable, project, author, body, action: 'reviewer'))
     end
 
-    def request_review(user)
-      body = "#{self.class.issuable_events[:review_requested]} #{user.to_reference}"
+    def request_review(user, has_unapproved)
+      body = ["#{self.class.issuable_events[:review_requested]} #{user.to_reference}"]
 
-      create_note(NoteSummary.new(noteable, project, author, body, action: 'reviewer'))
+      body << "removed approval" if has_unapproved
+
+      create_note(NoteSummary.new(noteable, project, author, body.to_sentence, action: 'reviewer'))
     end
 
     # Called when the contacts of an issuable are changed or removed

config/feature_flags/gitlab_com_derisk/workhorse_google_client.yml

@@ -1,8 +1,9 @@
 ---
 name: workhorse_google_client
+feature_issue_url: https://gitlab.com/groups/gitlab-org/-/epics/9457
 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/96891
 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/372596
 milestone: '15.6'
-type: development
-group: 'group::package registry'
+group: group::package registry
+type: gitlab_com_derisk
 default_enabled: false

lib/banzai/filter/emoji_filter.rb

@@ -9,7 +9,14 @@ module Banzai
     class EmojiFilter < HTML::Pipeline::Filter
       IGNORED_ANCESTOR_TAGS = %w[pre code tt].to_set
 
+      # Limit of how many emojis we will process.
+      # Protects against pathological number of emojis.
+      # For more information check: https://gitlab.com/gitlab-org/gitlab/-/issues/434803
+      EMOJI_LIMIT = 1000
+
       def call
+        @emoji_count = 0
+
         doc.xpath('descendant-or-self::text()').each do |node|
           content = node.to_html
           next if has_ancestor?(node, IGNORED_ANCESTOR_TAGS)
@@ -23,6 +30,7 @@ module Banzai
 
           node.replace(html)
         end
+
         doc
       end
 
@@ -32,10 +40,8 @@ module Banzai
       #
       # Returns a String with :emoji: replaced with gl-emoji unicode.
       def emoji_name_element_unicode_filter(text)
-        text.gsub(emoji_pattern) do |match|
-          name = Regexp.last_match(1)
-          emoji = TanukiEmoji.find_by_alpha_code(name)
-          Gitlab::Emoji.gl_emoji_tag(emoji)
+        scan_and_replace(text, emoji_pattern) do |matched_text|
+          TanukiEmoji.find_by_alpha_code(matched_text)
         end
       end
 
@@ -45,9 +51,8 @@ module Banzai
       #
       # Returns a String with unicode emoji replaced with gl-emoji unicode.
       def emoji_unicode_element_unicode_filter(text)
-        text.gsub(emoji_unicode_pattern) do |moji|
-          emoji = TanukiEmoji.find_by_codepoints(moji)
-          Gitlab::Emoji.gl_emoji_tag(emoji)
+        scan_and_replace(text, emoji_unicode_pattern) do |matched_text|
+          TanukiEmoji.find_by_codepoints(matched_text)
         end
       end
 
@@ -63,6 +68,39 @@ module Banzai
 
       private
 
+      # This performs the same function as a `gsub`. However this version
+      # allows us to break out of the replacement loop when the limit is
+      # reached. Benchmarking showed performance was roughly equivalent.
+      def scan_and_replace(text, pattern)
+        scanner = StringScanner.new(text)
+        buffer = +''
+
+        return text unless scanner.exist?(pattern)
+
+        until scanner.eos?
+          portion = scanner.scan_until(pattern)
+
+          if portion.nil?
+            buffer << scanner.rest
+            scanner.terminate
+            break
+          end
+
+          if emoji_limit_reached?(@emoji_count)
+            buffer << portion
+            buffer << scanner.rest
+            scanner.terminate
+            break
+          end
+
+          emoji = yield(scanner.matched)
+          @emoji_count += 1 if emoji
+          buffer << portion.sub(scanner.matched, Gitlab::Emoji.gl_emoji_tag(emoji))
+        end
+
+        buffer
+      end
+
       def emoji_pattern
         self.class.emoji_pattern
       end
@@ -70,6 +108,10 @@ module Banzai
       def emoji_unicode_pattern
         self.class.emoji_unicode_pattern
       end
+
+      def emoji_limit_reached?(count)
+        count >= EMOJI_LIMIT
+      end
     end
   end
 end

lib/banzai/filter/gollum_tags_filter.rb

@@ -66,7 +66,11 @@ module Banzai
 
           html = process_tag(Regexp.last_match(1))
 
-          node.replace(html) if html && html != node.content
+          next unless html && html != node.content
+
+          new_node = Banzai::Filter::SanitizationFilter.new(html).call
+          new_node = new_node&.children&.first&.add_class('gfm')
+          node.replace(new_node.to_html) if new_node
         end
 
         doc

lib/banzai/filter/references/abstract_reference_filter.rb

@@ -138,7 +138,7 @@ module Banzai
               yield_valid_link(node) do |link, inner_html|
                 if ref_pattern && link =~ ref_pattern_anchor
                   replace_link_node_with_href(node, index, link) do
-                    object_link_filter(link, ref_pattern, link_content: inner_html)
+                    object_link_filter(link, ref_pattern_anchor, link_content: inner_html)
                   end
 
                   next
@@ -148,7 +148,7 @@ module Banzai
 
                 if link == inner_html && inner_html =~ link_pattern_start
                   replace_link_node_with_text(node, index) do
-                    object_link_filter(inner_html, link_pattern, link_reference: true)
+                    object_link_filter(inner_html, link_pattern_start, link_reference: true)
                   end
 
                   next
@@ -156,7 +156,7 @@ module Banzai
 
                 if link =~ link_pattern_anchor
                   replace_link_node_with_href(node, index, link) do
-                    object_link_filter(link, link_pattern, link_content: inner_html, link_reference: true)
+                    object_link_filter(link, link_pattern_anchor, link_content: inner_html, link_reference: true)
                   end
 
                   next

lib/banzai/filter/references/reference_filter.rb

@@ -54,7 +54,7 @@ module Banzai
               yield_valid_link(node) do |link, inner_html|
                 if link =~ ref_pattern_start
                   replace_link_node_with_href(node, index, link) do
-                    object_link_filter(link, object_reference_pattern, link_content: inner_html)
+                    object_link_filter(link, ref_pattern_start, link_content: inner_html)
                   end
                 end
               end

lib/gitlab/ci/components/instance_path.rb

@@ -16,7 +16,7 @@ module Gitlab
         end
 
         def self.fqdn_prefix
-          "#{Settings.gitlab_ci['server_fqdn']}/"
+          "#{Gitlab.config.gitlab_ci.server_fqdn}/"
         end
 
         def initialize(address:)

lib/object_storage/direct_upload.rb

@@ -68,7 +68,7 @@ module ObjectStorage
         workhorse_aws_hash
       elsif config.azure?
         workhorse_azure_hash
-      elsif Feature.enabled?(:workhorse_google_client) && config.google?
+      elsif Feature.enabled?(:workhorse_google_client, Feature.current_request) && config.google?
         workhorse_google_hash
       else
         {}

spec/lib/banzai/filter/emoji_filter_spec.rb

@@ -98,4 +98,41 @@ RSpec.describe Banzai::Filter::EmojiFilter, feature_category: :team_planning do
 
     expect(doc.to_html).to match(/^This deserves a <gl-emoji.+>, big time\.\z/)
   end
+
+  context 'and protects against pathological number of emojis' do
+    context 'with hard limit' do
+      before do
+        stub_const('Banzai::Filter::EmojiFilter::EMOJI_LIMIT', 2)
+      end
+
+      it 'enforces limits on unicode emojis' do
+        doc = filter('⏯' * 3)
+
+        expect(doc.search('gl-emoji').count).to eq(2)
+        expect(doc.to_html).to end_with('⏯')
+      end
+
+      it 'enforces limits on named emojis' do
+        doc = filter(':play_pause: ' * 3)
+
+        expect(doc.search('gl-emoji').count).to eq(2)
+        expect(doc.to_html).to end_with(':play_pause: ')
+      end
+
+      # Since we convert unicode emojis first, those reach the limits
+      # first and `:play_pause:` is not converted because we're over limit.
+      it 'enforces limits on mixed emojis' do
+        doc = filter('⏯ :play_pause: ⏯')
+
+        expect(doc.search('gl-emoji').count).to eq(2)
+        expect(doc.to_html).to include(' :play_pause: ')
+      end
+    end
+
+    it 'limit keeps it from timing out' do
+      expect do
+        Timeout.timeout(1.second) { filter('⏯ :play_pause: ' * 500000) }
+      end.not_to raise_error
+    end
+  end
 end

spec/lib/banzai/filter/gollum_tags_filter_spec.rb

@@ -94,5 +94,19 @@ RSpec.describe Banzai::Filter::GollumTagsFilter, feature_category: :wiki do
 
       expect(doc.at_css('code').text).to eq '[[link-in-backticks]]'
     end
+
+    it "sanitizes the href attribute (case 1)" do
+      tag = '[[a|http:\'"injected=attribute><img/src="0"onerror="alert(0)">https://gitlab.com/gitlab-org/gitlab/-/issues/1]]'
+      doc = filter("See #{tag}", wiki: wiki)
+
+      expect(doc.at_css('a').to_html).to eq '<a href="http:\'%22injected=attribute&gt;&lt;img/src=%220%22onerror=%22alert(0)%22&gt;https://gitlab.com/gitlab-org/gitlab/-/issues/1" class="gfm">a</a>'
+    end
+
+    it "sanitizes the href attribute (case 2)" do
+      tag = '<i>[[a|\'"&gt;&lt;svg&gt;&lt;i/class=gl-show-field-errors&gt;&lt;input/title="&lt;script&gt;alert(0)&lt;/script&gt;"/&gt;&lt;/svg&gt;https://gitlab.com/gitlab-org/gitlab/-/issues/1]]'
+      doc = filter("See #{tag}", wiki: wiki)
+
+      expect(doc.at_css('i a').to_html).to eq "<a href=\"#{wiki.wiki_base_path}/\'%22&gt;&lt;svg&gt;&lt;i/class=gl-show-field-errors&gt;&lt;input/title=%22&lt;script&gt;alert(0)&lt;/script&gt;%22/&gt;&lt;/svg&gt;https://gitlab.com/gitlab-org/gitlab/-/issues/1\" class=\"gfm\">a</a>"
+    end
   end
 end

spec/lib/banzai/pipeline/full_pipeline_spec.rb

@@ -27,6 +27,24 @@ RSpec.describe Banzai::Pipeline::FullPipeline, feature_category: :team_planning
       expect(result).not_to include(link_label)
     end
 
+    it 'prevents xss by not replacing the same reference in one anchor multiple times' do
+      reference_link = ::Gitlab::Routing.url_helpers.project_issue_url(project, issue)
+      markdown = <<~TEXT
+        <div>
+        <a href="#{reference_link}<i>
+        <a alt='&quot;#{reference_link}'></a>
+        </i>">#{reference_link}<i>
+        <a alt='"#{reference_link}'></a></i></a>
+        </div>
+      TEXT
+
+      markdown.delete!("\n")
+
+      result = described_class.to_html(markdown, project: project)
+
+      expect(result).to include "<a alt='\"#{reference_link}'></a>"
+    end
+
     it 'escapes the data-original attribute on a reference' do
       markdown = %{[">bad things](#{issue.to_reference})}
       result = described_class.to_html(markdown, project: project)

spec/models/ci/catalog/resources/component_spec.rb

@@ -67,12 +67,13 @@ RSpec.describe Ci::Catalog::Resources::Component, type: :model, feature_category
       let(:component) { create(:ci_catalog_resource_component) }
 
       it 'generates the correct include path' do
-        expected_path = "#{Settings.gitlab_ci['component_fqdn']}/" \
+        expected_path = "#{Gitlab.config.gitlab_ci.server_fqdn}/" \
                         "#{component.project.full_path}/" \
                         "#{component.name}@" \
                         "#{component.version.version}"
 
         expect(component.include_path).to eq(expected_path)
+        expect(Gitlab.config.gitlab_ci.server_fqdn).not_to be_nil
       end
     end
   end

spec/models/container_repository_spec.rb

@@ -497,17 +497,7 @@ RSpec.describe ContainerRepository, :aggregate_failures, feature_category: :cont
           allow(ContainerRegistry::GitlabApiClient).to receive(:supports_gitlab_api?).and_return(true)
         end
 
-        context 'when the Gitlab API returns a tag' do
-          let_it_be(:tags_response) do
-            [
-              {
-                name: 'test',
-                digest: 'sha256:6c3c647c6eebdaab7c610cf7d66709b3',
-                size_bytes: 1234567892
-              }
-            ]
-          end
-
+        shared_examples 'returning an instantiated tag from the API response' do
           let_it_be(:response_body) do
             {
               pagination: {},
@@ -528,13 +518,52 @@ RSpec.describe ContainerRepository, :aggregate_failures, feature_category: :cont
             expect(tag).to be_a ContainerRegistry::Tag
             expect(tag).to have_attributes(
               repository: repository,
-              name: tags_response[0][:name],
+              name: 'test',
               digest: tags_response[0][:digest],
               total_size: tags_response[0][:size_bytes]
             )
           end
         end
 
+        context 'when the Gitlab API returns a tag' do
+          let_it_be(:tags_response) do
+            [
+              {
+                name: 'test',
+                digest: 'sha256:6c3c647c6eebdaab7c610cf7d66709b3',
+                size_bytes: 1234567892
+              }
+            ]
+          end
+
+          it_behaves_like 'returning an instantiated tag from the API response'
+        end
+
+        context 'when the Gitlab API returns multiple tags' do
+          let_it_be(:tags_response) do
+            [
+              {
+                name: 'a-test',
+                digest: 'sha256:6c3c647c6eebdaab7c610cf7d66709b3',
+                size_bytes: 1234567892
+              },
+              {
+                name: 'test',
+                digest: 'sha256:6c3c647c6eebdaab7c610cf7d66709b3',
+                size_bytes: 1234567892
+              },
+
+              {
+                name: 'test-a',
+                digest: 'sha256:6c3c647c6eebdaab7c610cf7d66709b3',
+                size_bytes: 1234567892
+              }
+            ]
+          end
+
+          it_behaves_like 'returning an instantiated tag from the API response'
+        end
+
         context 'when the Gitlab API does not return a tag' do
           before do
             allow(repository.gitlab_api_client).to receive(:tags).and_return({ pagination: {}, response_body: {} })

spec/requests/api/project_container_repositories_spec.rb

@@ -519,6 +519,49 @@ RSpec.describe API::ProjectContainerRepositories, feature_category: :container_r
                 it_behaves_like 'returning the tag'
               end
 
+              context 'when the Gitlab API returns multiple tags matching the name' do
+                let_it_be(:tags_response) do
+                  [
+                    {
+                      name: 'rootA',
+                      digest: 'sha256:4c8e63ca4cb663ce6c688cb06f1c372b088dac5b6d7ad7d49cd620d85cf72a15',
+                      config_digest: 'sha256:d7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac',
+                      size_bytes: 2319870,
+                      created_at: 1.minute.ago
+                    },
+                    {
+                      name: 'rootA-1',
+                      digest: 'sha256:4c8e63ca4cb663ce6c688cb06f1c372b088dac5b6d7ad7d49cd620d85cf72a15',
+                      config_digest: 'sha256:d7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac',
+                      size_bytes: 2319870,
+                      created_at: 1.minute.ago
+                    },
+                    {
+                      name: '1-rootA',
+                      digest: 'sha256:4c8e63ca4cb663ce6c688cb06f1c372b088dac5b6d7ad7d49cd620d85cf72a15',
+                      config_digest: 'sha256:d7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac',
+                      size_bytes: 2319870,
+                      created_at: 1.minute.ago
+                    }
+                  ]
+                end
+
+                let_it_be(:response_body) do
+                  {
+                    pagination: {},
+                    response_body: ::Gitlab::Json.parse(tags_response.to_json)
+                  }
+                end
+
+                before do
+                  allow_next_instance_of(ContainerRegistry::GitlabApiClient) do |client|
+                    allow(client).to receive(:tags).and_return(response_body)
+                  end
+                end
+
+                it_behaves_like 'returning the tag'
+              end
+
               context 'when the Gitlab API does not return a tag' do
                 before do
                   allow_next_instance_of(ContainerRegistry::GitlabApiClient) do |client|

spec/services/merge_requests/remove_approval_service_spec.rb

@@ -11,8 +11,9 @@ RSpec.describe MergeRequests::RemoveApprovalService, feature_category: :code_rev
 
     subject(:service) { described_class.new(project: project, current_user: user) }
 
-    def execute!
-      service.execute(merge_request)
+    def execute!(skip_updating_state: false, skip_system_note: false, skip_notification: false)
+      service.execute(merge_request, skip_updating_state: skip_updating_state, skip_system_note: skip_system_note,
+        skip_notification: skip_notification)
     end
 
     before do
@@ -71,6 +72,12 @@ RSpec.describe MergeRequests::RemoveApprovalService, feature_category: :code_rev
         }.from(false).to(true)
       end
 
+      it 'does not change reviewers state when skip_updating_state is true' do
+        expect { execute!(skip_updating_state: true) }.not_to change {
+          merge_request.merge_request_reviewers.reload.all?(&:unapproved?)
+        }
+      end
+
       it 'creates an unapproval note, triggers a web hook, and sends a notification' do
         expect(service).to receive(:execute_hooks).with(merge_request, 'unapproved')
         expect(SystemNoteService).to receive(:unapprove_mr)
@@ -79,6 +86,24 @@ RSpec.describe MergeRequests::RemoveApprovalService, feature_category: :code_rev
         execute!
       end
 
+      it 'does not trigger a web hook when skip_notification is true' do
+        expect(service).not_to receive(:execute_hooks)
+
+        execute!(skip_notification: true)
+      end
+
+      it 'does not send notification when skip_notification is true' do
+        expect(notification_service).not_to receive(:async)
+
+        execute!(skip_notification: true)
+      end
+
+      it 'does not create system note when skip_system_note is true' do
+        expect(SystemNoteService).not_to receive(:unapprove_mr)
+
+        execute!(skip_system_note: true)
+      end
+
       it 'tracks merge request unapprove action' do
         expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
           .to receive(:track_unapprove_mr_action).with(user: user)
@@ -86,6 +111,12 @@ RSpec.describe MergeRequests::RemoveApprovalService, feature_category: :code_rev
         execute!
       end
 
+      it 'does not track merge request unapprove action when skip_system_note is true' do
+        expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).not_to receive(:track_unapprove_mr_action)
+
+        execute!(skip_system_note: true)
+      end
+
       it_behaves_like 'triggers GraphQL subscription mergeRequestMergeStatusUpdated' do
         let(:action) { execute! }
       end

spec/services/merge_requests/request_review_service_spec.rb

@@ -74,7 +74,7 @@ RSpec.describe MergeRequests::RequestReviewService, feature_category: :code_revi
       it 'creates a sytem note' do
         expect(SystemNoteService)
           .to receive(:request_review)
-          .with(merge_request, project, current_user, user)
+          .with(merge_request, project, current_user, user, false)
 
         service.execute(merge_request, user)
       end
@@ -86,9 +86,9 @@ RSpec.describe MergeRequests::RequestReviewService, feature_category: :code_revi
       it 'calls MergeRequests::RemoveApprovalService' do
         expect_next_instance_of(
           MergeRequests::RemoveApprovalService,
-          project: project, current_user: current_user
+          project: project, current_user: user
         ) do |service|
-          expect(service).to receive(:execute).with(merge_request).and_return({ success: true })
+          expect(service).to receive(:execute).with(merge_request, skip_system_note: true, skip_notification: true, skip_updating_state: true).and_return({ success: true })
         end
 
         service.execute(merge_request, user)

spec/services/merge_requests/update_reviewer_state_service_spec.rb

@@ -62,7 +62,9 @@ RSpec.describe MergeRequests::UpdateReviewerStateService, feature_category: :cod
             MergeRequests::RemoveApprovalService,
             project: project, current_user: current_user
           ) do |service|
-            expect(service).to receive(:execute).with(merge_request).and_return({ success: true })
+            expect(service).to receive(:execute)
+              .with(merge_request, skip_system_note: true, skip_notification: true, skip_updating_state: true)
+              .and_return({ success: true })
           end
 
           expect(result[:status]).to eq :success
@@ -73,7 +75,9 @@ RSpec.describe MergeRequests::UpdateReviewerStateService, feature_category: :cod
             MergeRequests::RemoveApprovalService,
             project: project, current_user: current_user
           ) do |service|
-            expect(service).to receive(:execute).with(merge_request).and_return(nil)
+            expect(service).to receive(:execute)
+              .with(merge_request, skip_system_note: true, skip_notification: true, skip_updating_state: true)
+              .and_return(nil)
           end
 
           expect(result[:status]).to eq :error

spec/services/notes/quick_actions_service_spec.rb

@@ -369,7 +369,7 @@ RSpec.describe Notes::QuickActionsService, feature_category: :team_planning do
       end
 
       context 'when using work item reference' do
-        let_it_be(:note_text) { "/add_child #{child.to_reference(full: true)},#{second_child.to_reference(full: true)}" }
+        let_it_be(:note_text) { "/add_child #{child.to_reference(full: true)}, #{second_child.to_reference(full: true)}" }
 
         it_behaves_like 'adds child work items'
       end
@@ -380,7 +380,7 @@ RSpec.describe Notes::QuickActionsService, feature_category: :team_planning do
 
       context 'when using work item URL' do
         let_it_be(:project_path) { "#{Gitlab.config.gitlab.url}/#{project.full_path}" }
-        let_it_be(:url) { "#{project_path}/work_items/#{child.iid},#{project_path}/work_items/#{second_child.iid}" }
+        let_it_be(:url) { "#{project_path}/work_items/#{child.iid}, #{project_path}/work_items/#{second_child.iid}" }
         let_it_be(:note_text) { "/add_child #{url}" }
 
         it_behaves_like 'adds child work items'

spec/services/system_note_service_spec.rb

@@ -82,10 +82,10 @@ RSpec.describe SystemNoteService, feature_category: :shared do
 
     it 'calls IssuableService' do
       expect_next_instance_of(::SystemNotes::IssuablesService) do |service|
-        expect(service).to receive(:request_review).with(reviewer)
+        expect(service).to receive(:request_review).with(reviewer, true)
       end
 
-      described_class.request_review(noteable, project, author, reviewer)
+      described_class.request_review(noteable, project, author, reviewer, true)
     end
   end
 

spec/services/system_notes/issuables_service_spec.rb

@@ -214,18 +214,27 @@ RSpec.describe ::SystemNotes::IssuablesService, feature_category: :team_planning
   end
 
   describe '#request_review' do
-    subject(:request_review) { service.request_review(reviewer) }
+    subject(:request_review) { service.request_review(reviewer, unapproved) }
 
     let_it_be(:reviewer) { create(:user) }
     let_it_be(:noteable) { create(:merge_request, :simple, source_project: project, reviewers: [reviewer]) }
+    let(:unapproved) { false }
 
     it_behaves_like 'a system note' do
       let(:action) { 'reviewer' }
     end
 
-    it 'builds a correct phrase when a reviewer has been requested from a reviewer' do
+    it 'builds a correct phrase when a review has been requested from a reviewer' do
       expect(request_review.note).to eq "requested review from #{reviewer.to_reference}"
     end
+
+    context 'when unapproving' do
+      let(:unapproved) { true }
+
+      it 'builds a correct phrase when a review has been requested from a reviewer and the reviewer has been unapproved' do
+        expect(request_review.note).to eq "requested review from #{reviewer.to_reference} and removed approval"
+      end
+    end
   end
 
   describe '#change_issuable_contacts' do