Skip to content

Commit

Permalink
pip_audit, test: warn on Python path confusion (#451)
Browse files Browse the repository at this point in the history
* pip_audit, test: warn on Python path confusion

Closes #450.

Signed-off-by: William Woodruff <william@trailofbits.com>

* pip_audit, test: refactor check to use VIRTUAL_ENV

Signed-off-by: William Woodruff <william@trailofbits.com>

* pip_audit: remove commented code

Signed-off-by: William Woodruff <william@trailofbits.com>

* pip_audit, test: lintage

Signed-off-by: William Woodruff <william@trailofbits.com>

* CHANGELOG: record changes

Signed-off-by: William Woodruff <william@trailofbits.com>

* _cache: Remove remaining "Warning" prefix in log line

* _cli: refactor logging (#452)

* _cli: refactor logging

This is inspired by the refactor in
sigstore/sigstore-python#372.

Signed-off-by: William Woodruff <william@trailofbits.com>

* README: update `pip-audit --help`

Signed-off-by: William Woodruff <william@trailofbits.com>

Signed-off-by: William Woodruff <william@trailofbits.com>

* treewide: prep 2.4.11 (#453)

Signed-off-by: William Woodruff <william@trailofbits.com>
Co-authored-by: Alex Cameron <asc@tetsuo.sh>
  • Loading branch information
timothy-bartlett and tetsuo-cpp authored Dec 29, 2022
1 parent 2a31bb4 commit b41dd63
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 3 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ All versions prior to 0.0.9 are untracked.
specifier for its `requires-python` version
([#447](https://github.com/pypa/pip-audit/pull/447))

* Users are now warned if a `pip-audit` invocation is ambiguous, e.g.
if they've installed `pip-audit` globally but are asking for an audit
of a loaded virtual environment
([#451](https://github.com/pypa/pip-audit/pull/451))

## [2.4.10]

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion pip_audit/_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def _get_cache_dir(custom_cache_dir: Path | None, *, use_pip: bool = True) -> Pa
return pip_cache_dir
else:
logger.warning(
f"Warning: pip {_PIP_VERSION} doesn't support the `cache dir` subcommand, "
f"pip {_PIP_VERSION} doesn't support the `cache dir` subcommand, "
f"using {_PIP_AUDIT_INTERNAL_CACHE} instead"
)
return _PIP_AUDIT_INTERNAL_CACHE
Expand Down
29 changes: 28 additions & 1 deletion pip_audit/_dependency_source/pip.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"""

import logging
import os
import subprocess
import sys
from pathlib import Path
Expand Down Expand Up @@ -64,9 +65,35 @@ def __init__(
self._skip_editable = skip_editable
self.state = state

# NOTE: By default `pip_api` invokes `pip` through `sys.executable`, like so:
#
# {sys.executable} -m pip [args ...]
#
# This is the right decision 99% of the time, but it can result in unintuitive audits
# for users who have installed `pip-audit` globally but are trying to audit
# a loaded virtual environment, since `pip-audit`'s `sys.executable` will be the global
# Python and not the virtual environment's Python.
#
# To check for this, we check whether the Python that `pip_api` plans to use
# matches the active virtual environment's prefix. We do this instead of comparing
# against the $PATH-prioritized Python because that might be the same "effective"
# Python but with a different symlink (e.g. `<path>/python{,3,3.7}`). We *could*
# handle that case by resolving the symlinks, but that would then piece the
# virtual environment that we're attempting to detect.
effective_python = os.environ.get("PIPAPI_PYTHON_LOCATION", sys.executable)
venv_prefix = os.getenv("VIRTUAL_ENV")
if venv_prefix is not None and not effective_python.startswith(venv_prefix):
logger.warning(
f"pip-audit will run pip against {effective_python}, but you have "
f"a virtual environment loaded at {venv_prefix}. This may result in "
"unintuitive audits, since your local environment will not be audited. "
"You can forcefully override this behavior by setting PIPAPI_PYTHON_LOCATION "
"to the location of your virtual environment's Python interpreter."
)

if _PIP_VERSION < _MINIMUM_RELIABLE_PIP_VERSION:
logger.warning(
f"Warning: pip {_PIP_VERSION} is very old, and may not provide reliable "
f"pip {_PIP_VERSION} is very old, and may not provide reliable "
"dependency information! You are STRONGLY encouraged to upgrade to a "
"newer version of pip."
)
Expand Down
27 changes: 26 additions & 1 deletion test/dependency_source/test_pip.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,26 @@ def test_pip_source():
assert pytest_spec in specs


def test_pip_source_warns_about_confused_python(monkeypatch):
monkeypatch.setenv("PIPAPI_PYTHON_LOCATION", "/definitely/fake/path/python")
monkeypatch.setenv("VIRTUAL_ENV", "/definitely/fake/env")
logger = pretend.stub(warning=pretend.call_recorder(lambda s: None))
monkeypatch.setattr(pip, "logger", logger)

pip.PipSource()

assert logger.warning.calls == [
pretend.call(
"pip-audit will run pip against /definitely/fake/path/python, but you have "
"a virtual environment loaded at /definitely/fake/env. "
"This may result in unintuitive audits, since your local environment will not "
"be audited. You can forcefully override this behavior by setting "
"PIPAPI_PYTHON_LOCATION to the location of your virtual environment's Python "
"interpreter."
)
]


def test_pip_source_warns_about_old_pip(monkeypatch):
# Rather than hack around with virtualenvs and install a very old pip,
# simply lie about how old ours is.
Expand All @@ -34,7 +54,12 @@ def test_pip_source_warns_about_old_pip(monkeypatch):
monkeypatch.setattr(pip, "logger", logger)

pip.PipSource()
assert len(logger.warning.calls) == 1
assert logger.warning.calls == [
pretend.call(
"pip 1.0.0 is very old, and may not provide reliable dependency information! "
"You are STRONGLY encouraged to upgrade to a newer version of pip."
)
]


def test_pip_source_pip_api_failure(monkeypatch):
Expand Down

0 comments on commit b41dd63

Please sign in to comment.