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.

Sign up for GitHub

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

Jump to bottom

Update apispec package to use fecgov's forked repo #3362

Merged
merged 6 commits into from Sep 19, 2018
Merged

Update apispec package to use fecgov's forked repo #3362

merged 6 commits into from Sep 19, 2018

Conversation

lbeaufort
Copy link
Member

@lbeaufort lbeaufort commented Sep 4, 2018 •
edited

Resolves #3356 and Resolves #3280

apispec accepted my PR to fix the PyYAML vulnerability but we haven't heard back on another change that's preventing us from being able to run the latest validate_spec so I made the change on our end.

This PR uses our forked version until we can (hopefully) get apispec to make a change.

@lbeaufort lbeaufort force-pushed the feature/update-apispec branch 2 times, most recently from b60ce87 to 3ff5b24 Compare September 4, 2018 21:15
@lbeaufort lbeaufort changed the title Update apispec package to 0.39.0 [WIP] Update apispec package to 0.39.0 Sep 4, 2018
lbeaufort and others added 4 commits September 18, 2018 14:54
@lbeaufort
Update apispec package to 0.39.0
6c9956d 
Addresses #3356

- New `prance` package is used for `validate_spec`
- Additional imports needed for swagger tests
@lbeaufort
Update flask-apispec package to 0.7.0
e9d0728 
Addresses #3356
@lbeaufort
Upgrade to apispec 1.0.0b2
467ec5c
@lbeaufort
Remove unused variables
d01012a
@lbeaufort lbeaufort changed the title [WIP] Update apispec package to 0.39.0 [WIP] Update apispec package to use fecgov Sep 18, 2018
@lbeaufort lbeaufort changed the title [WIP] Update apispec package to use fecgov [WIP] Update apispec package to use fecgov's forked repo Sep 18, 2018
@lbeaufort lbeaufort force-pushed the feature/update-apispec branch 2 times, most recently from b19d642 to 65e0006 Compare September 18, 2018 20:01
@lbeaufort
Use fecgov's fork of apispec
e9e17e8 
Upgrade to latest version once marshmallow-code/apispec#282 is addressed

See https://github.com/fecgov/apispec/pull/1/files for forked changes
@lbeaufort lbeaufort changed the title [WIP] Update apispec package to use fecgov's forked repo Update apispec package to use fecgov's forked repo Sep 18, 2018
@lbeaufort lbeaufort mentioned this pull request Sep 18, 2018
Closed
3 tasks
vrajmohan
vrajmohan approved these changes Sep 18, 2018
View reviewed changes
Copy link
Contributor

@vrajmohan vrajmohan left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@qqss88
Copy link
Contributor

qqss88 commented Sep 19, 2018

Review done. Looks good to me too. Great job.

qqss88
qqss88 approved these changes Sep 19, 2018
View reviewed changes
Copy link
Contributor

@qqss88 qqss88 left a comment

Choose a reason for hiding this comment

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

approved. great job.

@vrajmohan vrajmohan merged commit 18131fe into develop Sep 19, 2018
@vrajmohan vrajmohan deleted the feature/update-apispec branch September 19, 2018 01:33
@lbeaufort
Copy link
Member Author

lbeaufort commented Sep 19, 2018 •
edited

@justin5p here's a summary of this issue:

The package that we use (apispec) was using PyYaml's yaml.load() function, which was flagged as a security vulnerability. I put in a change to apispec to use yaml.safe_load() which is the safe version of the load() function. Then, when upgrading to the latest version, we ran into a code problem that was preventing us from using the latest version of apispec. We put in an issue to their repo but haven't heard back, so we forked the latest version and made the code change on our end, then switched to using our forked copy.

At this time, both flagged packages (prance and apispec) use yaml.safe_load() and no longer present a security vulnerability.

Going forward, it would be best to keep an eye on the latest pyyaml release to see if they fix the load issue, and apispec to see if they fix the issue that caused us to need to fork their repo.

Please let me know if you have questions, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vrajmohan vrajmohan vrajmohan approved these changes

@qqss88 qqss88 qqss88 approved these changes

Assignees
No one assigned
Labels
Please review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Upgrade to latest apispec and flask-apispec versions [Med] Snyk - Arbitrary Code Execution (due 9/10)
3 participants
@lbeaufort @qqss88 @vrajmohan