pip-audit icon indicating copy to clipboard operation
pip-audit copied to clipboard

Report yanked versions of packages

Open sebastianw opened this issue 2 years ago • 7 comments

Is your feature request related to a problem? Please describe.

My requirements.txt contained a yanked version of cryptography. pip-audit did not warn about this.

Describe the solution you'd like

If possible pip-audit should (maybe optionally) warn/report yanked package versions. pip itself already warns about it when installing the package:

WARNING: The candidate selected for download or install is a yanked version: 'cryptography' candidate (version 38.0.2 at https://files.pythonhosted.org/packages/56/1e/2ffbbdddfe17308511cb2e06ac8c5aced9391dd5eea339e330a204edac34/cryptography-38.0.2-cp36-abi3-manylinux_2_28_x86_64.whl#sha256=55974e634712f7d054886a754a10c67b58e6a9d1c6c3d0d1181919e7fb336d0e (from https://pypi.org/simple/cryptography/) (requires-python:>=3.6))
Reason for being yanked: Regression in OpenSSL.

Describe alternatives you've considered

I did not consider any alternatives as none were obvious to me at first glance.

Additional context

Thank you for this great tool.

sebastianw avatar Oct 14 '22 13:10 sebastianw

Thanks for the report!

To clarify: you'd like pip-audit to warn you about package versions that don't have public vulnerabilities, but have been yanked by their maintainers? That seems like a reasonable addition to me; just wanted to make sure I understand.

woodruffw avatar Oct 14 '22 14:10 woodruffw

Thinking out loud: we can do this via the PEP 503 API, since it includes a data-yanked member when a particular distribution has been yanked.

We might want to stuff this behind another auditing flag; something like --fail-on-yanked (which would in turn be implied by the pre-existing --strict, maybe?)

woodruffw avatar Oct 14 '22 14:10 woodruffw

Thanks for the report!

To clarify: you'd like pip-audit to warn you about package versions that don't have public vulnerabilities, but have been yanked by their maintainers? That seems like a reasonable addition to me; just wanted to make sure I understand.

Yes, that is exactly what I want. 😃

sebastianw avatar Oct 14 '22 14:10 sebastianw

Excellent, thanks for confirming 🙂

CC @di and @tetsuo-cpp any objections to this functionality? IMO we should put it behind a flag, but otherwise I think it's a good addition.

woodruffw avatar Oct 14 '22 14:10 woodruffw

I think as long as this is opt-in, it's fine by me (since yanking does not necessarily mean there is a security issue, and this is primarily a security tool).

di avatar Oct 14 '22 14:10 di

Yep, agreed about opt-in. I'll take a stab at this in a bit.

(Thinking about it more, IMO we shouldn't have --strict imply this, since --strict currently controls only dependency collection errors, and yanked release do not cause dependency collection problems.)

Edit: Hmm, it's not 100% clear where this should go in pip-audit -- not all dependency sources hit PyPI so it doesn't make sense to put it there, and it doesn't exactly fit into the service/auditing phase either.

Edit 2: The data model is also a little murky here: individual distributions are marked as yanked, despite "yanking" being a thing that happens to entire versions. Actually, never mind, the JSON API shows a top-level yanked key for the release.

woodruffw avatar Oct 14 '22 14:10 woodruffw

I took an initial stab at this (https://github.com/pypa/pip-audit/compare/ww/yanked), but a couple of issues arose:

  • If we change the return type of our audit steps from Tuple[Dependency, List[VulnerabilityResult] to something like Tuple[Dependency, Yanked, List[VulnerabilityResult], we lose the (trivial) ability to turn the full audit into a mapping of Dependency => [VulnerabilityResult] via dict(...). This isn't a huge deal, but it produces a lot of code churn (especially in the unit tests).

  • The data model still isn't ideal here: I'm doing a check for "yanked" status in the PyPI vulnerability source since that's the easiest place to get it, but conceptually it belongs in dependency resolution. However, putting it there would require us to add additional network calls for some dependency resolvers (e.g. we'd have to check the simple respository API even when a requirements file is fully hashed and resolved).

woodruffw avatar Oct 14 '22 16:10 woodruffw