Ensure Grouped Security Updates are rebased correctly

Currently, when rebasing a grouped security update, we will default to
creating individual PRs for each dependency, since the updater does not
know how to refresh grouped updates.

This adds a basic strategy that reuses the refresh behavior for Version
Update groups.

common/lib/wildcard_matcher.rb

@@ -1,6 +1,8 @@
 # typed: strong
 # frozen_string_literal: true
 
+require "sorbet-runtime"
+
 class WildcardMatcher
   extend T::Sig
 

updater/lib/dependabot/updater/group_update_refreshing.rb

@@ -0,0 +1,60 @@
+# typed: false
+# frozen_string_literal: true
+
+# This module contains the methods required to refresh (upsert or recreate)
+# existing grouped pull requests.
+#
+# When included in an Operation it expects the following to be available:
+# - job: the current Dependabot::Job object
+# - dependency_snapshot: the Dependabot::DependencySnapshot of the current state
+# - error_handler: a Dependabot::UpdaterErrorHandler to report any problems to
+#
+module Dependabot
+  class Updater
+    module GroupUpdateRefreshing
+      def upsert_pull_request_with_error_handling(dependency_change, group)
+        if dependency_change.updated_dependencies.any?
+          upsert_pull_request(dependency_change, group)
+        else
+          Dependabot.logger.info("Dependencies are up to date, closing existing Pull Request")
+          close_pull_request(reason: :up_to_date, group: group)
+        end
+      rescue StandardError => e
+        error_handler.handle_job_error(error: e, dependency_group: dependency_snapshot.job_group)
+      end
+
+      # Having created the dependency_change, we need to determine the right strategy to apply it to the project:
+      # - Replace existing PR if the dependencies involved have changed
+      # - Update the existing PR if the dependencies and the target versions remain the same
+      # - Supersede the existing PR if the dependencies are the same but the target versions have changed
+      def upsert_pull_request(dependency_change, group)
+        if dependency_change.should_replace_existing_pr?
+          Dependabot.logger.info("Dependencies have changed, closing existing Pull Request")
+          close_pull_request(reason: :dependencies_changed, group: group)
+          Dependabot.logger.info("Creating a new pull request for '#{group.name}'")
+          service.create_pull_request(dependency_change, dependency_snapshot.base_commit_sha)
+        elsif dependency_change.matches_existing_pr?
+          Dependabot.logger.info("Updating pull request for '#{group.name}'")
+          service.update_pull_request(dependency_change, dependency_snapshot.base_commit_sha)
+        else
+          # If the changes do not match an existing PR, then we should open a new pull request and leave it to
+          # the backend to close the existing pull request with a comment that it has been superseded.
+          Dependabot.logger.info("Target versions have changed, existing Pull Request should be superseded")
+          Dependabot.logger.info("Creating a new pull request for '#{group.name}'")
+          service.create_pull_request(dependency_change, dependency_snapshot.base_commit_sha)
+        end
+      end
+
+      def close_pull_request(reason:, group:)
+        reason_string = reason.to_s.tr("_", " ")
+        Dependabot.logger.info(
+          "Telling backend to close pull request for the " \
+          "#{group.name} group " \
+          "(#{job.dependencies.join(', ')}) - #{reason_string}"
+        )
+
+        service.close_pull_request(job.dependencies, reason)
+      end
+    end
+  end
+end

updater/lib/dependabot/updater/operations.rb

@@ -4,6 +4,7 @@
 require "dependabot/updater/operations/create_group_security_update_pull_request"
 require "dependabot/updater/operations/create_security_update_pull_request"
 require "dependabot/updater/operations/group_update_all_versions"
+require "dependabot/updater/operations/refresh_group_security_update_pull_request"
 require "dependabot/updater/operations/refresh_group_update_pull_request"
 require "dependabot/updater/operations/refresh_security_update_pull_request"
 require "dependabot/updater/operations/refresh_version_update_pull_request"
@@ -33,6 +34,7 @@ module Operations
       OPERATIONS = [
         CreateGroupSecurityUpdatePullRequest,
         CreateSecurityUpdatePullRequest,
+        RefreshGroupSecurityUpdatePullRequest,
         RefreshSecurityUpdatePullRequest,
         RefreshGroupUpdatePullRequest,
         RefreshVersionUpdatePullRequest,

updater/lib/dependabot/updater/operations/refresh_group_security_update_pull_request.rb

@@ -0,0 +1,81 @@
+# typed: false
+# frozen_string_literal: true
+
+require "dependabot/updater/security_update_helpers"
+require "dependabot/updater/group_update_creation"
+require "dependabot/updater/group_update_refreshing"
+
+# This class implements our strategy for updating multiple, insecure dependencies
+# to a secure version. We attempt to make the smallest version update possible,
+# i.e. semver patch-level increase is preferred over minor-level increase.
+module Dependabot
+  class Updater
+    module Operations
+      class RefreshGroupSecurityUpdatePullRequest
+        include SecurityUpdateHelpers
+        include GroupUpdateCreation
+        include GroupUpdateRefreshing
+
+        def self.applies_to?(job:)
+          # If we haven't been given data for the vulnerable dependency,
+          # this strategy cannot act.
+          return false unless job.dependencies&.any?
+          return false unless job.security_updates_only?
+          return false unless job.dependencies.count > 1
+
+          job.updating_a_pull_request?
+        end
+
+        def self.tag_name
+          :update_security_group_pr
+        end
+
+        def initialize(service:, job:, dependency_snapshot:, error_handler:)
+          @service = service
+          @job = job
+          @dependency_snapshot = dependency_snapshot
+          @error_handler = error_handler
+        end
+
+        def perform
+          Dependabot.logger.info("Starting security update job for #{job.source.repo}")
+
+          target_dependencies = dependency_snapshot.job_dependencies
+
+          if target_dependencies.empty?
+            record_security_update_dependency_not_found
+          else
+            # make a temporary fake group to use the existing logic
+            group = Dependabot::DependencyGroup.new(
+              name: "#{job.package_manager} at #{job.source.directory || '/'} security update",
+              rules: {
+                "patterns" => "*" # The grouping is more dictated by the dependencies passed in.
+              }
+            )
+            target_dependencies.each do |dep|
+              group.dependencies << dep
+            end
+
+            dependency_change = compile_all_dependency_changes_for(group)
+
+            if dependency_change.updated_dependencies.any?
+              upsert_pull_request_with_error_handling(dependency_change, group)
+            else
+              Dependabot.logger.info("Nothing to update for Dependency Group: '#{group.name}'")
+            end
+
+            dependency_change
+          end
+        end
+
+        private
+
+        attr_reader :job,
+                    :service,
+                    :dependency_snapshot,
+                    :error_handler,
+                    :created_pull_requests
+      end
+    end
+  end
+end

updater/lib/dependabot/updater/operations/refresh_group_update_pull_request.rb

@@ -2,6 +2,7 @@
 # frozen_string_literal: true
 
 require "dependabot/updater/group_update_creation"
+require "dependabot/updater/group_update_refreshing"
 
 # This class implements our strategy for refreshing a single Pull Request which
 # updates all outdated Dependencies within a specific project folder that match
@@ -23,6 +24,7 @@ class Updater
     module Operations
       class RefreshGroupUpdatePullRequest
         include GroupUpdateCreation
+        include GroupUpdateRefreshing
 
         def self.applies_to?(job:)
           return false if job.security_updates_only?
@@ -71,7 +73,7 @@ def perform # rubocop:disable Metrics/AbcSize
             # Let's warn that the group is empty and then signal the PR should be closed
             # so users are informed this group is no longer actionable by Dependabot.
             warn_group_is_empty(dependency_snapshot.job_group)
-            close_pull_request(reason: :dependency_group_empty)
+            close_pull_request(reason: :dependency_group_empty, group: dependency_snapshot.job_group)
           else
             Dependabot.logger.info("Updating the '#{dependency_snapshot.job_group.name}' group")
 
@@ -87,7 +89,7 @@ def perform # rubocop:disable Metrics/AbcSize
 
             dependency_change = compile_all_dependency_changes_for(dependency_snapshot.job_group)
 
-            upsert_pull_request_with_error_handling(dependency_change)
+            upsert_pull_request_with_error_handling(dependency_change, dependency_snapshot.job_group)
           end
         end
 
@@ -97,50 +99,6 @@ def perform # rubocop:disable Metrics/AbcSize
                     :service,
                     :dependency_snapshot,
                     :error_handler
-
-        def upsert_pull_request_with_error_handling(dependency_change)
-          if dependency_change.updated_dependencies.any?
-            upsert_pull_request(dependency_change)
-          else
-            Dependabot.logger.info("Dependencies are up to date, closing existing Pull Request")
-            close_pull_request(reason: :up_to_date)
-          end
-        rescue StandardError => e
-          error_handler.handle_job_error(error: e, group: dependency_snapshot.job_group)
-        end
-
-        # Having created the dependency_change, we need to determine the right strategy to apply it to the project:
-        # - Replace existing PR if the dependencies involved have changed
-        # - Update the existing PR if the dependencies and the target versions remain the same
-        # - Supersede the existing PR if the dependencies are the same but the target versions have changed
-        def upsert_pull_request(dependency_change)
-          if dependency_change.should_replace_existing_pr?
-            Dependabot.logger.info("Dependencies have changed, closing existing Pull Request")
-            close_pull_request(reason: :dependencies_changed)
-            Dependabot.logger.info("Creating a new pull request for '#{dependency_snapshot.job_group.name}'")
-            service.create_pull_request(dependency_change, dependency_snapshot.base_commit_sha)
-          elsif dependency_change.matches_existing_pr?
-            Dependabot.logger.info("Updating pull request for '#{dependency_snapshot.job_group.name}'")
-            service.update_pull_request(dependency_change, dependency_snapshot.base_commit_sha)
-          else
-            # If the changes do not match an existing PR, then we should open a new pull request and leave it to
-            # the backend to close the existing pull request with a comment that it has been superseded.
-            Dependabot.logger.info("Target versions have changed, existing Pull Request should be superseded")
-            Dependabot.logger.info("Creating a new pull request for '#{dependency_snapshot.job_group.name}'")
-            service.create_pull_request(dependency_change, dependency_snapshot.base_commit_sha)
-          end
-        end
-
-        def close_pull_request(reason:)
-          reason_string = reason.to_s.tr("_", " ")
-          Dependabot.logger.info(
-            "Telling backend to close pull request for the " \
-            "#{dependency_snapshot.job_group.name} group " \
-            "(#{job.dependencies.join(', ')}) - #{reason_string}"
-          )
-
-          service.close_pull_request(job.dependencies, reason)
-        end
       end
     end
   end

updater/spec/dependabot/updater/operations_spec.rb

@@ -90,6 +90,20 @@
         .to be(Dependabot::Updater::Operations::CreateSecurityUpdatePullRequest)
     end
 
+    it "returns the RefreshGroupSecurityUpdatePullRequest class when the Job is for an existing security update for" \
+       " multiple dependencies" do
+      job = instance_double(Dependabot::Job,
+                            security_updates_only?: true,
+                            updating_a_pull_request?: true,
+                            dependencies: [anything, anything],
+                            dependency_group_to_refresh: anything,
+                            dependency_groups: [anything],
+                            is_a?: true)
+
+      expect(described_class.class_for(job: job))
+        .to be(Dependabot::Updater::Operations::RefreshGroupSecurityUpdatePullRequest)
+    end
+
     it "returns the RefreshSecurityUpdatePullRequest class when the Job is for an existing security update" do
       job = instance_double(Dependabot::Job,
                             security_updates_only?: true,