Skip to content

Commit

Permalink
Fix most errors spotted by ruff
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
legoktm committed Sep 19, 2024
1 parent 5c68e15 commit 6127b27
Show file tree
Hide file tree
Showing 73 changed files with 226 additions and 331 deletions.
9 changes: 3 additions & 6 deletions admin/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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(
Expand Down
15 changes: 7 additions & 8 deletions admin/securedrop_admin/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"):
Expand All @@ -75,7 +75,6 @@ def openssh_version() -> int:
return 0
except subprocess.CalledProcessError:
return 0
pass
return 0


Expand Down Expand Up @@ -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")

Expand All @@ -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

Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion admin/tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
6 changes: 3 additions & 3 deletions admin/tests/test_securedrop-admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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, "") == []
Expand Down
13 changes: 7 additions & 6 deletions devops/scripts/verify-mo.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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."""

Expand Down Expand 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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 ")
Expand Down
4 changes: 1 addition & 3 deletions journalist_gui/journalist_gui/SecureDropUpdater.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions molecule/testinfra/app-code/test_securedrop_app_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
10 changes: 3 additions & 7 deletions molecule/testinfra/app-code/test_securedrop_rqrequeue.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 1 addition & 4 deletions molecule/testinfra/app-code/test_securedrop_rqworker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 1 addition & 3 deletions molecule/testinfra/app/apache/test_apache_system_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down
6 changes: 2 additions & 4 deletions molecule/testinfra/app/test_app_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -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://")

Expand All @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion molecule/testinfra/app/test_apparmor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
11 changes: 5 additions & 6 deletions molecule/testinfra/app/test_appenv.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@
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"]
)
)
result = host.run(cmd)
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")
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion molecule/testinfra/app/test_ossec_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion molecule/testinfra/app/test_smoke.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
4 changes: 2 additions & 2 deletions molecule/testinfra/app/test_tor_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,15 @@ 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():
f = host.file(v2_url_filepath)
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():
Expand Down
Loading

0 comments on commit 6127b27

Please sign in to comment.