From 6127b272875d8c5c12859de02a6d42b5f2d333d6 Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Thu, 19 Sep 2024 18:01:42 -0400 Subject: [PATCH] Fix most errors spotted by ruff About half of these fixes were automatic, the rest I did by hand. I skipped removing % formatting from pretty_bad_protocol since there were like a hundred of them and it wasn't really worth it. --- admin/bootstrap.py | 9 +-- admin/securedrop_admin/__init__.py | 15 +++-- admin/tests/test_integration.py | 2 +- admin/tests/test_securedrop-admin.py | 6 +- devops/scripts/verify-mo.py | 13 +++-- .../roles/restore/files/compare_torrc.py | 9 +-- .../journalist_gui/SecureDropUpdater.py | 4 +- .../app-code/test_securedrop_app_code.py | 4 +- .../app-code/test_securedrop_rqrequeue.py | 10 +--- .../app-code/test_securedrop_rqworker.py | 5 +- .../test_securedrop_shredder_configuration.py | 10 +--- ...securedrop_source_deleter_configuration.py | 10 +--- .../app/apache/test_apache_system_config.py | 4 +- molecule/testinfra/app/test_app_network.py | 6 +- molecule/testinfra/app/test_apparmor.py | 2 +- molecule/testinfra/app/test_appenv.py | 11 ++-- molecule/testinfra/app/test_ossec_agent.py | 2 +- molecule/testinfra/app/test_smoke.py | 2 +- molecule/testinfra/app/test_tor_config.py | 4 +- .../testinfra/app/test_tor_hidden_services.py | 6 +- .../testinfra/common/test_fpf_apt_repo.py | 6 +- molecule/testinfra/common/test_grsecurity.py | 4 +- molecule/testinfra/common/test_user_config.py | 2 +- molecule/testinfra/mon/test_mon_network.py | 8 +-- molecule/testinfra/mon/test_ossec_server.py | 4 +- molecule/testinfra/mon/test_postfix.py | 7 +-- .../testinfra/ossec/test_journalist_mail.py | 10 ++-- pyproject.toml | 2 + ...da3fcab826a_delete_orphaned_submissions.py | 8 +-- .../ossec-common/var/ossec/checksdconfig.py | 2 +- securedrop/i18n.py | 5 +- securedrop/journalist_app/admin.py | 12 ++-- securedrop/journalist_app/main.py | 9 +-- securedrop/loaddata.py | 14 ++--- securedrop/manage.py | 34 +++++------ securedrop/management/submissions.py | 5 +- securedrop/models.py | 40 +++++-------- securedrop/passphrases.py | 35 +++++------- securedrop/pretty_bad_protocol/_meta.py | 6 +- securedrop/pretty_bad_protocol/_parsers.py | 24 +++----- securedrop/pretty_bad_protocol/_util.py | 4 +- securedrop/pretty_bad_protocol/gnupg.py | 56 ++++++++----------- securedrop/scripts/rqrequeue | 4 +- securedrop/scripts/shredder | 6 +- securedrop/scripts/source_deleter | 4 +- securedrop/source_app/main.py | 9 ++- securedrop/store.py | 5 +- securedrop/tests/conftest.py | 24 ++++---- securedrop/tests/functional/conftest.py | 8 +-- .../functional/pageslayout/test_journalist.py | 2 +- .../pageslayout/test_journalist_account.py | 2 +- .../pageslayout/test_journalist_admin.py | 4 +- .../pageslayout/test_journalist_col.py | 4 +- .../pageslayout/test_journalist_delete.py | 2 +- .../functional/pageslayout/test_source.py | 2 +- .../test_source_layout_torbrowser.py | 2 +- .../pageslayout/test_source_session_layout.py | 2 +- .../pageslayout/test_source_static_pages.py | 2 +- .../test_submit_and_retrieve_file.py | 2 +- .../tests/functional/test_admin_interface.py | 2 +- .../tests/functional/test_journalist.py | 2 +- .../tests/functional/test_source_warnings.py | 2 +- securedrop/tests/functional/web_drivers.py | 2 +- securedrop/tests/test_alembic.py | 2 +- securedrop/tests/test_journalist.py | 7 +-- securedrop/tests/test_journalist_api.py | 4 +- securedrop/tests/test_journalist_session.py | 2 +- .../tests/test_remove_pending_sources.py | 4 +- securedrop/tests/test_source.py | 5 +- securedrop/tests/test_store.py | 2 +- securedrop/tests/test_worker.py | 2 +- securedrop/tests/test_wsgi.py | 2 +- securedrop/tests/utils/instrument.py | 4 +- 73 files changed, 226 insertions(+), 331 deletions(-) diff --git a/admin/bootstrap.py b/admin/bootstrap.py index e59f3e3da79..838a57b4085 100755 --- a/admin/bootstrap.py +++ b/admin/bootstrap.py @@ -117,10 +117,7 @@ def is_missing_dependency() -> bool: return True # If any packages couldn't be found, it may point to an apt cache issue - if "Unable to locate package" in apt_query_result.stderr: - return True - - return False + return "Unable to locate package" in apt_query_result.stderr except subprocess.CalledProcessError as e: sdlog.error("Error checking apt dependencies") @@ -259,12 +256,12 @@ def install_pip_dependencies( ] ansible_ver = subprocess.run( - maybe_torify() + ansible_vercheck_cmd, text=True, capture_output=True + maybe_torify() + ansible_vercheck_cmd, text=True, capture_output=True, check=False ) if ansible_ver.stdout.startswith("2.9"): sdlog.info("Ansible is out-of-date, removing it.") delete_result = subprocess.run( - maybe_torify() + ansible_uninstall_cmd, capture_output=True, text=True + maybe_torify() + ansible_uninstall_cmd, capture_output=True, text=True, check=False ) if delete_result.returncode != 0: sdlog.error( diff --git a/admin/securedrop_admin/__init__.py b/admin/securedrop_admin/__init__.py index 99a3c7da3fc..ae69c6ba995 100755 --- a/admin/securedrop_admin/__init__.py +++ b/admin/securedrop_admin/__init__.py @@ -66,7 +66,7 @@ # Check OpenSSH version - ansible requires an extra argument for scp on OpenSSH 9 def openssh_version() -> int: try: - result = subprocess.run(["ssh", "-V"], capture_output=True, text=True) + result = subprocess.run(["ssh", "-V"], capture_output=True, text=True, check=False) if result.stderr.startswith("OpenSSH_9"): return 9 elif result.stderr.startswith("OpenSSH_8"): @@ -75,7 +75,6 @@ def openssh_version() -> int: return 0 except subprocess.CalledProcessError: return 0 - pass return 0 @@ -130,14 +129,14 @@ def validate(self, document: Document) -> bool: class ValidateTime(Validator): def validate(self, document: Document) -> bool: - if document.text.isdigit() and int(document.text) in range(0, 24): + if document.text.isdigit() and int(document.text) in range(24): return True raise ValidationError(message="Must be an integer between 0 and 23") class ValidateUser(Validator): def validate(self, document: Document) -> bool: text = document.text - if text != "" and text != "root" and text != "amnesia": + if text not in ("", "root", "amnesia"): return True raise ValidationError(message="Must not be root, amnesia or an empty string") @@ -160,8 +159,8 @@ def validate(self, document: Document) -> bool: raise ValidationError( message=( "DNS server(s) should be a space/comma-separated list " - "of up to {} IP addresses" - ).format(MAX_NAMESERVERS) + f"of up to {MAX_NAMESERVERS} IP addresses" + ) ) return True @@ -194,7 +193,7 @@ def validate(self, document: Document) -> bool: class ValidateYesNo(Validator): def validate(self, document: Document) -> bool: text = document.text.lower() - if text == "yes" or text == "no": + if text in ("yes", "no"): return True raise ValidationError(message="Must be either yes or no") @@ -792,7 +791,7 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: ) sdlog.error( "If you are certain you want to proceed, run:\n\n\t" - "./securedrop-admin --force {}\n".format(cmd_name) + f"./securedrop-admin --force {cmd_name}\n" ) sdlog.error("To apply the latest updates, run:\n\n\t" "./securedrop-admin update\n") sdlog.error( diff --git a/admin/tests/test_integration.py b/admin/tests/test_integration.py index 618b547659a..9ccff54b446 100644 --- a/admin/tests/test_integration.py +++ b/admin/tests/test_integration.py @@ -531,7 +531,7 @@ def test_sdconfig_enable_https_disable_pow_on_source_interface(): """ -@pytest.fixture() +@pytest.fixture def securedrop_git_repo(tmpdir): cwd = os.getcwd() os.chdir(str(tmpdir)) diff --git a/admin/tests/test_securedrop-admin.py b/admin/tests/test_securedrop-admin.py index 910a9421160..acd65999f2b 100644 --- a/admin/tests/test_securedrop-admin.py +++ b/admin/tests/test_securedrop-admin.py @@ -841,7 +841,7 @@ def verify_desc_consistency_optional(self, site_config, desc): if callable(default): default = default() assert site_config.user_prompt_config_one(desc, None) == default - assert type(default) == etype + assert type(default) is etype def verify_desc_consistency(self, site_config, desc): self.verify_desc_consistency_optional(site_config, desc) @@ -929,7 +929,7 @@ def verify_desc_consistency_allow_empty(self, site_config, desc): (var, default, etype, prompt, validator, transform, condition) = desc # verify the default passes validation assert site_config.user_prompt_config_one(desc, None) == default - assert type(default) == etype + assert type(default) is etype def verify_prompt_fingerprint(self, site_config, desc): self.verify_prompt_not_empty(site_config, desc) @@ -956,7 +956,7 @@ def verify_prompt_securedrop_supported_locales(self, site_config, desc): (var, default, etype, prompt, validator, transform, condition) = desc # verify the default passes validation assert site_config.user_prompt_config_one(desc, None) == default - assert type(default) == etype + assert type(default) is etype assert site_config.user_prompt_config_one(desc, "fr_FR en_US") == ["fr_FR", "en_US"] assert site_config.user_prompt_config_one(desc, ["fr_FR", "en_US"]) == ["fr_FR", "en_US"] assert site_config.user_prompt_config_one(desc, "") == [] diff --git a/devops/scripts/verify-mo.py b/devops/scripts/verify-mo.py index 8309e8f5c85..f7cf2ff77e5 100755 --- a/devops/scripts/verify-mo.py +++ b/devops/scripts/verify-mo.py @@ -21,7 +21,7 @@ import shlex import subprocess from pathlib import Path -from typing import Any, Iterator, Optional, Set +from typing import Iterator, Set import polib from translate.tools.pocompile import convertmo @@ -59,9 +59,9 @@ def __enter__(self) -> "CatalogVerifier": def __exit__( self, - exc_type: Optional[Any], - exc_value: Optional[Any], - traceback: Optional[Any], + exc_type: object, + exc_value: object, + traceback: object, ) -> None: """Clean up.""" @@ -113,11 +113,12 @@ def diffoscope_call( # because we want to inherit the Python virtual environment # in which we're invoked. # nosemgrep: python.lang.security.audit.subprocess-shell-true.subprocess-shell-true - return subprocess.run( + return subprocess.run( # noqa: S602 cmd, capture_output=True, env=os.environ, - shell=True, # noqa: S602 + shell=True, + check=False, ) def reproduce(self) -> None: diff --git a/install_files/ansible-base/roles/restore/files/compare_torrc.py b/install_files/ansible-base/roles/restore/files/compare_torrc.py index 53c34aae6cb..c46091e60bb 100644 --- a/install_files/ansible-base/roles/restore/files/compare_torrc.py +++ b/install_files/ansible-base/roles/restore/files/compare_torrc.py @@ -53,15 +53,12 @@ def strset(s): sys.exit(0) print( - "The Tor configuration on the app server offers version {} services.".format( - strset(server_versions) - ) + f"The Tor configuration on the app server offers version {strset(server_versions)} " + "services." ) print( - "The Tor configuration in this backup offers version {} services.".format( - strset(backup_versions) - ) + f"The Tor configuration in this backup offers version {strset(backup_versions)} services." ) print("\nIncompatible configuration: Restoring a backup including a different ") diff --git a/journalist_gui/journalist_gui/SecureDropUpdater.py b/journalist_gui/journalist_gui/SecureDropUpdater.py index b2d369283b7..75e80180350 100644 --- a/journalist_gui/journalist_gui/SecureDropUpdater.py +++ b/journalist_gui/journalist_gui/SecureDropUpdater.py @@ -19,9 +19,7 @@ def password_is_set(): pwd_flag = subprocess.check_output(["passwd", "--status"]).decode("utf-8").split()[1] - if pwd_flag == "NP": - return False - return True + return pwd_flag != "NP" def prevent_second_instance(app: QtWidgets.QApplication, name: str) -> None: diff --git a/molecule/testinfra/app-code/test_securedrop_app_code.py b/molecule/testinfra/app-code/test_securedrop_app_code.py index 245ef7683bd..d795cce67bf 100644 --- a/molecule/testinfra/app-code/test_securedrop_app_code.py +++ b/molecule/testinfra/app-code/test_securedrop_app_code.py @@ -52,7 +52,7 @@ def test_unwanted_packages_absent(host, package): assert not host.package(package).is_installed -@pytest.mark.skip_in_prod() +@pytest.mark.skip_in_prod def test_securedrop_application_test_locale(host): """ Ensure both SecureDrop DEFAULT_LOCALE and SUPPORTED_LOCALES are present. @@ -66,7 +66,7 @@ def test_securedrop_application_test_locale(host): assert "\nSUPPORTED_LOCALES = ['el', 'ar', 'en_US']\n" in securedrop_config.content_string -@pytest.mark.skip_in_prod() +@pytest.mark.skip_in_prod def test_securedrop_application_test_journalist_key(host): """ Ensure the SecureDrop Application GPG public key file is present. diff --git a/molecule/testinfra/app-code/test_securedrop_rqrequeue.py b/molecule/testinfra/app-code/test_securedrop_rqrequeue.py index 16b414cdfa3..d5f49ecc6db 100644 --- a/molecule/testinfra/app-code/test_securedrop_rqrequeue.py +++ b/molecule/testinfra/app-code/test_securedrop_rqrequeue.py @@ -17,13 +17,9 @@ def test_securedrop_rqrequeue_service(host): "Wants=redis-server.service", "", "[Service]", - 'Environment=PYTHONPATH="{}:{}"'.format( - securedrop_test_vars.securedrop_code, - securedrop_test_vars.securedrop_venv_site_packages, - ), - "ExecStart={}/python /var/www/securedrop/scripts/rqrequeue --interval 60".format( - securedrop_test_vars.securedrop_venv_bin - ), + f'Environment=PYTHONPATH="{securedrop_test_vars.securedrop_code}:{securedrop_test_vars.securedrop_venv_site_packages}"', + f"ExecStart={securedrop_test_vars.securedrop_venv_bin}/python /var/www/securedrop/" + "scripts/rqrequeue --interval 60", "PrivateDevices=yes", "PrivateTmp=yes", "ProtectSystem=full", diff --git a/molecule/testinfra/app-code/test_securedrop_rqworker.py b/molecule/testinfra/app-code/test_securedrop_rqworker.py index 1afdb20c475..88347d10cba 100644 --- a/molecule/testinfra/app-code/test_securedrop_rqworker.py +++ b/molecule/testinfra/app-code/test_securedrop_rqworker.py @@ -19,10 +19,7 @@ def test_securedrop_rqworker_service(host): "Wants=redis-server.service", "", "[Service]", - 'Environment=PYTHONPATH="{}:{}"'.format( - securedrop_test_vars.securedrop_code, - securedrop_test_vars.securedrop_venv_site_packages, - ), + f'Environment=PYTHONPATH="{securedrop_test_vars.securedrop_code}:{securedrop_test_vars.securedrop_venv_site_packages}"', f"ExecStart={securedrop_test_vars.securedrop_venv_bin}/rqworker -c rq_config", "PrivateDevices=yes", "PrivateTmp=yes", diff --git a/molecule/testinfra/app-code/test_securedrop_shredder_configuration.py b/molecule/testinfra/app-code/test_securedrop_shredder_configuration.py index a39164197f5..688d2b30c3a 100644 --- a/molecule/testinfra/app-code/test_securedrop_shredder_configuration.py +++ b/molecule/testinfra/app-code/test_securedrop_shredder_configuration.py @@ -16,13 +16,9 @@ def test_securedrop_shredder_service(host): "Description=SecureDrop shredder", "", "[Service]", - 'Environment=PYTHONPATH="{}:{}"'.format( - securedrop_test_vars.securedrop_code, - securedrop_test_vars.securedrop_venv_site_packages, - ), - "ExecStart={}/python /var/www/securedrop/scripts/shredder --interval 60".format( - securedrop_test_vars.securedrop_venv_bin - ), + f'Environment=PYTHONPATH="{securedrop_test_vars.securedrop_code}:{securedrop_test_vars.securedrop_venv_site_packages}"', + f"ExecStart={securedrop_test_vars.securedrop_venv_bin}/python /var/www/securedrop/" + "scripts/shredder --interval 60", "PrivateDevices=yes", "PrivateTmp=yes", "ProtectSystem=full", diff --git a/molecule/testinfra/app-code/test_securedrop_source_deleter_configuration.py b/molecule/testinfra/app-code/test_securedrop_source_deleter_configuration.py index 9de809a288c..a30077a3864 100644 --- a/molecule/testinfra/app-code/test_securedrop_source_deleter_configuration.py +++ b/molecule/testinfra/app-code/test_securedrop_source_deleter_configuration.py @@ -15,13 +15,9 @@ def test_securedrop_source_deleter_service(host): "Description=SecureDrop Source deleter", "", "[Service]", - 'Environment=PYTHONPATH="{}:{}"'.format( - securedrop_test_vars.securedrop_code, - securedrop_test_vars.securedrop_venv_site_packages, - ), - "ExecStart={}/python /var/www/securedrop/scripts/source_deleter --interval 10".format( - securedrop_test_vars.securedrop_venv_bin - ), + f'Environment=PYTHONPATH="{securedrop_test_vars.securedrop_code}:{securedrop_test_vars.securedrop_venv_site_packages}"', + f"ExecStart={securedrop_test_vars.securedrop_venv_bin}/python /var/www/securedrop/" + "scripts/source_deleter --interval 10", "PrivateDevices=yes", "PrivateTmp=yes", "ProtectSystem=full", diff --git a/molecule/testinfra/app/apache/test_apache_system_config.py b/molecule/testinfra/app/apache/test_apache_system_config.py index b40a0e9cc4b..1d5456ab0f6 100644 --- a/molecule/testinfra/app/apache/test_apache_system_config.py +++ b/molecule/testinfra/app/apache/test_apache_system_config.py @@ -90,9 +90,7 @@ def test_apache_ports_config(host, port): assert f.group == "root" assert f.mode == 0o644 - listening_regex = "^Listen {}:{}$".format( - re.escape(securedrop_test_vars.apache_listening_address), port - ) + listening_regex = f"^Listen {re.escape(securedrop_test_vars.apache_listening_address)}:{port}$" assert f.contains(listening_regex) diff --git a/molecule/testinfra/app/test_app_network.py b/molecule/testinfra/app/test_app_network.py index d3cbd5a193c..fb5474ce409 100644 --- a/molecule/testinfra/app/test_app_network.py +++ b/molecule/testinfra/app/test_app_network.py @@ -9,7 +9,7 @@ testinfra_hosts = [securedrop_test_vars.app_hostname] -@pytest.mark.skip_in_prod() +@pytest.mark.skip_in_prod def test_app_iptables_rules(host): local = host.get_host("local://") @@ -31,9 +31,7 @@ def test_app_iptables_rules(host): # Build iptables scrape cmd, purge comments + counters iptables = r"iptables-save | sed 's/ \[[0-9]*\:[0-9]*\]//g' | egrep -v '^#'" environment = os.environ.get("SECUREDROP_TESTINFRA_TARGET_HOST", "staging") - iptables_file = "{}/iptables-app-{}.j2".format( - os.path.dirname(os.path.abspath(__file__)), environment - ) + iptables_file = f"{os.path.dirname(os.path.abspath(__file__))}/iptables-app-{environment}.j2" # template out a local iptables jinja file jinja_iptables = Template(open(iptables_file).read()) diff --git a/molecule/testinfra/app/test_apparmor.py b/molecule/testinfra/app/test_apparmor.py index 2977bf6469f..5ce70ffaa30 100644 --- a/molecule/testinfra/app/test_apparmor.py +++ b/molecule/testinfra/app/test_apparmor.py @@ -103,7 +103,7 @@ def test_aastatus_unconfined(host): expected_unconfined = 0 unconfined_chk = str( - "{} processes are unconfined but have" " a profile defined".format(expected_unconfined) + f"{expected_unconfined} processes are unconfined but have" " a profile defined" ) with host.sudo(): aa_status_output = host.check_output("aa-status") diff --git a/molecule/testinfra/app/test_appenv.py b/molecule/testinfra/app/test_appenv.py index 17c0103141f..ead90c1a0ad 100644 --- a/molecule/testinfra/app/test_appenv.py +++ b/molecule/testinfra/app/test_appenv.py @@ -9,7 +9,7 @@ def test_app_pip_deps(host, exp_pip_pkg): """Ensure expected package versions are installed""" cmd = ( - "{}/bin/python3 -c \"from importlib.metadata import version; print(version('{}'))\"".format( # noqa + "{}/bin/python3 -c \"from importlib.metadata import version; print(version('{}'))\"".format( sdvars.securedrop_venv, exp_pip_pkg["name"] ) ) @@ -17,7 +17,7 @@ def test_app_pip_deps(host, exp_pip_pkg): assert result.stdout.strip() == exp_pip_pkg["version"] -@pytest.mark.skip_in_prod() +@pytest.mark.skip_in_prod def test_app_wsgi(host): """ensure logging is enabled for source interface in staging""" f = host.file("/var/www/source.wsgi") @@ -76,9 +76,8 @@ def test_app_code_venv(host): """ Ensure the securedrop-app-code virtualenv is correct. """ - cmd = """test -z $VIRTUAL_ENV && . {}/bin/activate && test "$VIRTUAL_ENV" = "{}" """.format( - sdvars.securedrop_venv, sdvars.securedrop_venv - ) + cmd = f"""test -z $VIRTUAL_ENV && . {sdvars.securedrop_venv}/bin/activate && " + "test "$VIRTUAL_ENV" = "{sdvars.securedrop_venv}" """ result = host.run(cmd) assert result.rc == 0 @@ -89,7 +88,7 @@ def test_supervisor_not_installed(host): assert host.package("supervisor").is_installed is False -@pytest.mark.skip_in_prod() +@pytest.mark.skip_in_prod def test_gpg_key_in_keyring(host): """ensure test gpg key is present in app keyring""" with host.sudo(sdvars.securedrop_user): diff --git a/molecule/testinfra/app/test_ossec_agent.py b/molecule/testinfra/app/test_ossec_agent.py index e972e8ab834..50d7071a727 100644 --- a/molecule/testinfra/app/test_ossec_agent.py +++ b/molecule/testinfra/app/test_ossec_agent.py @@ -39,7 +39,7 @@ def test_ossec_agent_installed(host): # Permissions don't match between Ansible and OSSEC deb packages postinst. -@pytest.mark.xfail() +@pytest.mark.xfail def test_ossec_keyfile_present(host): """ensure client keyfile for ossec-agent is present""" pattern = "^1024 {} {} [0-9a-f]{{64}}$".format( diff --git a/molecule/testinfra/app/test_smoke.py b/molecule/testinfra/app/test_smoke.py index 1f4c949ceab..b28361f1e2d 100644 --- a/molecule/testinfra/app/test_smoke.py +++ b/molecule/testinfra/app/test_smoke.py @@ -60,7 +60,7 @@ def test_redwood(host): assert len(parsed[2]) == 40 -@pytest.mark.skip_in_prod() +@pytest.mark.skip_in_prod def test_weak_submission_key(host): """ If the Submission Key is weak (e.g. has a SHA-1 signature), diff --git a/molecule/testinfra/app/test_tor_config.py b/molecule/testinfra/app/test_tor_config.py index c9422c87cff..ba6ce93fec1 100644 --- a/molecule/testinfra/app/test_tor_config.py +++ b/molecule/testinfra/app/test_tor_config.py @@ -68,7 +68,7 @@ def test_tor_torrc_sandbox(host): assert not f.contains("^.*Sandbox.*$") -@pytest.mark.skip_in_prod() +@pytest.mark.skip_in_prod def test_tor_v2_onion_url_file_absent(host): v2_url_filepath = "/var/lib/securedrop/source_v2_url" with host.sudo(): @@ -76,7 +76,7 @@ def test_tor_v2_onion_url_file_absent(host): assert not f.exists -@pytest.mark.skip_in_prod() +@pytest.mark.skip_in_prod def test_tor_v3_onion_url_readable_by_app(host): v3_url_filepath = "/var/lib/securedrop/source_v3_url" with host.sudo(): diff --git a/molecule/testinfra/app/test_tor_hidden_services.py b/molecule/testinfra/app/test_tor_hidden_services.py index ffe1431d6a2..38d22a16435 100644 --- a/molecule/testinfra/app/test_tor_hidden_services.py +++ b/molecule/testinfra/app/test_tor_hidden_services.py @@ -9,7 +9,7 @@ # Prod Tor services may have unexpected configs # TODO: read from admin workstation site-specific file if available -@pytest.mark.skip_in_prod() +@pytest.mark.skip_in_prod @pytest.mark.parametrize("tor_service", sdvars.tor_services) def test_tor_service_directories(host, tor_service): """ @@ -23,7 +23,7 @@ def test_tor_service_directories(host, tor_service): assert f.group == "debian-tor" -@pytest.mark.skip_in_prod() +@pytest.mark.skip_in_prod @pytest.mark.parametrize("tor_service", sdvars.tor_services) def test_tor_service_hostnames(host, tor_service): """ @@ -58,7 +58,7 @@ def test_tor_service_hostnames(host, tor_service): assert re.search(f"^{ths_hostname_regex_v3}$", f.content_string) -@pytest.mark.skip_in_prod() +@pytest.mark.skip_in_prod @pytest.mark.parametrize("tor_service", sdvars.tor_services) def test_tor_services_config(host, tor_service): """ diff --git a/molecule/testinfra/common/test_fpf_apt_repo.py b/molecule/testinfra/common/test_fpf_apt_repo.py index cfcde90a2c2..91b945ee438 100644 --- a/molecule/testinfra/common/test_fpf_apt_repo.py +++ b/molecule/testinfra/common/test_fpf_apt_repo.py @@ -28,9 +28,8 @@ def test_fpf_apt_repo_present(host): f = host.file("/etc/apt/sources.list.d/apt_test_freedom_press.list") else: f = host.file("/etc/apt/sources.list.d/apt_freedom_press.list") - repo_regex = r"^deb \[arch=amd64\] {} {} main$".format( - re.escape(test_vars.fpf_apt_repo_url), re.escape(host.system_info.codename) - ) + repo_regex = rf"^deb \[arch=amd64\] {re.escape(test_vars.fpf_apt_repo_url)} " + rf"{re.escape(host.system_info.codename)} main$" assert f.contains(repo_regex) @@ -61,7 +60,6 @@ def test_fpf_apt_repo_fingerprint(host): [ "pub 4096R/FC9F6818 2014-10-26 [expired: 2016-10-27]", "pub 4096R/00F4AD77 2016-10-20 [expired: 2017-10-20]", - "pub 4096R/00F4AD77 2016-10-20 [expired: 2017-10-20]", "pub 4096R/7B22E6A3 2021-05-10 [expired: 2022-07-04]", "pub 4096R/7B22E6A3 2021-05-10 [expired: 2023-07-04]", "pub 4096R/7B22E6A3 2021-05-10 [expired: 2024-07-08]", diff --git a/molecule/testinfra/common/test_grsecurity.py b/molecule/testinfra/common/test_grsecurity.py index 49364e7b45c..83d17f58eb8 100644 --- a/molecule/testinfra/common/test_grsecurity.py +++ b/molecule/testinfra/common/test_grsecurity.py @@ -113,9 +113,7 @@ def test_grsecurity_paxtest(host): paxtest_cmd += " | grep -P '^(Executable|Return)'" paxtest_results = host.check_output(paxtest_cmd) - paxtest_template_path = "{}/paxtest_results.j2".format( - os.path.dirname(os.path.abspath(__file__)) - ) + paxtest_template_path = f"{os.path.dirname(os.path.abspath(__file__))}/paxtest_results.j2" memcpy_result = "Killed" # Versions of paxtest newer than 0.9.12 or so will report diff --git a/molecule/testinfra/common/test_user_config.py b/molecule/testinfra/common/test_user_config.py index 3d703cd4428..339aaaa2d8a 100644 --- a/molecule/testinfra/common/test_user_config.py +++ b/molecule/testinfra/common/test_user_config.py @@ -88,7 +88,7 @@ def test_tmux_installed(host): assert host.package("tmux").is_installed -@pytest.mark.skip_in_prod() +@pytest.mark.skip_in_prod def test_sudoers_tmux_env_deprecated(host): """ Previous version of the Ansible config set the tmux config diff --git a/molecule/testinfra/mon/test_mon_network.py b/molecule/testinfra/mon/test_mon_network.py index a7a2bef3a81..92efb7e296f 100644 --- a/molecule/testinfra/mon/test_mon_network.py +++ b/molecule/testinfra/mon/test_mon_network.py @@ -9,7 +9,7 @@ testinfra_hosts = [securedrop_test_vars.monitor_hostname] -@pytest.mark.skip_in_prod() +@pytest.mark.skip_in_prod def test_mon_iptables_rules(host): local = host.get_host("local://") @@ -31,9 +31,7 @@ def test_mon_iptables_rules(host): # Build iptables scrape cmd, purge comments + counters iptables = r"iptables-save | sed 's/ \[[0-9]*\:[0-9]*\]//g' | egrep -v '^#'" environment = os.environ.get("SECUREDROP_TESTINFRA_TARGET_HOST", "staging") - iptables_file = "{}/iptables-mon-{}.j2".format( - os.path.dirname(os.path.abspath(__file__)), environment - ) + iptables_file = f"{os.path.dirname(os.path.abspath(__file__))}/iptables-mon-{environment}.j2" # template out a local iptables jinja file jinja_iptables = Template(open(iptables_file).read()) @@ -53,7 +51,7 @@ def test_mon_iptables_rules(host): assert iptables_expected == iptables -@pytest.mark.skip_in_prod() +@pytest.mark.skip_in_prod @pytest.mark.parametrize( "ossec_service", [ diff --git a/molecule/testinfra/mon/test_ossec_server.py b/molecule/testinfra/mon/test_ossec_server.py index 32534885a60..a515b836ef6 100644 --- a/molecule/testinfra/mon/test_ossec_server.py +++ b/molecule/testinfra/mon/test_ossec_server.py @@ -32,7 +32,7 @@ def test_ossec_service_start_style(host): # Permissions don't match between Ansible and OSSEC deb packages postinst. -@pytest.mark.xfail() +@pytest.mark.xfail @pytest.mark.parametrize( "keyfile", [ @@ -59,7 +59,7 @@ def test_ossec_keyfiles(host, keyfile): # Permissions don't match between Ansible and OSSEC deb packages postinst. -@pytest.mark.xfail() +@pytest.mark.xfail def test_procmail_log(host): """ Ensure procmail log file exist with proper ownership. diff --git a/molecule/testinfra/mon/test_postfix.py b/molecule/testinfra/mon/test_postfix.py index 3801bb9eafc..19dadd09460 100644 --- a/molecule/testinfra/mon/test_postfix.py +++ b/molecule/testinfra/mon/test_postfix.py @@ -37,11 +37,8 @@ def test_postfix_generic_maps(host): """ assert host.file("/etc/postfix/generic").exists assert host.file("/etc/postfix/generic").contains( - "^ossec@{} {}@{}".format( - securedrop_test_vars.monitor_hostname, - securedrop_test_vars.sasl_username, - securedrop_test_vars.sasl_domain, - ) + f"^ossec@{securedrop_test_vars.monitor_hostname} {securedrop_test_vars.sasl_username}@" + f"{securedrop_test_vars.sasl_domain}" ) assert host.file("/etc/postfix/main.cf").contains("^smtp_generic_maps") assert host.file("/etc/postfix/main.cf").contains( diff --git a/molecule/testinfra/ossec/test_journalist_mail.py b/molecule/testinfra/ossec/test_journalist_mail.py index bbdf850f846..f483e35000f 100644 --- a/molecule/testinfra/ossec/test_journalist_mail.py +++ b/molecule/testinfra/ossec/test_journalist_mail.py @@ -70,7 +70,7 @@ def test_procmail(self, host): ): # Look up CWD, in case tests move in the future current_dir = os.path.dirname(os.path.abspath(__file__)) - self.ansible(host, "copy", "dest=/tmp/{f} src={d}/{f}".format(f=f, d=current_dir)) + self.ansible(host, "copy", f"dest=/tmp/{f} src={current_dir}/{f}") assert self.run(host, "/var/ossec/process_submissions_today.sh forget") assert self.run(host, "postsuper -d ALL") assert self.run(host, f"cat /tmp/{f} | mail -s 'abc' root@localhost") @@ -96,10 +96,10 @@ def trigger(who, payload): assert self.run(host, f"! mailq | grep -q {who}@ossec.test") assert self.run( host, - """ + f""" ( echo 'Subject: TEST' ; echo ; echo -e '{payload}' ) | \ /var/ossec/send_encrypted_alarm.sh {who} - """.format(who=who, payload=payload), + """, ) assert self.wait_for_command(host, f"mailq | grep -q {who}@ossec.test") @@ -114,12 +114,12 @@ def trigger(who, payload): trigger(who, payload) assert self.run( host, - """ + f""" job=$(mailq | sed -n -e '2p' | cut -f1 -d ' ') postcat -q $job | tee /dev/stderr | \ gpg --homedir /var/ossec/.gnupg --decrypt 2>&1 | \ grep -q {expected} - """.format(expected=expected), + """, ) # # failure to encrypt must trigger an emergency mail to ossec contact diff --git a/pyproject.toml b/pyproject.toml index b13d7bf43af..8db51d45995 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -101,6 +101,8 @@ known-third-party = ["journalist_app", "management", "source_app", "tests"] "securedrop/pretty_bad_protocol/*.py" = [ # legacy code that still uses `assert` "S101", + # too much % formatting, not worth fixing for now + "UP031", ] [tool.mypy] diff --git a/securedrop/alembic/versions/3da3fcab826a_delete_orphaned_submissions.py b/securedrop/alembic/versions/3da3fcab826a_delete_orphaned_submissions.py index 19887fa014b..f81cc38be75 100644 --- a/securedrop/alembic/versions/3da3fcab826a_delete_orphaned_submissions.py +++ b/securedrop/alembic/versions/3da3fcab826a_delete_orphaned_submissions.py @@ -24,12 +24,12 @@ def raw_sql_grab_orphaned_objects(table_name: str) -> str: """Objects that have a source ID that doesn't exist in the sources table OR a NULL source ID should be deleted.""" - return ( # noqa: S608 - "SELECT id, filename, source_id FROM {table} " + return ( + f"SELECT id, filename, source_id FROM {table_name} " # noqa: S608 "WHERE source_id NOT IN (SELECT id FROM sources) " - "UNION SELECT id, filename, source_id FROM {table} " + f"UNION SELECT id, filename, source_id FROM {table_name} " "WHERE source_id IS NULL" - ).format(table=table_name) + ) def upgrade() -> None: diff --git a/securedrop/debian/ossec-common/var/ossec/checksdconfig.py b/securedrop/debian/ossec-common/var/ossec/checksdconfig.py index ecfb0859077..fa0b9c06ed8 100755 --- a/securedrop/debian/ossec-common/var/ossec/checksdconfig.py +++ b/securedrop/debian/ossec-common/var/ossec/checksdconfig.py @@ -32,7 +32,7 @@ def list_iptables_rules() -> dict: - result = subprocess.run(["iptables", "-S"], capture_output=True) + result = subprocess.run(["iptables", "-S"], capture_output=True, check=False) rules = result.stdout.decode("utf-8").splitlines() policies = [r for r in rules if r.startswith("-P")] input_rules = [r for r in rules if r.startswith("-A INPUT")] diff --git a/securedrop/i18n.py b/securedrop/i18n.py index 596ca74076a..0b281b6e924 100644 --- a/securedrop/i18n.py +++ b/securedrop/i18n.py @@ -114,9 +114,8 @@ def configure_babel(config: SecureDropConfig, app: Flask) -> Babel: # verify that Babel is only using the translations we told it about if list(babel.translation_directories) != [translations_directory]: raise ValueError( - "Babel translation directories ({}) do not match SecureDrop configuration ({})".format( - babel.translation_directories, [translations_directory] - ) + f"Babel translation directories ({babel.translation_directories}) do not match " + f"SecureDrop configuration ({[translations_directory]})" ) # register the function used to determine the locale of a request diff --git a/securedrop/journalist_app/admin.py b/securedrop/journalist_app/admin.py index b823fb0cdad..606ed43d58c 100644 --- a/securedrop/journalist_app/admin.py +++ b/securedrop/journalist_app/admin.py @@ -216,7 +216,7 @@ def add_user() -> Union[str, werkzeug.Response]: ), "error", ) - current_app.logger.error("Adding user " "'{}' failed: {}".format(username, e)) + current_app.logger.error("Adding user " f"'{username}' failed: {e}") if form_valid: if new_user.is_totp: @@ -423,9 +423,8 @@ def delete_user(user_id: int) -> werkzeug.Response: abort(403) elif not user: current_app.logger.error( - "Admin {} tried to delete nonexistent user with pk={}".format( - session.get_user().username, user_id - ) + f"Admin {session.get_user().username} tried to delete nonexistent user with " + f"pk={user_id}" ) abort(404) elif user.is_deleted_user(): @@ -456,9 +455,8 @@ def new_password(user_id: int) -> werkzeug.Response: if user.id == session.get_uid(): current_app.logger.error( - "Admin {} tried to change their password without validation.".format( - session.get_user().username - ) + f"Admin {session.get_user().username} tried to change their password without " + "validation." ) abort(403) diff --git a/securedrop/journalist_app/main.py b/securedrop/journalist_app/main.py index 1115bed5255..b6a319b0d14 100644 --- a/securedrop/journalist_app/main.py +++ b/securedrop/journalist_app/main.py @@ -138,9 +138,7 @@ def reply() -> werkzeug.Response: return redirect(url_for("col.col", filesystem_id=g.filesystem_id)) g.source.interaction_count += 1 - filename = "{}-{}-reply.gpg".format( - g.source.interaction_count, g.source.journalist_filename - ) + filename = f"{g.source.interaction_count}-{g.source.journalist_filename}-reply.gpg" EncryptionManager.get_default().encrypt_journalist_reply( for_source=g.source, reply_in=form.message.data, @@ -163,9 +161,8 @@ def reply() -> werkzeug.Response: # with responses to sources. It's possible the exception message # could contain information we don't want to write to disk. current_app.logger.error( - "Reply from '{}' (ID {}) failed: {}!".format( - session.get_user().username, session.get_uid(), exc.__class__ - ) + f"Reply from '{session.get_user().username}' (ID {session.get_uid()}) " + f"failed: {exc.__class__}!" ) else: flash( diff --git a/securedrop/loaddata.py b/securedrop/loaddata.py index 241e00f4353..ffd18091db6 100755 --- a/securedrop/loaddata.py +++ b/securedrop/loaddata.py @@ -381,16 +381,10 @@ def add_sources(args: argparse.Namespace, journalists: Tuple[Journalist, ...]) - add_reply(source, journalist_who_replied, journalist_who_saw) print( - "Created source {}/{} (codename: '{}', journalist designation '{}', " - "files: {}, messages: {}, replies: {})".format( - i, - args.source_count, - codename, - source.journalist_designation, - args.files_per_source, - args.messages_per_source, - args.replies_per_source if i <= replied_sources_count else 0, - ) + f"Created source {i}/{args.source_count} (codename: '{codename}', " + f"journalist designation '{source.journalist_designation}', " + f"files: {args.files_per_source}, messages: {args.messages_per_source}, " + f"replies: {args.replies_per_source if i <= replied_sources_count else 0})" ) diff --git a/securedrop/manage.py b/securedrop/manage.py index 01ce1512e23..169e0921acf 100755 --- a/securedrop/manage.py +++ b/securedrop/manage.py @@ -19,18 +19,18 @@ sys.path.insert(0, "/var/www/securedrop") -import qrcode # noqa: E402 -from sqlalchemy.orm.exc import NoResultFound # noqa: E402 +import qrcode +from sqlalchemy.orm.exc import NoResultFound if not os.environ.get("SECUREDROP_ENV"): os.environ["SECUREDROP_ENV"] = "dev" -from db import db # noqa: E402 -from management import SecureDropConfig, app_context # noqa: E402 -from management.run import run # noqa: E402 -from management.sources import remove_pending_sources # noqa: E402 -from management.submissions import ( # noqa: E402 +from db import db +from management import SecureDropConfig, app_context +from management.run import run +from management.sources import remove_pending_sources +from management.submissions import ( add_check_db_disconnect_parser, add_check_fs_disconnect_parser, add_delete_db_disconnect_parser, @@ -39,7 +39,7 @@ add_list_fs_disconnect_parser, add_were_there_submissions_today, ) -from models import FirstOrLastNameError, InvalidUsernameException, Journalist # noqa: E402 +from models import FirstOrLastNameError, InvalidUsernameException, Journalist logging.basicConfig(format="%(asctime)s %(levelname)s %(message)s") log = logging.getLogger(__name__) @@ -197,8 +197,8 @@ def _add_user(is_admin: bool = False, context: Optional[AppContext] = None) -> i if len(tmp_str) != 40: print( "The length of the secret is not correct. " - "Expected 40 characters, but received {}. " - "Try again.".format(len(tmp_str)) + f"Expected 40 characters, but received {len(tmp_str)}. " + "Try again." ) continue if otp_secret: @@ -239,7 +239,7 @@ def _add_user(is_admin: bool = False, context: Optional[AppContext] = None) -> i 'you will need to change the "Non-ASCII Font", which ' "is your profile's Text settings.\n\nCan't scan the " "barcode? Enter following shared secret manually:" - "\n{}\n".format(user.formatted_otp_secret) + f"\n{user.formatted_otp_secret}\n" ) return 0 @@ -249,11 +249,9 @@ def _get_username_to_delete() -> str: def _get_delete_confirmation(username: str) -> bool: - confirmation = obtain_input( - "Are you sure you want to delete user " '"{}" (y/n)?'.format(username) - ) + confirmation = obtain_input("Are you sure you want to delete user " f'"{username}" (y/n)?') if confirmation.lower() != "y": - print('Confirmation not received: user "{}" was NOT ' "deleted".format(username)) + print(f'Confirmation not received: user "{username}" was NOT ' "deleted") return False return True @@ -414,15 +412,13 @@ def set_clean_tmp_parser(subps: _SubParsersAction, name: str) -> None: default=default_days, type=int, help=( - "remove files not modified in a given number of DAYS " "(default {} days)".format( - default_days - ) + "remove files not modified in a given number of DAYS " f"(default {default_days} days)" ), ) parser.add_argument( "--directory", default=config.TEMP_DIR, - help=("remove old files from DIRECTORY " "(default {})".format(config.TEMP_DIR)), + help=("remove old files from DIRECTORY " f"(default {config.TEMP_DIR})"), ) parser.set_defaults(func=clean_tmp) diff --git a/securedrop/management/submissions.py b/securedrop/management/submissions.py index 5eee374d295..5217b6c4352 100644 --- a/securedrop/management/submissions.py +++ b/securedrop/management/submissions.py @@ -160,9 +160,8 @@ def delete_disconnected_fs_submissions(args: argparse.Namespace) -> None: time_elapsed += file_elapsed rate = bytes_deleted / time_elapsed print( - "elapsed: {:.2f}s rate: {:.1f} MB/s overall rate: {:.1f} MB/s".format( - file_elapsed, filesize / 1048576 / file_elapsed, rate / 1048576 - ) + f"elapsed: {file_elapsed:.2f}s rate: {filesize / 1048576 / file_elapsed:.1f} " + f"MB/s overall rate: {rate / 1048576:.1f} MB/s" ) else: print(f"Not removing {f}.") diff --git a/securedrop/models.py b/securedrop/models.py index c0cd4836c2d..badd5b7e590 100644 --- a/securedrop/models.py +++ b/securedrop/models.py @@ -35,12 +35,7 @@ def get_one_or_else( try: return query.one() except MultipleResultsFound as e: - logger.error( - "Found multiple while executing {} when one was expected: {}".format( - query, - e, - ) - ) + logger.error(f"Found multiple while executing {query} when one was expected: {e}") failure_method(500) except NoResultFound as e: logger.error(f"Found none when one was expected: {e}") @@ -87,7 +82,7 @@ def __init__( self.uuid = str(uuid.uuid4()) def __repr__(self) -> str: - return "" % (self.journalist_designation) + return f"" @property def journalist_filename(self) -> str: @@ -194,7 +189,7 @@ def __init__(self, source: Source, filename: str, storage: Storage) -> None: self.size = os.stat(storage.path(source.filesystem_id, filename)).st_size def __repr__(self) -> str: - return "" % (self.filename) + return f"" @property def is_file(self) -> bool: @@ -254,10 +249,7 @@ def seen(self) -> bool: If the submission has been downloaded or seen by any journalist, then the submission is considered seen. """ - if self.downloaded or self.seen_files.count() or self.seen_messages.count(): - return True - - return False + return bool(self.downloaded or self.seen_files.count() or self.seen_messages.count()) class Reply(db.Model): @@ -292,7 +284,7 @@ def __init__( self.size = os.stat(storage.path(source.filesystem_id, filename)).st_size def __repr__(self) -> str: - return "" % (self.filename) + return f"" def to_json(self) -> "Dict[str, Any]": seen_by = [r.journalist.uuid for r in SeenReply.query.filter(SeenReply.reply_id == self.id)] @@ -323,7 +315,7 @@ class SourceStar(db.Model): source_id = Column("source_id", Integer, ForeignKey("sources.id")) starred = Column("starred", Boolean, default=True) - def __eq__(self, other: "Any") -> bool: + def __eq__(self, other: object) -> bool: if isinstance(other, SourceStar): return ( self.source_id == other.source_id @@ -656,9 +648,8 @@ def throttle_login(cls, user: "Journalist") -> None: ) if len(attempts_within_period) > cls._MAX_LOGIN_ATTEMPTS_PER_PERIOD: raise LoginThrottledException( - "throttled ({} attempts in last {} seconds)".format( - len(attempts_within_period), cls._LOGIN_ATTEMPT_PERIOD - ) + f"throttled ({len(attempts_within_period)} attempts in last " + f"{cls._LOGIN_ATTEMPT_PERIOD} seconds)" ) @classmethod @@ -862,16 +853,11 @@ class InstanceConfig(db.Model): def __repr__(self) -> str: return ( - "".format( - self.version, - self.valid_until, - self.allow_document_uploads, - self.organization_name, - self.initial_message_min_len, - self.reject_message_with_codename, - ) + f"" ) def copy(self) -> "InstanceConfig": diff --git a/securedrop/passphrases.py b/securedrop/passphrases.py index 8f9d517a8a0..93282e88c38 100644 --- a/securedrop/passphrases.py +++ b/securedrop/passphrases.py @@ -46,12 +46,9 @@ def __init__( word_list_size = len(word_list) if word_list_size < self._WORD_LIST_MINIMUM_SIZE: raise InvalidWordListError( - "The word list for language '{}' only contains {} long-enough words;" - " minimum required is {} words.".format( - language, - word_list_size, - self._WORD_LIST_MINIMUM_SIZE, - ) + f"The word list for language '{language}' only contains {word_list_size}" + " long-enough words;" + f" minimum required is {self._WORD_LIST_MINIMUM_SIZE} words." ) # Ensure all words are ascii @@ -68,14 +65,11 @@ def __init__( longest_passphrase_length += self.PASSPHRASE_WORDS_COUNT # One space between each word if longest_passphrase_length >= self.MAX_PASSPHRASE_LENGTH: raise InvalidWordListError( - "Passphrases over the maximum length ({}) may be generated:" - " longest word in word list for language '{}' is '{}' and number of words per" - " passphrase is {}".format( - self.MAX_PASSPHRASE_LENGTH, - language, - longest_word, - self.PASSPHRASE_WORDS_COUNT, - ) + f"Passphrases over the maximum length ({self.MAX_PASSPHRASE_LENGTH}) " + "may be generated:" + f" longest word in word list for language '{language}' is '{longest_word}' " + "and number of words per" + f" passphrase is {self.PASSPHRASE_WORDS_COUNT}" ) # Ensure that passphrases shorter than what's supported can't be generated @@ -84,14 +78,11 @@ def __init__( shortest_passphrase_length += self.PASSPHRASE_WORDS_COUNT if shortest_passphrase_length <= self.MIN_PASSPHRASE_LENGTH: raise InvalidWordListError( - "Passphrases under the minimum length ({}) may be generated:" - " shortest word in word list for language '{}' is '{}' and number of words per" - " passphrase is {}".format( - self.MIN_PASSPHRASE_LENGTH, - language, - shortest_word, - self.PASSPHRASE_WORDS_COUNT, - ) + f"Passphrases under the minimum length ({self.MIN_PASSPHRASE_LENGTH}) " + "may be generated:" + f" shortest word in word list for language '{language}' is '{shortest_word}' " + "and number of words per" + f" passphrase is {self.PASSPHRASE_WORDS_COUNT}" ) @classmethod diff --git a/securedrop/pretty_bad_protocol/_meta.py b/securedrop/pretty_bad_protocol/_meta.py index 52509d8dc2c..55e68a18fa1 100644 --- a/securedrop/pretty_bad_protocol/_meta.py +++ b/securedrop/pretty_bad_protocol/_meta.py @@ -717,7 +717,7 @@ def _set_verbose(self, verbose): # type: ignore[no-untyped-def] self.verbose = verbose - def _collect_output(self, process, result, writer=None, stdin=None): # type: ignore[no-untyped-def] # noqa + def _collect_output(self, process, result, writer=None, stdin=None): # type: ignore[no-untyped-def] """Drain the subprocesses output streams, writing the collected output to the result. If a writer thread (writing to the subprocess) is given, make sure it's joined before returning. If a stdin stream is given, @@ -748,7 +748,7 @@ def _collect_output(self, process, result, writer=None, stdin=None): # type: ig stderr.close() stdout.close() - def _handle_io(self, args, file, result, passphrase=False, binary=False): # type: ignore[no-untyped-def] # noqa + def _handle_io(self, args, file, result, passphrase=False, binary=False): # type: ignore[no-untyped-def] """Handle a call to GPG - pass input data, collect output data.""" p = self._open_subprocess(args, passphrase) if not binary: @@ -1041,7 +1041,7 @@ def _encrypt( # type: ignore[no-untyped-def] return result - def _add_recipient_string(self, args, hidden_recipients, recipient): # type: ignore[no-untyped-def] # noqa + def _add_recipient_string(self, args, hidden_recipients, recipient): # type: ignore[no-untyped-def] if isinstance(hidden_recipients, (list, tuple)): if [s for s in hidden_recipients if recipient in str(s)]: args.append("--hidden-recipient %s" % recipient) diff --git a/securedrop/pretty_bad_protocol/_parsers.py b/securedrop/pretty_bad_protocol/_parsers.py index 47fa8ed3eb0..6e26e4d1fcf 100644 --- a/securedrop/pretty_bad_protocol/_parsers.py +++ b/securedrop/pretty_bad_protocol/_parsers.py @@ -230,9 +230,7 @@ def _is_hex(string): # type: ignore[no-untyped-def] :param str string: The string to check. """ - if HEXADECIMAL.match(string): - return True - return False + return bool(HEXADECIMAL.match(string)) def _sanitise(*args): # type: ignore[no-untyped-def] @@ -1004,9 +1002,7 @@ def __init__(self, gpg): # type: ignore[no-untyped-def] self.secring = None def __nonzero__(self): # type: ignore[no-untyped-def] - if self.fingerprint: - return True - return False + return bool(self.fingerprint) __bool__ = __nonzero__ @@ -1299,7 +1295,7 @@ def __init__(self, gpg): # type: ignore[no-untyped-def] #: Counts of all the status message results, :data:`_fields` which #: have appeared. - self.counts = OrderedDict(zip(self._fields, [int(0) for x in range(len(self._fields))])) + self.counts = OrderedDict(zip(self._fields, [0 for x in range(len(self._fields))])) #: A list of strings containing the fingerprints of the GnuPG keyIDs #: imported. @@ -1317,9 +1313,7 @@ def __nonzero__(self): # type: ignore[no-untyped-def] """ if self.counts["not_imported"] > 0: return False - if len(self.fingerprints) == 0: - return False - return True + return len(self.fingerprints) != 0 __bool__ = __nonzero__ @@ -1407,7 +1401,7 @@ def __init__(self, gpg): # type: ignore[no-untyped-def] #: Counts of all the status message results, :data:`_fields` which #: have appeared. - self.counts = OrderedDict(zip(self._fields, [int(0) for x in range(len(self._fields))])) + self.counts = OrderedDict(zip(self._fields, [0 for x in range(len(self._fields))])) #: A list of strings containing the fingerprints of the GnuPG keyIDs #: exported. @@ -1421,9 +1415,7 @@ def __nonzero__(self): # type: ignore[no-untyped-def] """ if self.counts["not_imported"] > 0: return False - if len(self.fingerprints) == 0: - return False - return True + return len(self.fingerprints) != 0 __bool__ = __nonzero__ @@ -1784,9 +1776,7 @@ def __init__(self, gpg): # type: ignore[no-untyped-def] self.data_filename = None def __nonzero__(self): # type: ignore[no-untyped-def] - if self.ok: - return True - return False + return self.ok __bool__ = __nonzero__ diff --git a/securedrop/pretty_bad_protocol/_util.py b/securedrop/pretty_bad_protocol/_util.py index d90b83082ae..72d81cb6560 100644 --- a/securedrop/pretty_bad_protocol/_util.py +++ b/securedrop/pretty_bad_protocol/_util.py @@ -399,7 +399,7 @@ def _threaded_copy_data(instream, outstream): # type: ignore[no-untyped-def] return copy_thread -def _which(executable, flags=os.X_OK, abspath_only=False, disallow_symlinks=False): # type: ignore[no-untyped-def] # noqa +def _which(executable, flags=os.X_OK, abspath_only=False, disallow_symlinks=False): # type: ignore[no-untyped-def] """Borrowed from Twisted's :mod:twisted.python.proutils . Search PATH for executable files with the given name. @@ -468,7 +468,7 @@ def _write_passphrase(stream, passphrase, encoding): # type: ignore[no-untyped- class InheritableProperty: """Based on the emulation of PyProperty_Type() in Objects/descrobject.c""" - def __init__(self, fget=None, fset=None, fdel=None, doc=None): # type: ignore[no-untyped-def] # noqa + def __init__(self, fget=None, fset=None, fdel=None, doc=None): # type: ignore[no-untyped-def] self.fget = fget self.fset = fset self.fdel = fdel diff --git a/securedrop/pretty_bad_protocol/gnupg.py b/securedrop/pretty_bad_protocol/gnupg.py index cf875c1cfd0..50ed25cea85 100644 --- a/securedrop/pretty_bad_protocol/gnupg.py +++ b/securedrop/pretty_bad_protocol/gnupg.py @@ -130,32 +130,20 @@ def __init__( # type: ignore[no-untyped-def] log.info( textwrap.dedent( - """ + f""" Initialised settings: - binary: {} - binary version: {} - homedir: {} - ignore_homedir_permissions: {} - keyring: {} - secring: {} - default_preference_list: {} - keyserver: {} - options: {} - verbose: {} - use_agent: {} - """.format( - self.binary, - self.binary_version, - self.homedir, - self.ignore_homedir_permissions, - self.keyring, - self.secring, - self.default_preference_list, - self.keyserver, - self.options, - str(self.verbose), - str(self.use_agent), - ) + binary: {self.binary} + binary version: {self.binary_version} + homedir: {self.homedir} + ignore_homedir_permissions: {self.ignore_homedir_permissions} + keyring: {self.keyring} + secring: {self.secring} + default_preference_list: {self.default_preference_list} + keyserver: {self.keyserver} + options: {self.options} + verbose: {str(self.verbose)} + use_agent: {str(self.use_agent)} + """ ) ) @@ -374,7 +362,7 @@ def recv_keys(self, *keyids, **kwargs): # type: ignore[no-untyped-def] else: log.error("No keyids requested for --recv-keys!") - def delete_keys(self, fingerprints, secret=False, subkeys=False): # type: ignore[no-untyped-def] # noqa + def delete_keys(self, fingerprints, secret=False, subkeys=False): # type: ignore[no-untyped-def] """Delete a key, or list of keys, from the current keyring. The keys must be referred to by their full fingerprints for GnuPG to @@ -410,7 +398,7 @@ def delete_keys(self, fingerprints, secret=False, subkeys=False): # type: ignor self._collect_output(p, result, stdin=p.stdin) return result - def export_keys(self, keyids, secret=False, subkeys=False, passphrase=None): # type: ignore[no-untyped-def] # noqa + def export_keys(self, keyids, secret=False, subkeys=False, passphrase=None): # type: ignore[no-untyped-def] """Export the indicated ``keyids``. :param str keyids: A keyid or fingerprint in any format that GnuPG will @@ -492,7 +480,7 @@ def list_packets(self, raw_data): # type: ignore[no-untyped-def] self._handle_io(args, _make_binary_stream(raw_data, self._encoding), result) return result - def sign_key(self, keyid, default_key=None, passphrase=None): # type: ignore[no-untyped-def] # noqa + def sign_key(self, keyid, default_key=None, passphrase=None): # type: ignore[no-untyped-def] """sign (an imported) public key - keyid, with default secret key >>> import gnupg @@ -588,7 +576,7 @@ def _parse_keys(self, result): # type: ignore[no-untyped-def] if keyword in valid_keywords: getattr(result, keyword)(L) - def expire(self, keyid, expiration_time="1y", passphrase=None, expire_subkeys=True): # type: ignore[no-untyped-def] # noqa + def expire(self, keyid, expiration_time="1y", passphrase=None, expire_subkeys=True): # type: ignore[no-untyped-def] """Changes GnuPG key expiration by passing in new time period (from now) through subprocess's stdin @@ -679,7 +667,7 @@ def gen_key(self, input): # type: ignore[no-untyped-def] return key - def gen_key_input(self, separate_keyring=False, save_batchfile=False, testing=False, **kwargs): # type: ignore[no-untyped-def] # noqa + def gen_key_input(self, separate_keyring=False, save_batchfile=False, testing=False, **kwargs): # type: ignore[no-untyped-def] """Generate a batch file for input to :meth:`~gnupg.GPG.gen_key`. The GnuPG batch file key generation feature allows unattended key @@ -958,11 +946,11 @@ def gen_key_input(self, separate_keyring=False, save_batchfile=False, testing=Fa else: docs = "" # docstring=None if run with "python -OO" links = "\n".join(x.strip() for x in docs.splitlines()[-2:]) - explain = """ -This directory was created by python-gnupg, on {}, and + explain = f""" +This directory was created by python-gnupg, on {_util.now()}, and it contains saved batch files, which can be given to GnuPG to automatically generate keys. Please see -{}""".format(_util.now(), links) # sometimes python is awesome. +{links}""" # sometimes python is awesome. with open(readme, "a+") as fh: [fh.write(line) for line in explain] @@ -1079,7 +1067,7 @@ def decrypt(self, message, **kwargs): # type: ignore[no-untyped-def] stream.close() return result - def decrypt_file(self, filename, always_trust=False, passphrase=None, output=None): # type: ignore[no-untyped-def] # noqa + def decrypt_file(self, filename, always_trust=False, passphrase=None, output=None): # type: ignore[no-untyped-def] """Decrypt the contents of a file-like object ``filename`` . :param str filename: A file-like object to decrypt. diff --git a/securedrop/scripts/rqrequeue b/securedrop/scripts/rqrequeue index c6ebc33d566..92a8838f044 100755 --- a/securedrop/scripts/rqrequeue +++ b/securedrop/scripts/rqrequeue @@ -8,8 +8,8 @@ import time sys.path.insert(0, "/var/www/securedrop") -from sdconfig import SecureDropConfig # noqa: E402 -from worker import requeue_interrupted_jobs # noqa: E402 +from sdconfig import SecureDropConfig +from worker import requeue_interrupted_jobs def parse_args(): diff --git a/securedrop/scripts/shredder b/securedrop/scripts/shredder index 0a5f2e9c8f5..7ebf322a04d 100755 --- a/securedrop/scripts/shredder +++ b/securedrop/scripts/shredder @@ -11,9 +11,9 @@ import time sys.path.insert(0, "/var/www/securedrop") -import journalist_app # noqa: E402 -from sdconfig import SecureDropConfig # noqa: E402 -from store import Storage # noqa: E402 +import journalist_app +from sdconfig import SecureDropConfig +from store import Storage def parse_args(): diff --git a/securedrop/scripts/source_deleter b/securedrop/scripts/source_deleter index 7082476729a..ec9c5b1595e 100755 --- a/securedrop/scripts/source_deleter +++ b/securedrop/scripts/source_deleter @@ -11,8 +11,8 @@ import time sys.path.insert(0, "/var/www/securedrop") -import journalist_app # noqa: E402 -from sdconfig import SecureDropConfig # noqa: E402 +import journalist_app +from sdconfig import SecureDropConfig def parse_args(): diff --git a/securedrop/source_app/main.py b/securedrop/source_app/main.py index 55be14ef380..605b4477c4c 100644 --- a/securedrop/source_app/main.py +++ b/securedrop/source_app/main.py @@ -169,9 +169,9 @@ def lookup(logged_in_source: SourceUser) -> str: ) reply.decrypted = decrypted_reply except UnicodeDecodeError: - current_app.logger.error("Could not decode reply %s" % reply.filename) + current_app.logger.error(f"Could not decode reply {reply.filename}") except FileNotFoundError: - current_app.logger.error("Reply file missing: %s" % reply.filename) + current_app.logger.error(f"Reply file missing: {reply.filename}") except (GpgDecryptError, RedwoodError) as e: current_app.logger.error(f"Could not decrypt reply {reply.filename}: {str(e)}") else: @@ -253,9 +253,8 @@ def submit(logged_in_source: SourceUser) -> werkzeug.Response: if not os.path.exists(Storage.get_default().path(logged_in_source.filesystem_id)): current_app.logger.debug( - "Store directory not found for source '{}', creating one.".format( - logged_in_source_in_db.journalist_designation - ) + f"Store directory not found for source " + f"'{logged_in_source_in_db.journalist_designation}', creating one." ) os.mkdir(Storage.get_default().path(logged_in_source.filesystem_id)) diff --git a/securedrop/store.py b/securedrop/store.py index cc89435b1ce..7cc4da8a030 100644 --- a/securedrop/store.py +++ b/securedrop/store.py @@ -157,9 +157,8 @@ def path(self, filesystem_id: str, filename: str = "") -> str: absolute = os.path.realpath(joined) if not self.verify(absolute): raise PathException( - """Could not resolve ("{}", "{}") to a path within the store.""".format( - filesystem_id, filename - ) + f'Could not resolve ("{filesystem_id}", "{filename}") to a path within ' + "the store." ) return absolute diff --git a/securedrop/tests/conftest.py b/securedrop/tests/conftest.py index 4822ad8e9a5..2c5cf42a955 100644 --- a/securedrop/tests/conftest.py +++ b/securedrop/tests/conftest.py @@ -99,7 +99,7 @@ def setup_journalist_key_and_gpg_folder() -> Generator[Tuple[str, Path], None, N shutil.rmtree(tmp_gpg_dir, ignore_errors=True) -@pytest.fixture() +@pytest.fixture def config( setup_journalist_key_and_gpg_folder: Tuple[str, Path], setup_rqworker: Tuple[str, str], @@ -119,7 +119,7 @@ def config( yield config -@pytest.fixture() +@pytest.fixture def alembic_config(config: SecureDropConfig) -> Generator[Path, None, None]: base_dir = path.join(path.dirname(__file__), "..") migrations_dir = path.join(base_dir, "alembic") @@ -148,12 +148,12 @@ def alembic_config(config: SecureDropConfig) -> Generator[Path, None, None]: os.environ["SECUREDROP_ENV"] = previous_env_value -@pytest.fixture() +@pytest.fixture def app_storage(config: SecureDropConfig) -> "Storage": return Storage(str(config.STORE_DIR), str(config.TEMP_DIR)) -@pytest.fixture() +@pytest.fixture def source_app(config: SecureDropConfig, app_storage: Storage) -> Generator[Flask, None, None]: with mock.patch("store.Storage.get_default") as mock_storage_global: mock_storage_global.return_value = app_storage @@ -168,7 +168,7 @@ def source_app(config: SecureDropConfig, app_storage: Storage) -> Generator[Flas db.drop_all() -@pytest.fixture() +@pytest.fixture def journalist_app(config: SecureDropConfig, app_storage: Storage) -> Generator[Flask, None, None]: with mock.patch("store.Storage.get_default") as mock_storage_global: mock_storage_global.return_value = app_storage @@ -183,7 +183,7 @@ def journalist_app(config: SecureDropConfig, app_storage: Storage) -> Generator[ db.drop_all() -@pytest.fixture() +@pytest.fixture def test_journo(journalist_app: Flask) -> Dict[str, Any]: with journalist_app.app_context(): user, password = utils.db_helper.init_journalist(is_admin=False) @@ -201,7 +201,7 @@ def test_journo(journalist_app: Flask) -> Dict[str, Any]: } -@pytest.fixture() +@pytest.fixture def test_admin(journalist_app: Flask) -> Dict[str, Any]: with journalist_app.app_context(): user, password = utils.db_helper.init_journalist(is_admin=True) @@ -216,7 +216,7 @@ def test_admin(journalist_app: Flask) -> Dict[str, Any]: } -@pytest.fixture() +@pytest.fixture def test_source(journalist_app: Flask, app_storage: Storage) -> Dict[str, Any]: with journalist_app.app_context(): passphrase = PassphraseGenerator.get_default().generate_passphrase() @@ -237,7 +237,7 @@ def test_source(journalist_app: Flask, app_storage: Storage) -> Dict[str, Any]: } -@pytest.fixture() +@pytest.fixture def test_submissions(journalist_app: Flask, app_storage: Storage) -> Dict[str, Any]: with journalist_app.app_context(): source, codename = utils.db_helper.init_source(app_storage) @@ -251,7 +251,7 @@ def test_submissions(journalist_app: Flask, app_storage: Storage) -> Dict[str, A } -@pytest.fixture() +@pytest.fixture def test_files(journalist_app, test_journo, app_storage): with journalist_app.app_context(): source, codename = utils.db_helper.init_source(app_storage) @@ -267,7 +267,7 @@ def test_files(journalist_app, test_journo, app_storage): } -@pytest.fixture() +@pytest.fixture def test_files_deleted_journalist(journalist_app, test_journo, app_storage): with journalist_app.app_context(): source, codename = utils.db_helper.init_source(app_storage) @@ -286,7 +286,7 @@ def test_files_deleted_journalist(journalist_app, test_journo, app_storage): } -@pytest.fixture() +@pytest.fixture def journalist_api_token(journalist_app, test_journo): with journalist_app.test_client() as app: valid_token = TOTP(test_journo["otp_secret"]).now() diff --git a/securedrop/tests/functional/conftest.py b/securedrop/tests/functional/conftest.py index 2feead5a08a..d40a000e61a 100644 --- a/securedrop/tests/functional/conftest.py +++ b/securedrop/tests/functional/conftest.py @@ -21,14 +21,14 @@ # Function-scoped so that tests can be run in parallel if needed -@pytest.fixture() +@pytest.fixture def firefox_web_driver() -> WebDriver: # type: ignore with get_web_driver(web_driver_type=WebDriverTypeEnum.FIREFOX) as web_driver: yield web_driver # Function-scoped so that tests can be run in parallel if needed -@pytest.fixture() +@pytest.fixture def tor_browser_web_driver() -> WebDriver: # type: ignore with get_web_driver(web_driver_type=WebDriverTypeEnum.TOR_BROWSER) as web_driver: yield web_driver @@ -216,7 +216,7 @@ def sd_servers( yield sd_servers_result -@pytest.fixture() +@pytest.fixture def sd_servers_with_clean_state( setup_journalist_key_and_gpg_folder: Tuple[str, Path], setup_rqworker: Tuple[str, Path], @@ -239,7 +239,7 @@ def sd_servers_with_clean_state( yield sd_servers_result -@pytest.fixture() +@pytest.fixture def sd_servers_with_submitted_file( setup_journalist_key_and_gpg_folder: Tuple[str, Path], setup_rqworker: Tuple[str, Path], diff --git a/securedrop/tests/functional/pageslayout/test_journalist.py b/securedrop/tests/functional/pageslayout/test_journalist.py index 34d4a170297..72a847f198f 100644 --- a/securedrop/tests/functional/pageslayout/test_journalist.py +++ b/securedrop/tests/functional/pageslayout/test_journalist.py @@ -21,7 +21,7 @@ @pytest.mark.parametrize("locale", list_locales()) -@pytest.mark.pagelayout() +@pytest.mark.pagelayout class TestJournalistLayout: def test_login_index_and_edit(self, locale, sd_servers, firefox_web_driver): # Given an SD server diff --git a/securedrop/tests/functional/pageslayout/test_journalist_account.py b/securedrop/tests/functional/pageslayout/test_journalist_account.py index 376bade7d1a..887ff0ba380 100644 --- a/securedrop/tests/functional/pageslayout/test_journalist_account.py +++ b/securedrop/tests/functional/pageslayout/test_journalist_account.py @@ -25,7 +25,7 @@ @pytest.mark.parametrize("locale", list_locales()) -@pytest.mark.pagelayout() +@pytest.mark.pagelayout class TestJournalistLayoutAccount: def test_account_edit_and_set_hotp_secret( self, locale, sd_servers_with_clean_state, firefox_web_driver diff --git a/securedrop/tests/functional/pageslayout/test_journalist_admin.py b/securedrop/tests/functional/pageslayout/test_journalist_admin.py index 90a6f85635d..40c44f4dd95 100644 --- a/securedrop/tests/functional/pageslayout/test_journalist_admin.py +++ b/securedrop/tests/functional/pageslayout/test_journalist_admin.py @@ -29,7 +29,7 @@ @pytest.mark.parametrize("locale", list_locales()) -@pytest.mark.pagelayout() +@pytest.mark.pagelayout class TestAdminLayoutAddAndEditUser: def test_admin_adds_user_hotp_and_edits_hotp( self, locale, sd_servers_with_clean_state, firefox_web_driver @@ -224,7 +224,7 @@ def _retry_2fa_pop_ups( @pytest.mark.parametrize("locale", list_locales()) -@pytest.mark.pagelayout() +@pytest.mark.pagelayout class TestAdminLayoutEditConfig: def test_admin_changes_logo(self, locale, sd_servers_with_clean_state, firefox_web_driver): # Given an SD server diff --git a/securedrop/tests/functional/pageslayout/test_journalist_col.py b/securedrop/tests/functional/pageslayout/test_journalist_col.py index dbf692e35de..fd39240ff89 100644 --- a/securedrop/tests/functional/pageslayout/test_journalist_col.py +++ b/securedrop/tests/functional/pageslayout/test_journalist_col.py @@ -37,7 +37,7 @@ def _create_source_and_submission_and_delete_source_key(config_in_use: SecureDro EncryptionManager.get_default().delete_source_key_pair(source_user.filesystem_id) -@pytest.fixture() +@pytest.fixture def sd_servers_with_deleted_source_key( setup_journalist_key_and_gpg_folder: Tuple[str, Path], setup_rqworker: Tuple[str, Path], @@ -64,7 +64,7 @@ def sd_servers_with_deleted_source_key( @pytest.mark.parametrize("locale", list_locales()) -@pytest.mark.pagelayout() +@pytest.mark.pagelayout class TestJournalistLayoutCol: def test_col_with_and_without_documents( self, locale, sd_servers_with_submitted_file, firefox_web_driver diff --git a/securedrop/tests/functional/pageslayout/test_journalist_delete.py b/securedrop/tests/functional/pageslayout/test_journalist_delete.py index e820e611b13..43b52494623 100644 --- a/securedrop/tests/functional/pageslayout/test_journalist_delete.py +++ b/securedrop/tests/functional/pageslayout/test_journalist_delete.py @@ -21,7 +21,7 @@ @pytest.mark.parametrize("locale", list_locales()) -@pytest.mark.pagelayout() +@pytest.mark.pagelayout class TestJournalistLayoutDelete: def test_delete_none(self, locale, sd_servers_with_submitted_file, firefox_web_driver): # Given an SD server with a file submitted by a source diff --git a/securedrop/tests/functional/pageslayout/test_source.py b/securedrop/tests/functional/pageslayout/test_source.py index fd239098002..dc212ce1962 100644 --- a/securedrop/tests/functional/pageslayout/test_source.py +++ b/securedrop/tests/functional/pageslayout/test_source.py @@ -21,7 +21,7 @@ @pytest.mark.parametrize("locale", list_locales()) -@pytest.mark.pagelayout() +@pytest.mark.pagelayout class TestSourceLayout: def test(self, locale, sd_servers_with_clean_state, tor_browser_web_driver): # Given a source user accessing the app from their browser diff --git a/securedrop/tests/functional/pageslayout/test_source_layout_torbrowser.py b/securedrop/tests/functional/pageslayout/test_source_layout_torbrowser.py index 8e7728a98f0..b90a45b821d 100644 --- a/securedrop/tests/functional/pageslayout/test_source_layout_torbrowser.py +++ b/securedrop/tests/functional/pageslayout/test_source_layout_torbrowser.py @@ -22,7 +22,7 @@ @pytest.mark.parametrize("locale", list_locales()) -@pytest.mark.pagelayout() +@pytest.mark.pagelayout class TestSourceLayoutTorBrowser: def test_index_and_logout(self, locale, sd_servers): # Given a source user accessing the app from their browser diff --git a/securedrop/tests/functional/pageslayout/test_source_session_layout.py b/securedrop/tests/functional/pageslayout/test_source_session_layout.py index e234ef25c64..151e7cffb62 100644 --- a/securedrop/tests/functional/pageslayout/test_source_session_layout.py +++ b/securedrop/tests/functional/pageslayout/test_source_session_layout.py @@ -36,7 +36,7 @@ def sd_servers_with_short_timeout( @pytest.mark.parametrize("locale", list_locales()) -@pytest.mark.pagelayout() +@pytest.mark.pagelayout class TestSourceAppSessionTimeout: def test_source_session_timeout(self, locale, sd_servers_with_short_timeout): # Given an SD server with a very short session timeout diff --git a/securedrop/tests/functional/pageslayout/test_source_static_pages.py b/securedrop/tests/functional/pageslayout/test_source_static_pages.py index 484af40ac2f..80b880f4be0 100644 --- a/securedrop/tests/functional/pageslayout/test_source_static_pages.py +++ b/securedrop/tests/functional/pageslayout/test_source_static_pages.py @@ -7,7 +7,7 @@ from version import __version__ -@pytest.mark.pagelayout() +@pytest.mark.pagelayout class TestSourceAppStaticPages: @pytest.mark.parametrize("locale", list_locales()) def test_notfound(self, locale, sd_servers): diff --git a/securedrop/tests/functional/pageslayout/test_submit_and_retrieve_file.py b/securedrop/tests/functional/pageslayout/test_submit_and_retrieve_file.py index 366af4b727f..563254c0d40 100644 --- a/securedrop/tests/functional/pageslayout/test_submit_and_retrieve_file.py +++ b/securedrop/tests/functional/pageslayout/test_submit_and_retrieve_file.py @@ -7,7 +7,7 @@ @pytest.mark.parametrize("locale", list_locales()) -@pytest.mark.pagelayout() +@pytest.mark.pagelayout class TestSubmitAndRetrieveFile: def test_submit_and_retrieve_happy_path( self, locale, sd_servers_with_clean_state, tor_browser_web_driver, firefox_web_driver diff --git a/securedrop/tests/functional/test_admin_interface.py b/securedrop/tests/functional/test_admin_interface.py index 4a017ce8fee..8ddd9de8f28 100644 --- a/securedrop/tests/functional/test_admin_interface.py +++ b/securedrop/tests/functional/test_admin_interface.py @@ -169,7 +169,7 @@ def _create_second_journalist(config_in_use: SecureDropConfig) -> None: db_session_for_sd_servers.commit() -@pytest.fixture() +@pytest.fixture def sd_servers_with_second_journalist( setup_journalist_key_and_gpg_folder: Tuple[str, Path], setup_rqworker: Tuple[str, Path], diff --git a/securedrop/tests/functional/test_journalist.py b/securedrop/tests/functional/test_journalist.py index 35fa9b4d848..68002330e2f 100644 --- a/securedrop/tests/functional/test_journalist.py +++ b/securedrop/tests/functional/test_journalist.py @@ -375,7 +375,7 @@ def one_source_no_files(): journ_app_nav.nav_helper.wait_for(one_source_no_files) -@pytest.fixture() +@pytest.fixture def sd_servers_with_missing_file( setup_journalist_key_and_gpg_folder: Tuple[str, Path], setup_rqworker: Tuple[str, Path], diff --git a/securedrop/tests/functional/test_source_warnings.py b/securedrop/tests/functional/test_source_warnings.py index 782735be3f6..e133327472d 100644 --- a/securedrop/tests/functional/test_source_warnings.py +++ b/securedrop/tests/functional/test_source_warnings.py @@ -8,7 +8,7 @@ from tests.functional.web_drivers import _FIREFOX_PATH -@pytest.fixture() +@pytest.fixture def orbot_web_driver(sd_servers): # Create new profile and driver with the orbot user agent orbot_user_agent = "Mozilla/5.0 (Android; Mobile; rv:52.0) Gecko/20100101 Firefox/52.0" diff --git a/securedrop/tests/functional/web_drivers.py b/securedrop/tests/functional/web_drivers.py index ee57ebc7d60..b7685b418e5 100644 --- a/securedrop/tests/functional/web_drivers.py +++ b/securedrop/tests/functional/web_drivers.py @@ -41,7 +41,7 @@ def _create_torbrowser_driver( ) -> TorBrowserDriver: logging.info("Creating TorBrowserDriver") log_file = open(_LOGFILE_PATH, "a") - log_file.write("\n\n[%s] Running Functional Tests\n" % str(datetime.now())) + log_file.write(f"\n\n[{datetime.now()}] Running Functional Tests\n") log_file.flush() # Don't use Tor when reading from localhost, and turn off private diff --git a/securedrop/tests/test_alembic.py b/securedrop/tests/test_alembic.py index d545d0563d0..0028ce6f5f7 100644 --- a/securedrop/tests/test_alembic.py +++ b/securedrop/tests/test_alembic.py @@ -25,7 +25,7 @@ WHITESPACE_REGEX = re.compile(r"\s+") -@pytest.fixture() +@pytest.fixture def _reset_db(config: SecureDropConfig) -> None: # The config fixture creates all the models in the DB, but most alembic tests expect an # empty DB, so we reset the DB via this fixture diff --git a/securedrop/tests/test_journalist.py b/securedrop/tests/test_journalist.py index 74569d6c8f5..ae1baaecd7a 100644 --- a/securedrop/tests/test_journalist.py +++ b/securedrop/tests/test_journalist.py @@ -1714,10 +1714,7 @@ def test_deleted_user_cannot_login(config, journalist_app, locale): ] with xfail_untranslated_messages(config, locale, msgids): ins.assert_message_flashed( - "{} {}".format( - gettext(msgids[0]), - gettext(msgids[1]), - ), + f"{gettext(msgids[0])} {gettext(msgids[1])}", "error", ) @@ -3500,7 +3497,7 @@ def test_download_selected_submissions_previously_downloaded( ) -@pytest.fixture() +@pytest.fixture def selected_missing_files(journalist_app, test_source, app_storage): """Fixture for the download tests with missing files in storage.""" source = Source.query.get(test_source["id"]) diff --git a/securedrop/tests/test_journalist_api.py b/securedrop/tests/test_journalist_api.py index 908833670ad..884a06a11eb 100644 --- a/securedrop/tests/test_journalist_api.py +++ b/securedrop/tests/test_journalist_api.py @@ -817,9 +817,7 @@ def test_authorized_user_can_add_reply( source = Source.query.get(source_id) - expected_filename = "{}-{}-reply.gpg".format( - source.interaction_count, source.journalist_filename - ) + expected_filename = f"{source.interaction_count}-{source.journalist_filename}-reply.gpg" expected_filepath = Path(app_storage.path(source.filesystem_id, expected_filename)) diff --git a/securedrop/tests/test_journalist_session.py b/securedrop/tests/test_journalist_session.py index 9711a90b621..2d1fa3fed45 100644 --- a/securedrop/tests/test_journalist_session.py +++ b/securedrop/tests/test_journalist_session.py @@ -13,7 +13,7 @@ NEW_PASSWORD = "another correct horse battery staple generic passphrase" -@pytest.fixture() +@pytest.fixture def redis(config): return Redis(**config.REDIS_KWARGS) diff --git a/securedrop/tests/test_remove_pending_sources.py b/securedrop/tests/test_remove_pending_sources.py index 11e012ab401..3b75d247e2e 100644 --- a/securedrop/tests/test_remove_pending_sources.py +++ b/securedrop/tests/test_remove_pending_sources.py @@ -16,7 +16,7 @@ def test_remove_pending_sources_none_pending(n, m, source_app, config, app_stora with source_app.app_context(): sources = [] - for i in range(0, n): + for i in range(n): source_user = create_source_user( db_session=db.session, source_passphrase=PassphraseGenerator.get_default().generate_passphrase(), @@ -53,7 +53,7 @@ def test_remove_pending_sources_all_pending(n, m, source_app, config, app_storag with source_app.app_context(): sources = [] - for i in range(0, n): + for i in range(n): source_user = create_source_user( db_session=db.session, source_passphrase=PassphraseGenerator.get_default().generate_passphrase(), diff --git a/securedrop/tests/test_source.py b/securedrop/tests/test_source.py index 2dfa8fa75ed..7410870776c 100644 --- a/securedrop/tests/test_source.py +++ b/securedrop/tests/test_source.py @@ -812,9 +812,8 @@ def test_login_with_overly_long_codename(source_app): assert resp.status_code == 200 text = resp.data.decode("utf-8") assert ( - "Field must be between 1 and {} characters long.".format( - PassphraseGenerator.MAX_PASSPHRASE_LENGTH - ) + f"Field must be between 1 and {PassphraseGenerator.MAX_PASSPHRASE_LENGTH} characters " + "long." ) in text diff --git a/securedrop/tests/test_store.py b/securedrop/tests/test_store.py index 24b540cec0b..5673796ff8c 100644 --- a/securedrop/tests/test_store.py +++ b/securedrop/tests/test_store.py @@ -20,7 +20,7 @@ from tests import utils -@pytest.fixture() +@pytest.fixture def test_storage() -> Generator[Storage, None, None]: # Setup the filesystem for the storage object with TemporaryDirectory() as data_dir_name: diff --git a/securedrop/tests/test_worker.py b/securedrop/tests/test_worker.py index 285e7dcded4..3785eea9129 100644 --- a/securedrop/tests/test_worker.py +++ b/securedrop/tests/test_worker.py @@ -28,7 +28,7 @@ def start_rq_worker(config, queue_name): "rq_config", queue_name, ], - preexec_fn=os.setsid, + preexec_fn=os.setsid, # noqa: PLW1509 ) diff --git a/securedrop/tests/test_wsgi.py b/securedrop/tests/test_wsgi.py index c2704a4e5ec..86ba7fdb76e 100644 --- a/securedrop/tests/test_wsgi.py +++ b/securedrop/tests/test_wsgi.py @@ -7,7 +7,7 @@ import pytest -@pytest.fixture() +@pytest.fixture def _journalist_pubkey(): """provision a valid public key for the JI startup check""" path = Path("/tmp/securedrop/journalist.pub") diff --git a/securedrop/tests/utils/instrument.py b/securedrop/tests/utils/instrument.py index cad11094678..e4bd49c7f0f 100644 --- a/securedrop/tests/utils/instrument.py +++ b/securedrop/tests/utils/instrument.py @@ -122,8 +122,8 @@ def assert_redirects(self, response, expected_location, message=None): """ valid_status_codes = (301, 302, 303, 305, 307) valid_status_code_str = ", ".join([str(code) for code in valid_status_codes]) - not_redirect = "HTTP Status {} expected but got {}".format( - valid_status_code_str, response.status_code + not_redirect = ( + f"HTTP Status {valid_status_code_str} expected but got {response.status_code}" ) assert response.status_code in (valid_status_codes, message) or not_redirect assert response.location == expected_location, message