pip_audit, test: warn on Python path confusion

[woodruffw]

Closes #450.

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

[woodruffw]

NB: This doesn't work quite as expected yet:

$ pip-audit
WARNING:pip_audit._dependency_source.pip:pip-audit will run pip against /Users/william/.pyenv/versions/3.11.0/bin/python3.11, but you have /Users/william/.pyenv/versions/3.11.0/bin/python in your PATH (usually indicating a virtual environment). 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 local Python interpreter.
Found 2 known vulnerabilities in 2 packages
Name       Version   ID                  Fix Versions
---------- --------- ------------------- ------------
certifi    2022.9.24 GHSA-43fp-rhv2-5gv8 2022.12.7
setuptools 65.5.0    GHSA-r9hx-vwmv-q579 65.5.1

(In this case it can't see that the two global versions are actually the same.)

[woodruffw]

I can induce the expected warning, however:

$ ~/.pyenv/versions/3.11.0/bin/pip-audit
WARNING:pip_audit._dependency_source.pip:pip-audit will run pip against /Users/william/.pyenv/versions/3.11.0/bin/python3.11, but you have /Users/william/devel/pip-audit/env/bin/python in your PATH (usually indicating a virtual environment). 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 local Python interpreter.
Found 2 known vulnerabilities in 2 packages
Name       Version   ID                  Fix Versions
---------- --------- ------------------- ------------
certifi    2022.9.24 GHSA-43fp-rhv2-5gv8 2022.12.7
setuptools 65.5.0    GHSA-r9hx-vwmv-q579 65.5.1

[woodruffw]

7f943b5 should be a better approach -- rather than checking whether sys.executable diverges from the $PATH Python, we check whether we're in a virtual environment but are executing with a sys.executable outside of that environment.

[woodruffw]

Example output, formatted for readability (global pip-audit, local virtual environment):

$ ~/.pyenv/versions/3.11.0/bin/pip-audit 
WARNING:pip_audit._dependency_source.pip:pip-audit will run pip against
/Users/william/.pyenv/versions/3.11.0/bin/python3.11, but you have a virtual environment 
loaded at /Users/william/devel/pip-audit/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 local Python interpreter.

Found 2 known vulnerabilities in 2 packages
Name       Version   ID                  Fix Versions
---------- --------- ------------------- ------------
certifi    2022.9.24 GHSA-43fp-rhv2-5gv8 2022.12.7
setuptools 65.5.0    GHSA-r9hx-vwmv-q579 65.5.1

[woodruffw]

Thinking about it some more: maybe we should just do what the user expects here (i.e. configure PIPAPI_PYTHON_LOCATION for them, based on VIRTUAL_ENV).

We could still emit a warning, but this would avoid users having to do their own manual workaround and would probably match ordinary user expectations (similar to how mypy or any other tool can be installed globally but used within a virtual environment).

[tetsuo-cpp]

Thinking about it some more: maybe we should just do what the user expects here (i.e. configure PIPAPI_PYTHON_LOCATION for them, based on VIRTUAL_ENV).

We could still emit a warning, but this would avoid users having to do their own manual workaround and would probably match ordinary user expectations (similar to how mypy or any other tool can be installed globally but used within a virtual environment).

Seems reasonable to me. If you find a file at ${VIRTUAL_ENV}/bin/python3, you can use that. I'll push a commit for that.

[tetsuo-cpp]

I thought about editing this to include the fact that we tried to look for a symlink in bin/ but wasn't sure how to phrase it as this is already getting complicated. I don't think it's all that important to communicate to the user since the corrective action doesn't change.

[tetsuo-cpp]

We could also maybe just fail here?

[woodruffw]

Yeah, I think just failing is probably appropriate -- the lack of a bin here suggests that the user intended the load a virtual environment (based on the env setting) but somehow ended up with a broken one, so we shouldn't try to guess at the correct behavior.

[woodruffw]

Hmm, there's another issue here: this check:

        if _PIP_VERSION < _MINIMUM_RELIABLE_PIP_VERSION:
            logger.warning(
                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."
            )

...doesn't actually do what we (or the user) expect, since _PIP_VERSION is just pip_api.PIP_VERSION and is therefore loaded eagerly on pip_api's import. So it doesn't see the updated PIPAPI_PYTHON_LOCATION.

[woodruffw]

Based on the above, I think we just want to go with the initial "warn" approach for now.

That'll allow us to get a bugfix out, and from there we can work on actually using the active virtual environment by default (since it'll require us to rethink how we use pip_api and possibly make some upstream changes).

[woodruffw]

For posterity, here's the tweaked approach based on @tetsuo-cpp's work (mostly tweaked to respect PIPAPI_PYTHON_LOCATION if the user has already overridden it):

        # 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.
        #
        # Rather than auditing the global environment (based on how `pip-audit` has been
        # installed), we detect the user's intent by checking for an active virtual
        # environment. If there's an active virtual environment and the user hasn't
        # already overriden `PIPAPI_PYTHON_LOCATION`, then we point `pip_api` at the
        # right Python path in that virtual environment.
        effective_python = os.getenv("PIPAPI_PYTHON_LOCATION")
        if effective_python is None:
            effective_python = sys.executable

            venv_prefix = os.getenv("VIRTUAL_ENV")
            if venv_prefix is not None and not effective_python.startswith(venv_prefix):
                venv_python = Path(venv_prefix) / "bin/python"

                if venv_python.exists():
                    os.environ["PIPAPI_PYTHON_LOCATION"] = str(venv_python)
                else:
                    raise PipSourceError(
                        f"pip-audit found a virtual environment at {venv_prefix}, but "
                        "couldn't find a Python interpreter in that virtual environment"
                    )

        if _PIP_VERSION < _MINIMUM_RELIABLE_PIP_VERSION:
            logger.warning(
                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."
            )

[tetsuo-cpp]

Based on the above, I think we just want to go with the initial "warn" approach for now.

Good idea. I completely missed the issue with PIP_VERSION. Let's just merge this for now.

[tetsuo-cpp]

LGTM!