From 061d3e9c5d72b1e928362d4664694d87dc7f383b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sun, 16 Apr 2023 13:23:17 +0200 Subject: [PATCH 1/6] Don't set a default value for ireq.hashes() function The default is potentially dangerous. --- src/pip/_internal/req/req_install.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index d01b24a9189..4a2242df62d 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -275,7 +275,7 @@ def has_hash_options(self) -> bool: """ return bool(self.hash_options) - def hashes(self, trust_internet: bool = True) -> Hashes: + def hashes(self, *, trust_internet: bool) -> Hashes: """Return a hash-comparer that considers my option- and URL-based hashes to be known-good. From 88ac144288a262220c9bbc73b753f6a816331c3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sun, 16 Apr 2023 13:24:02 +0200 Subject: [PATCH 2/6] Don't use deprecated git protocol in test --- tests/unit/test_req.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index c9742812be4..cb60f705d82 100644 --- a/tests/unit/test_req.py +++ b/tests/unit/test_req.py @@ -206,7 +206,7 @@ def test_unsupported_hashes(self, data: TestData) -> None: reqset = RequirementSet() reqset.add_unnamed_requirement( get_processed_req_from_line( - "git+git://github.com/pypa/pip-test-package --hash=sha256:123", + "git+https://github.com/pypa/pip-test-package --hash=sha256:123", lineno=1, ) ) @@ -229,7 +229,7 @@ def test_unsupported_hashes(self, data: TestData) -> None: match=( r"Can't verify hashes for these requirements because we don't " r"have a way to hash version control repositories:\n" - r" git\+git://github\.com/pypa/pip-test-package \(from -r " + r" git\+https://github\.com/pypa/pip-test-package \(from -r " r"file \(line 1\)\)\n" r"Can't verify hashes for these file:// requirements because " r"they point to directories:\n" From a1cd61b4fc28517dd6fac67ecbb4682cda6c815d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sat, 25 Mar 2023 15:54:16 +0100 Subject: [PATCH 3/6] Support immutable VCS references in hash-checking mode. --- docs/html/topics/secure-installs.md | 6 +++ news/6469.feature.rst | 1 + src/pip/_internal/exceptions.py | 26 ++++++++++ src/pip/_internal/operations/prepare.py | 68 +++++++++++++++++-------- src/pip/_internal/utils/hashes.py | 10 ++++ 5 files changed, 90 insertions(+), 21 deletions(-) create mode 100644 news/6469.feature.rst diff --git a/docs/html/topics/secure-installs.md b/docs/html/topics/secure-installs.md index f012842b2ac..8c74d39e87a 100644 --- a/docs/html/topics/secure-installs.md +++ b/docs/html/topics/secure-installs.md @@ -43,6 +43,12 @@ FooProject == 1.2 \ This prevents a surprising hash mismatch upon the release of a new version that matches the requirement specifier. +```{versionadded} 23.2 +VCS URLs that reference an commit hash are now supported in hash checking mode, +as pip considers the VCS provides the required source immutability guarantees. This is only +supported with `git` URLs at the time of writing. +``` + ### Forcing Hash-checking mode It is possible to force the hash checking mode to be enabled, by passing `--require-hashes` command-line option. diff --git a/news/6469.feature.rst b/news/6469.feature.rst new file mode 100644 index 00000000000..1a168e94e88 --- /dev/null +++ b/news/6469.feature.rst @@ -0,0 +1 @@ +Validate VCS urls in hash-checking mode using their commit hashes. diff --git a/src/pip/_internal/exceptions.py b/src/pip/_internal/exceptions.py index 7d92ba69983..cf86919d9ec 100644 --- a/src/pip/_internal/exceptions.py +++ b/src/pip/_internal/exceptions.py @@ -501,6 +501,32 @@ class VcsHashUnsupported(HashError): ) +class CacheEntryTypeHashNotSupported(HashError): + """A wheel cache entry was build from a URL that does not support hash checking.""" + + order = 0 + head = ( + "Can't verify hashes for these cached requirements because they are " + "from a URL that does not support hash checking:" + ) + + +class DependencyVcsHashNotSupported(HashError): + order = 0 + head = ( + "Can't verify hashes for these VCS requirements because they are " + "not user supplied, so we don't assume their VCS ref is trusted:" + ) + + +class MutableVcsRefHashNotSupported(HashError): + order = 0 + head = ( + "Can't verify hashes for these VCS requirements because their ref " + "is not immutable:" + ) + + class DirectoryUrlHashUnsupported(HashError): """A hash was provided for a version-control-system-based requirement, but we don't have a method for hashing those.""" diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 2273315234d..f929cf335c7 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -15,18 +15,21 @@ from pip._internal.distributions import make_distribution_for_install_requirement from pip._internal.distributions.installed import InstalledDistribution from pip._internal.exceptions import ( + CacheEntryTypeHashNotSupported, + DependencyVcsHashNotSupported, DirectoryUrlHashUnsupported, HashMismatch, HashUnpinned, InstallationError, MetadataInconsistent, + MutableVcsRefHashNotSupported, NetworkConnectionError, PreviousBuildDirError, VcsHashUnsupported, ) from pip._internal.index.package_finder import PackageFinder from pip._internal.metadata import BaseDistribution, get_metadata_distribution -from pip._internal.models.direct_url import ArchiveInfo +from pip._internal.models.direct_url import ArchiveInfo, VcsInfo from pip._internal.models.link import Link from pip._internal.models.wheel import Wheel from pip._internal.network.download import BatchDownloader, Downloader @@ -41,7 +44,7 @@ direct_url_for_editable, direct_url_from_link, ) -from pip._internal.utils.hashes import Hashes, MissingHashes +from pip._internal.utils.hashes import Hashes, MissingHashes, VcsHashes from pip._internal.utils.logging import indent_log from pip._internal.utils.misc import ( display_path, @@ -72,10 +75,14 @@ def _get_prepared_distribution( return abstract_dist.get_metadata_distribution() -def unpack_vcs_link(link: Link, location: str, verbosity: int) -> None: +def unpack_vcs_link( + link: Link, location: str, verbosity: int, hashes: Optional[Hashes] = None +) -> None: vcs_backend = vcs.get_backend_for_scheme(link.scheme) assert vcs_backend is not None vcs_backend.unpack(location, url=hide_url(link.url), verbosity=verbosity) + if hashes and not vcs_backend.is_immutable_rev_checkout(link.url, location): + raise MutableVcsRefHashNotSupported() class File: @@ -152,7 +159,7 @@ def unpack_url( """ # non-editable vcs urls if link.is_vcs: - unpack_vcs_link(link, location, verbosity=verbosity) + unpack_vcs_link(link, location, verbosity=verbosity, hashes=hashes) return None assert not link.is_existing_dir() @@ -335,6 +342,14 @@ def _get_linked_req_hashes(self, req: InstallRequirement) -> Hashes: # and raise some more informative errors than otherwise. # (For example, we can raise VcsHashUnsupported for a VCS URL # rather than HashMissing.) + + # Check that --hash is not used with VCS and local directories direct URLs. + if req.original_link: + if req.original_link.is_vcs and req.hashes(trust_internet=False): + raise VcsHashUnsupported() + if req.original_link.is_existing_dir() and req.hashes(trust_internet=False): + raise DirectoryUrlHashUnsupported() + if not self.require_hashes: return req.hashes(trust_internet=True) @@ -343,7 +358,9 @@ def _get_linked_req_hashes(self, req: InstallRequirement) -> Hashes: # report less-useful error messages for unhashable # requirements, complaining that there's no hash provided. if req.link.is_vcs: - raise VcsHashUnsupported() + if not req.user_supplied: + raise DependencyVcsHashNotSupported() + return VcsHashes() if req.link.is_existing_dir(): raise DirectoryUrlHashUnsupported() @@ -559,24 +576,33 @@ def _prepare_linked_requirement( assert link.is_file # We need to verify hashes, and we have found the requirement in the cache # of locally built wheels. - if ( - isinstance(req.download_info.info, ArchiveInfo) - and req.download_info.info.hashes - and hashes.has_one_of(req.download_info.info.hashes) - ): - # At this point we know the requirement was built from a hashable source - # artifact, and we verified that the cache entry's hash of the original - # artifact matches one of the hashes we expect. We don't verify hashes - # against the cached wheel, because the wheel is not the original. + if isinstance(req.download_info.info, ArchiveInfo): + if req.download_info.info.hashes and hashes.has_one_of( + req.download_info.info.hashes + ): + # At this point we know the requirement was built from a hashable + # source artifact, and we verified that the cache entry's hash of + # the original artifact matches one of the hashes we expect. We + # don't verify hashes against the cached wheel, because the wheel is + # not the original. + hashes = None + else: + logger.warning( + "The hashes of the source archive found in cache entry " + "don't match, ignoring cached built wheel " + "and re-downloading source." + ) + req.link = req.cached_wheel_source_link + link = req.link + elif isinstance(req.download_info.info, VcsInfo): + if not req.user_supplied: + raise DependencyVcsHashNotSupported() + # Don't verify hashes against the cached wheel: if it is in cache, + # it means it was built from a URL referencing an immutable commit + # hash. hashes = None else: - logger.warning( - "The hashes of the source archive found in cache entry " - "don't match, ignoring cached built wheel " - "and re-downloading source." - ) - req.link = req.cached_wheel_source_link - link = req.link + raise CacheEntryTypeHashNotSupported() self._ensure_link_req_src_dir(req, parallel_builds) diff --git a/src/pip/_internal/utils/hashes.py b/src/pip/_internal/utils/hashes.py index 843cffc6b3d..c29caa7999e 100644 --- a/src/pip/_internal/utils/hashes.py +++ b/src/pip/_internal/utils/hashes.py @@ -149,3 +149,13 @@ def __init__(self) -> None: def _raise(self, gots: Dict[str, "_Hash"]) -> "NoReturn": raise HashMissing(gots[FAVORITE_HASH].hexdigest()) + + +class VcsHashes(MissingHashes): + """A workalike for Hashes used for VCS references + + It never matches, and is used as a sentinel to indicate that we should + check the VCS reference is an immutable commit reference. + """ + + pass From 7aab44cc06c6bd1920c48c3de279e34ceae7df12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sun, 1 Oct 2023 16:50:43 +0200 Subject: [PATCH 4/6] Update src/pip/_internal/operations/prepare.py Co-authored-by: Ed Morley <501702+edmorley@users.noreply.github.com> --- src/pip/_internal/operations/prepare.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index f929cf335c7..df7ad628b0c 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -343,7 +343,7 @@ def _get_linked_req_hashes(self, req: InstallRequirement) -> Hashes: # (For example, we can raise VcsHashUnsupported for a VCS URL # rather than HashMissing.) - # Check that --hash is not used with VCS and local directories direct URLs. + # Check that --hash is not used with VCS and local directory direct URLs. if req.original_link: if req.original_link.is_vcs and req.hashes(trust_internet=False): raise VcsHashUnsupported() From c59af71f40a0bba60b104420e7ddd14ff18235e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sun, 1 Oct 2023 16:50:51 +0200 Subject: [PATCH 5/6] Update docs/html/topics/secure-installs.md Co-authored-by: Ed Morley <501702+edmorley@users.noreply.github.com> --- docs/html/topics/secure-installs.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/html/topics/secure-installs.md b/docs/html/topics/secure-installs.md index 8c74d39e87a..1ead18be9f7 100644 --- a/docs/html/topics/secure-installs.md +++ b/docs/html/topics/secure-installs.md @@ -44,9 +44,9 @@ FooProject == 1.2 \ This prevents a surprising hash mismatch upon the release of a new version that matches the requirement specifier. ```{versionadded} 23.2 -VCS URLs that reference an commit hash are now supported in hash checking mode, -as pip considers the VCS provides the required source immutability guarantees. This is only -supported with `git` URLs at the time of writing. +VCS URLs that reference a commit hash are now supported in hash checking mode, +since pip now trusts that the VCS provides the required source immutability guarantees. +This is currently only supported with `git` URLs. ``` ### Forcing Hash-checking mode From 80034904400b692becd23c277b9a7d71a77e3df8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sun, 1 Oct 2023 16:51:00 +0200 Subject: [PATCH 6/6] Update src/pip/_internal/exceptions.py Co-authored-by: Ed Morley <501702+edmorley@users.noreply.github.com> --- src/pip/_internal/exceptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pip/_internal/exceptions.py b/src/pip/_internal/exceptions.py index cf86919d9ec..725b9ee85d2 100644 --- a/src/pip/_internal/exceptions.py +++ b/src/pip/_internal/exceptions.py @@ -502,7 +502,7 @@ class VcsHashUnsupported(HashError): class CacheEntryTypeHashNotSupported(HashError): - """A wheel cache entry was build from a URL that does not support hash checking.""" + """A wheel cache entry was built from a URL that does not support hash checking.""" order = 0 head = (