From 0ffc54dca3dd0f64eb9498a37908ae756294da7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sat, 8 Apr 2023 17:04:37 +0200 Subject: [PATCH 1/3] Add tests about link hashes validation --- tests/functional/test_install.py | 97 ++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index 72c72f35c5d..b749372c13a 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -1,3 +1,4 @@ +import hashlib import os import re import ssl @@ -13,6 +14,7 @@ from pip._internal.cli.status_codes import ERROR, SUCCESS from pip._internal.models.index import PyPI, TestPyPI from pip._internal.utils.misc import rmtree +from pip._internal.utils.urls import path_to_url from tests.conftest import CertFactory from tests.lib import ( PipTestEnvironment, @@ -616,6 +618,101 @@ def test_hashed_install_failure(script: PipTestEnvironment, tmpdir: Path) -> Non assert len(result.files_created) == 0 +def test_link_hash_pass_require_hashes( + script: PipTestEnvironment, shared_data: TestData +) -> None: + """Test that a good hash in user provided direct URL is + considered valid for --require-hashes.""" + url = path_to_url(str(shared_data.packages.joinpath("simple-1.0.tar.gz"))) + url = ( + f"{url}#sha256=" + "393043e672415891885c9a2a0929b1af95fb866d6ca016b42d2e6ce53619b653" + ) + script.pip_install_local("--no-deps", "--require-hashes", url) + + +def test_bad_link_hash_install_failure( + script: PipTestEnvironment, shared_data: TestData +) -> None: + """Test that wrong hash in direct URL stop installation.""" + url = path_to_url(str(shared_data.packages.joinpath("simple-1.0.tar.gz"))) + url = f"{url}#sha256=invalidhash" + result = script.pip_install_local("--no-deps", url, expect_error=True) + assert "THESE PACKAGES DO NOT MATCH THE HASHES" in result.stderr + + +def test_link_hash_in_dep_fails_require_hashes( + script: PipTestEnvironment, tmp_path: Path, shared_data: TestData +) -> None: + """Test that a good hash in direct URL dependency is not considered + for --require-hashes.""" + # Create a project named pkga that depends on the simple-1.0.tar.gz with a direct + # URL including a hash. + simple_url = path_to_url(str(shared_data.packages.joinpath("simple-1.0.tar.gz"))) + simple_url_with_hash = ( + f"{simple_url}#sha256=" + "393043e672415891885c9a2a0929b1af95fb866d6ca016b42d2e6ce53619b653" + ) + project_path = tmp_path / "pkga" + project_path.mkdir() + project_path.joinpath("pyproject.toml").write_text( + textwrap.dedent( + f"""\ + [project] + name = "pkga" + version = "1.0" + dependencies = ["simple @ {simple_url_with_hash}"] + """ + ) + ) + # Build a wheel for pkga and compute its hash. + wheelhouse = tmp_path / "wheehouse" + wheelhouse.mkdir() + script.pip("wheel", "--no-deps", "-w", wheelhouse, project_path) + digest = hashlib.sha256( + wheelhouse.joinpath("pkga-1.0-py3-none-any.whl").read_bytes() + ).hexdigest() + # Install pkga from a requirements file with hash, using --require-hashes. + # This should fail because we have not provided a hash for the 'simple' dependency. + with requirements_file(f"pkga==1.0 --hash sha256:{digest}", tmp_path) as reqs_file: + result = script.pip( + "install", + "--no-build-isolation", + "--require-hashes", + "--no-index", + "-f", + wheelhouse, + "-r", + reqs_file, + expect_error=True, + ) + assert "Hashes are required in --require-hashes mode" in result.stderr + + +def test_bad_link_hash_in_dep_install_failure( + script: PipTestEnvironment, tmp_path: Path, shared_data: TestData +) -> None: + """Test that wrong hash in direct URL dependency stops installation.""" + url = path_to_url(str(shared_data.packages.joinpath("simple-1.0.tar.gz"))) + url = f"{url}#sha256=invalidhash" + project_path = tmp_path / "pkga" + project_path.mkdir() + project_path.joinpath("pyproject.toml").write_text( + textwrap.dedent( + f"""\ + [project] + name = "pkga" + version = "1.0" + dependencies = ["simple @ {url}"] + """ + ) + ) + result = script.pip_install_local( + "--no-build-isolation", project_path, expect_error=True + ) + assert "THESE PACKAGES DO NOT MATCH THE HASHES" in result.stderr, result.stderr + + def assert_re_match(pattern: str, text: str) -> None: assert re.search(pattern, text), f"Could not find {pattern!r} in {text!r}" From f5f0302516e4adc5b8541832da803784d44b0a0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sat, 8 Apr 2023 18:57:37 +0200 Subject: [PATCH 2/3] Fix --require-hashes trusting link hashes When a direct URL with hash is provided as a dependency, --require-hash incorrectly considered the link hash as trusted. --- news/11938.bugfix.rst | 3 +++ src/pip/_internal/req/req_install.py | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 news/11938.bugfix.rst diff --git a/news/11938.bugfix.rst b/news/11938.bugfix.rst new file mode 100644 index 00000000000..b299f8e4ff5 --- /dev/null +++ b/news/11938.bugfix.rst @@ -0,0 +1,3 @@ +When package A depends on package B provided as a direct URL dependency including a hash +embedded in the link, the ``--require-hashes`` option did not warn when user supplied hashes +were missing for package B. diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index baa6716381c..e2353f0321a 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -287,7 +287,12 @@ def hashes(self, trust_internet: bool = True) -> Hashes: """ good_hashes = self.hash_options.copy() - link = self.link if trust_internet else self.original_link + if trust_internet: + link = self.link + elif self.original_link and self.user_supplied: + link = self.original_link + else: + link = None if link and link.hash: good_hashes.setdefault(link.hash_name, []).append(link.hash) return Hashes(good_hashes) From 453a5a7e0738c9c0453a3a23db4ef74e9e4e41d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sat, 8 Apr 2023 19:16:50 +0200 Subject: [PATCH 3/3] Add test for combination of invalid link hash and good --hash --- tests/functional/test_install.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index b749372c13a..e50779688f1 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -634,13 +634,29 @@ def test_link_hash_pass_require_hashes( def test_bad_link_hash_install_failure( script: PipTestEnvironment, shared_data: TestData ) -> None: - """Test that wrong hash in direct URL stop installation.""" + """Test that wrong hash in direct URL stops installation.""" url = path_to_url(str(shared_data.packages.joinpath("simple-1.0.tar.gz"))) url = f"{url}#sha256=invalidhash" result = script.pip_install_local("--no-deps", url, expect_error=True) assert "THESE PACKAGES DO NOT MATCH THE HASHES" in result.stderr +def test_bad_link_hash_good_user_hash_install_success( + script: PipTestEnvironment, shared_data: TestData, tmp_path: Path +) -> None: + """Test that wrong hash in direct URL ignored when good --hash provided. + + This behaviour may be accidental? + """ + url = path_to_url(str(shared_data.packages.joinpath("simple-1.0.tar.gz"))) + url = f"{url}#sha256=invalidhash" + digest = "393043e672415891885c9a2a0929b1af95fb866d6ca016b42d2e6ce53619b653" + with requirements_file( + f"simple @ {url} --hash sha256:{digest}", tmp_path + ) as reqs_file: + script.pip_install_local("--no-deps", "--require-hashes", "-r", reqs_file) + + def test_link_hash_in_dep_fails_require_hashes( script: PipTestEnvironment, tmp_path: Path, shared_data: TestData ) -> None: