Crash on package @ git+... dependencies

[Zac-HD]

Bug description

Using package @ git+... dependencies crashes pip-audit with a traceback, when I'd expect it to output the usual report with those packages listed by name and skip-reason if unauditable.

Reproduction steps

# Some packages exist, and all is well
shed == 0.10.5

# Others *don't* exist or can't be fetched, that's reported nicely
this_might_exist_off_pypi == 0.0.1

# But if you have this awful kind of dep, you'll get a traceback!
hypothesis @ git+https://github.com/HypothesisWorks/hypothesis.git@bb6b55ad8d#subdirectory=hypothesis-python

pip-audit --no-deps -r requirements.txt

Platform information

• OS name and version: macOS Monterey
• pip-audit version (pip-audit -V): pip-audit 2.4.4
• Python version (python -V or python3 -V): Python 3.10.6
• pip version (pip -V or pip3 -V): pip 22.3

[di]

This doesn't crash for me:

$ cat requirements.txt
# Some packages exist, and all is well
shed == 0.10.5

# Others *don't* exist or can't be fetched, that's reported nicely
this_might_exist_off_pypi == 0.0.1

# But if you have this awful kind of dep, you'll get a traceback!
hypothesis @ git+https://github.com/HypothesisWorks/hypothesis.git@bb6b55ad8d#subdirectory=hypothesis-python

$ python -m pip_audit -r requirements.txt
No known vulnerabilities found
Name                      Skip Reason
------------------------- -----------------------------------------------------------------------------------------------------------------
this-might-exist-off-pypi Could not find project "this-might-exist-off-pypi" on any of the supplied index URLs: ['https://pypi.org/simple']

Can you provide the traceback you're getting?

[Zac-HD]

$ pip-audit --no-deps -r requirements.txt

WARNING:pip_audit._cli:--no-deps is supported, but users are encouraged to fully hash their pinned dependencies
WARNING:pip_audit._cli:Consider using a tool like `pip-compile`: https://pip-tools.readthedocs.io/en/latest/#using-hashes

Traceback (most recent call last):
  File "/opt/homebrew/Caskroom/miniforge/base/envs/py310/bin/pip-audit", line 8, in <module>
    sys.exit(audit())
  File "/opt/homebrew/Caskroom/miniforge/base/envs/py310/lib/python3.10/site-packages/pip_audit/_cli.py", line 432, in audit
    for (spec, vulns) in auditor.audit(source):
  File "/opt/homebrew/Caskroom/miniforge/base/envs/py310/lib/python3.10/site-packages/pip_audit/_audit.py", line 66, in audit
    for dep, vulns in self._service.query_all(specs):
  File "/opt/homebrew/Caskroom/miniforge/base/envs/py310/lib/python3.10/site-packages/pip_audit/_service/interface.py", line 150, in query_all
    for spec in specs:
  File "/opt/homebrew/Caskroom/miniforge/base/envs/py310/lib/python3.10/site-packages/pip_audit/_dependency_source/requirement.py", line 114, in collect
    for _, dep in self._collect_cached_deps(filename, reqs):
  File "/opt/homebrew/Caskroom/miniforge/base/envs/py310/lib/python3.10/site-packages/pip_audit/_dependency_source/requirement.py", line 312, in _collect_cached_deps
    for req, dep in self._collect_preresolved_deps(
  File "/opt/homebrew/Caskroom/miniforge/base/envs/py310/lib/python3.10/site-packages/pip_audit/_dependency_source/requirement.py", line 259, in _collect_preresolved_deps
    raise RequirementSourceError(
pip_audit._dependency_source.requirement.RequirementSourceError: requirement hypothesis is not pinned, URL requirements must be pinned with #egg=your_package_name==your_package_version: hypothesis@ git+https://github.com/HypothesisWorks/hypothesis.git@bb6b55ad8d#subdirectory=hypothesis-python from git+https://github.com/HypothesisWorks/hypothesis.git@bb6b55ad8d#subdirectory=hypothesis-python (from RequirementLine(line_number=8, line='hypothesis @ git+https://github.com/HypothesisWorks/hypothesis.git@bb6b55ad8d#subdirectory=hypothesis-python', filename=PosixPath('requirements.txt')))

(added newlines for clarity, otherwise unedited)

I suspect it's relevant that we're using a private index, and don't have https://pypi.org/simple in pip.conf 🤔

[woodruffw]

Thanks for the additional context @Zac-HD! I'll have time to attempt to repro this (and hopefully come up with a quick fix) in the coming days.

[woodruffw]

One small thing to note (that I don't think is causing the bug, but might be helpful):

I suspect it's relevant that we're using a private index, and don't have https://pypi.org/simple in pip.conf 🤔

If you're trying to avoid all network interaction with PyPI, then you'll probably want -s osv as well. By default, pip-audit uses PyPI's JSON API for vulnerability information; with -s osv it'll use the OSV's (also online) DB instead.

[woodruffw]

Did some digging here, and it looks like this error only happens with --no-deps. I think that's why @di couldn't repro it.

In particular, it looks like it happens because --no-deps requires every dependency to be fully pinned, but the URL-style requirement you're providing doesn't include a version specification.

In other words, this:

hypothesis @ git+https://github.com/HypothesisWorks/hypothesis.git@bb6b55ad8d#subdirectory=hypothesis-python

should be this:

hypothesis @ git+https://github.com/HypothesisWorks/hypothesis.git@bb6b55ad8d#egg=hypothesis==6.56.3&subdirectory=hypothesis-python

...however, that still fails with the same error, which makes me think it might be a bug in our requirements parser. Will continue looking.

[woodruffw]

Yep, this looks like a case of pip-requirements-parser not handling the egg fragment correctly:

{'extras': [],
 'global_options': [],
 'has_egg_fragment': True,
 'hash_options': [],
 'install_options': [],
 'invalid_options': {},
 'is_archive': False,
 'is_constraint': False,
 'is_editable': False,
 'is_local_path': True,
 'is_name_at_url': True,
 'is_pinned': False,
 'is_url': True,
 'is_vcs_url': True,
 'is_wheel': False,
 'link': 'git+https://github.com/HypothesisWorks/hypothesis.git@bb6b55ad8d#egg=hypothesis==6.56.3&subdirectory=hypothesis-python',
 'marker': None,
 'name': 'hypothesis',
 'requirement_line': {'line': 'hypothesis @ '
                              'git+https://github.com/HypothesisWorks/hypothesis.git@bb6b55ad8d#egg=hypothesis==6.56.3&subdirectory=hypothesis-python',
                      'line_number': 8},
 'specifier': []}

...or us using their API wrong. I'm going to file an upstream issue to get some clarification, but this is definitely a bug on our side at the minimum!

[woodruffw]

Going further, I'm not 100% sure our current guidance in that exception (#egg=package_name==package_version) is actually correct -- I can't find a reference anywhere in pip's documentation for that syntax, and I can't find it in pip's source either. The only supported #egg=... form seems to be without the package version suffix, meaning that VCS URL dependencies cannot be meaningfully pinned.

cc @tetsuo-cpp since you wrote this bit of the code: do you know where the #egg=package_name==package_version guidance came from?

[woodruffw]

Yep, confirmed: that syntax is definitely not official or supported: pypa/pip#5384

It seems like the only way to support "pinned" dependencies with VCS URLs is to use VCS-specific tagging (e.g. git tags or revs). So we need to update the error here to clarify that --no-deps can't be used with requirements inputs that contain VCS URLs.

[Zac-HD]

So we need to update the error here to clarify that --no-deps can't be used with requirements inputs that contain VCS URLs.

Could we instead skip VCS URLs in --no-deps mode, as is reported for not-on-PyPI packages more generally?

My motivation is that I want to check a requirements file produced by pip-compile for known vulnerabilities, without giving sdist package authors arbitary code execution on my machine (by having previously installed the package, or building a wheel locally; I know that producing the lockfile did this at some point). It's not much bash to create a vcs-free requirements file to audit, but having it built in would reduce the friction for marginal users.

[woodruffw]

Could we instead skip VCS URLs in --no-deps mode, as is reported for not-on-PyPI packages more generally?

My motivation is that I want to check a requirements file produced by pip-compile for known vulnerabilities, without giving sdist package authors arbitary code execution on my machine (by having previously installed the package, or building a wheel locally; I know that producing the lockfile did this at some point). It's not much bash to create a vcs-free requirements file to audit, but having it built in would reduce the friction for marginal users.

I'm of two minds on this. On one hand, we explicitly discourage pip-audit users from treating it as separate from pip install in terms of package trust -- there are use flows that happen to avoid arbitrary package script execution during audits, but that behavior isn't permanently guaranteed (although it's unlikely that it'll change).

On the other hand, I agree that skipping URL deps makes sense -- they're fundamentally outside of the PyPI ecosystem, so auditing them against PyPI makes very little sense. Except that they have sub-dependencies, and we do want to audit those...

Here's what I'm currently thinking: we could add another option, something like --skip-urls (subject to a better name), which would work both with and without --no-deps. This would allow users to explicitly opt into this skipping behavior while preserving the current default behavior.

cc @di for thoughts.

[woodruffw]

Thought about this some more: we currently skip deps in every mode if they aren't available on PyPI and URL dependencies are trivially not on PyPI, so it makes sense to skip them as well (and allow users to fail with --strict). So I think we should make this the default behavior, remove the error case, and avoid adding a new option.

[woodruffw]

I just confirmed that pip install --no-deps -r requirements.txt does install a VCS dependency but skips its subdependencies, so #382 (comment) is probably the closest appropriate behavior here. I'll adjust the PR.

[tetsuo-cpp]

cc @tetsuo-cpp since you wrote this bit of the code: do you know where the #egg=package_name==package_version guidance came from?

Not sure whether this is relevant anymore, but I'll reply anyway. Both pip-api and pip-requirements-parser parse the version out of the egg fragment so I just assumed it was part of the official syntax. It does appear to be some kind of informal convention, but I agree that it makes more sense to just skip them.

[woodruffw]

Not sure whether this is relevant anymore, but I'll reply anyway. Both pip-api and pip-requirements-parser parse the version out of the egg fragment so I just assumed it was part of the official syntax. It does appear to be some kind of informal convention, but I agree that it makes more sense to just skip them.

Yeah, that's interesting -- pip install doesn't complain if I include the ==version specifier, but it also doesn't seem to respect it (I put in a definitely wrong version number, and it still installed the version actually present at that git rev). So this is probably an overly permissive parsing bug on pip's part (and both parsers' parts as well).

Edit: Opened pypa/pip#11567 to track that behavior.

[seankfh]

I'm bumping into this issue attempting to list a forked git repo as a dependency. I use pip-compile to get the dependency into requirements.txt as follows:

textract @ git+https://github.com/seankfh/textract.git@0c80ff5727061587442fc5a1886c668d53e8d16d
    # via -r requirements.in

And when pip-audit runs through pre-commit it outputs the following:

pip_audit._dependency_source.requirement.RequirementSourceError: requirement textract is not pinned, URL requirements must be pinned with #egg=your_package_name==your_package_version: textract@ git+https://github.com/seankfh/textract.git@0c80ff5727061587442fc5a1886c668d53e8d16d from git+https://github.com/seankfh/textract.git@0c80ff5727061587442fc5a1886c668d53e8d16d (from RequirementLine(line_number=219, line='textract @ git+https://github.com/seankfh/textract.git@0c80ff5727061587442fc5a1886c668d53e8d16d', filename=PosixPath('requirements.txt')))

I've always used --no-deps because pip-audit will hang on my Debian system without it. Any ideas?

[woodruffw]

Thanks for telling us @seankfh!

Would you mind opening a separate issue for that? The top-level issue here was a crash, not a behavioral consideration around how --no-deps interacts with URL requirements. A separate issue would help us better track and triage the behavior you're seeing, especially since there's been a few releases since we closed this.

(And please include any details about that hang you're seeing!)