diff --git a/CHANGELOG.md b/CHANGELOG.md index e9bcd0af..dbdb5698 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ All versions prior to 0.0.9 are untracked. ## [Unreleased] +### Changed + +* Further simplified `pip-audit`'s dependency resolution to remove inconsistent + behaviour when using hashed requirements or the `--no-deps` flag + ([#540](https://github.com/pypa/pip-audit/pull/540)) + ## [2.5.2] ### Fixed diff --git a/pip_audit/_dependency_source/__init__.py b/pip_audit/_dependency_source/__init__.py index 3d19e722..3e8eb6fc 100644 --- a/pip_audit/_dependency_source/__init__.py +++ b/pip_audit/_dependency_source/__init__.py @@ -2,13 +2,7 @@ Dependency source interfaces and implementations for `pip-audit`. """ -from .interface import ( - PYPI_URL, - DependencyFixError, - DependencySource, - DependencySourceError, - InvalidRequirementSpecifier, -) +from .interface import PYPI_URL, DependencyFixError, DependencySource, DependencySourceError from .pip import PipSource, PipSourceError from .pyproject import PyProjectSource from .requirement import RequirementSource @@ -18,7 +12,6 @@ "DependencyFixError", "DependencySource", "DependencySourceError", - "InvalidRequirementSpecifier", "PipSource", "PipSourceError", "PyProjectSource", diff --git a/pip_audit/_dependency_source/interface.py b/pip_audit/_dependency_source/interface.py index f6504e78..8655ecc0 100644 --- a/pip_audit/_dependency_source/interface.py +++ b/pip_audit/_dependency_source/interface.py @@ -57,10 +57,3 @@ class DependencyFixError(Exception): """ pass - - -class InvalidRequirementSpecifier(DependencySourceError): - """ - A `DependencySourceError` specialized for the case of a non-PEP 440 requirements - specifier. - """ diff --git a/pip_audit/_dependency_source/requirement.py b/pip_audit/_dependency_source/requirement.py index ab62ef29..d32d1734 100644 --- a/pip_audit/_dependency_source/requirement.py +++ b/pip_audit/_dependency_source/requirement.py @@ -14,7 +14,6 @@ from typing import IO, Iterator from packaging.specifiers import SpecifierSet -from packaging.version import Version from pip_requirements_parser import InstallRequirement, InvalidRequirementLine, RequirementsFile from pip_audit._dependency_source import ( @@ -22,11 +21,10 @@ DependencyFixError, DependencySource, DependencySourceError, - InvalidRequirementSpecifier, ) from pip_audit._fix import ResolvedFixVersion from pip_audit._service import Dependency -from pip_audit._service.interface import ResolvedDependency, SkippedDependency +from pip_audit._service.interface import ResolvedDependency from pip_audit._state import AuditState from pip_audit._virtual_env import VirtualEnv, VirtualEnvError @@ -120,30 +118,11 @@ def collect(self) -> Iterator[Dependency]: os.unlink(t) def _collect_from_files(self, filenames: list[os.PathLike]) -> Iterator[Dependency]: - # Figure out whether we have a fully resolved set of dependencies. - reqs: list[InstallRequirement] = [] - require_hashes: bool = self._require_hashes - for filename in filenames: - rf = RequirementsFile.from_file(filename) - if len(rf.invalid_lines) > 0: - invalid = rf.invalid_lines[0] - raise InvalidRequirementSpecifier( - f"requirement file {filename} contains invalid specifier at " - f"line {invalid.line_number}: {invalid.error_message}" - ) - - # If one or more requirements have a hash, this implies `--require-hashes`. - require_hashes = require_hashes or any(req.hash_options for req in rf.requirements) - reqs.extend(rf.requirements) - - # If the user has supplied `--no-deps` or there are hashed requirements, we should assume - # that we have a fully resolved set of dependencies and we should waste time by invoking - # `pip`. - if self._no_deps or require_hashes: - yield from self._collect_preresolved_deps(iter(reqs), require_hashes) - return - ve_args = [] + if self._no_deps: + ve_args.append("--no-deps") + if self._require_hashes: + ve_args.append("--require-hashes") for filename in filenames: ve_args.extend(["-r", str(filename)]) @@ -259,61 +238,6 @@ def _recover_files(self, tmp_files: list[IO[str]]) -> None: logger.warning(f"encountered an exception during file recovery: {e}") continue - def _collect_preresolved_deps( - self, reqs: Iterator[InstallRequirement], require_hashes: bool - ) -> Iterator[Dependency]: - """ - Collect pre-resolved (pinned) dependencies. - """ - req_names: set[str] = set() - for req in reqs: - if not req.hash_options and require_hashes: - raise RequirementSourceError(f"requirement {req.dumps()} does not contain a hash") - if req.req is None: - # PEP 508-style URL requirements don't have a pre-declared version, even - # when hashed; the `#egg=name==version` syntax is non-standard and not supported - # by `pip` itself. - # - # In this case, we can't audit the dependency so we should signal to the - # caller that we're skipping it. - yield SkippedDependency( - name=req.requirement_line.line, - skip_reason="could not deduce package version from URL requirement", - ) - continue - if self._skip_editable and req.is_editable: - yield SkippedDependency(name=req.name, skip_reason="requirement marked as editable") - if req.marker is not None and not req.marker.evaluate(): - continue # pragma: no cover - - # This means we have a duplicate requirement for the same package - if req.name in req_names: - raise RequirementSourceError( - f"package {req.name} has duplicate requirements: {str(req)}" - ) - req_names.add(req.name) - - # NOTE: URL dependencies cannot be pinned, so skipping them - # makes sense (under the same principle of skipping dependencies - # that can't be found on PyPI). This is also consistent with - # what `pip --no-deps` does (installs the URL dependency, but - # not any subdependencies). - if req.is_url: - yield SkippedDependency( - name=req.name, - skip_reason="URL requirements cannot be pinned to a specific package version", - ) - elif not req.specifier: - raise RequirementSourceError(f"requirement {req.name} is not pinned: {str(req)}") - else: - pinned_specifier = PINNED_SPECIFIER_RE.match(str(req.specifier)) - if pinned_specifier is None: - raise RequirementSourceError( - f"requirement {req.name} is not pinned to an exact version: {str(req)}" - ) - - yield ResolvedDependency(req.name, Version(pinned_specifier.group("version"))) - class RequirementSourceError(DependencySourceError): """A requirements-parsing specific `DependencySourceError`.""" diff --git a/test/dependency_source/test_requirement.py b/test/dependency_source/test_requirement.py index dcd5a0f2..a991fcb8 100644 --- a/test/dependency_source/test_requirement.py +++ b/test/dependency_source/test_requirement.py @@ -16,7 +16,7 @@ requirement, ) from pip_audit._fix import ResolvedFixVersion -from pip_audit._service import ResolvedDependency, SkippedDependency +from pip_audit._service import ResolvedDependency from pip_audit._state import AuditState from pip_audit._virtual_env import VirtualEnv, VirtualEnvError @@ -125,6 +125,27 @@ def test_requirement_source_git(req_file): assert ResolvedDependency(name="uWSGI", version=Version("2.0.20")) in specs +@pytest.mark.online +def test_requirement_source_url(req_file): + source = _init_requirement( + [ + ( + req_file(), + "https://github.com/pallets/flask/archive/refs/tags/2.0.1.tar.gz\n", + ) + ], + ) + + specs = list(source.collect()) + assert ( + ResolvedDependency( + name="Flask", + version=Version("2.0.1"), + ) + in specs + ) + + @pytest.mark.online def test_requirement_source_multiple_indexes(req_file): source = _init_requirement( @@ -319,7 +340,35 @@ def mock_replace(*_args, **_kwargs): assert expected_req == f.read().strip() +@pytest.mark.online def test_requirement_source_require_hashes(req_file): + source = _init_requirement( + [ + ( + req_file(), + "wheel==0.38.1 " + "--hash=sha256:7a95f9a8dc0924ef318bd55b616112c70903192f524d120acc614f59547a9e1f\n" + "setuptools==67.0.0 " + "--hash=sha256:9d790961ba6219e9ff7d9557622d2fe136816a264dd01d5997cfc057d804853d", + ) + ], + require_hashes=True, + ) + + specs = list(source.collect()) + assert specs == [ + ResolvedDependency(name="wheel", version=Version("0.38.1")), + ResolvedDependency(name="setuptools", version=Version("67.0.0")), + ] + + +@pytest.mark.online +def test_requirement_source_require_hashes_not_fully_resolved(req_file): + # When using `--require-hashes`, `pip` requires a fully resolved list of requirements. If it + # finds a subdependency that is not listed in the requirements file, it will raise an error. + # + # In the case of Flask, this package has lots of subdependencies that aren't listed here so we + # expect an error. source = _init_requirement( [ ( @@ -331,12 +380,12 @@ def test_requirement_source_require_hashes(req_file): require_hashes=True, ) - specs = list(source.collect()) - assert specs == [ResolvedDependency("flask", Version("2.0.1"))] + with pytest.raises(DependencySourceError): + list(source.collect()) def test_requirement_source_require_hashes_missing(req_file): - source = _init_requirement([(req_file(), "flask==2.0.1")], require_hashes=True) + source = _init_requirement([(req_file(), "wheel==0.38.1")], require_hashes=True) # All requirements must be hashed when collecting with `require-hashes` with pytest.raises(DependencySourceError): @@ -348,9 +397,9 @@ def test_requirement_source_require_hashes_inferred(req_file): [ ( req_file(), - "flask==2.0.1 " - "--hash=sha256:a6209ca15eb63fc9385f38e452704113d679511d9574d09b2cf9183ae7d20dc9\n" - "requests==2.0", + "wheel==0.38.1 " + "--hash=sha256:7a95f9a8dc0924ef318bd55b616112c70903192f524d120acc614f59547a9e1f\n" + "setuptools==67.0.0", ) ] ) @@ -365,10 +414,10 @@ def test_requirement_source_require_hashes_unpinned(req_file): [ ( req_file(), - "flask==2.0.1 " - "--hash=sha256:a6209ca15eb63fc9385f38e452704113d679511d9574d09b2cf9183ae7d20dc9\n" - "requests>=1.0 " - "--hash=sha256:requests-hash", + "wheel==0.38.1 " + "--hash=sha256:7a95f9a8dc0924ef318bd55b616112c70903192f524d120acc614f59547a9e1f\n" + "setuptools<=67.0.0 " + "--hash=sha256:9d790961ba6219e9ff7d9557622d2fe136816a264dd01d5997cfc057d804853d", ) ] ) @@ -379,109 +428,30 @@ def test_requirement_source_require_hashes_unpinned(req_file): list(source.collect()) -def test_requirement_source_no_deps(req_file): - source = _init_requirement([(req_file(), "flask==2.0.1")], no_deps=True) - - specs = list(source.collect()) - assert specs == [ResolvedDependency("flask", Version("2.0.1"))] - - -def test_requirement_source_no_deps_unpinned(req_file): - source = _init_requirement([(req_file(), "flask\nrequests==1.0")], no_deps=True) - - # When dependency resolution is disabled, all requirements must be pinned. - with pytest.raises(DependencySourceError): - list(source.collect()) - - -def test_requirement_source_no_deps_not_exact_version(req_file): - source = _init_requirement([(req_file(), "flask==1.0\nrequests>=1.0")], no_deps=True) - - # When dependency resolution is disabled, all requirements must be pinned. - with pytest.raises(DependencySourceError): - list(source.collect()) - - -def test_requirement_source_no_deps_unpinned_url(req_file): - source = _init_requirement( - [ - ( - req_file(), - "https://github.com/pallets/flask/archive/refs/tags/2.0.1.tar.gz#egg=flask\n", - ) - ], - no_deps=True, - ) - - assert list(source.collect()) == [ - SkippedDependency( - name="flask", - skip_reason="URL requirements cannot be pinned to a specific package version", - ) - ] - - -def test_requirement_source_no_deps_editable_with_egg_fragment(req_file): - source = _init_requirement([(req_file(), "-e file:flask.py#egg=flask==2.0.1")], no_deps=True) - - specs = list(source.collect()) - assert ( - SkippedDependency( - name="flask", - skip_reason="URL requirements cannot be pinned to a specific package version", - ) - in specs - ) - - -def test_requirement_source_no_deps_editable_without_egg_fragment(req_file): - source = _init_requirement([(req_file(), "-e file:flask.py")], no_deps=True) - - specs = list(source.collect()) - assert ( - SkippedDependency( - name="-e file:flask.py", - skip_reason="could not deduce package version from URL requirement", - ) - in specs - ) - - -def test_requirement_source_no_deps_non_editable_without_egg_fragment(req_file): +def test_requirement_source_require_hashes_incorrect_hash(req_file): source = _init_requirement( [ ( req_file(), - "git+https://github.com/unbit/uwsgi.git@1bb9ad77c6d2d310c2d6d1d9ad62de61f725b824", + "wheel==0.38.1 " + "--hash=sha256:7a95f9a8dc0924ef318bd55b616112c70903192f524d120acc614f59547a9e1f\n" + "setuptools<=67.0.0 " + "--hash=sha256:setuptools-hash", ) - ], - no_deps=True, + ] ) - specs = list(source.collect()) - assert ( - SkippedDependency( - name="git+https://github.com/unbit/uwsgi.git@1bb9ad77c6d2d310c2d6d1d9ad62de61f725b824", - skip_reason="could not deduce package version from URL requirement", - ) - in specs - ) + # The `setuptools` hash is incorrect. + with pytest.raises(DependencySourceError): + list(source.collect()) -def test_requirement_source_no_deps_editable_skip(req_file): - source = _init_requirement( - [(req_file(), "-e file:flask.py#egg=flask==2.0.1")], no_deps=True, skip_editable=True - ) +@pytest.mark.online +def test_requirement_source_no_deps(req_file): + source = _init_requirement([(req_file(), "flask==2.0.1")], no_deps=True) specs = list(source.collect()) - assert SkippedDependency(name="flask", skip_reason="requirement marked as editable") in specs - - -def test_requirement_source_no_deps_duplicate_dependencies(req_file): - source = _init_requirement([(req_file(), "flask==1.0\nflask==1.0")], no_deps=True) - - with pytest.raises(DependencySourceError): - list(source.collect()) + assert specs == [ResolvedDependency("Flask", Version("2.0.1"))] def test_requirement_source_no_double_open(monkeypatch, req_file): @@ -550,7 +520,7 @@ def test_requirement_source_fix_explicit_subdep(monkeypatch, req_file): assert len(logger.warning.calls) == 1 -def test_requirement_source_fix_explicit_subdep_multiple_reqs(monkeypatch, req_file): +def test_requirement_source_fix_explicit_subdep_multiple_reqs(req_file): # Recreate the vulnerable subdependency case. source = _init_requirement([(req_file(), "flask==2.0.1")]) flask_deps = source.collect()