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

[pyyaml] allow patching of unsafe pyyaml operations #3808

Merged
merged 1 commit into from
Jan 30, 2019
Merged

Conversation

truthbk
Copy link
Member

@truthbk truthbk commented Jan 8, 2019

Note: Please remember to review the Datadog Contribution Guidelines
if you have not yet done so.

What does this PR do?

Given the state of pyyaml package and the CVE-2017-18342 (which does not affect us), let's just make things a little safer for users who may be using pyyaml in their custom checks. This PR monkey patches unsafe operations, and points to the safe counterparts.

Motivation

Make custom checks safer if they use pyyaml.

Testing Guidelines

An overview on testing
is available in our contribution guidelines.

Additional Notes

We should make this very clear in the changelog.

[utils] changing yaml to ddyaml to avoid clash

[pyyaml] allow reversing of monkey patch
@truthbk truthbk added this to the 5.31.0 milestone Jan 8, 2019
@olivielpeau olivielpeau modified the milestones: 5.31.0, 5.32.0 Jan 8, 2019
Copy link
Contributor

@remeh remeh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

@truthbk truthbk merged commit f2c90aa into master Jan 30, 2019
@truthbk truthbk deleted the jaime/yamlsafety branch January 30, 2019 10:11
truthbk added a commit that referenced this pull request Jul 22, 2020
truthbk added a commit that referenced this pull request Jul 29, 2020
* [requirements] bump pyyaml to 5.3.1

* Revert "[pyyaml] allow patching of unsafe pyyaml operations (#3808)"

This reverts commit f2c90aa.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants