Add more helpful error messaging when a vulnerable dependency cannot be upgraded

npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker.rb

@@ -96,13 +96,19 @@ def requirements_update_strategy
       end
 
       def conflicting_dependencies
-        ConflictingDependencyResolver.new(
+        conflicts = ConflictingDependencyResolver.new(
           dependency_files: dependency_files,
           credentials: credentials
         ).conflicting_dependencies(
           dependency: dependency,
           target_version: lowest_security_fix_version
         )
+
+        vulnerable = [vulnerability_audit].select do |hash|
+          !hash["fix_available"] && hash["explanation"]
+        end
+
+        conflicts + vulnerable
       end
 
       private

npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/vulnerability_auditor.rb

@@ -21,6 +21,7 @@ def initialize(dependency_files:, credentials:, allow_removal: false)
           @allow_removal = allow_removal
         end
 
+        # rubocop:disable Metrics/MethodLength
         # Finds any dependencies in the `package-lock.json` or `npm-shrinkwrap.json` that have
         # a subdependency on the given dependency that is locked to a vuln version range.
         #
@@ -41,6 +42,7 @@ def initialize(dependency_files:, credentials:, allow_removal: false)
         #       dependency on the blocking dependency
         #   * :top_level_ancestors [Array<String>] the names of all top-level dependencies with a transitive
         #     dependency on the dependency
+        #   * :explanation [String] an explanation for why the project failed the vulnerability auditor run
         def audit(dependency:, security_advisories:)
           fix_unavailable = {
             "dependency_name" => dependency.name,
@@ -74,21 +76,36 @@ def audit(dependency:, security_advisories:)
               function: "npm:vulnerabilityAuditor",
               args: [Dir.pwd, vuln_versions]
             )
-            return fix_unavailable unless viable_audit_result?(audit_result, security_advisories)
+
+            validation_result = validate_audit_result(audit_result, security_advisories)
+            unless viable_audit_result?(validation_result)
+              fix_unavailable["explanation"] = explain_fix_unavailable(validation_result, dependency)
+              return fix_unavailable
+            end
 
             audit_result
           end
         rescue SharedHelpers::HelperSubprocessFailed => e
           log_helper_subprocess_failure(dependency, e)
           fix_unavailable
         end
+        # rubocop:enable Metrics/MethodLength
 
         private
 
         attr_reader :dependency_files, :credentials
 
-        def viable_audit_result?(audit_result, security_advisories)
-          validation_result = validate_audit_result(audit_result, security_advisories)
+        def explain_fix_unavailable(validation_result, dependency)
+          case validation_result
+          when :fix_unavailable, :dependency_still_vulnerable, :downgrades_dependencies
+            "No patched version available for #{dependency.name}"
+          when :vulnerable_dependency_removed
+            "#{dependency.name} was removed in the update. Dependabot is not able to " \
+            "deal with this yet, but you can still upgrade manually."
+          end
+        end
+
+        def viable_audit_result?(validation_result)
           return true if validation_result == :viable
 
           Dependabot.logger.info("VulnerabilityAuditor: audit result not viable: #{validation_result}")

npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/vulnerability_auditor_spec.rb

@@ -110,7 +110,11 @@
 
           expect(Dependabot.logger).to receive(:info).with(/audit result not viable: vulnerable_dependency_removed/i)
           expect(subject.audit(dependency: dependency, security_advisories: security_advisories)).
-            to include("fix_available" => false)
+            to include(
+              "fix_available" => false,
+              "explanation" => "#{dependency.name} was removed in the update. " \
+                               "Dependabot is not able to deal with this yet, but you can still upgrade manually."
+            )
         end
       end
     end
@@ -137,7 +141,10 @@
 
         expect(Dependabot.logger).to receive(:info).with(/audit result not viable: dependency_still_vulnerable/i)
         expect(subject.audit(dependency: dependency, security_advisories: security_advisories)).
-          to include("fix_available" => false)
+          to include(
+            "fix_available" => false,
+            "explanation" => "No patched version available for #{dependency.name}"
+          )
       end
     end
 
@@ -172,7 +179,10 @@
 
         expect(Dependabot.logger).to receive(:info).with(/audit result not viable: downgrades_dependencies/i)
         expect(subject.audit(dependency: dependency, security_advisories: security_advisories)).
-          to include("fix_available" => false)
+          to include(
+            "fix_available" => false,
+            "explanation" => "No patched version available for #{dependency.name}"
+          )
       end
     end
 

npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker_spec.rb

@@ -1491,6 +1491,119 @@
     end
   end
 
+  describe "#conflicting_dependencies" do
+    let(:registry_listing_url) { "https://registry.npmjs.org/locked-transitive-dependency" }
+    let(:options) { { npm_transitive_security_updates: true } }
+    let(:credentials) do
+      [{
+        "type" => "git_source",
+        "host" => "github.com",
+        "username" => "x-access-token",
+        "password" => "token"
+      }]
+    end
+
+    let(:dependency) do
+      Dependabot::Dependency.new(
+        name: "@dependabot-fixtures/npm-transitive-dependency",
+        version: dependency_version,
+        requirements: [],
+        package_manager: "npm_and_yarn"
+      )
+    end
+
+    context "with a conflicting dependency" do
+      let(:dependency_files) { project_dependency_files("npm8/locked_transitive_dependency") }
+      let(:dependency_version) { "1.0.0" }
+      let(:target_version) { Dependabot::NpmAndYarn::Version.new("1.0.1") }
+
+      it "delegates to the ConflictingDependencyResolver and VulnerabilityAuditor and explains the conflict", :vcr do
+        expect(described_class::ConflictingDependencyResolver).
+          to receive(:new).
+          with(
+            dependency_files: dependency_files,
+            credentials: credentials
+          ).and_call_original
+
+        expect(described_class::VulnerabilityAuditor).
+          to receive(:new).
+          with(
+            dependency_files: dependency_files,
+            credentials: credentials,
+            allow_removal: false
+          ).and_call_original
+
+        conflicting_dependencies_result = checker.send(:conflicting_dependencies)
+
+        expect(conflicting_dependencies_result.count).to eq(1)
+        expect(conflicting_dependencies_result.first).
+          to eq(
+            "explanation" => "@dependabot-fixtures/npm-parent-dependency@2.0.0 requires " \
+                             "@dependabot-fixtures/npm-transitive-dependency@1.0.0 via " \
+                             "@dependabot-fixtures/npm-intermediate-dependency@0.0.1",
+            "name" => "@dependabot-fixtures/npm-intermediate-dependency",
+            "requirement" => "1.0.0",
+            "version" => "0.0.1"
+          )
+      end
+    end
+
+    context "with a conflicting dependency and an unsatisfiable vulnerablity" do
+      let(:dependency_files) { project_dependency_files("npm8/locked_transitive_dependency") }
+      let(:dependency_version) { "1.0.0" }
+      let(:target_version) { Dependabot::NpmAndYarn::Version.new("1.0.1") }
+      let(:security_advisories) do
+        [
+          Dependabot::SecurityAdvisory.new(
+            dependency_name: "@dependabot-fixtures/npm-transitive-dependency",
+            package_manager: "npm_and_yarn",
+            vulnerable_versions: ["< 1.0.2"]
+          )
+        ]
+      end
+
+      it "delegates to the ConflictingDependencyResolver and VulnerabilityAuditor and explains the conflict", :vcr do
+        expect(described_class::ConflictingDependencyResolver).
+          to receive(:new).
+          with(
+            dependency_files: dependency_files,
+            credentials: credentials
+          ).and_call_original
+
+        expect(described_class::VulnerabilityAuditor).
+          to receive(:new).
+          with(
+            dependency_files: dependency_files,
+            credentials: credentials,
+            allow_removal: false
+          ).and_call_original
+
+        conflicting_dependencies_result = checker.send(:conflicting_dependencies)
+
+        expect(conflicting_dependencies_result.count).to eq(2)
+
+        expect(conflicting_dependencies_result.first).
+          to eq(
+            "explanation" => "@dependabot-fixtures/npm-parent-dependency@2.0.0 requires " \
+                             "@dependabot-fixtures/npm-transitive-dependency@1.0.0 via " \
+                             "@dependabot-fixtures/npm-intermediate-dependency@0.0.1",
+            "name" => "@dependabot-fixtures/npm-intermediate-dependency",
+            "requirement" => "1.0.0",
+            "version" => "0.0.1"
+          )
+
+        expect(conflicting_dependencies_result.last).
+          to eq(
+            "dependency_name" => "@dependabot-fixtures/npm-transitive-dependency",
+            "explanation" => "No patched version available for @dependabot-fixtures/npm-transitive-dependency",
+            "fix_available" => false,
+            "fix_updates" => [],
+            "top_level_ancestors" => []
+          )
+      end
+    end
+  end
+
   context "when types dependency specified" do
     let(:registry_listing_url) { "https://registry.npmjs.org/jquery" }
     let(:registry_response) do