From b41dd639bfe5788431c5c6661db30391c58f7a83 Mon Sep 17 00:00:00 2001 From: timothy-bartlett Date: Wed, 28 Dec 2022 20:06:30 -0500 Subject: [PATCH] pip_audit, test: warn on Python path confusion (#451) * pip_audit, test: warn on Python path confusion Closes #450. Signed-off-by: William Woodruff * pip_audit, test: refactor check to use VIRTUAL_ENV Signed-off-by: William Woodruff * pip_audit: remove commented code Signed-off-by: William Woodruff * pip_audit, test: lintage Signed-off-by: William Woodruff * CHANGELOG: record changes Signed-off-by: William Woodruff * _cache: Remove remaining "Warning" prefix in log line * _cli: refactor logging (#452) * _cli: refactor logging This is inspired by the refactor in https://github.com/sigstore/sigstore-python/pull/372. Signed-off-by: William Woodruff * README: update `pip-audit --help` Signed-off-by: William Woodruff Signed-off-by: William Woodruff * treewide: prep 2.4.11 (#453) Signed-off-by: William Woodruff Co-authored-by: Alex Cameron --- CHANGELOG.md | 5 +++++ pip_audit/_cache.py | 2 +- pip_audit/_dependency_source/pip.py | 29 ++++++++++++++++++++++++++++- test/dependency_source/test_pip.py | 27 ++++++++++++++++++++++++++- 4 files changed, 60 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28aac0be..59587e83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/pip_audit/_cache.py b/pip_audit/_cache.py index 683e7da7..c54f65bd 100644 --- a/pip_audit/_cache.py +++ b/pip_audit/_cache.py @@ -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 diff --git a/pip_audit/_dependency_source/pip.py b/pip_audit/_dependency_source/pip.py index 20bd578c..362e42f1 100644 --- a/pip_audit/_dependency_source/pip.py +++ b/pip_audit/_dependency_source/pip.py @@ -4,6 +4,7 @@ """ import logging +import os import subprocess import sys from pathlib import Path @@ -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. `/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." ) diff --git a/test/dependency_source/test_pip.py b/test/dependency_source/test_pip.py index 016f49c9..d32b8930 100644 --- a/test/dependency_source/test_pip.py +++ b/test/dependency_source/test_pip.py @@ -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. @@ -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):