address safety, mypy, and black errors

[sssoleileraaa]

Description

This PR proposes the following change:

• When specifying a 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
• There is now a venv makefile target
• Fix safety error that was in a specific version of a package we were pining in dev-requirements.txt (also now we are no longer pinning this package because it is unnecessary) by upgrading dev dependencies (this required a workaround for Requirements generated with pip-tools > 5.0.0 not parseable by build scripts securedrop-builder#225 in the makefile target update-pip-requirements)
• Fix black formatting errors
• Fix mypy errors around assigning a variable an incompatible type

Safety error

Checking file ./dev-requirements.txt
+==============================================================================+
|                                                                              |
|                               /$$$$$$            /$$                         |
|                              /$$__  $$          | $$                         |
|           /$$$$$$$  /$$$$$$ | $$  \__//$$$$$$  /$$$$$$   /$$   /$$           |
|          /$$_____/ |____  $$| $$$$   /$$__  $$|_  $$_/  | $$  | $$           |
|         |  $$$$$$   /$$$$$$$| $$_/  | $$$$$$$$  | $$    | $$  | $$           |
|          \____  $$ /$$__  $$| $$    | $$_____/  | $$ /$$| $$  | $$           |
|          /$$$$$$$/|  $$$$$$$| $$    |  $$$$$$$  |  $$$$/|  $$$$$$$           |
|         |_______/  \_______/|__/     \_______/   \___/   \____  $$           |
|                                                          /$$  | $$           |
|                                                         |  $$$$$$/           |
|  by pyup.io                                              \______/            |
|                                                                              |
+==============================================================================+
| REPORT                                                                       |
| checked 34 packages, using free DB (updated once a month)                    |
+============================+===========+==========================+==========+
| package                    | installed | affected                 | ID       |
+============================+===========+==========================+==========+
| pip                        | 20.2.3    | <21.1                    | 40291    |
+==============================================================================+
| Pip 21.1 stops splitting on unicode separators in git references, which      |
| could be maliciously used to install a different revision on the repository. |
| See: <https://github.com/pypa/pip/issues/9827>. Additionally, pip 21.1       |
| updates urllib3 to 1.26.4 to fix CVE-2021-28363.                             |
+==============================================================================+

Test Plan

0. ensure only dev dependencies have changed and that code changes in proxy.py are non-functional changes
1. install and run the proxy from this pr and run through basic functional test plan
2. delete .venv directory
3. make venv
4. source .venv/bin/activate

• make check has no errors
• make update-pip-requirements runs without error

[sssoleileraaa]

See PR description for updates around changes and test plan

[sssoleileraaa]

Temporarily moving this to draft until freedomofpress/securedrop-builder#248 is merged so that I can undo the small workaround and use the latest pip-tools when updating requirements for all environments.

[emkll]

By using pip-compile with the --upgrade flag, we will always upgrade all the dependencies. This is a departure from the current method of updating only the dependencies we need to update, and pinning the lowest version in *requirements.in. One such example (updating the dependencies identified here) can be found in https://github.com/freedomofpress/securedrop-proxy/compare/test-88

The rationale is that it makes updates less likely to break applications, because we only update the dependencies when required, and only update the dependencies that need to be updated. By carefully crafting requirements.in, we ensure that only the minimal amount of packages are updated when pip is compiled (partially due to requirement to review the production requirements, and building wheels in securedrop-debian-packaging)

Because we probably want to use the same pip-compile commands across the board, if we do adopt this approach here, it may make sense to adopt it elsewhere. What are the advantages of upgrading all the dependencies each time we update the requirements file?

[emkll]

This will install pip and wheel without locking versions nor hashes, which has downsides:

1. Since we don't pin versions, an update to pip or wheel can break
2. We have no way of tracking or validating the integrity of the packages that are installed (we do install pip on L12)

Is there a reason this is required here?

[sssoleileraaa]

venv is a new makefile target to create a dev virtual environment that can install all the dependencies defined in dev-requirements.txt. the reason to update wheel here is to fix the following error when you create a virtual env for the proxy (perhaps this is why the venv target was not included even though it is a standard across most of our projects):

error: invalid command 'bdist_wheel'

notice also how we do pip install --upgrade pip when running make bandit since this is dev only

[emkll]

we should probably use pip-tools as pinned by the requirements file, in other words, use the requirements installed in the .venv

[sssoleileraaa]

this is a workaround for the issue we've had for a while in our packaging code that makes it so we don't support pip-tools>5.0.0. the reason this PR is in draft mode is to remove this workaround, since I fixed the debian packaging logic here: freedomofpress/securedrop-builder#248 <-- this should be reviewed first.

[sssoleileraaa]

By using pip-compile with the --upgrade flag, we will always upgrade all the dependencies. This is a departure from the current method of updating only the dependencies we need to update, and pinning the lowest version in *requirements.in. One such example (updating the dependencies identified here) can be found in https://github.com/freedomofpress/securedrop-proxy/compare/test-88

So the PR proposes the following:

When specifying a 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 >=.

In the PR, you'll notice this is a new rule being proposed for dev-only. That's why we still run pip-compile --generate-hashes --output-file requirements.txt requirements.in without the --upgrade option. The pros are that we use the latest packages with security fixes when developing our code so that when we get safety alerts on packages it'll most likely be related to requirements.txt (our prod dependencies). We also have less noisy security alerts for all our test and dev dependencies. Another benefit is that we find more bugs in our code when using the latest linters, as you can see in this PR for example.

The rationale is that it makes updates less likely to break applications, because we only update the dependencies when required, and only update the dependencies that need to be updated. By carefully crafting requirements.in, we ensure that only the minimal amount of packages are updated when pip is compiled (partially due to requirement to review the production requirements, and building wheels in securedrop-debian-packaging)

Nothing has changed in requirements.in - this is a dev only change.

Because we probably want to use the same pip-compile commands across the board, if we do adopt this approach here, it may make sense to adopt it elsewhere. What are the advantages of upgrading all the dependencies each time we update the requirements file?

That is a securedrop-debian-packaging#225 workaround that will go away once our packaging bug is fixed (it's fixed in this PR: freedomofpress/securedrop-builder#248, which should be reviewed first). Once that is fixed, I will remove that simple workaround and will move this PR out of draft mode.

[sssoleileraaa]

As mentioned before, once freedomofpress/securedrop-builder#248 is merged this PR will be ready for review (I removed the workaround here that freedomofpress/securedrop-builder#248 directly fixed). So this is now ready for a final review.

[emkll]

Thanks @creviera for the changes here, LGTM. As discussed, we will try using the --upgrade flag on the dev requirements to update them more regularly.