remove mac reqs and update pillow

[sssoleileraaa]

Description

• Removes dev requirements file for macs since none of our maintainers are developing/supporting mac
• Fix safety error:

+==============================================================================+
|                                                                              |
|                               /$$$$$$            /$$                         |
|                              /$$__  $$          | $$                         |
|           /$$$$$$$  /$$$$$$ | $$  \__//$$$$$$  /$$$$$$   /$$   /$$           |
|          /$$_____/ |____  $$| $$$$   /$$__  $$|_  $$_/  | $$  | $$           |
|         |  $$$$$$   /$$$$$$$| $$_/  | $$$$$$$$  | $$    | $$  | $$           |
|          \____  $$ /$$__  $$| $$    | $$_____/  | $$ /$$| $$  | $$           |
|          /$$$$$$$/|  $$$$$$$| $$    |  $$$$$$$  |  $$$$/|  $$$$$$$           |
|         |_______/  \_______/|__/     \_______/   \___/   \____  $$           |
|                                                          /$$  | $$           |
|                                                         |  $$$$$$/           |
|  by pyup.io                                              \______/            |
|                                                                              |
...
+==============================================================================+
| pillow                     | 8.1.2     | <8.2.0                   | 40595    |
+==============================================================================+
| An issue was discovered in Pillow before 8.2.0. For EPS data, the readline   |
| implementation used in EPSImageFile has to deal with any combination of \r   |
| and \n as line endings. It used an accidentally quadratic method of          |
| accumulating lines while looking for a line ending. A malicious EPS file     |
| could use this to perform a DoS of Pillow in the open phase, before an image |
| was accepted for opening.                                                    |
+==============================================================================+
make: *** [Makefile:106: safety] Error 1

Follow up PR will address the same issue addressed here: freedomofpress/securedrop-proxy#88:

• When specifying a dev-only python dependency in a .in file, do not require a specific version unless there's reason to use a specific version. Doing so will help alleviate tedium when updating dependencies in the future. If you need to specify a version, lean towards using >=.
• Running make update-pip-requirements will now use pip-complie --upgrade for dev dependencies
• Fix black and mypy errors (mostly assigning variables incompatible types)

Test Plan

• CI passes
• Safety error is fixed
• mac files and mac-only code fully removed

Checklist

If these changes modify code paths involving cryptography, the opening of files in VMs or network (via the RPC service) traffic, Qubes testing in the staging environment is required. For fine tuning of the graphical user interface, testing in any environment in Qubes is required. Please check as applicable:

• I have tested these changes in the appropriate Qubes environment
• I do not have an appropriate Qubes OS workstation set up (the reviewer will need to test these changes)
• These changes should not need testing in Qubes

If these changes add or remove files other than client code, the AppArmor profile may need to be updated. Please check as applicable:

• I have updated the AppArmor profile
• No update to the AppArmor profile is required for these changes
• I don't know and would appreciate guidance

If these changes modify the database schema, you should include a database migration. Please check as applicable:

• I have written a migration and upgraded a test database based on main and confirmed that the migration applies cleanly
• I have written a migration but have not upgraded a test database based on main and would like the reviewer to do so
• I need help writing a database migration
• No database schema changes are needed

[rmol]

Using --upgrade when compiling dev requirements as is currently done means the dev virtualenv can end up with higher versions of production requirements than will be used in production. The discrepancies could lead to problems that are hard to diagnose.

I'm all for --upgrade, but we should probably compile just the dev requirements into dev-requirements.txt, then build the dev virtualenv from the two separate requirements files.

If there's consensus, we should probably make the same change in securedrop-proxy.

[kushaldas]

Using --upgrade when compiling dev requirements as is currently done means the dev virtualenv can end up with higher versions of production requirements than will be used in production. The discrepancies could lead to problems that are hard to diagnose.

I also agree wtih @rmol here, we use the pinned lock files so that we can choose to upgrade the dependencies selectively. I would prefer to keep that same way so that we can manage the dependencies better.

[sssoleileraaa]

I'm all for --upgrade, but we should probably compile just the dev requirements into dev-requirements.txt, then build the dev virtualenv from the two separate requirements files.

There is consensus here, as discussed in standup today, to do something similar to freedomofpress/securedrop-proxy#88 where we use the latest security and code linters and no longer unnecessarily specify out-of-date dev-only versions in the .in file for the dev-ONLY dependencies. This PR only updates pillow via the make update-pip-requirements script so it should look similar to other dev dependency updates. It looks like I did not specify a version for pillow consistent with how we've done this in the past, so I will push an update to pin pillow using >= but nothing else should change other than what changes when you run make update-pip-dependencies. The change to use the latest linters and other dev-only dependencies will be upcoming in the next PR.

[sssoleileraaa]

@rmol see how --upgrade is only used for dev dependencies here: https://github.com/freedomofpress/securedrop-proxy/blob/9abadaa6922c21a985db454f1fe1afa79a03d6d2/Makefile#L55-L57, it's also not currently used in the securedrop-client repo (it is not added as part of this PR either)

please let me know if there is something you see wrong with how this is done in the proxy

Update

I think I found the problem in the proxy repo, so I opened an issue there (this is a bug): freedomofpress/securedrop-proxy#153

[sssoleileraaa]

Note: freedomofpress/securedrop-builder#248 allows us to support multi-line pip-tools comments

[rmol]

I may be changing my mind about using the blanket --upgrade. There's unavoidable potential for version discrepancies even if we compile production and development requirements separately, e.g. both want requests, which is a dependency of both semgrep and securedrop-sdk (and explicitly listed in the production requirements.in). I think the followup PRs here and in securedrop-proxy are going to need more thought.

It looks like this PR is currently still upgrading more than Pillow, including prod requirements like MarkupSafe. To get this merged, we can just upgrade Pillow by running this manually instead (with the requirements files as of this branch's parent):

pip-compile --verbose --rebuild --generate-hashes --annotate --allow-unsafe --output-file "requirements/dev-requirements.txt" "requirements/requirements.in" "requirements/dev-requirements.in" --upgrade-package pillow==8.3.1

While keeping the Mac requirement removal, of course.

[sssoleileraaa]

@rmol if you're around today, let's talk plz. I've been using make update-pip-requirements

securedrop-client/Makefile

Lines 123 to 126 in 8e6ece0

	update-pip-requirements: ## Updates all Python requirements files via pip-compile for Linux.
	pip-compile --verbose --rebuild --generate-hashes --annotate --allow-unsafe --output-file "requirements/dev-requirements.txt" "requirements/requirements.in" "requirements/dev-requirements.in"
	pip-compile --verbose --rebuild --generate-hashes --annotate --output-file "requirements/requirements.txt" "requirements/requirements.in"

-- there's no usage of --upgrade anywhere, so not sure if i'm missing something. make update-pip-requirements does update both dev and prod dependencies (it also runs pip-compile --verbose --rebuild --generate-hashes --annotate --output-file "requirements/requirements.txt" "requirements/requirements.in"). if you're suggesting that we no longer use make update-pip-requirements then let's talk about this. also let's discuss freedomofpress/securedrop-proxy#153 if you and others are up for that and some documentation around how we plan to simplify our dev dependency story?

[sssoleileraaa]

I definitely could learn a lot from both you and kushal so would love to have another team discussion!

[rmol]

Looks good. Just had one clarifying nit in the README change.

[rmol]

The (Linux) after dev-requirements.txt can be removed now.