[npm] fix failure to attempt parent update if unfixed transitive update is available

bin/dry-run.rb

@@ -463,6 +463,17 @@ def handle_dependabot_error(error:, dependency:)
 end
 # rubocop:enable Metrics/MethodLength
 
+def log_conflicting_dependencies(conflicting_dependencies)
+  return unless conflicting_dependencies.any?
+
+  puts " => The update is not possible because of the following conflicting " \
+       "dependencies:"
+
+  conflicting_dependencies.each do |conflicting_dep|
+    puts "   #{conflicting_dep['explanation']}"
+  end
+end
+
 StackProf.start(raw: true) if $options[:profile]
 
 $network_trace_count = 0
@@ -715,16 +726,7 @@ def security_fix?(dependency)
       puts "    (no update possible 🙅‍♀️)"
     end
 
-    conflicting_dependencies = checker.conflicting_dependencies
-    if conflicting_dependencies.any?
-      puts " => The update is not possible because of the following conflicting " \
-           "dependencies:"
-
-      conflicting_dependencies.each do |conflicting_dep|
-        puts "   #{conflicting_dep['explanation']}"
-      end
-    end
-
+    log_conflicting_dependencies(checker.conflicting_dependencies)
     next
   end
 
@@ -737,6 +739,13 @@ def security_fix?(dependency)
     next
   end
 
+  if $options[:security_updates_only] &&
+     updated_deps.none? { |d| security_fix?(d) }
+    puts "    (updated version is still vulnerable 🚨)"
+    log_conflicting_dependencies(checker.conflicting_dependencies)
+    next
+  end
+
   # Removal is only supported for transitive dependencies which are removed as a
   # side effect of the parent update
   deps_to_update = updated_deps.reject(&:removed?)
@@ -750,11 +759,6 @@ def security_fix?(dependency)
     d.version == d.previous_version
   end
 
-  if $options[:security_updates_only] &&
-     updated_deps.none? { |d| security_fix?(d) }
-    puts "    (updated version is still vulnerable 🚨)"
-  end
-
   if $options[:write]
     updated_files.each do |updated_file|
       path = File.join(dependency_files_cache_dir, updated_file.name)

npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker.rb

@@ -46,8 +46,8 @@ def lowest_resolvable_security_fix_version
         raise "Dependency not vulnerable!" unless vulnerable?
         # NOTE: we currently don't resolve transitive/sub-dependencies as
         # npm/yarn don't provide any control over updating to a specific
-        # sub-dependency
-        return latest_resolvable_version unless dependency.top_level?
+        # sub-dependency version
+        return latest_resolvable_transitive_security_fix_version_with_no_unlock unless dependency.top_level?
 
         # TODO: Might want to check resolvability here?
         lowest_security_fix_version
@@ -213,6 +213,16 @@ def build_updated_dependency(update_details)
         )
       end
 
+      def latest_resolvable_transitive_security_fix_version_with_no_unlock
+        fix_possible = Dependabot::UpdateCheckers::VersionFilters.filter_vulnerable_versions(
+          [latest_resolvable_version].compact,
+          security_advisories
+        ).any?
+        return nil unless fix_possible
+
+        latest_resolvable_version
+      end
+
       def latest_resolvable_version_with_no_unlock_for_git_dependency
         reqs = dependency.requirements.filter_map do |r|
           next if r.fetch(:requirement).nil?

npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker_spec.rb

@@ -213,6 +213,39 @@
           end
         end
       end
+
+      context "when a transitive dependency is able to update without unlocking its parent but is still vulnerable",
+              :vcr do
+        let(:dependency_files) { project_dependency_files("npm8/transitive_dependency_locked_but_updateable") }
+        let(:registry_listing_url) { "https://registry.npmjs.org/transitive-dependency-locked-but-updateable" }
+
+        let(:security_advisories) do
+          [
+            Dependabot::SecurityAdvisory.new(
+              dependency_name: "@dependabot-fixtures/npm-transitive-dependency-with-more-versions",
+              package_manager: "npm_and_yarn",
+              vulnerable_versions: ["< 2.0.0"]
+            )
+          ]
+        end
+        let(:dependency_version) { "1.0.0" }
+        let(:dependency) do
+          Dependabot::Dependency.new(
+            name: "@dependabot-fixtures/npm-transitive-dependency-with-more-versions",
+            version: dependency_version,
+            requirements: [],
+            package_manager: "npm_and_yarn"
+          )
+        end
+
+        it "can't update without unlocking" do
+          expect(subject).to eq(false)
+        end
+
+        it "allows full unlocking" do
+          expect(checker.can_update?(requirements_to_unlock: :all)).to eq(true)
+        end
+      end
     end
 
     context "for a scoped package name" do
@@ -1459,6 +1492,64 @@
           )
         end
       end
+
+      context "when a transitive dependency is able to update without unlocking its parent but is still vulnerable" do
+        let(:dependency_files) { project_dependency_files("npm8/transitive_dependency_locked_but_updateable") }
+        let(:registry_listing_url) { "https://registry.npmjs.org/transitive-dependency-locked-but-updateable" }
+
+        let(:security_advisories) do
+          [
+            Dependabot::SecurityAdvisory.new(
+              dependency_name: "@dependabot-fixtures/npm-transitive-dependency-with-more-versions",
+              package_manager: "npm_and_yarn",
+              vulnerable_versions: ["< 2.0.0"]
+            )
+          ]
+        end
+        let(:dependency_version) { "1.0.0" }
+        let(:dependency) do
+          Dependabot::Dependency.new(
+            name: "@dependabot-fixtures/npm-transitive-dependency-with-more-versions",
+            version: dependency_version,
+            requirements: [],
+            package_manager: "npm_and_yarn"
+          )
+        end
+
+        it "correctly updates the transitive dependency by unlocking the parent" do
+          expect(checker.send(:updated_dependencies_after_full_unlock)).to eq([
+            Dependabot::Dependency.new(
+              name: "@dependabot-fixtures/npm-transitive-dependency-with-more-versions",
+              package_manager: "npm_and_yarn",
+              previous_requirements: [],
+              previous_version: "1.0.0",
+              requirements: [],
+              version: "2.0.0"
+            ),
+            Dependabot::Dependency.new(
+              name: "@dependabot-fixtures/npm-parent-dependency-with-more-versions",
+              package_manager: "npm_and_yarn",
+              previous_requirements: [{
+                requirement: "^1.0.0",
+                file: "package.json",
+                groups: ["dependencies"],
+                source: {
+                  type: "registry",
+                  url: "https://registry.npmjs.org"
+                }
+              }],
+              previous_version: "1.0.0",
+              requirements: [{
+                requirement: "^1.0.0",
+                file: "package.json",
+                groups: ["dependencies"],
+                source: nil
+              }],
+              version: "1.0.1"
+            )
+          ])
+        end
+      end
     end
   end
 

npm_and_yarn/spec/fixtures/projects/npm8/transitive_dependency_locked_but_updateable/package.json

@@ -0,0 +1,14 @@
+{
+  "name": "transitive-dependency-locked-but-updateable",
+  "version": "1.0.0",
+  "description": "",
+  "main": "index.js",
+  "scripts": {
+    "test": "echo \"Error: no test specified\" && exit 1"
+  },
+  "author": "",
+  "license": "ISC",
+  "dependencies": {
+    "@dependabot-fixtures/npm-parent-dependency-with-more-versions": "^1.0.0"
+  }
+}