Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report yanked versions of packages #381

Open
sebastianw opened this issue Oct 14, 2022 · 7 comments
Open

Report yanked versions of packages #381

sebastianw opened this issue Oct 14, 2022 · 7 comments
Assignees
Labels
component:cli CLI components enhancement New feature or request

Comments

@sebastianw
Copy link

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 sebastianw added the enhancement New feature or request label Oct 14, 2022
@woodruffw
Copy link
Member

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
Copy link
Member

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?)

@sebastianw
Copy link
Author

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. 😃

@woodruffw
Copy link
Member

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.

@di
Copy link
Sponsor Member

di commented Oct 14, 2022

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).

@woodruffw
Copy link
Member

woodruffw commented Oct 14, 2022

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 woodruffw self-assigned this Oct 14, 2022
@woodruffw woodruffw added the component:cli CLI components label Oct 14, 2022
@woodruffw
Copy link
Member

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:cli CLI components enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants