diff --git a/npm_and_yarn/README.md b/npm_and_yarn/README.md index db238e13286..61d2fb77927 100644 --- a/npm_and_yarn/README.md +++ b/npm_and_yarn/README.md @@ -2,6 +2,25 @@ Yarn and npm support for [`dependabot-core`][core-repo]. +### Native npm transitive updates + +`npm update` accepts package names, not per-package version constraints. Dependabot +checks changed installations in v1, v2, and v3 lockfiles, including nested packages +and aliases. It also checks the effective replacement of a removed installation, +so deduplication cannot bypass the policy. Required installations must retain +their package identity and a valid version. + +The resolver rejects versions above its allowable version or inside an ignore +range; the writer raises `UpdateNotPossible` if npm exceeds the requested version. +Existing unchanged effective versions are preserved. +An allowed root version does not make an update safe if another changed occurrence +exceeds the bound. + +An out-of-policy native result is rejected, not relabelled as a lower version. +Dependabot does not add manifest overrides to force an older candidate, since +overrides replace the parent package's requirements. Selecting an older compatible +candidate requires native npm support for per-package constraints. + ### Running locally 1. Start a development shell @@ -15,4 +34,8 @@ Yarn and npm support for [`dependabot-core`][core-repo]. [dependabot-core-dev] ~ $ cd npm_and_yarn && rspec ``` +The npm spec helper clears the active npm version selector and per-directory +registrations after each example. Configure required selectors within each +example so randomized runs remain independent. + [core-repo]: https://github.com/dependabot/dependabot-core diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser/json_lock/record.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser/json_lock/record.rb index f448999c58d..cf7a76cf12e 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser/json_lock/record.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser/json_lock/record.rb @@ -69,6 +69,37 @@ def legacy_entries entries(object.fetch("dependencies", {}), "#{@context}.dependencies") end + sig { returns(T::Hash[String, Record]) } + def package_entries + entries(object.fetch("packages", {}), "#{@context}.packages") + end + + # Keep modern packages authoritative, including an empty table in a v2 lockfile. + # Legacy dependency trees describe the same installation paths recursively. + sig { returns(T::Hash[String, Record]) } + def installed_entries + return package_entries if object.key?("packages") + + legacy_entries.each_with_object({ "" => self }) do |(name, record), installed| + record.installed_entries.each do |path, child| + installed[File.join("node_modules", name, path).delete_suffix("/")] = child + end + end + end + + sig { returns(T::Array[String]) } + def dependency_names + %w(requires dependencies devDependencies optionalDependencies peerDependencies).flat_map do |field| + value = object[field] + next [] if value.nil? || (field == "requires" && value == true) + + # v1 dependencies are installations; its requirements live in "requires". + object_value(value, "#{@context}.#{field}").filter_map do |name, requirement| + name if requirement.is_a?(String) + end + end + end + sig { params(name: String).returns(T.nilable(Record)) } def legacy_entry(name) dependencies = object["dependencies"] diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater.rb index 9a02b096711..264ce176177 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater.rb @@ -239,7 +239,21 @@ def top_level_dependency_update_not_required?(dependency) sig { returns(T::Hash[String, String]) } def run_current_npm_update - run_npm_updater(top_level_dependencies: top_level_dependencies, sub_dependencies: sub_dependencies) + updated_files = run_npm_updater( + top_level_dependencies: top_level_dependencies, + sub_dependencies: sub_dependencies + ) + # The previous-state diagnostic must not enforce the new update's policy. + invalid_dependencies = sub_dependencies.reject do |dependency| + NativeHelpers.npm_subdependency_update_allowed?( + lockfile: lockfile, + updated_content: updated_files.fetch(lockfile_basename), + dependency: dependency + ) + end + raise Dependabot::UpdateNotPossible, invalid_dependencies.map(&:name) if invalid_dependencies.any? + + updated_files end sig { returns(T::Hash[String, String]) } diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn/native_helpers.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn/native_helpers.rb index 59a3af52105..fbab6969168 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn/native_helpers.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn/native_helpers.rb @@ -3,6 +3,7 @@ require "sorbet-runtime" require "dependabot/npm_and_yarn/helpers" +require "dependabot/npm_and_yarn/file_parser/json_lock" module Dependabot module NpmAndYarn @@ -74,6 +75,233 @@ def self.run_npm_audit_fix_command(min_release_age_arg: nil) Helpers.run_npm_command(command, fingerprint: fingerprint) end + # npm update accepts names, not version constraints. Check the actual + # changed occurrences before accepting its result, including audit fallback. + sig do + params( + lockfile: DependencyFile, + updated_content: String, + dependency: Dependency, + ignored_versions: T::Array[String] + ).returns(T::Boolean) + end + def self.npm_subdependency_update_allowed?(lockfile:, updated_content:, dependency:, ignored_versions: []) + allowable_version = dependency.version + allowable = Version.new(allowable_version) if allowable_version + ignored = ignored_versions.flat_map { |req| dependency.requirement_class.requirements_array(req) } + + changed_npm_dependency_records(lockfile, updated_content, dependency.name).all? do |record| + version = npm_record_version(record) + next false unless Version.semver_for(version) + + candidate = Version.new(version) + next false if allowable && candidate > allowable + + ignored.none? { |requirement| requirement.satisfied_by?(candidate) } + end + end + + sig do + params(lockfile: DependencyFile, updated_content: String, dependency_name: String) + .returns(T::Array[T.nilable(FileParser::JsonLock::Record)]) + end + def self.changed_npm_dependency_records(lockfile, updated_content, dependency_name) + updated_lockfile = lockfile.dup + updated_lockfile.content = updated_content + before = FileParser::JsonLock.new(lockfile).parsed.installed_entries + after = FileParser::JsonLock.new(updated_lockfile).parsed.installed_entries + + changed = T.let([], T::Array[T.nilable(FileParser::JsonLock::Record)]) + after.each do |path, record| + next unless npm_dependency_record?(path, record, dependency_name) + + previous = before[path] + if previous && npm_record_name(path, previous) != npm_record_name(path, record) + changed << nil + next + end + next if previous && npm_record_version(previous) == npm_record_version(record) + + changed << record + end + changed + removed_npm_dependency_replacements(before, after, dependency_name) + end + private_class_method :changed_npm_dependency_records + + sig do + params( + before: T::Hash[String, FileParser::JsonLock::Record], + after: T::Hash[String, FileParser::JsonLock::Record], + dependency_name: String + ).returns(T::Array[T.nilable(FileParser::JsonLock::Record)]) + end + def self.removed_npm_dependency_replacements(before, after, dependency_name) + replacements = T.let([], T::Array[T.nilable(FileParser::JsonLock::Record)]) + before.each do |path, record| + next unless npm_dependency_record?(path, record, dependency_name) + next if after[path] && npm_dependency_record?(path, T.must(after[path]), dependency_name) + + installed_name = T.must(path.split("node_modules/").last) + npm_replacement_consumers(before, after, path).each do |consumer_path| + replacement = npm_replacement_record(after, consumer_path, path, record) + version = npm_record_version(replacement) + previous_version = npm_consumed_version(before, consumer_path, installed_name) + next if Version.semver_for(version) && version == previous_version + + replacements << replacement + end + end + replacements + end + private_class_method :removed_npm_dependency_replacements + + sig do + params( + entries: T::Hash[String, FileParser::JsonLock::Record], + consumer_path: String, + original_path: String, + original: FileParser::JsonLock::Record + ).returns(T.nilable(FileParser::JsonLock::Record)) + end + def self.npm_replacement_record(entries, consumer_path, original_path, original) + name = T.must(original_path.split("node_modules/").last) + replacement_path = npm_dependency_path(entries, consumer_path, name) + return unless replacement_path + + replacement = entries.fetch(replacement_path) + # Missing or differently named installations do not prove a safe replacement. + replacement if npm_record_name(replacement_path, replacement) == npm_record_name(original_path, original) + end + private_class_method :npm_replacement_record + + sig do + params( + entries: T::Hash[String, FileParser::JsonLock::Record], + consumer_path: String, + name: String + ).returns(T.nilable(String)) + end + def self.npm_consumed_version(entries, consumer_path, name) + return unless npm_consumer_requires?(entries, consumer_path, name) + + path = npm_dependency_path(entries, consumer_path, name) + npm_record_version(entries.fetch(path)) if path + end + private_class_method :npm_consumed_version + + sig do + params( + entries: T::Hash[String, FileParser::JsonLock::Record], + consumer_path: String, + name: String + ).returns(T::Boolean) + end + def self.npm_consumer_requires?(entries, consumer_path, name) + entries[consumer_path]&.dependency_names&.include?(name) || false + end + private_class_method :npm_consumer_requires? + + sig do + params( + before: T::Hash[String, FileParser::JsonLock::Record], + after: T::Hash[String, FileParser::JsonLock::Record], + path: String + ).returns(T::Array[String]) + end + def self.npm_replacement_consumers(before, after, path) + prefix, _, installed_name = path.rpartition("node_modules/") + consumers = before.filter_map do |consumer_path, consumer| + next unless consumer.dependency_names.include?(installed_name) + + consumer_path if npm_dependency_path(before, consumer_path, installed_name) == path + end + if consumers.empty? + # Without requirement metadata, only a removed parent or a proven + # in-policy replacement makes the removal safe. + parent_path = prefix.delete_suffix("/") + consumers = !parent_path.empty? && !after.key?(parent_path) ? [] : [parent_path] + else + consumers.select! { |consumer_path| npm_consumer_requires?(after, consumer_path, installed_name) } + end + + consumers + npm_new_consumers(before, after, path) + end + private_class_method :npm_replacement_consumers + + sig do + params( + before: T::Hash[String, FileParser::JsonLock::Record], + after: T::Hash[String, FileParser::JsonLock::Record], + path: String + ).returns(T::Array[String]) + end + def self.npm_new_consumers(before, after, path) + # A parent may itself have moved during deduplication. New consumers + # must not acquire an out-of-policy installation either. + installed_name = T.must(path.split("node_modules/").last) + after.filter_map do |consumer_path, consumer| + next if npm_consumer_requires?(before, consumer_path, installed_name) + next unless consumer.dependency_names.include?(installed_name) + + replacement_path = npm_dependency_path(after, consumer_path, installed_name) + next unless replacement_path + next unless npm_record_name(replacement_path, after.fetch(replacement_path)) == + npm_record_name(path, before.fetch(path)) + + consumer_path + end + end + private_class_method :npm_new_consumers + + sig do + params( + entries: T::Hash[String, FileParser::JsonLock::Record], + parent_path: String, + name: String + ).returns(T.nilable(String)) + end + def self.npm_dependency_path(entries, parent_path, name) + loop do + candidate = parent_path.empty? ? "node_modules/#{name}" : "#{parent_path}/node_modules/#{name}" + return candidate if File.basename(parent_path) != "node_modules" && entries.key?(candidate) + return if parent_path.empty? + + # Node searches ancestor directories, skipping node_modules/node_modules. + # Keep scoped names and workspace directory prefixes in these paths. + parent = File.dirname(parent_path) + return if parent == parent_path + + parent_path = parent == "." ? "" : parent + end + end + private_class_method :npm_dependency_path + + sig { params(path: String, record: FileParser::JsonLock::Record, name: String).returns(T::Boolean) } + def self.npm_dependency_record?(path, record, name) + path.include?("node_modules/") && + (path.split("node_modules/").last == name || npm_record_name(path, record) == name) + end + private_class_method :npm_dependency_record? + + sig { params(path: String, record: FileParser::JsonLock::Record).returns(String) } + def self.npm_record_name(path, record) + name = record.name + return name if name + + version = record.version + return version.delete_prefix("npm:").rpartition("@").first if version&.start_with?("npm:") + + T.must(path.split("node_modules/").last) + end + private_class_method :npm_record_name + + sig { params(record: T.nilable(FileParser::JsonLock::Record)).returns(T.nilable(String)) } + def self.npm_record_version(record) + version = record&.version + version&.start_with?("npm:") ? version.rpartition("@").last : version + end + private_class_method :npm_record_version + # Masks the varying cooldown day count out of the telemetry fingerprint while # keeping the security `=0` bypass distinguishable (mirrors the npm lockfile # updater's `fingerprint_min_release_age_arg`). diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver.rb index 78ef86d6340..34dd66cd6b7 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver.rb @@ -122,6 +122,8 @@ def update_subdependency_in_lockfile(lockfile) sig { params(updated_lockfiles: T::Array[Dependabot::DependencyFile]).returns(T.nilable(Gem::Version)) } def version_from_updated_lockfiles(updated_lockfiles) + return unless npm_updates_allowed?(updated_lockfiles) + updated_files = dependency_files - dependency_files_builder.lockfiles + updated_lockfiles @@ -143,6 +145,23 @@ def version_from_updated_lockfiles(updated_lockfiles) ) end + sig { params(updated_lockfiles: T::Array[Dependabot::DependencyFile]).returns(T::Boolean) } + def npm_updates_allowed?(updated_lockfiles) + allowed = updated_lockfiles.all? do |updated| + next true unless updated.name.end_with?("package-lock.json", "npm-shrinkwrap.json") + + original = dependency_files_builder.lockfiles.find { |lockfile| lockfile.path == updated.path } + NativeHelpers.npm_subdependency_update_allowed?( + lockfile: T.must(original), + updated_content: T.must(updated.content), + dependency: updated_dependency, + ignored_versions: ignored_versions + ) + end + Dependabot.logger.info("npm resolved #{dependency.name} outside the allowed versions") unless allowed + allowed + end + sig { returns(PnpmTransitiveUpdate) } def pnpm_update @pnpm_update ||= PnpmTransitiveUpdate.new(dependency: dependency, latest_allowable_version: allowable_version) diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/dependency_grapher_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/dependency_grapher_spec.rb index 697bf9bca0c..a8cdd2d3bde 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/dependency_grapher_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/dependency_grapher_spec.rb @@ -915,8 +915,6 @@ def grapher_for(directory, npm_engine_constraint) Dependabot::NpmAndYarn::Helpers.npm_version_selector = "99" end - after { Dependabot::NpmAndYarn::Helpers.npm_version_selector = nil } - it "activates each directory's own npm engine selector before generating its ephemeral lockfile" do selectors_seen = [] allow(Dependabot::NpmAndYarn::DependencyGrapher::LockfileGenerator).to receive(:new) do |**kwargs| diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_parser/json_lock_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_parser/json_lock_spec.rb index a6f82567fa1..970a7648023 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_parser/json_lock_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_parser/json_lock_spec.rb @@ -157,6 +157,48 @@ end end + describe "#parsed.installed_entries" do + subject(:installed) { reader.parsed.installed_entries } + + context "with nested legacy installations" do + let(:data) do + { + "lockfileVersion" => 1, + "requires" => true, + "dependencies" => { + "@scope/parent" => { + "version" => "1.0.0", + "requires" => { "@scope/child" => "^2.0.0" }, + "dependencies" => { "@scope/child" => { "version" => "2.0.0" } } + } + } + } + end + + it "preserves installation paths and separates requirements from installed entries" do + expect(installed.keys).to eq( + ["", "node_modules/@scope/parent", "node_modules/@scope/parent/node_modules/@scope/child"] + ) + expect(installed.fetch("node_modules/@scope/parent").dependency_names).to eq(["@scope/child"]) + expect(installed.fetch("").dependency_names).to be_empty + end + end + + context "with a v2 lockfile containing stale legacy installations" do + let(:data) { super().merge("lockfileVersion" => 2, "dependencies" => { "stale" => entry }) } + + it "uses only the packages table" do + expect(installed.keys).to eq(["node_modules/example"]) + end + + context "when the packages table is empty" do + let(:data) { super().merge("packages" => {}) } + + it { is_expected.to be_empty } + end + end + end + describe "#legacy_dependencies" do let(:data) { { "dependencies" => { "example" => entry }, "packages" => { "ignored" => {} } } } 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 e91442fce47..15df4893d26 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 @@ -215,8 +215,6 @@ allow(Dependabot::NpmAndYarn::Helpers).to receive(:register_npm_version_selector).and_call_original end - after { Dependabot::NpmAndYarn::Helpers.npm_version_selector = nil } - it "activates the selected npm major for the update process" do parser.ecosystem diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater_spec.rb index ac0533ae114..60dcca6e6eb 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater_spec.rb @@ -380,6 +380,19 @@ end end + context "when native npm exceeds a requested subdependency version" do + let(:files) { project_dependency_files("npm8/subdependency_update") } + let(:dependency_name) { "acorn" } + let(:version) { "5.6.0" } + let(:previous_version) { "5.5.3" } + let(:requirements) { [] } + let(:previous_requirements) { [] } + + it "does not publish an out-of-policy lockfile" do + expect { updated_npm_lock_content }.to raise_error(Dependabot::UpdateNotPossible) + end + end + context "when updating a subdependency in a workspace repo" do let(:files) { project_dependency_files("npm8/workspace_subdependency_update") } @@ -473,6 +486,27 @@ expect { updated_npm_lock_content } .to raise_error(Dependabot::InconsistentRegistryResponse) end + + context "when also updating a subdependency" do + let(:files) { project_dependency_files("npm8/subdependency_update") } + let(:dependencies) do + [ + dependency, + Dependabot::Dependency.new( + name: "acorn", + version: "5.7.4", + previous_version: "5.5.3", + requirements: [], + previous_requirements: [], + package_manager: "npm_and_yarn" + ) + ] + end + + it "preserves the original registry failure classification" do + expect { updated_npm_lock_content }.to raise_error(Dependabot::InconsistentRegistryResponse) + end + end end context "with a dependency that can't be found" do diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb index 12d19418f91..2e20f91c7e1 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb @@ -1096,31 +1096,11 @@ .to eq("2.0.2") end - context "when updating to lowest required version" do - let(:dependency_name) { "extend" } + context "when native npm exceeds the requested version" do let(:version) { "2.0.1" } - let(:previous_version) { "2.0.0" } - let(:requirements) { [] } - let(:previous_requirements) { nil } - - it "updates only relevant lockfiles" do - expect(updated_files.map(&:name)) - .to contain_exactly("packages/package1/package-lock.json", "packages/package3/yarn.lock") - - package1_npm_lock = - updated_files - .find { |f| f.name == "packages/package1/package-lock.json" } - package3_yarn_lock = - updated_files.find { |f| f.name == "packages/package3/yarn.lock" } - parsed_package1_npm_lock = JSON.parse(package1_npm_lock.content) - - expect(package3_yarn_lock.content) - .to include("extend@~2.0.0:\n version \"2.0.2\"") - # TODO: Change this to 2.0.1 once npm supports updating to specific - # sub dependency versions - expect(parsed_package1_npm_lock["packages"]["node_modules/extend"]["version"]) - .to eq("2.0.2") + it "rejects the update rather than returning partial lockfile changes" do + expect { updated_files }.to raise_error(Dependabot::UpdateNotPossible) end end @@ -1504,22 +1484,6 @@ end end - context "with a sub-dependency" do - let(:files) { project_dependency_files("npm8/subdependency_update") } - - let(:dependency_name) { "acorn" } - let(:version) { "5.7.3" } - let(:previous_version) { "5.5.3" } - let(:requirements) { [] } - let(:previous_requirements) { [] } - - it "updates the version" do - parsed_npm_lock = JSON.parse(updated_npm_lock.content) - expect(parsed_npm_lock["dependencies"]["acorn"]["version"]) - .to eq("5.7.4") - end - end - context "with a sub-dependency and non-standard indentation" do let(:files) { project_dependency_files("npm8/subdependency_update_tab_indentation") } @@ -2658,26 +2622,11 @@ .to eq("2.0.2") end - context "when updating to lowest required version" do - let(:dependency_name) { "extend" } + context "when native npm exceeds the requested version" do let(:version) { "2.0.1" } - let(:previous_version) { "2.0.0" } - let(:requirements) { [] } - let(:previous_requirements) { nil } - - it "updates only relevant lockfiles" do - expect(updated_files.map(&:name)) - .to contain_exactly("packages/package1/package-lock.json") - - package1_npm_lock = - updated_files - .find { |f| f.name == "packages/package1/package-lock.json" } - parsed_package1_npm_lock = JSON.parse(package1_npm_lock.content) - # TODO: Change this to 2.0.1 once npm supports updating to specific - # sub dependency versions - expect(parsed_package1_npm_lock["dependencies"]["extend"]["version"]) - .to eq("2.0.2") + it "rejects lockfile changes above the requested version" do + expect { updated_files }.to raise_error(Dependabot::UpdateNotPossible) 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 index 3f501b3f296..88370c9eb44 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/helpers_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/helpers_spec.rb @@ -16,8 +16,6 @@ [Dependabot::DependencyFile.new(name: "package.json", content: "{}", directory: "/modern")] end - after { described_class.npm_version_selector = nil } - it "restores the selector registered for each manifest directory" do described_class.register_npm_version_selector("/legacy", "9") described_class.register_npm_version_selector("/modern", "10") @@ -31,8 +29,6 @@ end describe "::run_npm_command" do - after { described_class.npm_version_selector = nil } - context "when an npm selector is configured" do before { described_class.npm_version_selector = "10" } @@ -116,7 +112,6 @@ describe "::npm_version" do before { described_class.npm_version_selector = "10" } - after { described_class.npm_version_selector = nil } it "returns the Corepack-managed npm version" do allow(Dependabot::SharedHelpers).to receive(:run_shell_command) diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/native_helpers_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/native_helpers_spec.rb index 33c1273b08f..3eb45c6ea27 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/native_helpers_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/native_helpers_spec.rb @@ -6,6 +6,424 @@ require "dependabot/npm_and_yarn/native_helpers" RSpec.describe Dependabot::NpmAndYarn::NativeHelpers do + describe ".npm_subdependency_update_allowed?" do + subject(:allowed) do + described_class.npm_subdependency_update_allowed?( + lockfile: lockfile, + updated_content: updated_content, + dependency: dependency, + ignored_versions: ignored_versions + ) + end + + let(:dependency) do + Dependabot::Dependency.new(name: "child", version: "1.2.0", requirements: [], package_manager: "npm_and_yarn") + end + let(:ignored_versions) { [] } + let(:original_packages) do + { + "node_modules/child" => { "version" => "1.0.0" }, + "node_modules/parent/node_modules/child" => { "version" => "2.0.0" } + } + end + let(:updated_packages) { original_packages.merge("node_modules/child" => { "version" => "1.2.0" }) } + let(:lockfile) do + Dependabot::DependencyFile.new( + name: "package-lock.json", content: JSON.generate(lockfileVersion: 3, packages: original_packages) + ) + end + let(:updated_content) { JSON.generate(lockfileVersion: 3, packages: updated_packages) } + + it "accepts the bound and leaves an unchanged higher occurrence alone" do + expect(allowed).to be(true) + end + + context "when a nested occurrence exceeds the bound" do + let(:updated_packages) do + super().merge("node_modules/parent/node_modules/child" => { "version" => "2.1.0" }) + end + + it { is_expected.to be(false) } + end + + context "when npm adds an aliased occurrence above the bound" do + let(:updated_packages) do + super().merge("node_modules/@scope/alias" => { "name" => "child", "version" => "2.0.0" }) + end + + it { is_expected.to be(false) } + end + + context "when an existing alias changes package identity" do + let(:dependency) do + Dependabot::Dependency.new(name: "alias", version: "1.2.0", requirements: [], package_manager: "npm_and_yarn") + end + let(:original_packages) { { "node_modules/alias" => { "name" => "child", "version" => "1.0.0" } } } + let(:updated_name) { "unrelated" } + let(:updated_packages) { { "node_modules/alias" => { "name" => updated_name, "version" => "1.2.0" } } } + + it "rejects a different package even when its version is within the bound" do + expect(allowed).to be(false) + end + + context "when the effective package identity is preserved" do + let(:updated_name) { "child" } + + it { is_expected.to be(true) } + + context "when npm converts a legacy alias to a modern package record" do + let(:lockfile) do + Dependabot::DependencyFile.new( + name: "package-lock.json", + content: JSON.generate(lockfileVersion: 1, dependencies: { alias: { version: "npm:child@1.0.0" } }) + ) + end + + it { is_expected.to be(true) } + end + end + end + + context "when an ignored range is below the upper bound" do + let(:ignored_versions) { [">= 1.1.0, < 1.3.0"] } + + it { is_expected.to be(false) } + end + + context "when only unchanged occurrences match an ignore rule" do + let(:ignored_versions) { [">= 2"] } + + it { is_expected.to be(true) } + end + + context "when an updated occurrence has no valid version" do + let(:updated_packages) { super().merge("node_modules/child" => { "version" => "not-semver" }) } + + it { is_expected.to be(false) } + end + + context "with a v2 lockfile containing a stale legacy section" do + let(:updated_content) do + JSON.generate( + lockfileVersion: 2, + packages: updated_packages, + dependencies: { child: { version: "1.0.0" } } + ) + end + let(:updated_packages) { super().merge("node_modules/child" => { "version" => "1.3.0" }) } + + it { is_expected.to be(false) } + end + + context "with a legacy v1 lockfile" do + let(:original_dependencies) do + { + "child" => { "version" => "1.0.0" }, + "parent" => { "version" => "1.0.0", "dependencies" => { "child" => { "version" => "2.0.0" } } } + } + end + let(:updated_dependencies) { original_dependencies.merge("child" => { "version" => "1.2.0" }) } + let(:lockfile) do + Dependabot::DependencyFile.new( + name: "package-lock.json", content: JSON.generate(lockfileVersion: 1, dependencies: original_dependencies) + ) + end + let(:updated_content) { JSON.generate(lockfileVersion: 1, dependencies: updated_dependencies) } + + it "accepts a compliant change beside an unchanged higher nested occurrence" do + expect(allowed).to be(true) + end + + context "when the top-level installation exceeds the bound" do + let(:updated_dependencies) { super().merge("child" => { "version" => "1.3.0" }) } + + it { is_expected.to be(false) } + end + + context "when a nested installation exceeds the bound" do + let(:updated_dependencies) do + super().merge( + "parent" => { "version" => "1.0.0", "dependencies" => { "child" => { "version" => "2.1.0" } } } + ) + end + + it { is_expected.to be(false) } + end + + context "when an ignored range is below the bound" do + let(:ignored_versions) { [">= 1.1.0, < 1.3.0"] } + + it { is_expected.to be(false) } + end + + context "when only the unchanged nested installation is ignored" do + let(:ignored_versions) { [">= 2"] } + + it { is_expected.to be(true) } + end + + context "when only a nested installation changes into an ignored range" do + let(:ignored_versions) { [">= 1.1.0, < 1.3.0"] } + let(:updated_dependencies) do + original_dependencies.merge( + "parent" => { "version" => "1.0.0", "dependencies" => { "child" => { "version" => "1.2.0" } } } + ) + end + + it { is_expected.to be(false) } + + context "without the ignored range" do + let(:ignored_versions) { [] } + + it { is_expected.to be(true) } + end + end + + context "when npm converts the lockfile and adds package names" do + let(:ignored_versions) { [">= 2"] } + let(:updated_content) do + JSON.generate( + lockfileVersion: 2, + packages: { + "node_modules/child" => { "name" => "child", "version" => "1.2.0" }, + "node_modules/parent" => { "name" => "parent", "version" => "1.0.0" }, + "node_modules/parent/node_modules/child" => { "name" => "child", "version" => "2.0.0" } + }, + dependencies: original_dependencies + ) + end + + it "compares effective versions and names rather than legacy metadata" do + expect(allowed).to be(true) + end + end + + context "when a scoped nested alias is updated" do + let(:updated_dependencies) do + super().merge( + "@scope/parent" => { + "version" => "1.0.0", "dependencies" => { "@scope/alias" => { "version" => "npm:child@1.3.0" } } + } + ) + end + + it { is_expected.to be(false) } + + context "with a compliant alias version" do + let(:updated_dependencies) do + super().merge( + "@scope/parent" => { + "version" => "1.0.0", "dependencies" => { "@scope/alias" => { "version" => "npm:child@1.2.0" } } + } + ) + end + + it { is_expected.to be(true) } + end + + context "when the aliased package itself is scoped" do + let(:dependency) do + Dependabot::Dependency.new( + name: "@scope/child", version: "1.2.0", requirements: [], package_manager: "npm_and_yarn" + ) + end + let(:updated_dependencies) do + super().merge("@scope/alias" => { "version" => "npm:@scope/child@1.3.0" }) + end + + it { is_expected.to be(false) } + end + end + end + + shared_examples "a removed nested occurrence" do + it "rejects rebinding to an unchanged higher installation" do + expect(allowed).to be(false) + end + + context "with a compliant hoisted installation" do + let(:hoisted_version) { "1.2.0" } + + it { is_expected.to be(true) } + + context "when the hoisted version is ignored" do + let(:ignored_versions) { [">= 1.1.0, < 1.3.0"] } + + it { is_expected.to be(false) } + end + end + + context "with an invalid hoisted version" do + let(:hoisted_version) { "not-semver" } + + it { is_expected.to be(false) } + end + + context "with an empty hoisted version" do + let(:hoisted_version) { "" } + + it { is_expected.to be(false) } + end + + context "without a replacement" do + let(:remove_hoisted) { true } + + it { is_expected.to be(false) } + end + + context "when the parent no longer requires the dependency" do + let(:updated_requirements) { {} } + + it { is_expected.to be(true) } + end + + context "when the parent is also removed" do + let(:remove_parent) { true } + + it { is_expected.to be(true) } + end + + context "when deduplication preserves the effective higher version" do + let(:nested_version) { "2.0.0" } + let(:ignored_versions) { [">= 2"] } + + it { is_expected.to be(true) } + end + end + + context "when a nested occurrence is removed" do + let(:hoisted_version) { "2.0.0" } + let(:nested_version) { "1.0.0" } + let(:parent_name) { "parent" } + let(:installed_name) { "child" } + let(:updated_requirements) { { installed_name => ">=1" } } + let(:remove_parent) { false } + let(:remove_hoisted) { false } + let(:original_packages) do + { + "node_modules/#{installed_name}" => { "name" => "child", "version" => hoisted_version }, + "node_modules/#{parent_name}" => { "version" => "1.0.0", "dependencies" => { installed_name => ">=1" } }, + "node_modules/#{parent_name}/node_modules/#{installed_name}" => + { "name" => "child", "version" => nested_version } + } + end + let(:updated_packages) do + packages = original_packages.except("node_modules/#{parent_name}/node_modules/#{installed_name}") + packages["node_modules/#{parent_name}"] = { "version" => "1.0.0", "dependencies" => updated_requirements } + packages.delete("node_modules/#{parent_name}") if remove_parent + packages.delete("node_modules/#{installed_name}") if remove_hoisted + packages + end + + it_behaves_like "a removed nested occurrence" + + context "with scoped parents and aliases" do + let(:parent_name) { "@scope/parent" } + let(:installed_name) { "@scope/alias" } + + it_behaves_like "a removed nested occurrence" + end + + context "with a nearer compliant installation and an unchanged higher root installation" do + let(:parent_name) { "@scope/grandparent/node_modules/parent" } + let(:original_packages) do + super().merge( + "node_modules/@scope/grandparent" => { "version" => "1.0.0" }, + "node_modules/@scope/grandparent/node_modules/child" => { "version" => "1.2.0" } + ) + end + + it { is_expected.to be(true) } + end + + context "when the parent moves to a different installation path" do + let(:remove_parent) { true } + let(:updated_packages) do + super().merge( + "node_modules/other-parent" => { "version" => "1.0.0", "dependencies" => updated_requirements } + ) + end + + it { is_expected.to be(false) } + + context "with a compliant replacement" do + let(:hoisted_version) { "1.2.0" } + + it { is_expected.to be(true) } + end + end + + context "when the replacement has a different package identity" do + let(:hoisted_version) { "1.2.0" } + let(:updated_packages) do + super().merge("node_modules/child" => { "name" => "unrelated", "version" => "1.2.0" }) + end + + it { is_expected.to be(false) } + end + + context "with no requirement metadata proving the removal is safe" do + let(:original_packages) do + super().merge("node_modules/#{parent_name}" => { "version" => "1.0.0" }) + end + let(:updated_requirements) { {} } + + it { is_expected.to be(false) } + end + + context "with a legacy v1 lockfile" do + let(:original_dependencies) do + { + "child" => { "version" => hoisted_version }, + "parent" => { + "version" => "1.0.0", "requires" => { "child" => ">=1" }, + "dependencies" => { "child" => { "version" => nested_version } } + } + } + end + let(:updated_dependencies) do + dependencies = original_dependencies.merge( + "parent" => { "version" => "1.0.0", "requires" => updated_requirements } + ) + dependencies.delete("parent") if remove_parent + dependencies.delete("child") if remove_hoisted + dependencies + end + let(:lockfile) do + Dependabot::DependencyFile.new( + name: "package-lock.json", content: JSON.generate(lockfileVersion: 1, dependencies: original_dependencies) + ) + end + let(:updated_content) { JSON.generate(lockfileVersion: 1, dependencies: updated_dependencies) } + + it_behaves_like "a removed nested occurrence" + end + end + + context "when a workspace occurrence is removed" do + let(:nearer_version) { "2.0.0" } + let(:original_packages) do + { + "node_modules/child" => { "version" => "1.2.0" }, + "packages/node_modules/child" => { "version" => nearer_version }, + "packages/app" => { "version" => "1.0.0", "dependencies" => { "child" => ">=1" } }, + "packages/app/node_modules/child" => { "version" => "1.0.0" } + } + end + let(:updated_packages) { original_packages.except("packages/app/node_modules/child") } + + it "checks nearer ancestor installations before the root" do + expect(allowed).to be(false) + end + + context "when the nearer installation is compliant" do + let(:nearer_version) { "1.2.0" } + + it { is_expected.to be(true) } + end + end + end + describe ".run_pnpm_audit_fix_command" do before do allow(Dependabot::NpmAndYarn::Helpers).to receive(:run_pnpm_command) do |command, **| diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/package_manager_helper_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/package_manager_helper_spec.rb index e51f58629e0..660e731d290 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/package_manager_helper_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/package_manager_helper_spec.rb @@ -226,8 +226,6 @@ allow(Dependabot::NpmAndYarn::Helpers).to receive(:package_manager_install) end - after { Dependabot::NpmAndYarn::Helpers.npm_version_selector = nil } - it "activates the cached latest version for that major" do expect(helper.setup("npm")).to eq("10") expect(Dependabot::NpmAndYarn::Helpers).to have_received(:package_manager_install) diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver_spec.rb index fcac65594fd..a24ce01d8de 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver_spec.rb @@ -351,9 +351,23 @@ end let(:latest_allowable_version) { "6.0.2" } - it "calls run_npm_updater" do - expect(resolver).to receive(:run_npm_updater).and_call_original - expect(latest_resolvable_version).to eq(Gem::Version.new("5.7.4")) + it "rejects an update when a higher nested occurrence exceeds the cap" do + expect(latest_resolvable_version).to be_nil + end + + context "when native npm exceeds the allowable version" do + let(:latest_allowable_version) { "5.6.0" } + + it "rejects the actual lockfile instead of reporting a lower version" do + expect(latest_resolvable_version).to be_nil + end + end + + context "when native npm selects an ignored version below the allowable version" do + let(:ignored_versions) { [">= 5.7.0, < 6"] } + let(:latest_allowable_version) { "7.0.0" } + + it { is_expected.to be_nil } end context "when resolving a security update" do @@ -428,103 +442,6 @@ latest_resolvable_version end end - - context "when all_versions metadata is present" do - let(:all_version_deps) do - [ - Dependabot::Dependency.new( - name: "acorn", - version: "5.5.3", - requirements: [], - package_manager: "npm_and_yarn" - ), - Dependabot::Dependency.new( - name: "acorn", - version: "5.6.0", - requirements: [], - package_manager: "npm_and_yarn" - ), - Dependabot::Dependency.new( - name: "acorn", - version: "5.7.4", - requirements: [], - package_manager: "npm_and_yarn" - ) - ] - end - - let(:parsed_dep) do - Dependabot::Dependency.new( - name: "acorn", - version: "5.7.4", - requirements: [], - package_manager: "npm_and_yarn", - metadata: { all_versions: all_version_deps } - ) - end - - before do - parser = instance_double(Dependabot::NpmAndYarn::FileParser) - allow(Dependabot::NpmAndYarn::FileParser).to receive(:new).and_return(parser) - allow(parser).to receive(:parse).and_return([parsed_dep]) - end - - it "returns the highest version from all_versions within the allowable range" do - expect(latest_resolvable_version).to eq(Gem::Version.new("5.7.4")) - end - - context "when latest_allowable_version caps the result" do - let(:latest_allowable_version) { "5.6.0" } - - it "returns the highest version at or below the allowable version" do - expect(latest_resolvable_version).to eq(Gem::Version.new("5.6.0")) - end - end - - context "when no all_versions candidate is above the current version" do - let(:all_version_deps) do - [ - Dependabot::Dependency.new( - name: "acorn", - version: "5.5.3", - requirements: [], - package_manager: "npm_and_yarn" - ) - ] - end - - it "falls back to the combined parsed version" do - expect(latest_resolvable_version).to eq(Gem::Version.new("5.7.4")) - end - end - - context "when all_versions metadata is empty" do - let(:parsed_dep) do - Dependabot::Dependency.new( - name: "acorn", - version: "5.7.4", - requirements: [], - package_manager: "npm_and_yarn", - metadata: { all_versions: [] } - ) - end - - it "falls back to the combined parsed version" do - expect(latest_resolvable_version).to eq(Gem::Version.new("5.7.4")) - end - end - - context "when the experiment is disabled" do - before do - allow(Dependabot::Experiments).to receive(:enabled?) - .with(:enable_audit_fix_fallback).and_return(false) - end - - it "returns the combined parsed version without checking all_versions" do - expect(latest_resolvable_version).to eq(Gem::Version.new("5.7.4")) - end - end - end end context "with a npm6 package-lock.json" do @@ -540,9 +457,17 @@ end let(:latest_allowable_version) { "6.0.2" } - # NOTE: The latest vision is 6.0.2, but we can't reach it as other - # dependencies constrain us - it { is_expected.to eq(Gem::Version.new("5.7.4")) } + it "rejects a native update that moves a nested occurrence beyond the allowable version" do + expect(latest_resolvable_version).to be_nil + end + + context "when all changed occurrences are within the allowable version" do + let(:latest_allowable_version) { "6.4.2" } + + it "returns the highest allowable updated occurrence" do + expect(latest_resolvable_version).to eq(Gem::Version.new("6.4.2")) + end + end end context "when sub-dependency is bundled" do @@ -580,7 +505,9 @@ end let(:latest_allowable_version) { "6.0.2" } - it { is_expected.to eq(Gem::Version.new("5.7.4")) } + it "rejects a native update that moves a nested occurrence beyond the allowable version" do + expect(latest_resolvable_version).to be_nil + end end context "when updating a sub-dependency across both yarn and npm lockfiles" do diff --git a/npm_and_yarn/spec/spec_helper.rb b/npm_and_yarn/spec/spec_helper.rb index 9f90cbd01dd..ff77ed1ddbd 100644 --- a/npm_and_yarn/spec/spec_helper.rb +++ b/npm_and_yarn/spec/spec_helper.rb @@ -11,6 +11,13 @@ def require_common_spec(path) require "#{common_dir}/spec/spec_helper.rb" +RSpec.configure do |config| + config.after do + Thread.current[:npm_and_yarn_npm_version_selector] = nil + Thread.current[:npm_and_yarn_npm_version_selectors] = nil + end +end + def create_dependency( name:, version:,