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

5071 SNYK - Upgrade Pillow (and Wagtail) #5138

Merged
merged 1 commit into from
Mar 29, 2022
Merged

Conversation

rfultz
Copy link
Contributor

@rfultz rfultz commented Mar 25, 2022

Summary

Upgrading Pillow to address a vulnerability, which required a Wagtail upgrade. Luckily, the Wagtail upgrade addressed only the Pillow vulnerability and only changed the required version

Required reviewers

  • one front-end or UX
  • should probably check on dev or feature so we can play with image uploads (Pillow being an image handler)

Impacted areas of the application

Pillow and Wagtail

Screenshots

None

Related PRs

None

How to test

Testing vulnerabilities is always tricky, like proving an absence. The vulnerability reads,

Affected versions of this package are vulnerable to Improper Input Validation. When the path to the temporary directory on Linux or macOS contained a space, this would break removal of the temporary image file after im.show() (and related actions), and potentially remove an unrelated file.

so to fully test it, we'd need to use a Linux or Mac, use a temporary directory with a space in its path, trigger Wagtail to use im.show(), and then check to make sure no unrelated files were removed?

For this review, it seems like the most we can do is make sure nothing major is broken, like making sure image uploads work (outside of localhost?) as expected?

  • pull the branch
  • ./manage.py runserver
  • make sure things are working as expected
  • push to dev or feature and make sure image uploads work there? I'm open to ideas

Copy link
Contributor

@johnnyporkchops johnnyporkchops left a comment

Choose a reason for hiding this comment

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

Pushed this branch to feature space and tested image uploads successfully.

  • Make sure we rm task for feature deploy before merging

Copy link
Member

@patphongs patphongs left a comment

Choose a reason for hiding this comment

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

I reverted and squashed the the temp push to feature commit. I also tested on feature and all looks good, thanks @rfultz

@codecov-commenter
Copy link

Codecov Report

Merging #5138 (80f65dc) into develop (fc95ab1) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #5138      +/-   ##
===========================================
- Coverage    74.95%   74.93%   -0.02%     
===========================================
  Files          125      125              
  Lines         8120     8120              
  Branches       648      648              
===========================================
- Hits          6086     6085       -1     
- Misses        2034     2035       +1     
Impacted Files Coverage Δ
fec/fec/static/js/modules/calendar.js 92.08% <0.00%> (-0.72%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc95ab1...80f65dc. Read the comment docs.

@patphongs patphongs merged commit e57c821 into develop Mar 29, 2022
@rfultz rfultz deleted the feature/5071-snyk-pillow branch July 22, 2024 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SNYK: Medium] Improper Input Validation (Due 04/15/2022)
4 participants