Merge pull request #9556 from dependabot/jurre/hash-private-packages

Improve handling for hashing unknown packages

.editorconfig

@@ -15,3 +15,7 @@ indent_style = tab
 
 [*.php]
 indent_size = 4
+
+[*.py]
+indent_size = 4
+max_line_length = 80

python/helpers/lib/hasher.py

@@ -1,17 +1,26 @@
 import hashin
 import json
 import plette
+import traceback
 from poetry.factory import Factory
 
 
-def get_dependency_hash(dependency_name, dependency_version, algorithm):
-    hashes = hashin.get_package_hashes(
-        dependency_name,
-        version=dependency_version,
-        algorithm=algorithm
-    )
-
-    return json.dumps({"result": hashes["hashes"]})
+def get_dependency_hash(dependency_name, dependency_version, algorithm,
+                        index_url=hashin.DEFAULT_INDEX_URL):
+    try:
+        hashes = hashin.get_package_hashes(
+            dependency_name,
+            version=dependency_version,
+            algorithm=algorithm,
+            index_url=index_url
+        )
+        return json.dumps({"result": hashes["hashes"]})
+    except hashin.PackageNotFoundError as e:
+        return json.dumps({
+            "error": repr(e),
+            "error_class:": e.__class__.__name__,
+            "trace:": ''.join(traceback.format_stack())
+        })
 
 
 def get_pipfile_hash(directory):

python/lib/dependabot/python/file_updater.rb

@@ -105,18 +105,31 @@ def updated_pip_compile_based_files
         PipCompileFileUpdater.new(
           dependencies: dependencies,
           dependency_files: dependency_files,
-          credentials: credentials
+          credentials: credentials,
+          index_urls: pip_compile_index_urls
         ).updated_dependency_files
       end
 
       def updated_requirement_based_files
         RequirementFileUpdater.new(
           dependencies: dependencies,
           dependency_files: dependency_files,
-          credentials: credentials
+          credentials: credentials,
+          index_urls: pip_compile_index_urls
         ).updated_dependency_files
       end
 
+      def pip_compile_index_urls
+        if credentials.any?(&:replaces_base?)
+          credentials.select(&:replaces_base?).map { |cred| AuthedUrlBuilder.authed_url(credential: cred) }
+        else
+          urls = credentials.map { |cred| AuthedUrlBuilder.authed_url(credential: cred) }
+          # If there are no credentials that replace the base, we need to
+          # ensure that the base URL is included in the list of extra-index-urls.
+          [nil, *urls]
+        end
+      end
+
       def check_required_files
         filenames = dependency_files.map(&:name)
         return if filenames.any? { |name| name.end_with?(".txt", ".in") }

python/lib/dependabot/python/file_updater/pip_compile_file_updater.rb

@@ -34,10 +34,11 @@ class PipCompileFileUpdater
         attr_reader :dependency_files
         attr_reader :credentials
 
-        def initialize(dependencies:, dependency_files:, credentials:)
+        def initialize(dependencies:, dependency_files:, credentials:, index_urls: nil)
           @dependencies = dependencies
           @dependency_files = dependency_files
           @credentials = credentials
+          @index_urls = index_urls
           @build_isolation = true
         end
 
@@ -265,7 +266,8 @@ def freeze_dependency_requirement(file)
             content: file.content,
             dependency_name: dependency.name,
             old_requirement: old_req[:requirement],
-            new_requirement: "==#{dependency.version}"
+            new_requirement: "==#{dependency.version}",
+            index_urls: @index_urls
           ).updated_content
         end
 
@@ -283,7 +285,8 @@ def update_dependency_requirement(file)
             content: file.content,
             dependency_name: dependency.name,
             old_requirement: old_req[:requirement],
-            new_requirement: new_req[:requirement]
+            new_requirement: new_req[:requirement],
+            index_urls: @index_urls
           ).updated_content
         end
 
@@ -389,11 +392,29 @@ def deps_to_augment_hashes_for(updated_content, original_content)
         end
 
         def package_hashes_for(name:, version:, algorithm:)
-          SharedHelpers.run_helper_subprocess(
-            command: "pyenv exec python3 #{NativeHelpers.python_helper_path}",
-            function: "get_dependency_hash",
-            args: [name, version, algorithm]
-          ).map { |h| "--hash=#{algorithm}:#{h['hash']}" }
+          index_urls = @index_urls || [nil]
+          hashes = []
+
+          index_urls.each do |index_url|
+            args = [name, version, algorithm]
+            args << index_url if index_url
+
+            begin
+              native_helper_hashes = SharedHelpers.run_helper_subprocess(
+                command: "pyenv exec python3 #{NativeHelpers.python_helper_path}",
+                function: "get_dependency_hash",
+                args: args
+              ).map { |h| "--hash=#{algorithm}:#{h['hash']}" }
+
+              hashes.concat(native_helper_hashes)
+            rescue SharedHelpers::HelperSubprocessFailed => e
+              raise unless e.error_class.include?("PackageNotFoundError")
+
+              next
+            end
+          end
+
+          hashes
         end
 
         def hash_separator(requirement_string)

python/lib/dependabot/python/file_updater/requirement_file_updater.rb

@@ -16,10 +16,11 @@ class RequirementFileUpdater
         attr_reader :dependency_files
         attr_reader :credentials
 
-        def initialize(dependencies:, dependency_files:, credentials:)
+        def initialize(dependencies:, dependency_files:, credentials:, index_urls: nil)
           @dependencies = dependencies
           @dependency_files = dependency_files
           @credentials = credentials
+          @index_urls = index_urls
         end
 
         def updated_dependency_files
@@ -58,7 +59,8 @@ def updated_requirement_or_setup_file_content(new_req, old_req)
             dependency_name: dependency.name,
             old_requirement: old_req.fetch(:requirement),
             new_requirement: new_req.fetch(:requirement),
-            new_hash_version: dependency.version
+            new_hash_version: dependency.version,
+            index_urls: @index_urls
           ).updated_content
         end
 

python/lib/dependabot/python/file_updater/requirement_replacer.rb

@@ -12,13 +12,16 @@ module Dependabot
   module Python
     class FileUpdater
       class RequirementReplacer
+        PACKAGE_NOT_FOUND_ERROR = "PackageNotFoundError"
+
         def initialize(content:, dependency_name:, old_requirement:,
-                       new_requirement:, new_hash_version: nil)
+                       new_requirement:, new_hash_version: nil, index_urls: nil)
           @content          = content
           @dependency_name  = normalise(dependency_name)
           @old_requirement  = old_requirement
           @new_requirement  = new_requirement
           @new_hash_version = new_hash_version
+          @index_urls = index_urls
         end
 
         def updated_content
@@ -137,11 +140,28 @@ def hash_separator(requirement)
         end
 
         def package_hashes_for(name:, version:, algorithm:)
-          SharedHelpers.run_helper_subprocess(
-            command: "pyenv exec python3 #{NativeHelpers.python_helper_path}",
-            function: "get_dependency_hash",
-            args: [name, version, algorithm]
-          ).map { |h| "--hash=#{algorithm}:#{h['hash']}" }
+          index_urls = @index_urls || [nil]
+
+          index_urls.map do |index_url|
+            args = [name, version, algorithm]
+            args << index_url unless index_url.nil?
+
+            begin
+              result = SharedHelpers.run_helper_subprocess(
+                command: "pyenv exec python3 #{NativeHelpers.python_helper_path}",
+                function: "get_dependency_hash",
+                args: args
+              )
+            rescue SharedHelpers::HelperSubprocessFailed => e
+              raise unless e.message.include?("PackageNotFoundError")
+
+              next
+            end
+
+            return result.map { |h| "--hash=#{algorithm}:#{h['hash']}" } if result.is_a?(Array)
+          end
+
+          raise Dependabot::DependencyFileNotResolvable, "Unable to find hashes for package #{name}"
         end
 
         def original_dependency_declaration_string(old_req)

python/spec/dependabot/python/file_updater/pip_compile_file_updater_spec.rb

@@ -551,4 +551,85 @@
       end
     end
   end
+
+  describe "#package_hashes_for" do
+    let(:name) { "package_name" }
+    let(:version) { "1.0.0" }
+    let(:algorithm) { "sha256" }
+
+    context "when index_urls is not set" do
+      let(:updater) do
+        described_class.new(
+          dependencies: [],
+          dependency_files: [],
+          credentials: []
+        )
+      end
+
+      before do
+        allow(Dependabot::SharedHelpers).to receive(:run_helper_subprocess).and_return([{ "hash" => "123abc" }])
+      end
+
+      it "returns hash" do
+        result = updater.send(:package_hashes_for, name: name, version: version, algorithm: algorithm)
+        expect(result).to eq(["--hash=sha256:123abc"])
+      end
+    end
+
+    context "when multiple index_urls are set" do
+      let(:updater) do
+        described_class.new(
+          dependencies: [],
+          dependency_files: [],
+          credentials: [],
+          index_urls: [nil, "http://example.com"]
+        )
+      end
+
+      before do
+        allow(Dependabot::SharedHelpers).to receive(:run_helper_subprocess)
+          .and_return([{ "hash" => "123abc" }], [{ "hash" => "312cba" }])
+      end
+
+      it "returns returns two hashes" do
+        result = updater.send(:package_hashes_for, name: name, version: version, algorithm: algorithm)
+        expect(result).to eq(%w(--hash=sha256:123abc --hash=sha256:312cba))
+      end
+    end
+
+    context "when multiple index_urls are set but package does not exist in PyPI" do
+      let(:updater) do
+        described_class.new(
+          dependencies: [],
+          dependency_files: [],
+          credentials: [],
+          index_urls: [nil, "http://example.com"]
+        )
+      end
+
+      before do
+        allow(Dependabot::SharedHelpers).to receive(:run_helper_subprocess).with({
+          args: %w(package_name 1.0.0 sha256),
+          command: "pyenv exec python3 /opt/python/run.py",
+          function: "get_dependency_hash"
+        }).and_raise(
+          Dependabot::SharedHelpers::HelperSubprocessFailed.new(
+            message: "Error message", error_context: {}, error_class: "PackageNotFoundError"
+          )
+        )
+
+        allow(Dependabot::SharedHelpers).to receive(:run_helper_subprocess)
+          .with({
+            args: %w(package_name 1.0.0 sha256 http://example.com),
+            command: "pyenv exec python3 /opt/python/run.py",
+            function: "get_dependency_hash"
+          }).and_return([{ "hash" => "123abc" }])
+      end
+
+      it "returns returns two hashes" do
+        result = updater.send(:package_hashes_for, name: name, version: version, algorithm: algorithm)
+        expect(result).to eq(["--hash=sha256:123abc"])
+      end
+    end
+  end
 end

python/spec/dependabot/python/file_updater/requirement_file_updater_spec.rb

@@ -90,6 +90,50 @@
         its(:content) { is_expected.to include "psycopg2==2.8.1  # Comment!\n" }
       end
 
+      context "with an unknown package" do
+        let(:dependency) do
+          Dependabot::Dependency.new(
+            name: "some_unknown_package",
+            version: "24.3.3",
+            requirements: [{
+              file: "requirements.txt",
+              requirement: updated_requirement_string,
+              groups: [],
+              source: nil
+            }],
+            previous_requirements: [{
+              file: "requirements.txt",
+              requirement: previous_requirement_string,
+              groups: [],
+              source: nil
+            }],
+            package_manager: "pip"
+          )
+        end
+
+        let(:requirements_fixture_name) { "hashes_unknown_package.txt" }
+        let(:previous_requirement_string) { "==24.3.3" }
+        let(:updated_requirement_string) { "==24.4.0" }
+
+        context "when package is not in default index" do
+          it "raises an error" do
+            expect { updated_files }.to raise_error(Dependabot::DependencyFileNotResolvable)
+          end
+        end
+
+        context "when package is in default index" do
+          before do
+            allow(Dependabot::SharedHelpers).to receive(:run_helper_subprocess)
+              .and_return([{ "hash" => "1234567890abcdef" }])
+          end
+
+          its(:content) do
+            is_expected.to include "some_unknown_package==24.4.0"
+            is_expected.to include "--hash=sha256:1234567890abcdef"
+          end
+        end
+      end
+
       context "when there is a range" do
         context "with a space after the comma" do
           let(:requirements_fixture_name) { "version_between_bounds.txt" }

python/spec/dependabot/python/file_updater_spec.rb

@@ -391,5 +391,37 @@
         updated_files.each { |f| expect(f).to be_a(Dependabot::DependencyFile) }
       end
     end
+
+    describe "#pip_compile_index_urls" do
+      let(:instance) do
+        described_class.new(
+          dependencies: [],
+          dependency_files: [],
+          credentials: credentials
+        )
+      end
+
+      let(:credentials) { [double(replaces_base?: replaces_base)] }
+      let(:replaces_base) { false }
+
+      before do
+        allow_any_instance_of(Dependabot::Python::FileUpdater).to receive(:check_required_files).and_return(true)
+        allow(Dependabot::Python::AuthedUrlBuilder).to receive(:authed_url).and_return("authed_url")
+      end
+
+      context "when credentials replace base" do
+        let(:replaces_base) { true }
+
+        it "returns authed urls for these credentials" do
+          expect(instance.send(:pip_compile_index_urls)).to eq(["authed_url"])
+        end
+      end
+
+      context "when credentials do not replace base" do
+        it "returns nil and authed urls for all credentials" do
+          expect(instance.send(:pip_compile_index_urls)).to eq([nil, "authed_url"])
+        end
+      end
+    end
   end
 end