-
-
Notifications
You must be signed in to change notification settings - Fork 809
Upgrade h11 due to vulnerability #2643
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
base: master
Are you sure you want to change the base?
Conversation
# DO NOT MERGE
Hey all. I'm not sure what this means, but might imply an issue with the test. |
As I mentioned before, and I've actually consulted a security expert in the ecosystem... This is NOT needed on uvicorn. The point is that uvicorn already allows you to bump h11 on your application, and the vulnerability was in a dependency. As a user, you should have a security auditing process to make sure you bump h11. I'll leave this open for a bit - I may merge if a lot of people complain. 🤷♂️ |
Hey @Kludex - thank you for replying. thank you for maintaining this project - it is wonderful! (with and without this pr ;) |
I think it would be great to merge this as other security tools that are scanning the stated dependencies of the project rather than perhaps lockfiles will continue to report the package as vulnerable generating unnecessary noise. It would also simplify things for new users by getting them started with a dependency that hasn't been flagged as vulnerable |
This is the same discussion as encode/httpx#3564 (comment) If users are not auditing their codebase and upgrading uvicorn, they won't see a version bump here regardless. New users should not get the old vulnerable h11 version, all the Python package resolvers I know of default to using the latest compatible version of dependencies. |
So whatever the technical rights or wrongs CodeQL (for example) is flagging this package as having a Critical vulnerability and if updating the metadata helps avoid having to explain why this is a false positive isn't a net win for everyone? |
It's not a net win for users that need to use an older version of h11 and are then needlessly blocked from upgrading uvicorn. This sounds like a problem with CodeQL — it seems absurd to flag this package as having a critical vulnerability because it allows you to install it alongside a vulnerable package version. For example, you can |
@zanieb - what you're saying makes sense. In your opinion, under what circumstances is it appropriate to update a dependency requirement version in |
When needed. If I need to use |
@Kludex - what about a case when you know uvicorn is certainly introducing a vulnerability (say you require |
Well... If the restriction FORCES the user to install a vulnerable package to use uvicorn, then the bump is also needed... This is not relevant here tho... 👀 |
I think we’ve clarified the different perspectives:
You consider the first approach the best policy. |
This is not about me. This comment reflects my thoughts: #2643 (comment) |
I meant - under this policy, it makes sense to not merge this PR. |
Regarding this comment: Example of such a (rare) case:
In this situation, Having said all that, these scenarios do not invalidate the original considerations—for instance, the desire to let users who need an older version of Personally, I would lean toward a more “secure” or “restrictive” approach, but I recognize that the alternative is a valid and reasonable policy as well. Ultimately, I believe these decisions are best made by the maintainers, not by commenters like myself. |
Summary
A known vulnerability exists in h11, which is a dependency: GHSA-vqfr-h8mv-ghfj
Checklist
I've added a test for each change that was introduced, andI tried as much as possible to make a single atomic change.I've updated the documentation accordingly.