diff --git a/updater/lib/dependabot/job.rb b/updater/lib/dependabot/job.rb index 4dee011b16a..d17ae4201ea 100644 --- a/updater/lib/dependabot/job.rb +++ b/updater/lib/dependabot/job.rb @@ -28,7 +28,6 @@ module Dependabot class Job # rubocop:disable Metrics/ClassLength extend T::Sig - include Dependabot::Updater::UpdateTypeHelper TOP_LEVEL_DEPENDENCY_TYPES = T.let(%w(direct production development).freeze, T::Array[String]) PERMITTED_KEYS = T.let( @@ -301,7 +300,10 @@ def reject_external_code? # was vulnerable instead of the current version. This prevents security updates # from being filtered out after the dependency has already been updated in group scenarios. # - # rubocop:disable Metrics/AbcSize + # NOTE: update-types (semver-based filtering) is handled in ignore_conditions_for, + # not here — allowed_update? runs pre-resolution when only the current version is + # known, and semver-level filtering needs version ranges computed from that version. + # # rubocop:disable Metrics/PerceivedComplexity # rubocop:disable Metrics/CyclomaticComplexity sig { params(dependency: Dependency, check_previous_version: T::Boolean).returns(T::Boolean) } @@ -324,15 +326,6 @@ def allowed_update?(dependency, check_previous_version: false) condition_name = update.fetch("dependency-name", dependency.name) next false unless name_match?(condition_name, dependency.name) - # Check update-types (semver-based filtering) - security updates bypass this - allowed_update_types = update.fetch("update-types", nil) - if allowed_update_types.is_a?(Array) && !allowed_update_types.empty? && !security_update - dep_update_type = update_type_for_dependency(dependency) - config_type = "version-update:semver-#{dep_update_type}" if dep_update_type - normalized_types = allowed_update_types.filter_map { |t| t.is_a?(String) ? t.downcase.strip : nil } - next false if config_type && !normalized_types.include?(config_type) - end - # Check the dependency-type (defaulting to all) dep_type = update.fetch("dependency-type", "all") next false if dep_type == "indirect" && @@ -348,7 +341,6 @@ def allowed_update?(dependency, check_previous_version: false) true end end - # rubocop:enable Metrics/AbcSize # rubocop:enable Metrics/PerceivedComplexity # rubocop:enable Metrics/CyclomaticComplexity @@ -443,10 +435,16 @@ def security_advisories_for(dependency) sig { params(dependency: Dependabot::Dependency).returns(T::Array[String]) } def ignore_conditions_for(dependency) - update_config.ignored_versions_for( + conditions = update_config.ignored_versions_for( dependency, security_updates_only: security_updates_only? ) + + # Supplement with implicit ignore ranges derived from allow update-types. + # allow update-types cannot be checked in allowed_update? because it runs + # pre-resolution when only the current version is known. Version ranges + # only need the current version — the same mechanism as ignore update-types. + conditions + ignored_versions_from_allowed_update_types(dependency) end # TODO: Present Dependabot::Config::IgnoreCondition in calling code @@ -485,6 +483,101 @@ def completely_ignored?(dependency) ignore_conditions_for(dependency).any?(Dependabot::Config::IgnoreCondition::ALL_VERSIONS) end + # Derives implicit ignore version ranges from allow rules that specify update-types. + # For example, if allow says only "semver-patch", this computes ignore ranges for + # major and minor — using the same mechanism as IgnoreCondition#versions_by_type. + sig { params(dependency: Dependabot::Dependency).returns(T::Array[String]) } + def ignored_versions_from_allowed_update_types(dependency) + return [] if security_updates_only? + + permitted_types = collect_permitted_update_types(dependency) + return [] if permitted_types.empty? + + disallowed_types = Dependabot::Updater::UpdateTypeHelper::ALL_SEMVER_UPDATE_TYPES - permitted_types + + version = version_for_dependency(dependency) + return [] unless version + + disallowed_types.flat_map do |t| + case t + when Dependabot::Config::IgnoreCondition::PATCH_VERSION_TYPE + version.ignored_patch_versions + when Dependabot::Config::IgnoreCondition::MINOR_VERSION_TYPE + version.ignored_minor_versions + when Dependabot::Config::IgnoreCondition::MAJOR_VERSION_TYPE + version.ignored_major_versions + else + [] + end + end.compact + end + + # Collects the union of update-types from all matching allow rules for a dependency. + # Returns empty if no matching rules specify update-types (meaning no filtering needed). + # If any matching rule lacks update-types, it permits all types — returns empty. + sig { params(dependency: Dependabot::Dependency).returns(T::Array[String]) } + def collect_permitted_update_types(dependency) + matching_rules = matching_allow_rules(dependency) + return [] if matching_rules.empty? + + # If any matching rule lacks update-types, it permits all types + return [] if matching_rules.any? { |r| allow_rule_permits_all_types?(r) } + + matching_rules + .flat_map { |r| r.fetch("update-types", []) } + .filter_map { |t| t.is_a?(String) ? t.downcase.strip : nil } + .select { |t| Dependabot::Updater::UpdateTypeHelper::ALL_SEMVER_UPDATE_TYPES.include?(t) } + .uniq + end + + sig { params(dependency: Dependabot::Dependency).returns(T::Array[T::Hash[String, T.untyped]]) } + def matching_allow_rules(dependency) + allowed_updates.select do |update| + allow_rule_matches_dependency?(update, dependency) + end + end + + sig { params(update: T::Hash[String, T.untyped], dependency: Dependabot::Dependency).returns(T::Boolean) } + def allow_rule_matches_dependency?(update, dependency) + condition_name = update.fetch("dependency-name", nil) + return false if condition_name && !name_match?(condition_name, dependency.name) + + dep_type = update.fetch("dependency-type", nil) + return true if dep_type.nil? || dep_type == "all" + + # Indirect deps don't match top-level type rules (matching allowed_update? behavior) + return false if dependency.requirements.none? && TOP_LEVEL_DEPENDENCY_TYPES.include?(dep_type) + + case dep_type + when "production" then dependency.production? + when "development" then !dependency.production? + when "direct" then dependency.requirements.any? + when "indirect" then dependency.requirements.none? + else true + end + end + + sig { params(rule: T::Hash[String, T.untyped]).returns(T::Boolean) } + def allow_rule_permits_all_types?(rule) + !rule.key?("update-types") || !rule["update-types"].is_a?(Array) || rule["update-types"].empty? + end + + sig { params(dependency: Dependabot::Dependency).returns(T.nilable(Dependabot::Version)) } + def version_for_dependency(dependency) + version_str = dependency.version + return nil if version_str.nil? || version_str.empty? + + version_class = begin + Dependabot::Utils.version_class_for_package_manager(dependency.package_manager) + rescue StandardError + Dependabot::Version + end + + return nil unless version_class.correct?(version_str) + + version_class.new(version_str) + end + sig { void } def register_experiments experiments.entries.each do |name, value| diff --git a/updater/lib/dependabot/updater/update_type_helper.rb b/updater/lib/dependabot/updater/update_type_helper.rb index ad3e4ec2c06..ded409d9fbe 100644 --- a/updater/lib/dependabot/updater/update_type_helper.rb +++ b/updater/lib/dependabot/updater/update_type_helper.rb @@ -2,6 +2,7 @@ # frozen_string_literal: true require "sorbet-runtime" +require "dependabot/config/ignore_condition" require "dependabot/utils" module Dependabot @@ -12,6 +13,19 @@ class Updater module UpdateTypeHelper extend T::Sig + SEMVER_MAJOR = "major" + SEMVER_MINOR = "minor" + SEMVER_PATCH = "patch" + + ALL_SEMVER_UPDATE_TYPES = T.let( + [ + Dependabot::Config::IgnoreCondition::MAJOR_VERSION_TYPE, + Dependabot::Config::IgnoreCondition::MINOR_VERSION_TYPE, + Dependabot::Config::IgnoreCondition::PATCH_VERSION_TYPE + ].freeze, + T::Array[String] + ) + # Represents semantic version components (major, minor, patch) class SemverParts < T::Struct const :major, Integer @@ -58,11 +72,11 @@ def classify_semver_update(prev, curr) curr_parts = semver_parts(curr) return nil if prev_parts.nil? || curr_parts.nil? - return "major" if curr_parts.major > prev_parts.major - return "minor" if curr_parts.major == prev_parts.major && curr_parts.minor > prev_parts.minor - return "patch" if curr_parts.major == prev_parts.major && - curr_parts.minor == prev_parts.minor && - curr_parts.patch > prev_parts.patch + return SEMVER_MAJOR if curr_parts.major > prev_parts.major + return SEMVER_MINOR if curr_parts.major == prev_parts.major && curr_parts.minor > prev_parts.minor + return SEMVER_PATCH if curr_parts.major == prev_parts.major && + curr_parts.minor == prev_parts.minor && + curr_parts.patch > prev_parts.patch Dependabot.logger.info( "Could not classify semver update: #{prev_parts.major}.#{prev_parts.minor}.#{prev_parts.patch} -> " \ diff --git a/updater/spec/dependabot/job_spec.rb b/updater/spec/dependabot/job_spec.rb index 9df296dab69..5a1c2affbde 100644 --- a/updater/spec/dependabot/job_spec.rb +++ b/updater/spec/dependabot/job_spec.rb @@ -456,6 +456,10 @@ end context "with update-types in allow block" do + # update-types filtering is handled by ignore_conditions_for (via version ranges), + # NOT by allowed_update?. allowed_update? only checks dependency-name, dependency-type, + # and update-type (security). So dependencies with update-types should still pass + # allowed_update? as long as name/type match. let(:dependency) do Dependabot::Dependency.new( name: "business", @@ -466,20 +470,7 @@ ) end - context "when allowing minor updates and dependency has minor update" do - let(:allowed_updates) do - [ - { - "dependency-name" => "business", - "update-types" => ["version-update:semver-minor"] - } - ] - end - - it { is_expected.to be(true) } - end - - context "when allowing only patch updates but dependency has minor update" do + context "when allow rule has update-types and dependency name matches" do let(:allowed_updates) do [ { @@ -489,118 +480,7 @@ ] end - it { is_expected.to be(false) } - end - - context "when allowing multiple update types including minor" do - let(:allowed_updates) do - [ - { - "dependency-name" => "business", - "update-types" => ["version-update:semver-patch", "version-update:semver-minor"] - } - ] - end - - it { is_expected.to be(true) } - end - - context "with a major version update" do - let(:dependency) do - Dependabot::Dependency.new( - name: "business", - package_manager: "bundler", - version: "2.0.0", - previous_version: "1.8.0", - requirements: requirements - ) - end - - context "when allowing major updates" do - let(:allowed_updates) do - [ - { - "dependency-name" => "business", - "update-types" => ["version-update:semver-major"] - } - ] - end - - it { is_expected.to be(true) } - end - - context "when only allowing minor updates" do - let(:allowed_updates) do - [ - { - "dependency-name" => "business", - "update-types" => ["version-update:semver-minor"] - } - ] - end - - it { is_expected.to be(false) } - end - end - - context "with a patch version update" do - let(:dependency) do - Dependabot::Dependency.new( - name: "business", - package_manager: "bundler", - version: "1.8.1", - previous_version: "1.8.0", - requirements: requirements - ) - end - - let(:allowed_updates) do - [ - { - "dependency-name" => "business", - "update-types" => ["version-update:semver-patch"] - } - ] - end - - it { is_expected.to be(true) } - end - - context "when combining update-types with dependency-type" do - let(:allowed_updates) do - [ - { - "dependency-name" => "business", - "dependency-type" => "direct", - "update-types" => ["version-update:semver-minor", "version-update:semver-patch"] - } - ] - end - - it { is_expected.to be(true) } - end - - context "when dependency has no previous version" do - let(:dependency) do - Dependabot::Dependency.new( - name: "business", - package_manager: "bundler", - version: "1.9.0", - previous_version: nil, - requirements: requirements - ) - end - - let(:allowed_updates) do - [ - { - "dependency-name" => "business", - "update-types" => ["version-update:semver-minor"] - } - ] - end - - it "allows update when semver type cannot be determined" do + it "allows the update (update-types filtering is handled by ignore_conditions_for)" do expect(allowed_update).to be(true) end end @@ -628,35 +508,7 @@ ] end - it "treats empty update-types as no filtering" do - expect(allowed_update).to be(true) - end - end - - context "when one allow rule has update-types and another does not" do - let(:dependency) do - Dependabot::Dependency.new( - name: "business", - package_manager: "bundler", - version: "2.0.0", - previous_version: "1.0.0", - requirements: requirements - ) - end - - let(:allowed_updates) do - [ - { - "dependency-name" => "business", - "update-types" => ["version-update:semver-patch"] - }, - { - "dependency-name" => "business" - } - ] - end - - it "allows if any rule matches (second rule has no update-types filter)" do + it "allows the update" do expect(allowed_update).to be(true) end end @@ -682,7 +534,7 @@ ] end - it "bypasses update-types filtering for security updates" do + it "allows security update regardless of update-types" do expect(allowed_update).to be(true) end end @@ -1152,6 +1004,195 @@ end end + describe "#ignore_conditions_for" do + subject(:ignored) { job.ignore_conditions_for(dependency) } + + let(:dependency) do + Dependabot::Dependency.new( + name: "business", + package_manager: "bundler", + version: "1.8.0", + requirements: [{ file: "Gemfile", requirement: "~> 1.8.0", groups: [], source: nil }] + ) + end + + context "with no ignore conditions and no allow update-types" do + let(:allowed_updates) do + [{ "dependency-type" => "direct", "update-type" => "all" }] + end + + it "returns an empty array" do + expect(ignored).to be_empty + end + end + + context "with explicit ignore conditions" do + let(:attributes) do + super().merge( + ignore_conditions: [ + { + "dependency-name" => "business", + "version-requirement" => "> 2.0.0" + } + ] + ) + end + + it "returns the explicit ignore ranges" do + expect(ignored).to include("> 2.0.0") + end + end + + context "with allow update-types" do + context "when allowing only patch updates" do + let(:allowed_updates) do + [{ "dependency-name" => "business", "update-types" => ["version-update:semver-patch"] }] + end + + it "generates ignore ranges for major and minor" do + expect(ignored).to include(a_string_matching(/>= 2/)) + expect(ignored).to include(a_string_matching(/>= 1\.9/)) + expect(ignored).not_to include(a_string_matching(/> 1\.8\.0, < 1\.9/)) + end + end + + context "when allowing only minor updates" do + let(:allowed_updates) do + [{ "dependency-name" => "business", "update-types" => ["version-update:semver-minor"] }] + end + + it "generates ignore ranges for major and patch" do + expect(ignored).to include(a_string_matching(/>= 2/)) + expect(ignored).to include(a_string_matching(/> 1\.8\.0, < 1\.9/)) + expect(ignored).not_to include(a_string_matching(/>= 1\.9/)) + end + end + + context "when allowing minor and patch" do + let(:allowed_updates) do + [{ + "dependency-name" => "business", + "update-types" => ["version-update:semver-minor", "version-update:semver-patch"] + }] + end + + it "generates ignore ranges only for major" do + expect(ignored).to include(a_string_matching(/>= 2/)) + expect(ignored).not_to include(a_string_matching(/>= 1\.9/)) + expect(ignored).not_to include(a_string_matching(/> 1\.8\.0/)) + end + end + + context "when allow rule has no update-types" do + let(:allowed_updates) do + [{ "dependency-name" => "business", "dependency-type" => "direct", "update-type" => "all" }] + end + + it "does not generate any implicit ignore ranges" do + expect(ignored).to be_empty + end + end + + context "when one allow rule has update-types and another does not" do + let(:allowed_updates) do + [ + { "dependency-name" => "business", "update-types" => ["version-update:semver-patch"] }, + { "dependency-name" => "business" } + ] + end + + it "does not generate ignore ranges (rule without update-types permits all)" do + expect(ignored).to be_empty + end + end + + context "when dependency name does not match" do + let(:allowed_updates) do + [{ "dependency-name" => "other-dep", "update-types" => ["version-update:semver-patch"] }] + end + + it "does not generate ignore ranges" do + expect(ignored).to be_empty + end + end + + context "when security_updates_only is true" do + let(:security_updates_only) { true } + let(:dependencies) { ["business"] } + let(:allowed_updates) do + [{ "dependency-name" => "business", "update-types" => ["version-update:semver-patch"] }] + end + + it "does not generate ignore ranges" do + expect(ignored).to be_empty + end + end + + context "with dependency-type filtering" do + let(:allowed_updates) do + [{ "dependency-type" => "production", "update-types" => ["version-update:semver-patch"] }] + end + + context "when dependency is production" do + let(:dependency) do + Dependabot::Dependency.new( + name: "business", + package_manager: "bundler", + version: "1.8.0", + requirements: [{ file: "Gemfile", requirement: "~> 1.8.0", groups: ["default"], source: nil }] + ) + end + + it "generates ignore ranges for major and minor" do + expect(ignored).to include(a_string_matching(/>= 2/)) + expect(ignored).to include(a_string_matching(/>= 1\.9/)) + end + end + + context "when dependency is development" do + let(:dependency) do + Dependabot::Dependency.new( + name: "business", + package_manager: "bundler", + version: "1.8.0", + requirements: [{ file: "Gemfile", requirement: "~> 1.8.0", groups: ["development"], source: nil }] + ) + end + + it "does not generate ignore ranges (rule does not match)" do + expect(ignored).to be_empty + end + end + end + + context "with wildcard dependency name" do + let(:allowed_updates) do + [{ "dependency-name" => "busi*", "update-types" => ["version-update:semver-patch"] }] + end + + it "matches and generates ignore ranges" do + expect(ignored).to include(a_string_matching(/>= 2/)) + end + end + + context "when combining with explicit ignore conditions" do + let(:attributes) do + super().merge( + ignore_conditions: [{ "dependency-name" => "business", "version-requirement" => "> 1.8.1" }] + ) + end + let(:allowed_updates) do + [{ "dependency-name" => "business", "update-types" => ["version-update:semver-patch"] }] + end + + it "includes both explicit ignore and implicit allow-derived ranges" do + expect(ignored).to include("> 1.8.1") + expect(ignored).to include(a_string_matching(/>= 2/)) + end + end + end + end + describe "#allowed_update? with check_previous_version parameter" do subject(:allowed_update_with_completed) do job.allowed_update?(dependency, check_previous_version: check_previous_version) diff --git a/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb b/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb index b6a4ac9567b..a6464726d91 100644 --- a/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb +++ b/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb @@ -403,5 +403,62 @@ ) end end + + context "when allow rules specify update-types (patch only)" do + let(:job_definition) do + job_definition_fixture("bundler/version_updates/allow_update_types_patch_only") + end + let(:dependency_files) do + original_bundler_files(fixture: "bundler_allow_update_types") + end + + it "passes ignored_versions that block major and minor updates to the UpdateChecker" do + update_all_versions.send(:update_checker_for, dependency, raise_on_ignored: false) + + expect(stub_update_checker_class).to have_received(:new).with( + hash_including( + ignored_versions: a_collection_containing_exactly( + a_string_matching(/>= 5/), + a_string_matching(/>= 4\.1/) + ) + ) + ) + end + end + + context "when allow rules specify update-types (minor and patch)" do + let(:job_definition) do + job_definition_fixture("bundler/version_updates/allow_update_types_minor_and_patch") + end + let(:dependency_files) do + original_bundler_files(fixture: "bundler_allow_update_types") + end + + it "passes ignored_versions that block only major updates to the UpdateChecker" do + update_all_versions.send(:update_checker_for, dependency, raise_on_ignored: false) + + expect(stub_update_checker_class).to have_received(:new).with( + hash_including( + ignored_versions: a_collection_containing_exactly( + a_string_matching(/>= 5/) + ) + ) + ) + end + end + + context "when allow rules have no update-types" do + let(:job_definition) do + job_definition_fixture("bundler/version_updates/pull_request_simple") + end + + it "passes no implicit ignored_versions to the UpdateChecker" do + update_all_versions.send(:update_checker_for, dependency, raise_on_ignored: false) + + expect(stub_update_checker_class).to have_received(:new).with( + hash_including(ignored_versions: []) + ) + end + end end end diff --git a/updater/spec/fixtures/bundler_allow_update_types/original/Gemfile b/updater/spec/fixtures/bundler_allow_update_types/original/Gemfile new file mode 100644 index 00000000000..b446f56370e --- /dev/null +++ b/updater/spec/fixtures/bundler_allow_update_types/original/Gemfile @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +source "https://rubygems.org" + +gem "dummy-pkg-a", "~> 4.0.0" diff --git a/updater/spec/fixtures/bundler_allow_update_types/original/Gemfile.lock b/updater/spec/fixtures/bundler_allow_update_types/original/Gemfile.lock new file mode 100644 index 00000000000..6e93a55084d --- /dev/null +++ b/updater/spec/fixtures/bundler_allow_update_types/original/Gemfile.lock @@ -0,0 +1,13 @@ +GEM + remote: https://rubygems.org/ + specs: + dummy-pkg-a (4.0.0) + +PLATFORMS + ruby + +DEPENDENCIES + dummy-pkg-a (~> 4.0.0) + +BUNDLED WITH + 2.5.0 diff --git a/updater/spec/fixtures/job_definitions/bundler/version_updates/allow_update_types_minor_and_patch.yaml b/updater/spec/fixtures/job_definitions/bundler/version_updates/allow_update_types_minor_and_patch.yaml new file mode 100644 index 00000000000..382230552ef --- /dev/null +++ b/updater/spec/fixtures/job_definitions/bundler/version_updates/allow_update_types_minor_and_patch.yaml @@ -0,0 +1,45 @@ +job: + allowed-updates: + - dependency-name: "dummy-pkg-a" + update-types: + - "version-update:semver-minor" + - "version-update:semver-patch" + commit-message-options: + prefix: + prefix-development: + include-scope: + credentials-metadata: + - type: git_source + host: github.com + debug: + dependencies: + dependency-groups: [] + dependency-group-to-refresh: + existing-pull-requests: [] + existing-group-pull-requests: [] + experiments: + record-ecosystem-versions: true + record-update-job-unknown-error: true + proxy-cached: true + dependency-change-validation: true + ignore-conditions: [] + lockfile-only: false + max-updater-run-time: 2700 + package-manager: bundler + proxy-log-response-body-on-auth-failure: true + requirements-update-strategy: bump_versions_if_necessary + reject-external-code: false + security-advisories: [] + security-updates-only: false + source: + provider: github + repo: dsp-testing/dependabot-allow-update-types + branch: + directory: "/." + api-endpoint: https://api.github.com/ + hostname: github.com + commit: mock-sha + updating-a-pull-request: false + update-subdependencies: false + vendor-dependencies: false + repo-private: true diff --git a/updater/spec/fixtures/job_definitions/bundler/version_updates/allow_update_types_patch_only.yaml b/updater/spec/fixtures/job_definitions/bundler/version_updates/allow_update_types_patch_only.yaml new file mode 100644 index 00000000000..4daf1a0f844 --- /dev/null +++ b/updater/spec/fixtures/job_definitions/bundler/version_updates/allow_update_types_patch_only.yaml @@ -0,0 +1,44 @@ +job: + allowed-updates: + - dependency-name: "dummy-pkg-a" + update-types: + - "version-update:semver-patch" + commit-message-options: + prefix: + prefix-development: + include-scope: + credentials-metadata: + - type: git_source + host: github.com + debug: + dependencies: + dependency-groups: [] + dependency-group-to-refresh: + existing-pull-requests: [] + existing-group-pull-requests: [] + experiments: + record-ecosystem-versions: true + record-update-job-unknown-error: true + proxy-cached: true + dependency-change-validation: true + ignore-conditions: [] + lockfile-only: false + max-updater-run-time: 2700 + package-manager: bundler + proxy-log-response-body-on-auth-failure: true + requirements-update-strategy: bump_versions_if_necessary + reject-external-code: false + security-advisories: [] + security-updates-only: false + source: + provider: github + repo: dsp-testing/dependabot-allow-update-types + branch: + directory: "/." + api-endpoint: https://api.github.com/ + hostname: github.com + commit: mock-sha + updating-a-pull-request: false + update-subdependencies: false + vendor-dependencies: false + repo-private: true