added missing digest support to recipes #2351
Conversation
There was a problem hiding this comment.
Very nice!
Looking good to me, thanks 🙏
Couple of notes for a potential follow up PR.
- add tests to the
TestRecipeclass oftests/test_recipe.py - fix a couple of DRY issues
|
Thanks so much. Additional note: It turns out pypi is recommending sha256, and also providing md5 and blake2-256. Should probably change the 3 algorithm choices here to match those. |
| expected_md5 = self.md5sum | ||
| expected_digests = {} | ||
| for alg in set(hashlib.algorithms_guaranteed) | set(('md5', 'sha512', 'blake2b')): | ||
| expected_digest = getattr(self, alg + 'sum') if hasattr(self, alg + 'sum') else None |
There was a problem hiding this comment.
@fuzzyTew fyi you could shorten
getattr(self, alg + 'sum') if hasattr(self, alg + 'sum') else Noneto
getattr(self, alg + 'sum', None)
There was a problem hiding this comment.
Thanks, I didn't see that. If you have any interest / effort, I may have trouble getting to this for a while.
The biggest issue with my code is that I provided no way to set the digest length, so you can't do 256-bit blake2 which pypi uses.
Might be reasonable also to add a note regarding other security norms of the system not yet being as tight as the new algorithms.
EDIT: btw I am fuzzytew
This adds support for all the hash fragment types pypi may use: https://pip.pypa.io/en/stable/reference/pip_install/#hashes-from-pypi .
Allowing cryptographically secure hashes can help system administrators defend against compromise.
Documentation is only added for sha512sum and blake2bsum to maintain backwards compatibility in the interface and stay somewhat concise. These two algorithms are rated highly.