-
Notifications
You must be signed in to change notification settings - Fork 659
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
update pyyaml to >=5.1 #2064
update pyyaml to >=5.1 #2064
Conversation
Signed-off-by: redshiftzero <jen@freedom.press>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, thanks!
setup.cfg
Outdated
@@ -80,7 +80,7 @@ install_requires = | |||
Jinja2 >= 2.10.1 | |||
pexpect >= 4.6.0, < 5 | |||
psutil == 5.4.6; sys_platform!="win32" and sys_platform!="cygwin" | |||
PyYAML == 3.13 | |||
PyYAML >= 5.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also get a < 6
?
Assuming semantic versioning ... but we should make sure 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, can we also get a changelog entry for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Marking for #2064 (comment) and #2064 (review).
Closes #1806
CVE-2017-18342 flags the previously unsafe default yaml load function in pyyaml. Since version 1.0.6 of this project, it looks like the safe loader (
yaml.safe_load
) was already used everywhere. This PR just updates to a version of pyyaml that does not have the CVE associated with it.