diff --git a/common/lib/dependabot/dependency.rb b/common/lib/dependabot/dependency.rb index d1e773b11ab..fc8dcd48485 100644 --- a/common/lib/dependabot/dependency.rb +++ b/common/lib/dependabot/dependency.rb @@ -37,11 +37,11 @@ def self.register_name_normaliser(package_manager, name_builder) attr_reader :name, :version, :requirements, :package_manager, :previous_version, :previous_requirements, - :subdependency_metadata + :subdependency_metadata, :metadata def initialize(name:, requirements:, package_manager:, version: nil, previous_version: nil, previous_requirements: nil, - subdependency_metadata: [], removed: false) + subdependency_metadata: [], removed: false, metadata: {}) @name = name @version = version @requirements = requirements.map { |req| symbolize_keys(req) } @@ -54,6 +54,7 @@ def initialize(name:, requirements:, package_manager:, version: nil, map { |h| symbolize_keys(h) } end @removed = removed + @metadata = symbolize_keys(metadata || {}) check_values end @@ -105,6 +106,15 @@ def display_name display_name_builder.call(name) end + # Returns all detected versions of the dependency. Only ecosystems that + # support this feature will return more than the current version. + def all_versions + all_versions = metadata[:all_versions] + return [version].compact unless all_versions + + all_versions.filter_map(&:version) + end + def ==(other) other.instance_of?(self.class) && to_h == other.to_h end diff --git a/common/lib/dependabot/file_parsers/base/dependency_set.rb b/common/lib/dependabot/file_parsers/base/dependency_set.rb index 9d199942750..ee466fe3b92 100644 --- a/common/lib/dependabot/file_parsers/base/dependency_set.rb +++ b/common/lib/dependabot/file_parsers/base/dependency_set.rb @@ -14,34 +14,42 @@ def initialize(dependencies = [], case_sensitive: false) raise ArgumentError, "must be an array of Dependency objects" end - @dependencies = dependencies @case_sensitive = case_sensitive + @dependencies = Hash.new { |hsh, key| hsh[key] = DependencySlot.new } + dependencies.each { |dep| self << dep } end - attr_reader :dependencies + def dependencies + @dependencies.values.filter_map(&:combined) + end def <<(dep) raise ArgumentError, "must be a Dependency object" unless dep.is_a?(Dependency) - existing_dependency = dependency_for_name(dep.name) + @dependencies[key_for_dependency(dep)] << dep + self + end - return self if existing_dependency&.to_h == dep.to_h + def +(other) + raise ArgumentError, "must be a DependencySet" unless other.is_a?(DependencySet) - if existing_dependency - dependencies[dependencies.index(existing_dependency)] = - combined_dependency(existing_dependency, dep) - else - dependencies << dep + other_names = other.dependencies.map(&:name) + other_names.each do |name| + all_versions = other.all_versions_for_name(name) + all_versions.each { |dep| self << dep } end self end - def +(other) - raise ArgumentError, "must be a DependencySet" unless other.is_a?(DependencySet) + def all_versions_for_name(name) + key = key_for_name(name) + @dependencies.key?(key) ? @dependencies[key].all_versions : [] + end - other.dependencies.each { |dep| self << dep } - self + def dependency_for_name(name) + key = key_for_name(name) + @dependencies.key?(key) ? @dependencies[key].combined : nil end private @@ -50,41 +58,98 @@ def case_sensitive? @case_sensitive end - def dependency_for_name(name) - return dependencies.find { |d| d.name == name } if case_sensitive? + def key_for_name(name) + case_sensitive? ? name : name.downcase + end - dependencies.find { |d| d.name&.downcase == name&.downcase } + def key_for_dependency(dep) + key_for_name(dep.name) end - def combined_dependency(old_dep, new_dep) - package_manager = old_dep.package_manager - v_cls = Utils.version_class_for_package_manager(package_manager) - - # If we already have a requirement use the existing version - # (if present). Otherwise, use whatever the lowest version is - new_version = - if old_dep.requirements.any? then old_dep.version || new_dep.version - elsif !v_cls.correct?(new_dep.version) then old_dep.version - elsif !v_cls.correct?(old_dep.version) then new_dep.version - elsif v_cls.new(new_dep.version) > v_cls.new(old_dep.version) - old_dep.version + # There can only be one entry per dependency name in a `DependencySet`. Each entry + # is assigned a `DependencySlot`. + # + # In some ecosystems (like `npm_and_yarn`), however, multiple versions of a + # dependency may be encountered and added to the set. The `DependencySlot` retains + # all added versions and presents a single unified dependency for the entry + # that combines the attributes of these versions. + # + # The combined dependency is accessible via `DependencySet#dependencies` or + # `DependencySet#dependency_for_name`. The list of individual versions of the + # dependency is accessible via `DependencySet#all_versions_for_name`. + class DependencySlot + attr_reader :all_versions, :combined + + def initialize + @all_versions = [] + @combined = nil + end + + def <<(dep) + return self if @all_versions.include?(dep) + + @combined = if @combined + combined_dependency(@combined, dep) + else + Dependency.new( + name: dep.name, + version: dep.version, + requirements: dep.requirements, + package_manager: dep.package_manager, + subdependency_metadata: dep.subdependency_metadata + ) + end + + index_of_same_version = + @all_versions.find_index { |other| other.version == dep.version } + + if index_of_same_version.nil? + @all_versions << dep else - new_dep.version + same_version = @all_versions[index_of_same_version] + @all_versions[index_of_same_version] = combined_dependency(same_version, dep) end - subdependency_metadata = ( - (old_dep.subdependency_metadata || []) + - (new_dep.subdependency_metadata || []) - ).uniq - - Dependency.new( - name: old_dep.name, - version: new_version, - requirements: (old_dep.requirements + new_dep.requirements).uniq, - package_manager: package_manager, - subdependency_metadata: subdependency_metadata - ) + self + end + + private + + # Produces a new dependency by merging the attributes of `old_dep` with those of + # `new_dep`. Requirements and subdependency metadata will be combined and deduped. + # The version of the combined dependency is determined by the logic below. + def combined_dependency(old_dep, new_dep) + version = if old_dep.top_level? # Prefer a direct dependency over a transitive one + old_dep.version || new_dep.version + elsif !version_class.correct?(new_dep.version) + old_dep.version + elsif !version_class.correct?(old_dep.version) + new_dep.version + elsif version_class.new(new_dep.version) > version_class.new(old_dep.version) + old_dep.version + else + new_dep.version + end + requirements = (old_dep.requirements + new_dep.requirements).uniq + subdependency_metadata = ( + (old_dep.subdependency_metadata || []) + + (new_dep.subdependency_metadata || []) + ).uniq + + Dependency.new( + name: old_dep.name, + version: version, + requirements: requirements, + package_manager: old_dep.package_manager, + subdependency_metadata: subdependency_metadata + ) + end + + def version_class + @version_class ||= Utils.version_class_for_package_manager(@combined.package_manager) + end end + private_constant :DependencySlot end end end diff --git a/common/spec/dependabot/dependency_spec.rb b/common/spec/dependabot/dependency_spec.rb index 47d7454baa6..45a17786241 100644 --- a/common/spec/dependabot/dependency_spec.rb +++ b/common/spec/dependabot/dependency_spec.rb @@ -270,4 +270,104 @@ it { is_expected.to eq(nil) } end end + + describe "#metadata" do + it "stores metadata given to initialize" do + dependency = described_class.new( + name: "dep", + requirements: [], + package_manager: "dummy", + metadata: { foo: 42 } + ) + expect(dependency.metadata).to eq(foo: 42) + end + + it "is mutable" do + dependency = described_class.new( + name: "dep", + requirements: [], + package_manager: "dummy", + metadata: { foo: 42 } + ) + + dependency.metadata[:all_versions] = [] + expect(dependency.metadata).to eq(foo: 42, all_versions: []) + end + + it "is not serialized" do + dependency = described_class.new( + name: "dep", + requirements: [], + package_manager: "dummy", + metadata: { foo: 42 } + ) + expect(dependency.to_h.keys).not_to include("metadata") + end + + it "isn't utilized by the equality operator" do + dependency1 = described_class.new( + name: "dep", + requirements: [], + package_manager: "dummy", + metadata: { foo: 42 } + ) + dependency2 = described_class.new( + name: "dep", + requirements: [], + package_manager: "dummy", + metadata: { foo: 43 } + ) + expect(dependency1).to eq(dependency2) + end + end + + describe "#all_versions" do + it "returns an empty array by default" do + dependency = described_class.new( + name: "dep", + requirements: [], + package_manager: "dummy" + ) + + expect(dependency.all_versions).to eq([]) + end + + it "returns the dependency version if all_version metadata isn't present" do + dependency = described_class.new( + name: "dep", + requirements: [], + package_manager: "dummy", + version: "1.0.0" + ) + + expect(dependency.all_versions).to eq(["1.0.0"]) + end + + it "returns all_version metadata if present" do + dependency = described_class.new( + name: "dep", + requirements: [], + package_manager: "dummy", + version: "1.0.0", + metadata: { + all_versions: [ + described_class.new( + name: "dep", + requirements: [], + package_manager: "dummy", + version: "1.0.0" + ), + described_class.new( + name: "dep", + requirements: [], + package_manager: "dummy", + version: "2.0.0" + ) + ] + } + ) + + expect(dependency.all_versions).to eq(["1.0.0", "2.0.0"]) + end + end end diff --git a/common/spec/dependabot/file_parsers/base/dependency_set_spec.rb b/common/spec/dependabot/file_parsers/base/dependency_set_spec.rb index bd0fe52a273..f6c4b0161b8 100644 --- a/common/spec/dependabot/file_parsers/base/dependency_set_spec.rb +++ b/common/spec/dependabot/file_parsers/base/dependency_set_spec.rb @@ -234,4 +234,134 @@ end end end + + context "when multiple versions of a dependency are added" do + let(:foo_v1) do + Dependabot::Dependency.new( + name: "foo", + version: "1.0", + requirements: [], + package_manager: "dummy" + ) + end + + let(:foo_v1_1) do # rubocop:disable Naming/VariableNumber + Dependabot::Dependency.new( + name: "foo", + version: "1.1", + requirements: [{ + requirement: "^1", + file: "Dummyfile", + groups: nil, + source: nil + }], + package_manager: "dummy" + ) + end + + let(:foo_v1_1_alt) do + Dependabot::Dependency.new( + name: "foo", + version: "1.1", + requirements: [{ + requirement: "^1", + file: "Dummyfile.lock", + groups: ["prod"], + source: { + type: "registry", + url: "https://registry.dummy.org" + } + }], + package_manager: "dummy" + ) + end + + let(:foo_sha) do + Dependabot::Dependency.new( + name: "foo", + version: "d5ac0584ee9ae7bd9288220a39780f155b9ad4c8", + requirements: [{ + requirement: "^1", + file: "Dummyfile.lock", + groups: ["prod"], + source: { + type: "git", + url: "https://github.com/acme-inc/foo", + branch: nil, + ref: "main" + } + }], + package_manager: "dummy" + ) + end + + it "merges each into a single combined dependency" do + dependency_set = described_class.new << foo_v1 << foo_sha << foo_v1_1 + + expect(dependency_set.dependency_for_name("foo")).to eq( + Dependabot::Dependency.new( + name: "foo", + version: "1.0", + requirements: ( + foo_v1.requirements + + foo_sha.requirements + + foo_v1_1.requirements + ).uniq, + package_manager: "dummy" + ) + ) + end + + it "returns all versions in the order they were added by default" do + dependency_set = described_class.new << foo_v1_1 << foo_sha << foo_v1 + expect(dependency_set.all_versions_for_name("foo")).to eq([foo_v1_1, foo_sha, foo_v1]) + end + + it "preserves all versions when combined with another dependency set" do + set_a = described_class.new << foo_v1 + set_b = described_class.new << foo_sha << foo_v1_1 << foo_v1_1_alt + combined_set = set_a + set_b + + expect(combined_set.dependency_for_name("foo")).to eq( + Dependabot::Dependency.new( + name: "foo", + version: "1.0", + requirements: ( + foo_v1.requirements + + foo_sha.requirements + + foo_v1_1.requirements + + foo_v1_1_alt.requirements + ).uniq, + package_manager: "dummy" + ) + ) + + expect(combined_set.all_versions_for_name("foo")).to eq([ + foo_v1, + foo_sha, + Dependabot::Dependency.new( + name: "foo", + version: "1.1", + package_manager: "dummy", + requirements: (foo_v1_1.requirements + foo_v1_1_alt.requirements).uniq + ) + ]) + end + + context "and the same version is added multiple times" do + it "combines each into the the existing version" do + dependency_set = described_class.new << foo_v1_1 << foo_v1_1_alt << foo_sha + + expect(dependency_set.all_versions_for_name("foo")).to eq([ + Dependabot::Dependency.new( + name: "foo", + version: "1.1", + package_manager: "dummy", + requirements: (foo_v1_1.requirements + foo_v1_1_alt.requirements).uniq + ), + foo_sha + ]) + end + end + end end diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser.rb index 7e72b6eb590..287383e2292 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser.rb @@ -7,6 +7,7 @@ require "dependabot/file_parsers" require "dependabot/file_parsers/base" require "dependabot/shared_helpers" +require "dependabot/npm_and_yarn/helpers" require "dependabot/npm_and_yarn/native_helpers" require "dependabot/npm_and_yarn/version" require "dependabot/git_metadata_fetcher" @@ -36,7 +37,8 @@ def parse dependency_set = DependencySet.new dependency_set += manifest_dependencies dependency_set += lockfile_dependencies - dependencies = dependency_set.dependencies + + dependencies = Helpers.dependencies_with_all_versions_metadata(dependency_set) # TODO: Currently, Dependabot can't handle dependencies that have both # a git source *and* a non-git source. Fix that! diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser/lockfile_parser.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser/lockfile_parser.rb index 81da602137f..5f5fab1aff6 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser/lockfile_parser.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser/lockfile_parser.rb @@ -17,7 +17,8 @@ def parse dependency_set += yarn_lock_dependencies if yarn_locks.any? dependency_set += package_lock_dependencies if package_locks.any? dependency_set += shrinkwrap_dependencies if shrinkwraps.any? - dependency_set.dependencies + + Helpers.dependencies_with_all_versions_metadata(dependency_set) end def lockfile_details(dependency_name:, requirement:, manifest_name:) diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn/helpers.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn/helpers.rb index 832de4beabd..70c99a60149 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn/helpers.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn/helpers.rb @@ -40,6 +40,29 @@ def self.run_yarn_commands(*commands) end commands.each { |cmd| SharedHelpers.run_shell_command(cmd) } end + + def self.dependencies_with_all_versions_metadata(dependency_set) + working_set = Dependabot::NpmAndYarn::FileParser::DependencySet.new + dependencies = [] + + names = dependency_set.dependencies.map(&:name) + names.each do |name| + all_versions = dependency_set.all_versions_for_name(name) + all_versions.each do |dep| + metadata_versions = dep.metadata.fetch(:all_versions, []) + if metadata_versions.any? + metadata_versions.each { |a| working_set << a } + else + working_set << dep + end + end + dependency = working_set.dependency_for_name(name) + dependency.metadata[:all_versions] = working_set.all_versions_for_name(name) + dependencies << dependency + end + + dependencies + end end end end diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker.rb index a3da496044f..ef343afa96d 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker.rb @@ -4,6 +4,7 @@ require "dependabot/update_checkers" require "dependabot/update_checkers/base" require "dependabot/shared_helpers" +require "set" module Dependabot module NpmAndYarn @@ -16,6 +17,20 @@ class UpdateChecker < Dependabot::UpdateCheckers::Base require_relative "update_checker/conflicting_dependency_resolver" require_relative "update_checker/vulnerability_auditor" + def up_to_date? + return false if security_update? && + dependency.version && + version_class.correct?(dependency.version) && + vulnerable_versions.any? && + !vulnerable_versions.include?(version_class.new(dependency.version)) + + super + end + + def vulnerable? + super || vulnerable_versions.any? + end + def latest_version @latest_version ||= if git_dependency? @@ -125,6 +140,18 @@ def vulnerability_audit ) end + def vulnerable_versions + @vulnerable_versions ||= + begin + all_versions = dependency.all_versions. + filter_map { |v| version_class.new(v) if version_class.correct?(v) } + + all_versions.select do |v| + security_advisories.any? { |advisory| advisory.vulnerable?(v) } + end + end + end + def latest_version_resolvable_with_full_unlock? return false unless latest_version @@ -382,6 +409,10 @@ def library? ).library? end + def security_update? + security_advisories.any? + end + def dependency_source_details original_source(dependency) end diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_parser_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_parser_spec.rb index 6bc9c71385a..3abe3eaf129 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_parser_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_parser_spec.rb @@ -1400,5 +1400,35 @@ ]) end end + + context "with multiple versions of a dependency" do + subject { parser.parse } + let(:files) { project_dependency_files("npm8/transitive_dependency_multiple_versions") } + + it "stores all versions of the dependency in its metadata" do + name = "@dependabot-fixtures/npm-transitive-dependency" + dependency = subject.find { |dep| dep.name == name } + + expect(dependency.metadata[:all_versions]).to eq([ + Dependabot::Dependency.new( + name: name, + version: "1.0.1", + requirements: [{ + requirement: "1.0.1", + file: "package.json", + groups: ["dependencies"], + source: { type: "registry", url: "https://registry.npmjs.org" } + }], + package_manager: "npm_and_yarn" + ), + Dependabot::Dependency.new( + name: name, + version: "1.0.0", + requirements: [], + package_manager: "npm_and_yarn" + ) + ]) + end + end end end diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/helpers_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/helpers_spec.rb new file mode 100644 index 00000000000..478f154acc1 --- /dev/null +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/helpers_spec.rb @@ -0,0 +1,155 @@ +# frozen_string_literal: true + +require "spec_helper" +require "dependabot/npm_and_yarn/file_parser" +require "dependabot/npm_and_yarn/helpers" + +RSpec.describe Dependabot::NpmAndYarn::Helpers do + describe "::dependencies_with_all_versions_metadata" do + let(:foo_a) do + Dependabot::Dependency.new( + name: "foo", + version: "0.0.1", + requirements: [{ + requirement: "^0.0.1", + file: "package.json", + groups: nil, + source: nil + }], + package_manager: "npm_and_yarn" + ) + end + + let(:foo_b) do + Dependabot::Dependency.new( + name: "foo", + version: "0.0.2", + requirements: [{ + requirement: "^0.0.1", + file: "package-lock.json", + groups: ["dependencies"], + source: { type: "registry", url: "https://registry.npmjs.org" } + }], + package_manager: "npm_and_yarn" + ) + end + + let(:foo_c) do + Dependabot::Dependency.new( + name: "foo", + version: "0.0.3", + requirements: [{ + requirement: "^0.0.3", + file: "package-lock.json", + groups: ["dependencies"], + source: { type: "registry", url: "https://registry.npmjs.org" } + }], + package_manager: "npm_and_yarn" + ) + end + + let(:bar_a) do + Dependabot::Dependency.new( + name: "bar", + version: "0.2.1", + requirements: [{ + requirement: "^0.2.1", + file: "package.json", + groups: ["dependencies"], + source: nil + }], + package_manager: "npm_and_yarn" + ) + end + + let(:bar_b) do + Dependabot::Dependency.new( + name: "bar", + version: "0.2.2", + requirements: [{ + requirement: "^0.2.1", + file: "package-lock.json", + groups: ["dependencies"], + source: { type: "registry", url: "https://registry.npmjs.org" } + }], + package_manager: "npm_and_yarn" + ) + end + + let(:bar_c) do + Dependabot::Dependency.new( + name: "bar", + version: "0.2.3", + requirements: [{ + requirement: "^0.2.3", + file: "package-lock.json", + groups: ["dependencies"], + source: { type: "registry", url: "https://registry.npmjs.org" } + }], + package_manager: "npm_and_yarn" + ) + end + + it "returns flattened list of dependencies populated with :all_versions metadata" do + dependency_set = Dependabot::NpmAndYarn::FileParser::DependencySet.new + dependency_set << foo_a << bar_a << foo_c << bar_c << foo_b << bar_b + + expect(described_class.dependencies_with_all_versions_metadata(dependency_set)).to eq([ + Dependabot::Dependency.new( + name: "foo", + version: "0.0.1", + requirements: (foo_a.requirements + foo_c.requirements + foo_b.requirements).uniq, + package_manager: "npm_and_yarn", + metadata: { all_versions: [foo_a, foo_c, foo_b] } + ), + Dependabot::Dependency.new( + name: "bar", + version: "0.2.1", + requirements: (bar_a.requirements + bar_c.requirements + bar_b.requirements).uniq, + package_manager: "npm_and_yarn", + metadata: { all_versions: [bar_a, bar_c, bar_b] } + ) + ]) + end + + context "when dependencies in set already have :all_versions metadata" do + it "correctly merges existing metadata into new metadata" do + dependency_set = Dependabot::NpmAndYarn::FileParser::DependencySet.new + dependency_set << foo_a + dependency_set << Dependabot::Dependency.new( + name: "foo", + version: "0.0.3", + requirements: (foo_c.requirements + foo_b.requirements).uniq, + package_manager: "npm_and_yarn", + metadata: { all_versions: [foo_c, foo_b] } + ) + dependency_set << bar_c + dependency_set << bar_b + dependency_set << Dependabot::Dependency.new( + name: "bar", + version: "0.2.1", + requirements: bar_a.requirements, + package_manager: "npm_and_yarn", + metadata: { all_versions: [bar_a] } + ) + + expect(described_class.dependencies_with_all_versions_metadata(dependency_set)).to eq([ + Dependabot::Dependency.new( + name: "foo", + version: "0.0.1", + requirements: (foo_a.requirements + foo_c.requirements + foo_b.requirements).uniq, + package_manager: "npm_and_yarn", + metadata: { all_versions: [foo_a, foo_c, foo_b] } + ), + Dependabot::Dependency.new( + name: "bar", + version: "0.2.3", + requirements: (bar_c.requirements + bar_b.requirements + bar_a.requirements).uniq, + package_manager: "npm_and_yarn", + metadata: { all_versions: [bar_c, bar_b, bar_a] } + ) + ]) + end + end + end +end diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker_spec.rb index f62c1f2bc05..d15e25a6fc7 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker_spec.rb @@ -65,6 +65,82 @@ Dependabot::Experiments.register(:yarn_berry, true) end + describe "#vulnerable?" do + context "when the dependency has multiple versions" do + let(:dependency) do + Dependabot::Dependency.new( + name: "foo", + version: "1.0.0", + requirements: (foo_v1.requirements + foo_v2.requirements).uniq, + package_manager: "npm_and_yarn", + metadata: { all_versions: [foo_v1, foo_v2] } + ) + end + + let(:foo_v1) do + Dependabot::Dependency.new( + name: "foo", + version: "1.0.0", + requirements: [{ + file: "package.json", + requirement: "^1.0.0", + groups: nil, + source: nil + }], + package_manager: "npm_and_yarn" + ) + end + + let(:foo_v2) do + Dependabot::Dependency.new( + name: "foo", + version: "2.0.0", + requirements: [{ + file: "package-lock.json", + requirement: "^2.0.0", + groups: ["dependencies"], + source: { type: "registry", url: "https://registry.npmjs.org" } + }], + package_manager: "npm_and_yarn" + ) + end + + context "if any of the versions is vulnerable" do + let(:security_advisories) do + [ + Dependabot::SecurityAdvisory.new( + dependency_name: "foo", + package_manager: "npm_and_yarn", + vulnerable_versions: [">=2.0.0 <2.0.3"], + safe_versions: [">=1.0.0 <2.0.0", ">=2.0.3"] + ) + ] + end + + it "returns true" do + expect(checker.vulnerable?).to eq(true) + end + end + + context "if none of the versions is vulnerable" do + let(:security_advisories) do + [ + Dependabot::SecurityAdvisory.new( + dependency_name: "foo", + package_manager: "npm_and_yarn", + vulnerable_versions: ["<1.0.0"], + safe_versions: [">=1.0.0"] + ) + ] + end + + it "returns false" do + expect(checker.vulnerable?).to eq(false) + end + end + end + end + describe "#up_to_date?", :vcr do context "with no lockfile" do let(:dependency_files) { project_dependency_files("npm6/peer_dependency_typescript_no_lockfile") } diff --git a/npm_and_yarn/spec/fixtures/projects/npm8/transitive_dependency_multiple_versions/package-lock.json b/npm_and_yarn/spec/fixtures/projects/npm8/transitive_dependency_multiple_versions/package-lock.json new file mode 100644 index 00000000000..1bf36eab904 --- /dev/null +++ b/npm_and_yarn/spec/fixtures/projects/npm8/transitive_dependency_multiple_versions/package-lock.json @@ -0,0 +1,57 @@ +{ + "name": "transitive-dependency-multiple-versions", + "version": "1.0.0", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "name": "transitive-dependency-multiple-versions", + "version": "1.0.0", + "license": "ISC", + "dependencies": { + "@dependabot-fixtures/npm-intermediate-dependency": "0.0.1", + "@dependabot-fixtures/npm-transitive-dependency": "1.0.1" + } + }, + "node_modules/@dependabot-fixtures/npm-intermediate-dependency": { + "version": "0.0.1", + "resolved": "https://registry.npmjs.org/@dependabot-fixtures/npm-intermediate-dependency/-/npm-intermediate-dependency-0.0.1.tgz", + "integrity": "sha512-/N77Dzpfg8BIfFgpJrMk86ueUYTVhmpc4RobuHpIpKSc3GZr4Ltu4au92brnUGk66UkzgrMmtgqRXO8OrOspKQ==", + "dependencies": { + "@dependabot-fixtures/npm-transitive-dependency": "1.0.0" + } + }, + "node_modules/@dependabot-fixtures/npm-intermediate-dependency/node_modules/@dependabot-fixtures/npm-transitive-dependency": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/@dependabot-fixtures/npm-transitive-dependency/-/npm-transitive-dependency-1.0.0.tgz", + "integrity": "sha512-nFbzQH0TRgdzSA2/FH6MPnxZDpD+5Bgz00aD5Edgbc1wY/k8VC9s7lnk22dBTgJLwoY7MgbrnAf9rAvN08hHVg==" + }, + "node_modules/@dependabot-fixtures/npm-transitive-dependency": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/@dependabot-fixtures/npm-transitive-dependency/-/npm-transitive-dependency-1.0.1.tgz", + "integrity": "sha512-nWQzJEqSqKZu+mgNSVdsO69NG6vCGIN9FuM+Vip5nqItqrNeQoITZM6/q6+tqgdM48XkQEOUpEiYpAdoMbxniw==" + } + }, + "dependencies": { + "@dependabot-fixtures/npm-intermediate-dependency": { + "version": "0.0.1", + "resolved": "https://registry.npmjs.org/@dependabot-fixtures/npm-intermediate-dependency/-/npm-intermediate-dependency-0.0.1.tgz", + "integrity": "sha512-/N77Dzpfg8BIfFgpJrMk86ueUYTVhmpc4RobuHpIpKSc3GZr4Ltu4au92brnUGk66UkzgrMmtgqRXO8OrOspKQ==", + "requires": { + "@dependabot-fixtures/npm-transitive-dependency": "1.0.0" + }, + "dependencies": { + "@dependabot-fixtures/npm-transitive-dependency": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/@dependabot-fixtures/npm-transitive-dependency/-/npm-transitive-dependency-1.0.0.tgz", + "integrity": "sha512-nFbzQH0TRgdzSA2/FH6MPnxZDpD+5Bgz00aD5Edgbc1wY/k8VC9s7lnk22dBTgJLwoY7MgbrnAf9rAvN08hHVg==" + } + } + }, + "@dependabot-fixtures/npm-transitive-dependency": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/@dependabot-fixtures/npm-transitive-dependency/-/npm-transitive-dependency-1.0.1.tgz", + "integrity": "sha512-nWQzJEqSqKZu+mgNSVdsO69NG6vCGIN9FuM+Vip5nqItqrNeQoITZM6/q6+tqgdM48XkQEOUpEiYpAdoMbxniw==" + } + } +} diff --git a/npm_and_yarn/spec/fixtures/projects/npm8/transitive_dependency_multiple_versions/package.json b/npm_and_yarn/spec/fixtures/projects/npm8/transitive_dependency_multiple_versions/package.json new file mode 100644 index 00000000000..7f274fa3c25 --- /dev/null +++ b/npm_and_yarn/spec/fixtures/projects/npm8/transitive_dependency_multiple_versions/package.json @@ -0,0 +1,16 @@ +{ + "name": "transitive-dependency-multiple-versions", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "keywords": [], + "author": "", + "license": "ISC", + "dependencies": { + "@dependabot-fixtures/npm-intermediate-dependency": "0.0.1", + "@dependabot-fixtures/npm-transitive-dependency": "1.0.1" + } +} diff --git a/updater/lib/dependabot/job.rb b/updater/lib/dependabot/job.rb index 29c071836d0..04b186dd9c2 100644 --- a/updater/lib/dependabot/job.rb +++ b/updater/lib/dependabot/job.rb @@ -112,8 +112,9 @@ def vulnerable?(dependency) version_class_for_package_manager(dependency.package_manager) return false unless version_class.correct?(dependency.version) - version = version_class.new(dependency.version) - security_advisories.any? { |a| a.vulnerable?(version) } + all_versions = dependency.all_versions. + filter_map { |v| version_class.new(v) if version_class.correct?(v) } + security_advisories.any? { |a| all_versions.any? { |v| a.vulnerable?(v) } } end def security_fix?(dependency) diff --git a/updater/spec/dependabot/job_spec.rb b/updater/spec/dependabot/job_spec.rb index 1483037dc5e..eb8d1d3a6b7 100644 --- a/updater/spec/dependabot/job_spec.rb +++ b/updater/spec/dependabot/job_spec.rb @@ -158,6 +158,53 @@ it { is_expected.to eq(true) } end + + context "for a security fix that doesn't apply" do + let(:security_advisories) do + [ + { + "dependency-name" => "business", + "affected-versions" => ["> 1.8.0"], + "patched-versions" => [], + "unaffected-versions" => [] + } + ] + end + + it { is_expected.to eq(false) } + end + + context "for a security fix that doesn't apply to some versions" do + let(:security_advisories) do + [ + { + "dependency-name" => "business", + "affected-versions" => ["> 1.8.0"], + "patched-versions" => [], + "unaffected-versions" => [] + } + ] + end + + it "should be allowed" do + dependency.metadata[:all_versions] = [ + Dependabot::Dependency.new( + name: dependency_name, + package_manager: "bundler", + version: "1.8.0", + requirements: [] + ), + Dependabot::Dependency.new( + name: dependency_name, + package_manager: "bundler", + version: "1.9.0", + requirements: [] + ) + ] + + is_expected.to eq(true) + end + end end context "and a dependency whitelist that includes the dependency" do