Skip to content

Commit

Permalink
Reintroduce dependency optimization (#626)
Browse files Browse the repository at this point in the history
* Reintroduce preresolved dependencies optimization

* Fix lint

* Fix CHANGELOG

* _cli, requirement: Introduce `--disable-pip` flag

* README: Update help text

* requirement: Add back the `--require-hashes` flag in the dependency
resolution path

* test: Fix tests and fill in test coverage

* CHANGELOG: Add more detail to changelog

* Update pip_audit/_cli.py

---------

Co-authored-by: William Woodruff <william@trailofbits.com>
Co-authored-by: Alex Cameron <asc@tetsuo.sh>
Co-authored-by: William Woodruff <william@yossarian.net>
  • Loading branch information
4 people authored Jun 28, 2023
1 parent 2263bd1 commit d93c5c8
Show file tree
Hide file tree
Showing 7 changed files with 289 additions and 11 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ All versions prior to 0.0.9 are untracked.

## [Unreleased]

### Changed

* Added option to skip dependency resolution via `pip` with the `--disable-pip`
flag. This option can only be used with hashed requirements files or when the
`--no-deps` flag has been provided.
([#610](https://github.com/pypa/pip-audit/pull/610))

## [2.5.6]

### Fixed
Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ usage: pip-audit [-h] [-V] [-l] [-r REQUIREMENT] [-f FORMAT] [-s SERVICE] [-d]
[--path PATH] [-v] [--fix] [--require-hashes]
[--index-url INDEX_URL] [--extra-index-url URL]
[--skip-editable] [--no-deps] [-o FILE] [--ignore-vuln ID]
[--disable-pip]
[project_path]

audit the Python environment for dependencies with known vulnerabilities
Expand Down Expand Up @@ -206,6 +207,9 @@ optional arguments:
--ignore-vuln ID ignore a specific vulnerability by its vulnerability
ID; this option can be used multiple times (default:
[])
--disable-pip don't use `pip` for dependency resolution; this can
only be used with hashed requirements files or if the
`--no-deps` flag has been provided (default: False)
```
<!-- @end-pip-audit-help@ -->
Expand Down
13 changes: 13 additions & 0 deletions pip_audit/_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,13 @@ def _parser() -> argparse.ArgumentParser: # pragma: no cover
"this option can be used multiple times"
),
)
parser.add_argument(
"--disable-pip",
action="store_true",
help="don't use `pip` for dependency resolution; "
"this can only be used with hashed requirements files or if the `--no-deps` flag has been "
"provided",
)
return parser


Expand Down Expand Up @@ -380,11 +387,16 @@ def audit() -> None: # pragma: no cover
parser.error("The --extra-index-url flag can only be used with --requirement (-r)")
elif args.no_deps:
parser.error("The --no-deps flag can only be used with --requirement (-r)")
elif args.disable_pip:
parser.error("The --disable-pip flag can only be used with --requirement (-r)")

# Nudge users to consider alternate workflows.
if args.require_hashes and args.no_deps:
logger.warning("The --no-deps flag is redundant when used with --require-hashes")

if args.no_deps and args.disable_pip:
logger.warning("The --no-deps flag is redundant when used with --disable-pip")

if args.require_hashes and isinstance(service, OsvService):
logger.warning(
"The --require-hashes flag with --service osv only enforces hash presence NOT hash "
Expand Down Expand Up @@ -414,6 +426,7 @@ def audit() -> None: # pragma: no cover
req_files,
require_hashes=args.require_hashes,
no_deps=args.no_deps,
disable_pip=args.disable_pip,
skip_editable=args.skip_editable,
index_url=args.index_url,
extra_index_urls=args.extra_index_urls,
Expand Down
9 changes: 8 additions & 1 deletion pip_audit/_dependency_source/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@
Dependency source interfaces and implementations for `pip-audit`.
"""

from .interface import PYPI_URL, DependencyFixError, DependencySource, DependencySourceError
from .interface import (
PYPI_URL,
DependencyFixError,
DependencySource,
DependencySourceError,
InvalidRequirementSpecifier,
)
from .pip import PipSource, PipSourceError
from .pyproject import PyProjectSource
from .requirement import RequirementSource
Expand All @@ -12,6 +18,7 @@
"DependencyFixError",
"DependencySource",
"DependencySourceError",
"InvalidRequirementSpecifier",
"PipSource",
"PipSourceError",
"PyProjectSource",
Expand Down
9 changes: 9 additions & 0 deletions pip_audit/_dependency_source/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,12 @@ class DependencyFixError(Exception):
"""

pass


class InvalidRequirementSpecifier(DependencySourceError):
"""
A `DependencySourceError` specialized for the case of a non-PEP 440 requirements
specifier.
"""

pass
103 changes: 98 additions & 5 deletions pip_audit/_dependency_source/requirement.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,18 @@

from packaging.specifiers import SpecifierSet
from packaging.utils import canonicalize_name
from packaging.version import Version
from pip_requirements_parser import InstallRequirement, InvalidRequirementLine, RequirementsFile

from pip_audit._dependency_source import DependencyFixError, DependencySource, DependencySourceError
from pip_audit._dependency_source import (
DependencyFixError,
DependencySource,
DependencySourceError,
InvalidRequirementSpecifier,
)
from pip_audit._fix import ResolvedFixVersion
from pip_audit._service import Dependency
from pip_audit._service.interface import ResolvedDependency
from pip_audit._service.interface import ResolvedDependency, SkippedDependency
from pip_audit._state import AuditState
from pip_audit._virtual_env import VirtualEnv, VirtualEnvError

Expand All @@ -40,6 +46,7 @@ def __init__(
*,
require_hashes: bool = False,
no_deps: bool = False,
disable_pip: bool = False,
skip_editable: bool = False,
index_url: str | None = None,
extra_index_urls: list[str] = [],
Expand All @@ -53,10 +60,14 @@ def __init__(
`require_hashes` controls the hash policy: if `True`, dependency collection
will fail unless all requirements include hashes.
`no_deps` controls the dependency resolution policy: if `True`,
`disable_pip` controls the dependency resolution policy: if `True`,
dependency resolution is not performed and the inputs are checked
and treated as "frozen".
`no_deps` controls whether dependency resolution can be disabled even without
hashed requirements (which implies a fully resolved requirements file): if `True`,
`disable_pip` is allowed without a hashed requirements file.
`skip_editable` controls whether requirements marked as "editable" are skipped.
By default, editable requirements are not skipped.
Expand All @@ -69,6 +80,7 @@ def __init__(
self._filenames = filenames
self._require_hashes = require_hashes
self._no_deps = no_deps
self._disable_pip = disable_pip
self._skip_editable = skip_editable
self._index_url = index_url
self._extra_index_urls = extra_index_urls
Expand Down Expand Up @@ -123,9 +135,35 @@ def collect(self) -> Iterator[Dependency]:
t.unlink()

def _collect_from_files(self, filenames: list[Path]) -> 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._disable_pip:
if not self._no_deps and not require_hashes:
raise RequirementSourceError(
"the --disable-pip flag can only be used with a hashed requirements files or "
"if the --no-deps flag has been provided"
)
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:
Expand Down Expand Up @@ -246,6 +284,61 @@ 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`."""
Expand Down
Loading

0 comments on commit d93c5c8

Please sign in to comment.