Pin Pillow to 10.1.0 to make Snyk happy

[chosak]

Snyk reports a vulnerability in Pillow versions < 10.0.0. We don't pin Pillow, which means that Wagtail in practice pulls in the most recent version, which is greater than 10.0.0, BUT for some reason this doesn't satisfy Snyk.

Snyk keeps reporting this vulnerability on all of our cf.gov PRs. In order to make Snyk happy, let's pin Pillow to the currently installed version, which is 10.1.0.

Screenshots

Here's what Snyk reports:

Even though in practice we are already getting Snyk 10.1.0:

$ /srv/cfgov/current/venv/bin/pip freeze | grep illow
Pillow @ file:///srv/cfgov/versions/20231129110504/wheels/Pillow-10.1.0-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl#sha256=fa1d323703cfdac2036af05191b969b910d8f115cf53093125e4058f62012c9a

Checklist

• PR has an informative and human-readable title
• Changes are limited to a single goal (no scope creep)
• Code follows the standards laid out in the CFPB development guidelines

[lfatty]

Looks great!

[chosak]

Snyk is still reporting this issue:

Note they claim to have tested commit e4af342 which is this commit, which pins Pillow. @lfatty any idea why Snyk is still failing?

[willbarton]

I don't think I like pinning Pillow ourselves, given that it's pinned upstream from us.

An alternative to doing that might be use pip-compile, which'll do that pinning (and hashing) of all upstream dependencies automatically. But. I don't think that'd help here since Snyk doesn't recognize this at all?

[chosak]

@willbarton I agree it's not optimal. My goal is to make Snyk happy so our PRs don't keep failing this check. It's just unclear to me how Snyk is actually checking this - for example the current failure shows the vulnerability coming from [email protected] > [email protected] > [email protected], but:

• [email protected] doesn't actually specify [email protected], it specifies wagtail>=2.15
• [email protected] doesn't actually specify [email protected], it specifies Pillow>=9.1.0,<11.0.0

My best guess is that maybe at some point Snyk scanned [email protected] and at that time the latest version of Wagtail was 5.0.5, and at that time the latest version of Pillow was 9.5.0? I can't find any way to get Snyk to do a total re-assessment of all of its dependencies.

it's pinned upstream from us

From Snyk's POV it doesn't actually see that upstream pin because it checks libraries.txt separately from wagtail.txt, so it doesn't see our Wagtail pin.

Would you be alright with trying this PR to see if it satisfies Snyk? If not, we might need to remove Snyk or find a way to permanently ignore this error.

[willbarton]

We can try I guess?

That kind of ambiguity though is exactly what the pip-compile workflow I tried to introduce should avoid though because it means Snyk would be scanning the full set of dependencies as pinned and hashed the last time we compiled them.

[chosak]

Thanks @willbarton. Let me try this and if Snyk doesn't recognize it, I'll revert. Agree we should revisit #7874.

[chosak]

Frustratingly, this didn't work - Snyk still reports the same vulnerability.

But I figured out why it is doing that! It turns out, buried in the documentation, that when Snyk scans using requirements files, you can only choose Python versions 2.7.16 or 3.7.4. In our case, because we've chosen Python 3, we're testing with 3.7.4 (not 3.8 which we actually use in production).

The latest Pillow version supported for Python 3.7 is... the vulnerable 9.5.0! Support for 3.7 was removed in Pillow 10.0.0 although unfortunately this isn't mentioned in the release notes. So this is where the Pillow vulnerability is coming from.

Snyk does support more recent versions of Python if you use Pipenv, see docs here. Alternatively if you use Poetry they scan the exact dependency tree you provide.

Given that @willbarton, any thoughts on how to proceed? It seems like I could remove this pin and permanently ignore Pillow in Snyk (sorry @lfatty):

Then we'd no longer have the red X on all of our PRs, but Snyk would essentially be testing versions we're not even using. Snyk doesn't seem particularly useful for Python testing if it's not even testing using our version of Python..

[willbarton]

I don't love ignoring Pillow, because that's where a lot of our regular vulnerabilities on the Python side come from.

I also... the Python version thing boggles my mind. We could use Poetry, I guess, instead of pip-compile, but that'd be a vastly larger disruption to our workflow, without much benefit (IMHO) aside from making Snyk happy?

My preferred approach would be for Snyk to support vanilla Python > 3.7!! That's bonkers. Aside from that, the alternatives seem entirely to make Snyk happy at the cost of extra complexity for us. So... I really don't know.

Snyk doesn't seem particularly useful for Python testing if it's not even testing using our version of Python..

💯

[willbarton]

But I figured out why it is doing that! It turns out, buried in the documentation, that when Snyk scans using requirements files, you can only choose Python versions 2.7.16 or 3.7.4. In our case, because we've chosen Python 3, we're testing with 3.7.4 (not 3.8 which we actually use in production).

Just for added emphasis, this is MIND BOGGLING to me. Python is on 3.12!! And Snyk isn't supporting over 3.7!! 3.7 was EOL'ed in July!