diff --git a/updater/lib/dependabot/dependency_snapshot.rb b/updater/lib/dependabot/dependency_snapshot.rb index a1041eb5686..26a04b7c7ef 100644 --- a/updater/lib/dependabot/dependency_snapshot.rb +++ b/updater/lib/dependabot/dependency_snapshot.rb @@ -27,6 +27,32 @@ def self.create_from_job_definition(job:, job_definition:) attr_reader :base_commit_sha, :dependency_files, :dependencies + # Returns the subset of all project dependencies which are permitted + # by the project configuration. + def allowed_dependencies + @allowed_dependencies ||= dependencies.select { |d| job.allowed_update?(d) } + end + + # Returns the subset of all project dependencies which are specifically + # requested to be updated by the job definition. + def job_dependencies + return [] unless job.dependencies&.any? + return @job_dependencies if defined? @job_dependencies + + # Gradle, Maven and Nuget dependency names can be case-insensitive and + # the dependency name in the security advisory often doesn't match what + # users have specified in their manifest. + # + # It's technically possibly to publish case-sensitive npm packages to a + # private registry but shouldn't cause problems here as job.dependencies + # is set either from an existing PR rebase/recreate or a security + # advisory. + job_dependency_names = job.dependencies.map(&:downcase) + @job_dependencies = dependencies.select do |dep| + job_dependency_names.include?(dep.name.downcase) + end + end + private def initialize(job:, base_commit_sha:, dependency_files:) diff --git a/updater/lib/dependabot/environment.rb b/updater/lib/dependabot/environment.rb index 88f0bcc2cf7..135a4ae979c 100644 --- a/updater/lib/dependabot/environment.rb +++ b/updater/lib/dependabot/environment.rb @@ -38,6 +38,10 @@ def self.github_actions? @github_actions ||= environment_variable("GITHUB_ACTIONS", false) end + def self.deterministic_updates? + @deterministic_updates ||= environment_variable("UPDATER_DETERMINISTIC", false) + end + def self.job_definition @job_definition ||= JSON.parse(File.read(job_path)) end diff --git a/updater/lib/dependabot/job.rb b/updater/lib/dependabot/job.rb index 4e9f23b4fd2..82b9d070a7a 100644 --- a/updater/lib/dependabot/job.rb +++ b/updater/lib/dependabot/job.rb @@ -134,6 +134,14 @@ def reject_external_code? @reject_external_code end + # TODO: Remove vulnerability checking + # + # This method does too much, let's make it focused on _just_ determining + # if the given dependency is within the configurations allowed_updates. + # + # The calling operation should be responsible for checking vulnerability + # separately, if required. + # # rubocop:disable Metrics/PerceivedComplexity def allowed_update?(dependency) allowed_updates.any? do |update| diff --git a/updater/lib/dependabot/updater.rb b/updater/lib/dependabot/updater.rb index a4a7e5e9973..9fd1795a952 100644 --- a/updater/lib/dependabot/updater.rb +++ b/updater/lib/dependabot/updater.rb @@ -69,6 +69,7 @@ def run error_handler: error_handler ).perform rescue *ErrorHandler::RUN_HALTING_ERRORS.keys => e + # TODO: Drop this into Security-specific operations if e.is_a?(Dependabot::AllVersionsIgnored) && !job.security_updates_only? error = StandardError.new( "Dependabot::AllVersionsIgnored was unexpectedly raised for a non-security update job" @@ -581,34 +582,19 @@ def existing_pull_request(updated_dependencies) end def dependencies - all_deps = dependency_snapshot.dependencies - # Rebases and security updates have dependencies, version updates don't - if job.dependencies - # Gradle, Maven and Nuget dependency names can be case-insensitive and - # the dependency name in the security advisory often doesn't match what - # users have specified in their manifest. - # - # It's technically possibly to publish case-sensitive npm packages to a - # private registry but shouldn't cause problems here as job.dependencies - # is set either from an existing PR rebase/recreate or a security - # advisory. - job_dependencies = job.dependencies.map(&:downcase) - return all_deps.select do |dep| - job_dependencies.include?(dep.name.downcase) - end + return dependency_snapshot.job_dependencies if job.dependencies + + if dependency_snapshot.dependencies.any? && dependency_snapshot.allowed_dependencies.none? + Dependabot.logger.info("Found no dependencies to update after filtering allowed updates") + return [] end - allowed_deps = all_deps.select { |d| job.allowed_update?(d) } + allowed_deps = dependency_snapshot.allowed_dependencies # Return dependencies in a random order, with top-level dependencies # considered first so that dependency runs which time out don't always hit # the same dependencies - allowed_deps = allowed_deps.shuffle unless ENV["UPDATER_DETERMINISTIC"] - - if all_deps.any? && allowed_deps.none? - Dependabot.logger.info("Found no dependencies to update after filtering allowed " \ - "updates") - end + allowed_deps = allowed_deps.shuffle unless Environment.deterministic_updates? # Consider updating vulnerable deps first. Only consider the first 10, # though, to ensure they don't take up the entire update run diff --git a/updater/lib/dependabot/updater/operations/group_update_all_versions.rb b/updater/lib/dependabot/updater/operations/group_update_all_versions.rb index 34aa7abcac3..db192c7ba29 100644 --- a/updater/lib/dependabot/updater/operations/group_update_all_versions.rb +++ b/updater/lib/dependabot/updater/operations/group_update_all_versions.rb @@ -68,19 +68,12 @@ def perform :error_handler def dependencies - all_deps = dependency_snapshot.dependencies - - allowed_deps = all_deps.select { |d| job.allowed_update?(d) } - # Return dependencies in a random order, with top-level dependencies - # considered first so that dependency runs which time out don't always hit - # the same dependencies - allowed_deps = allowed_deps.shuffle unless ENV["UPDATER_DETERMINISTIC"] - - if all_deps.any? && allowed_deps.none? + if dependency_snapshot.dependencies.any? && dependency_snapshot.allowed_dependencies.none? Dependabot.logger.info("Found no dependencies to update after filtering allowed updates") + return [] end - allowed_deps + dependency_snapshot.allowed_dependencies end # Returns a Dependabot::DependencyChange object that encapsulates the diff --git a/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb b/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb index 87505479c9a..560b2cb0d16 100644 --- a/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb @@ -44,18 +44,7 @@ def perform :created_pull_requests def dependencies - # Gradle, Maven and Nuget dependency names can be case-insensitive and - # the dependency name in the security advisory often doesn't match what - # users have specified in their manifest. - # - # It's technically possibly to publish case-sensitive npm packages to a - # private registry but shouldn't cause problems here as job.dependencies - # is set either from an existing PR rebase/recreate or a security - # advisory. - job_dependencies = job.dependencies.map(&:downcase) - dependency_snapshot.dependencies.select do |dep| - job_dependencies.include?(dep.name.downcase) - end + dependency_snapshot.job_dependencies end # rubocop:disable Metrics/AbcSize diff --git a/updater/lib/dependabot/updater/operations/update_all_versions.rb b/updater/lib/dependabot/updater/operations/update_all_versions.rb index 5ca4f84762a..8fb797b25e3 100644 --- a/updater/lib/dependabot/updater/operations/update_all_versions.rb +++ b/updater/lib/dependabot/updater/operations/update_all_versions.rb @@ -39,19 +39,16 @@ def perform :created_pull_requests def dependencies - all_deps = dependency_snapshot.dependencies - - allowed_deps = all_deps.select { |d| job.allowed_update?(d) } - # Return dependencies in a random order, with top-level dependencies - # considered first so that dependency runs which time out don't always hit - # the same dependencies - allowed_deps = allowed_deps.shuffle unless ENV["UPDATER_DETERMINISTIC"] - - if all_deps.any? && allowed_deps.none? + if dependency_snapshot.dependencies.any? && dependency_snapshot.allowed_dependencies.none? Dependabot.logger.info("Found no dependencies to update after filtering allowed updates") + return [] end - allowed_deps + if Environment.deterministic_updates? + dependency_snapshot.allowed_dependencies + else + dependency_snapshot.allowed_dependencies.shuffle + end end def check_and_create_pr_with_error_handling(dependency)