From b3f5c732bc6f0de0938999535787faa989774038 Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Thu, 2 Apr 2026 23:26:25 -0700 Subject: [PATCH 1/2] Parse Job inputs into typed structs instead of untyped hashes Replaces T::Hash[String, T.untyped] with T::ImmutableStruct for the three main untyped collections in the Job class: - AllowedUpdate: dependency_name, dependency_type, update_type, update_types fields replace hash key lookups like update.fetch("dependency-name") - IgnoreCondition (Job-scoped): dependency_name, version_requirement, update_types, source fields replace ic["dependency-name"] etc. - SecurityAdvisoryEntry: dependency_name, affected_versions, patched_versions, unaffected_versions replace adv["affected-versions"] Each struct has a from_hash class method that parses the raw JSON hash at the boundary (in the Job constructor). Downstream code gets typed field access instead of T.untyped hash lookups. --- updater/lib/dependabot/dependency_change.rb | 9 +- updater/lib/dependabot/job.rb | 85 ++++++++++--------- updater/lib/dependabot/job/allowed_update.rb | 44 ++++++++++ .../lib/dependabot/job/ignore_condition.rb | 34 ++++++++ .../dependabot/job/security_advisory_entry.rb | 28 ++++++ .../refresh_security_update_pull_request.rb | 2 +- ...fresh_security_update_pull_request_spec.rb | 16 +++- 7 files changed, 173 insertions(+), 45 deletions(-) create mode 100644 updater/lib/dependabot/job/allowed_update.rb create mode 100644 updater/lib/dependabot/job/ignore_condition.rb create mode 100644 updater/lib/dependabot/job/security_advisory_entry.rb diff --git a/updater/lib/dependabot/dependency_change.rb b/updater/lib/dependabot/dependency_change.rb index a540a5a4e10..2e047935e95 100644 --- a/updater/lib/dependabot/dependency_change.rb +++ b/updater/lib/dependabot/dependency_change.rb @@ -95,7 +95,14 @@ def pr_message dependency_group: dependency_group, pr_message_max_length: pr_message_max_length, pr_message_encoding: pr_message_encoding, - ignore_conditions: job.ignore_conditions, + ignore_conditions: job.ignore_conditions.map do |ic| + { + "dependency-name" => ic.dependency_name, + "version-requirement" => ic.version_requirement, + "source" => ic.source, + "updated-at" => ic.updated_at + }.compact + end, notices: notices ).message diff --git a/updater/lib/dependabot/job.rb b/updater/lib/dependabot/job.rb index 313babe8f4e..6c71d3df94e 100644 --- a/updater/lib/dependabot/job.rb +++ b/updater/lib/dependabot/job.rb @@ -15,6 +15,10 @@ require "dependabot/package/release_cooldown_options" require "dependabot/updater/update_type_helper" +require "dependabot/job/allowed_update" +require "dependabot/job/ignore_condition" +require "dependabot/job/security_advisory_entry" + # Describes a single Dependabot workload within the GitHub-integrated Service # # This primarily acts as a value class to hold inputs for various Core objects @@ -62,7 +66,7 @@ class Job # rubocop:disable Metrics/ClassLength T::Array[Symbol] ) - sig { returns(T::Array[T::Hash[String, T.untyped]]) } + sig { returns(T::Array[Dependabot::Job::AllowedUpdate]) } attr_reader :allowed_updates sig { returns(T::Array[Dependabot::Credential]) } @@ -86,7 +90,7 @@ class Job # rubocop:disable Metrics/ClassLength sig { returns(String) } attr_reader :command - sig { returns(T::Array[T.untyped]) } + sig { returns(T::Array[Dependabot::Job::IgnoreCondition]) } attr_reader :ignore_conditions sig { returns(String) } @@ -95,7 +99,7 @@ class Job # rubocop:disable Metrics/ClassLength sig { returns(T.nilable(Dependabot::RequirementsUpdateStrategy)) } attr_reader :requirements_update_strategy - sig { returns(T::Array[T.untyped]) } + sig { returns(T::Array[Dependabot::Job::SecurityAdvisoryEntry]) } attr_reader :security_advisories sig { returns(T::Boolean) } @@ -168,8 +172,11 @@ def self.standardise_keys(hash) def initialize(attributes) # rubocop:disable Metrics/AbcSize,Metrics/MethodLength @id = T.let(attributes.fetch(:id), String) @command = T.let(attributes.fetch(:command, ""), String) - @allowed_updates = T.let(attributes.fetch(:allowed_updates), T::Array[T.untyped]) - @commit_message_options = T.let( + @allowed_updates = T.let( + attributes.fetch(:allowed_updates).map { |h| AllowedUpdate.from_hash(h) }, + T::Array[AllowedUpdate] + ) + @commit_message_options = T.let( attributes.fetch(:commit_message_options, {}), T.nilable(T::Hash[T.untyped, T.untyped]) ) @@ -195,7 +202,10 @@ def initialize(attributes) # rubocop:disable Metrics/AbcSize,Metrics/MethodLengt attributes.fetch(:experiments, {}), T.nilable(T::Hash[T.untyped, T.untyped]) ) - @ignore_conditions = T.let(attributes.fetch(:ignore_conditions), T::Array[T.untyped]) + @ignore_conditions = T.let( + attributes.fetch(:ignore_conditions).map { |h| Job::IgnoreCondition.from_hash(h) }, + T::Array[Job::IgnoreCondition] + ) @package_manager = T.let(attributes.fetch(:package_manager), String) @reject_external_code = T.let(attributes.fetch(:reject_external_code, false), T::Boolean) @repo_contents_path = T.let(attributes.fetch(:repo_contents_path, nil), T.nilable(String)) @@ -207,7 +217,10 @@ def initialize(attributes) # rubocop:disable Metrics/AbcSize,Metrics/MethodLengt T.nilable(Dependabot::RequirementsUpdateStrategy) ) - @security_advisories = T.let(attributes.fetch(:security_advisories), T::Array[T.untyped]) + @security_advisories = T.let( + attributes.fetch(:security_advisories).map { |h| SecurityAdvisoryEntry.from_hash(h) }, + T::Array[SecurityAdvisoryEntry] + ) @security_updates_only = T.let(attributes.fetch(:security_updates_only), T::Boolean) @source = T.let(build_source(attributes.fetch(:source)), Dependabot::Source) @token = T.let(attributes.fetch(:token, nil), T.nilable(String)) @@ -327,18 +340,17 @@ def allowed_update?(dependency, check_previous_version: false) allowed_updates.any? do |update| # Check the update-type (defaulting to all) - update_type = update.fetch("update-type", "all") # NOTE: Preview supports specifying a "security" update type whereas # native will say "security-updates-only" - security_update = update_type == "security" || security_updates_only? + security_update = update.update_type == "security" || security_updates_only? next false if security_update && !vulnerable?(dependency, check_previous_version) # Check the dependency-name (defaulting to matching) - condition_name = update.fetch("dependency-name", dependency.name) + condition_name = update.dependency_name || dependency.name next false unless name_match?(condition_name, dependency.name) # Check the dependency-type (defaulting to all) - dep_type = update.fetch("dependency-type", "all") + dep_type = update.dependency_type next false if dep_type == "indirect" && dependency.requirements.any? # In dependabot-api, dependency-type is defaulting to "direct" not "all". Ignoring @@ -428,18 +440,16 @@ def commit_message_options def security_advisories_for(dependency) relevant_advisories = security_advisories - .select { |adv| adv.fetch("dependency-name").casecmp(dependency.name).zero? } + .select { |adv| T.must(adv.dependency_name.casecmp(dependency.name)).zero? } - relevant_advisories.map do |adv| - vulnerable_versions = adv["affected-versions"] || [] - safe_versions = (adv["patched-versions"] || []) + - (adv["unaffected-versions"] || []) + requirement_class = Dependabot::Utils.requirement_class_for_package_manager(package_manager) + relevant_advisories.map do |adv| Dependabot::SecurityAdvisory.new( dependency_name: dependency.name, package_manager: package_manager, - vulnerable_versions: vulnerable_versions, - safe_versions: safe_versions + vulnerable_versions: adv.affected_versions.flat_map { |v| requirement_class.requirements_array(v) }, + safe_versions: adv.patched_versions + adv.unaffected_versions ) end end @@ -470,16 +480,14 @@ def ignore_conditions_for(dependency) # were created via "@dependabot ignore version" commands sig { params(dependency: Dependabot::Dependency).void } def log_ignore_conditions_for(dependency) - conditions = ignore_conditions.select { |ic| name_match?(ic["dependency-name"], dependency.name) } + conditions = ignore_conditions.select { |ic| name_match?(ic.dependency_name, dependency.name) } if conditions.any? Dependabot.logger.info("Ignored versions:") conditions.each do |ic| - unless ic["version-requirement"].nil? - Dependabot.logger.info(" #{ic['version-requirement']} - from #{ic['source']}") - end + Dependabot.logger.info(" #{ic.version_requirement} - from #{ic.source}") unless ic.version_requirement.nil? - ic["update-types"]&.each do |update_type| - msg = " #{update_type} - from #{ic['source']}" + ic.update_types&.each do |update_type| + msg = " #{update_type} - from #{ic.source}" msg += " (doesn't apply to security update)" if security_updates_only? Dependabot.logger.info(msg) end @@ -577,26 +585,26 @@ def collect_permitted_update_types(dependency) return [] if matching_rules.any? { |r| allow_rule_permits_all_types?(r) } matching_rules - .flat_map { |r| r.fetch("update-types", []) } + .flat_map(&: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]]) } + sig { params(dependency: Dependabot::Dependency).returns(T::Array[AllowedUpdate]) } 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) } + sig { params(update: AllowedUpdate, dependency: Dependabot::Dependency).returns(T::Boolean) } def allow_rule_matches_dependency?(update, dependency) - condition_name = update.fetch("dependency-name", nil) + condition_name = update.dependency_name 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" + dep_type = update.dependency_type + return true if 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) @@ -610,9 +618,9 @@ def allow_rule_matches_dependency?(update, dependency) end end - sig { params(rule: T::Hash[String, T.untyped]).returns(T::Boolean) } + sig { params(rule: AllowedUpdate).returns(T::Boolean) } def allow_rule_permits_all_types?(rule) - !rule.key?("update-types") || !rule["update-types"].is_a?(Array) || rule["update-types"].empty? + rule.update_types.empty? end sig { params(dependency: Dependabot::Dependency).returns(T.nilable(Dependabot::Version)) } @@ -731,17 +739,16 @@ def clean_directories(source_details) def calculate_update_config update_config_ignore_conditions = ignore_conditions.map do |ic| Dependabot::Config::IgnoreCondition.new( - dependency_name: T.let(ic["dependency-name"], String), - versions: T.let([ic["version-requirement"]].compact, T::Array[String]), - update_types: T.let(ic["update-types"], T.nilable(T::Array[String])) + dependency_name: ic.dependency_name, + versions: [ic.version_requirement].compact, + update_types: ic.update_types ) end - update_config = Dependabot::Config::UpdateConfig.new( - ignore_conditions: T.let(update_config_ignore_conditions, T::Array[Dependabot::Config::IgnoreCondition]), - exclude_paths: T.let(exclude_paths, T.nilable(T::Array[String])) + Dependabot::Config::UpdateConfig.new( + ignore_conditions: update_config_ignore_conditions, + exclude_paths: exclude_paths ) - T.let(update_config, Dependabot::Config::UpdateConfig) end end end diff --git a/updater/lib/dependabot/job/allowed_update.rb b/updater/lib/dependabot/job/allowed_update.rb new file mode 100644 index 00000000000..e71be504a9b --- /dev/null +++ b/updater/lib/dependabot/job/allowed_update.rb @@ -0,0 +1,44 @@ +# typed: strict +# frozen_string_literal: true + +require "sorbet-runtime" + +module Dependabot + class Job + # Parsed representation of an allowed update rule from the job definition. + # + # Replaces the raw T::Hash[String, T.untyped] with a typed struct so + # downstream code gets compile-time checked field access instead of + # hash key lookups that return T.untyped. + class AllowedUpdate < T::ImmutableStruct + extend T::Sig + + const :dependency_name, T.nilable(String) + const :dependency_type, String, default: "all" + const :update_type, String, default: "all" + const :update_types, T::Array[String], default: [] + + sig { params(hash: T::Hash[String, T.untyped]).returns(AllowedUpdate) } + def self.from_hash(hash) + new( + dependency_name: hash["dependency-name"], + dependency_type: hash.fetch("dependency-type", "all"), + update_type: hash.fetch("update-type", "all"), + update_types: hash.fetch("update-types", []) || [] + ) + end + + # Temporary bridge: returns the original hash format for code that + # still expects hash access. Remove once all callers are migrated. + sig { returns(T::Hash[String, T.untyped]) } + def to_hash + h = T.let({}, T::Hash[String, T.untyped]) + h["dependency-name"] = dependency_name if dependency_name + h["dependency-type"] = dependency_type + h["update-type"] = update_type + h["update-types"] = update_types unless update_types.empty? + h + end + end + end +end diff --git a/updater/lib/dependabot/job/ignore_condition.rb b/updater/lib/dependabot/job/ignore_condition.rb new file mode 100644 index 00000000000..9d0a9a2431d --- /dev/null +++ b/updater/lib/dependabot/job/ignore_condition.rb @@ -0,0 +1,34 @@ +# typed: strict +# frozen_string_literal: true + +require "sorbet-runtime" + +module Dependabot + class Job + # Parsed representation of an ignore condition from the job definition. + # + # The raw JSON uses string keys like "dependency-name" and "version-requirement". + # This struct parses those into typed fields at the boundary so the rest of + # the codebase gets compile-time field access. + class IgnoreCondition < T::ImmutableStruct + extend T::Sig + + const :dependency_name, String + const :version_requirement, T.nilable(String) + const :update_types, T.nilable(T::Array[String]) + const :source, T.nilable(String) + const :updated_at, T.nilable(String) + + sig { params(hash: T::Hash[String, T.untyped]).returns(IgnoreCondition) } + def self.from_hash(hash) + new( + dependency_name: hash.fetch("dependency-name"), + version_requirement: hash["version-requirement"], + update_types: hash["update-types"], + source: hash["source"], + updated_at: hash["updated-at"] + ) + end + end + end +end diff --git a/updater/lib/dependabot/job/security_advisory_entry.rb b/updater/lib/dependabot/job/security_advisory_entry.rb new file mode 100644 index 00000000000..58e8699e2a9 --- /dev/null +++ b/updater/lib/dependabot/job/security_advisory_entry.rb @@ -0,0 +1,28 @@ +# typed: strict +# frozen_string_literal: true + +require "sorbet-runtime" + +module Dependabot + class Job + # Parsed representation of a security advisory from the job definition. + class SecurityAdvisoryEntry < T::ImmutableStruct + extend T::Sig + + const :dependency_name, String + const :affected_versions, T::Array[String], default: [] + const :patched_versions, T::Array[String], default: [] + const :unaffected_versions, T::Array[String], default: [] + + sig { params(hash: T::Hash[String, T.untyped]).returns(SecurityAdvisoryEntry) } + def self.from_hash(hash) + new( + dependency_name: hash.fetch("dependency-name"), + affected_versions: hash.fetch("affected-versions", []) || [], + patched_versions: hash.fetch("patched-versions", []) || [], + unaffected_versions: hash.fetch("unaffected-versions", []) || [] + ) + end + end + end +end diff --git a/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb b/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb index a2cf25e227e..94dc35e2f16 100644 --- a/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb @@ -326,7 +326,7 @@ def close_pull_request(reason:) sig { returns(String) } def security_advisory_dependency - T.cast(job.security_advisories.first, T::Hash[String, String])["dependency-name"].to_s + job.security_advisories.first&.dependency_name.to_s end end end diff --git a/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb index df788933f17..5e9e6febcd3 100644 --- a/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/refresh_security_update_pull_request_spec.rb @@ -227,7 +227,9 @@ allow(job).to receive_messages( allowed_update?: true, dependencies: ["dummy-pkg-a"], - security_advisories: [{ "dependency-name" => "dummy-pkg-a" }] + security_advisories: [ + Dependabot::Job::SecurityAdvisoryEntry.from_hash({ "dependency-name" => "dummy-pkg-a" }) + ] ) end @@ -302,7 +304,9 @@ ) allow(job).to receive_messages( allowed_update?: true, - security_advisories: [{ "dependency-name" => "dummy-pkg-a" }] + security_advisories: [ + Dependabot::Job::SecurityAdvisoryEntry.from_hash({ "dependency-name" => "dummy-pkg-a" }) + ] ) end @@ -332,7 +336,9 @@ ) allow(job).to receive_messages( allowed_update?: true, - security_advisories: [{ "dependency-name" => "dummy-pkg-a" }] + security_advisories: [ + Dependabot::Job::SecurityAdvisoryEntry.from_hash({ "dependency-name" => "dummy-pkg-a" }) + ] ) end @@ -362,7 +368,9 @@ ) allow(job).to receive_messages( allowed_update?: true, - security_advisories: [{ "dependency-name" => "Dummy-Pkg-A" }] + security_advisories: [ + Dependabot::Job::SecurityAdvisoryEntry.from_hash({ "dependency-name" => "Dummy-Pkg-A" }) + ] ) end From 4415c06f548fb0565e2e52f9fa3836e1566be241 Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Mon, 8 Jun 2026 14:21:35 -0700 Subject: [PATCH 2/2] Fall back to "all" when dependency-type or update-type is null --- updater/lib/dependabot/job/allowed_update.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/updater/lib/dependabot/job/allowed_update.rb b/updater/lib/dependabot/job/allowed_update.rb index e71be504a9b..04d6199a408 100644 --- a/updater/lib/dependabot/job/allowed_update.rb +++ b/updater/lib/dependabot/job/allowed_update.rb @@ -22,8 +22,8 @@ class AllowedUpdate < T::ImmutableStruct def self.from_hash(hash) new( dependency_name: hash["dependency-name"], - dependency_type: hash.fetch("dependency-type", "all"), - update_type: hash.fetch("update-type", "all"), + dependency_type: hash.fetch("dependency-type", "all") || "all", + update_type: hash.fetch("update-type", "all") || "all", update_types: hash.fetch("update-types", []) || [] ) end