Merge pull request #7233 from freedomofpress/ruff-0.6

Upgrade ruff, remove black

.githooks/pre-commit

@@ -10,12 +10,9 @@ if [[ -n "$PY_FILES" ]]; then
     if [[ ! -v VIRTUAL_ENV ]]; then
         source .venv/bin/activate
     fi
-    echo "Checking:"
-    echo "$PY_FILES"
-    # Run black against changed python files for this commit
-    black --check --diff ${PY_FILES//$'\n'/ }
     # Run ruff (against all files, it's fast enough)
-    ruff check . --diff && echo "ruff passed!"
+    ruff format . && ruff check . --diff \
+        && echo "ruff passed!"
 else
     exit 0
 fi

Makefile

@@ -61,16 +61,6 @@ update-pip-requirements: update-admin-pip-requirements update-python3-requiremen
 #
 #################
 
-.PHONY: check-black
-check-black: ## Check Python source code formatting with black
-	@echo "███ Running black check..."
-	@black --check --diff .
-	@echo
-
-.PHONY: black
-black: ## Update Python source code formatting with black
-	@black securedrop .
-
 .PHONY: ansible-config-lint
 ansible-config-lint: ## Run custom Ansible linting tasks.
 	@echo "███ Linting Ansible configuration..."
@@ -94,15 +84,19 @@ app-lint-full: ## Test pylint compliance, with no checks disabled.
 	@echo
 
 .PHONY: check-ruff
-check-ruff:  ## Lint Python source files.
+check-ruff:  ## Check linting and formatting of Python source files.
 	@echo "███ Running ruff..."
-	@ruff check . --show-source
+	@ruff format . --diff
+	@ruff check .
 	@echo
 
 .PHONY: ruff
 ruff: ## Update Python source file formatting.
+	@ruff format .
 	@ruff check . --fix
 
+fix: ruff ## Apply automatic fixes.
+
 # The --disable=names is required to use the BEM syntax
 # # https://csswizardry.com/2013/01/mindbemding-getting-your-head-round-bem-syntax/
 .PHONY: html-lint
@@ -139,7 +133,7 @@ yamllint:  ## Lint YAML files (does not validate syntax!).
 # While the order mostly doesn't matter here, keep "check-ruff" first, since it
 # gives the broadest coverage and runs (and therefore fails) fastest.
 .PHONY: lint
-lint: check-ruff ansible-config-lint app-lint check-black html-lint shellcheck typelint yamllint check-strings check-supported-locales check-desktop-files ## Runs all lint checks
+lint: check-ruff ansible-config-lint app-lint html-lint shellcheck typelint yamllint check-strings check-supported-locales check-desktop-files ## Runs all lint checks
 
 .PHONY: safety
 safety:  ## Run `safety check` to check python dependencies for vulnerabilities.

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")
@@ -185,7 +182,6 @@ def envsetup(args: argparse.Namespace, virtualenv_dir: str = VENV_DIR) -> None:
 
     # virtualenv doesnt exist? Install dependencies and create
     if not os.path.exists(virtualenv_dir):
-
         # Technically you can create a virtualenv from within python
         # but pip can only be run over Tor on Tails, and debugging that
         # along with instaling a third-party dependency is not worth
@@ -260,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(

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")
 
@@ -769,7 +768,6 @@ def wrapper(*args: Any, **kwargs: Any) -> Any:
 
             update_status, latest_tag = check_for_updates(cli_args)
             if update_status is True:
-
                 # Useful for troubleshooting
                 branch_status = get_git_branch(cli_args)
 
@@ -793,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(

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))

admin/tests/test_securedrop-admin.py

@@ -76,9 +76,7 @@ def test_update_check_decorator_when_no_update_needed(self, caplog):
             "securedrop_admin.check_for_updates", side_effect=[[False, "1.5.0"]]
         ) as mocked_check, mock.patch(
             "securedrop_admin.get_git_branch", side_effect=["develop"]
-        ), mock.patch(
-            "sys.exit"
-        ) as mocked_exit:
+        ), mock.patch("sys.exit") as mocked_exit:
             # The decorator itself interprets --force
             args = argparse.Namespace(force=False)
             rv = securedrop_admin.update_check_required("update_check_test")(lambda _: 100)(args)
@@ -101,9 +99,7 @@ def test_update_check_decorator_when_update_needed(self, caplog):
             "securedrop_admin.check_for_updates", side_effect=[[True, "1.5.0"]]
         ) as mocked_check, mock.patch(
             "securedrop_admin.get_git_branch", side_effect=["bad_branch"]
-        ), mock.patch(
-            "sys.exit"
-        ) as mocked_exit:
+        ), mock.patch("sys.exit") as mocked_exit:
             # The decorator itself interprets --force
             args = argparse.Namespace(force=False)
             securedrop_admin.update_check_required("update_check_test")(lambda _: _)(args)
@@ -125,9 +121,7 @@ def test_update_check_decorator_when_skipped(self, caplog):
             "securedrop_admin.check_for_updates", side_effect=[[True, "1.5.0"]]
         ) as mocked_check, mock.patch(
             "securedrop_admin.get_git_branch", side_effect=["develop"]
-        ), mock.patch(
-            "sys.exit"
-        ) as mocked_exit:
+        ), mock.patch("sys.exit") as mocked_exit:
             # The decorator itself interprets --force
             args = argparse.Namespace(force=True)
             rv = securedrop_admin.update_check_required("update_check_test")(lambda _: 100)(args)
@@ -847,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)
@@ -935,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)
@@ -962,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, "") == []

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:

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 ")

install_files/ansible-base/roles/tails-config/files/securedrop_init.py

@@ -207,7 +207,6 @@
             )
 
             if "OK" in verify_proc:
-
                 # Updating the cert chain requires sudo privileges
                 os.setresgid(0, 0, -1)
                 os.setresuid(0, 0, -1)

journalist_gui/journalist_gui/SecureDropUpdater.py

@@ -17,16 +17,12 @@
 
 
 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:
-
     # Null byte triggers abstract namespace
     IDENTIFIER = "\0" + name
     ALREADY_BOUND_ERRNO = 98

journalist_gui/test_gui.py

@@ -101,7 +101,6 @@ def test_default_tab(self):
         assert self.window.tabWidget.currentIndex() == 0
 
     def test_output_tab(self):
-
         tab = self.window.tabWidget.tabBar()
         QTest.mouseClick(tab, Qt.LeftButton)
         assert self.window.tabWidget.currentIndex() == self.window.tabWidget.indexOf(

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.

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",

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",

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",