From 0ee64f2fe865ad491c69a7afb5af3d5c2822a19e Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:08:30 -0800 Subject: [PATCH 01/55] Fixed F401 (unused-import) --- management/auth.py | 2 +- management/backup.py | 2 +- management/cli.py | 2 +- management/daemon.py | 4 ++-- management/dns_update.py | 2 +- management/mailconfig.py | 2 +- management/ssl_certificates.py | 1 - management/status_checks.py | 3 +-- management/wsgi.py | 2 +- tests/fail2ban.py | 3 +-- tests/test_dns.py | 2 +- 11 files changed, 11 insertions(+), 14 deletions(-) diff --git a/management/auth.py b/management/auth.py index c576d01c..d450720d 100644 --- a/management/auth.py +++ b/management/auth.py @@ -1,4 +1,4 @@ -import base64, os, os.path, hmac, json, secrets +import base64, hmac, json, secrets from datetime import timedelta from expiringdict import ExpiringDict diff --git a/management/backup.py b/management/backup.py index c2ef7676..0d2a0c07 100755 --- a/management/backup.py +++ b/management/backup.py @@ -7,7 +7,7 @@ # 4) The stopped services are restarted. # 5) STORAGE_ROOT/backup/after-backup is executed if it exists. -import os, os.path, shutil, glob, re, datetime, sys +import os, os.path, re, datetime, sys import dateutil.parser, dateutil.relativedelta, dateutil.tz import rtyaml from exclusiveprocess import Lock diff --git a/management/cli.py b/management/cli.py index b32089c6..cbf19fc0 100755 --- a/management/cli.py +++ b/management/cli.py @@ -6,7 +6,7 @@ # root API key. This file is readable only by root, so this # tool can only be used as root. -import sys, getpass, urllib.request, urllib.error, json, re, csv +import sys, getpass, urllib.request, urllib.error, json, csv def mgmt(cmd, data=None, is_json=False): # The base URL for the management daemon. (Listens on IPv4 only.) diff --git a/management/daemon.py b/management/daemon.py index 47548531..5081aea3 100755 --- a/management/daemon.py +++ b/management/daemon.py @@ -11,11 +11,11 @@ # service mailinabox start # when done debugging, start it up again import os, os.path, re, json, time -import multiprocessing.pool, subprocess +import multiprocessing.pool from functools import wraps -from flask import Flask, request, render_template, abort, Response, send_from_directory, make_response +from flask import Flask, request, render_template, Response, send_from_directory, make_response import auth, utils from mailconfig import get_mail_users, get_mail_users_ex, get_admins, add_mail_user, set_mail_password, remove_mail_user diff --git a/management/dns_update.py b/management/dns_update.py index 9a768ea8..64b55dec 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -4,7 +4,7 @@ # and mail aliases and restarts nsd. ######################################################################## -import sys, os, os.path, urllib.parse, datetime, re, hashlib, base64 +import sys, os, os.path, datetime, re, hashlib, base64 import ipaddress import rtyaml import dns.resolver diff --git a/management/mailconfig.py b/management/mailconfig.py index 2fcb9703..4068b393 100755 --- a/management/mailconfig.py +++ b/management/mailconfig.py @@ -9,7 +9,7 @@ # Python 3 in setup/questions.sh to validate the email # address entered by the user. -import subprocess, shutil, os, sqlite3, re +import os, sqlite3, re import utils from email_validator import validate_email as validate_email_, EmailNotValidError import idna diff --git a/management/ssl_certificates.py b/management/ssl_certificates.py index 19f0266a..f3e83a15 100755 --- a/management/ssl_certificates.py +++ b/management/ssl_certificates.py @@ -4,7 +4,6 @@ import os, os.path, re, shutil, subprocess, tempfile from utils import shell, safe_domain_name, sort_domains -import idna # SELECTING SSL CERTIFICATES FOR USE IN WEB diff --git a/management/status_checks.py b/management/status_checks.py index e8faf6fb..927e6102 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -4,11 +4,10 @@ # TLS certificates have been signed, etc., and if not tells the user # what to do next. -import sys, os, os.path, re, subprocess, datetime, multiprocessing.pool +import sys, os, os.path, re, datetime, multiprocessing.pool import asyncio import dns.reversename, dns.resolver -import dateutil.parser, dateutil.tz import idna import psutil import postfix_mta_sts_resolver.resolver diff --git a/management/wsgi.py b/management/wsgi.py index 86cf3af4..9cf21c4b 100644 --- a/management/wsgi.py +++ b/management/wsgi.py @@ -1,5 +1,5 @@ from daemon import app -import auth, utils +import utils app.logger.addHandler(utils.create_syslog_handler()) diff --git a/tests/fail2ban.py b/tests/fail2ban.py index cb55c51f..9979507f 100644 --- a/tests/fail2ban.py +++ b/tests/fail2ban.py @@ -6,7 +6,7 @@ # try to log in to. ###################################################################### -import sys, os, time, functools +import sys, os, time # parse command line @@ -163,7 +163,6 @@ def run_test(testfunc, args, count, within_seconds, parallel): # run testfunc sequentially and still get to count requests within # the required time. So we split the requests across threads. - import requests.exceptions from multiprocessing import Pool restart_fail2ban_service() diff --git a/tests/test_dns.py b/tests/test_dns.py index 25c64bf1..62cf5e78 100755 --- a/tests/test_dns.py +++ b/tests/test_dns.py @@ -7,7 +7,7 @@ # where ipaddr is the IP address of your Mail-in-a-Box # and hostname is the domain name to check the DNS for. -import sys, re, difflib +import sys, re import dns.reversename, dns.resolver if len(sys.argv) < 3: From cb922ec286c99c01c75fa526af27c78c64869838 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:08:56 -0800 Subject: [PATCH 02/55] Fixed UP015 (redundant-open-modes): Unnecessary open mode parameters --- management/auth.py | 2 +- management/backup.py | 4 ++-- management/cli.py | 2 +- management/dns_update.py | 10 +++++----- management/status_checks.py | 6 +++--- management/utils.py | 4 ++-- management/web_update.py | 6 +++--- tools/editconf.py | 2 +- tools/parse-nginx-log-bootstrap-accesses.py | 2 +- 9 files changed, 19 insertions(+), 19 deletions(-) diff --git a/management/auth.py b/management/auth.py index d450720d..653d0485 100644 --- a/management/auth.py +++ b/management/auth.py @@ -22,7 +22,7 @@ class AuthService: def init_system_api_key(self): """Write an API key to a local file so local processes can use the API""" - with open(self.key_path, 'r') as file: + with open(self.key_path) as file: self.key = file.read() def authenticate(self, request, env, login_only=False, logout=False): diff --git a/management/backup.py b/management/backup.py index 0d2a0c07..6ec4d28a 100755 --- a/management/backup.py +++ b/management/backup.py @@ -578,7 +578,7 @@ def get_backup_config(env, for_save=False, for_ui=False): # Merge in anything written to custom.yaml. try: - with open(os.path.join(backup_root, 'custom.yaml'), 'r') as f: + with open(os.path.join(backup_root, 'custom.yaml')) as f: custom_config = rtyaml.load(f) if not isinstance(custom_config, dict): raise ValueError() # caught below config.update(custom_config) @@ -604,7 +604,7 @@ def get_backup_config(env, for_save=False, for_ui=False): config["target"] = "file://" + config["file_target_directory"] ssh_pub_key = os.path.join('/root', '.ssh', 'id_rsa_miab.pub') if os.path.exists(ssh_pub_key): - with open(ssh_pub_key, 'r') as f: + with open(ssh_pub_key) as f: config["ssh_pub_key"] = f.read() return config diff --git a/management/cli.py b/management/cli.py index cbf19fc0..5e2e331c 100755 --- a/management/cli.py +++ b/management/cli.py @@ -47,7 +47,7 @@ def read_password(): return first def setup_key_auth(mgmt_uri): - with open('/var/lib/mailinabox/api.key', 'r') as f: + with open('/var/lib/mailinabox/api.key') as f: key = f.read().strip() auth_handler = urllib.request.HTTPBasicAuthHandler() diff --git a/management/dns_update.py b/management/dns_update.py index 64b55dec..0869e77d 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -455,7 +455,7 @@ def build_sshfp_records(): # specify that port to sshkeyscan. port = 22 - with open('/etc/ssh/sshd_config', 'r') as f: + with open('/etc/ssh/sshd_config') as f: for line in f: s = line.rstrip().split() if len(s) == 2 and s[0] == 'Port': @@ -606,7 +606,7 @@ def get_dns_zonefile(zone, env): raise ValueError("%s is not a domain name that corresponds to a zone." % zone) nsd_zonefile = "/etc/nsd/zones/" + fn - with open(nsd_zonefile, "r") as f: + with open(nsd_zonefile) as f: return f.read() ######################################################################## @@ -676,7 +676,7 @@ def hash_dnssec_keys(domain, env): oldkeyfn = os.path.join(env['STORAGE_ROOT'], 'dns/dnssec', keyfn + ".private") keydata.append(keytype) keydata.append(keyfn) - with open(oldkeyfn, "r") as fr: + with open(oldkeyfn) as fr: keydata.append( fr.read() ) keydata = "".join(keydata).encode("utf8") return hashlib.sha1(keydata).hexdigest() @@ -704,7 +704,7 @@ def sign_zone(domain, zonefile, env): # Use os.umask and open().write() to securely create a copy that only # we (root) can read. oldkeyfn = os.path.join(env['STORAGE_ROOT'], 'dns/dnssec', keyfn + ext) - with open(oldkeyfn, "r") as fr: + with open(oldkeyfn) as fr: keydata = fr.read() keydata = keydata.replace("_domain_", domain) prev_umask = os.umask(0o77) # ensure written file is not world-readable @@ -815,7 +815,7 @@ def write_opendkim_tables(domains, env): def get_custom_dns_config(env, only_real_records=False): try: - with open(os.path.join(env['STORAGE_ROOT'], 'dns/custom.yaml'), 'r') as f: + with open(os.path.join(env['STORAGE_ROOT'], 'dns/custom.yaml')) as f: custom_dns = rtyaml.load(f) if not isinstance(custom_dns, dict): raise ValueError() # caught below except: diff --git a/management/status_checks.py b/management/status_checks.py index 927e6102..81038300 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -212,7 +212,7 @@ def check_ssh_password(env, output): # the configuration file. if not os.path.exists("/etc/ssh/sshd_config"): return - with open("/etc/ssh/sshd_config", "r") as f: + with open("/etc/ssh/sshd_config") as f: sshd = f.read() if re.search("\nPasswordAuthentication\s+yes", sshd) \ or not re.search("\nPasswordAuthentication\s+no", sshd): @@ -600,7 +600,7 @@ def check_dnssec(domain, env, output, dns_zonefiles, is_checking_primary=False): # record that we suggest using is for the KSK (and that's how the DS records were generated). # We'll also give the nice name for the key algorithm. dnssec_keys = load_env_vars_from_file(os.path.join(env['STORAGE_ROOT'], 'dns/dnssec/%s.conf' % alg_name_map[ds_alg])) - with open(os.path.join(env['STORAGE_ROOT'], 'dns/dnssec/' + dnssec_keys['KSK'] + '.key'), 'r') as f: + with open(os.path.join(env['STORAGE_ROOT'], 'dns/dnssec/' + dnssec_keys['KSK'] + '.key')) as f: dnsssec_pubkey = f.read().split("\t")[3].split(" ")[3] expected_ds_records[ (ds_keytag, ds_alg, ds_digalg, ds_digest) ] = { @@ -963,7 +963,7 @@ def run_and_output_changes(env, pool): # Load previously saved status checks. cache_fn = "/var/cache/mailinabox/status_checks.json" if os.path.exists(cache_fn): - with open(cache_fn, 'r') as f: + with open(cache_fn) as f: try: prev = json.load(f) except json.JSONDecodeError: diff --git a/management/utils.py b/management/utils.py index b5ca7e59..baf99143 100644 --- a/management/utils.py +++ b/management/utils.py @@ -14,7 +14,7 @@ def load_env_vars_from_file(fn): # Load settings from a KEY=VALUE file. import collections env = collections.OrderedDict() - with open(fn, 'r') as f: + with open(fn) as f: for line in f: env.setdefault(*line.strip().split("=", 1)) return env @@ -36,7 +36,7 @@ def load_settings(env): import rtyaml fn = os.path.join(env['STORAGE_ROOT'], 'settings.yaml') try: - with open(fn, "r") as f: + with open(fn) as f: config = rtyaml.load(f) if not isinstance(config, dict): raise ValueError() # caught below return config diff --git a/management/web_update.py b/management/web_update.py index e23bb2d8..46a6f2a0 100644 --- a/management/web_update.py +++ b/management/web_update.py @@ -63,7 +63,7 @@ def get_web_domains_with_root_overrides(env): root_overrides = { } nginx_conf_custom_fn = os.path.join(env["STORAGE_ROOT"], "www/custom.yaml") if os.path.exists(nginx_conf_custom_fn): - with open(nginx_conf_custom_fn, 'r') as f: + with open(nginx_conf_custom_fn) as f: custom_settings = rtyaml.load(f) for domain, settings in custom_settings.items(): for type, value in [('redirect', settings.get('redirects', {}).get('/')), @@ -78,7 +78,7 @@ def do_web_update(env): # Helper for reading config files and templates def read_conf(conf_fn): - with open(os.path.join(os.path.dirname(__file__), "../conf", conf_fn), "r") as f: + with open(os.path.join(os.path.dirname(__file__), "../conf", conf_fn)) as f: return f.read() # Build an nginx configuration file. @@ -156,7 +156,7 @@ def make_domain_config(domain, templates, ssl_certificates, env): hsts = "yes" nginx_conf_custom_fn = os.path.join(env["STORAGE_ROOT"], "www/custom.yaml") if os.path.exists(nginx_conf_custom_fn): - with open(nginx_conf_custom_fn, 'r') as f: + with open(nginx_conf_custom_fn) as f: yaml = rtyaml.load(f) if domain in yaml: yaml = yaml[domain] diff --git a/tools/editconf.py b/tools/editconf.py index ec112b02..ea4d9020 100755 --- a/tools/editconf.py +++ b/tools/editconf.py @@ -76,7 +76,7 @@ for setting in settings: found = set() buf = "" -with open(filename, "r") as f: +with open(filename) as f: input_lines = list(f) while len(input_lines) > 0: diff --git a/tools/parse-nginx-log-bootstrap-accesses.py b/tools/parse-nginx-log-bootstrap-accesses.py index c61ed79e..6207b2e9 100755 --- a/tools/parse-nginx-log-bootstrap-accesses.py +++ b/tools/parse-nginx-log-bootstrap-accesses.py @@ -38,7 +38,7 @@ for date, ip in accesses: # Since logs are rotated, store the statistics permanently in a JSON file. # Load in the stats from an existing file. if os.path.exists(outfn): - with open(outfn, "r") as f: + with open(outfn) as f: existing_data = json.load(f) for date, count in existing_data: if date not in by_date: From 49124cc9caee228cf5595381203b667e82c8750f Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:10:25 -0800 Subject: [PATCH 03/55] Fixed PLR6201 (literal-membership): Use a `set` literal when testing for membership --- management/auth.py | 2 +- management/backup.py | 2 +- management/cli.py | 6 +++--- management/daemon.py | 6 +++--- management/dns_update.py | 10 +++++----- management/mail_log.py | 12 ++++++------ management/mailconfig.py | 2 +- management/ssl_certificates.py | 2 +- management/status_checks.py | 18 +++++++++--------- management/web_update.py | 2 +- tools/editconf.py | 2 +- 11 files changed, 32 insertions(+), 32 deletions(-) diff --git a/management/auth.py b/management/auth.py index 653d0485..2d5b8a36 100644 --- a/management/auth.py +++ b/management/auth.py @@ -48,7 +48,7 @@ class AuthService: return username, password username, password = parse_http_authorization_basic(request.headers.get('Authorization', '')) - if username in (None, ""): + if username in {None, ""}: raise ValueError("Authorization header invalid.") if username.strip() == "" and password.strip() == "": diff --git a/management/backup.py b/management/backup.py index 6ec4d28a..bb6dff9a 100755 --- a/management/backup.py +++ b/management/backup.py @@ -556,7 +556,7 @@ def backup_set_custom(env, target, target_user, target_pass, min_age): # Validate. try: - if config["target"] not in ("off", "local"): + if config["target"] not in {"off", "local"}: # these aren't supported by the following function, which expects a full url in the target key, # which is what is there except when loading the config prior to saving list_target_files(config) diff --git a/management/cli.py b/management/cli.py index 5e2e331c..115ed9f2 100755 --- a/management/cli.py +++ b/management/cli.py @@ -91,7 +91,7 @@ elif sys.argv[1] == "user" and len(sys.argv) == 2: print("*", end='') print() -elif sys.argv[1] == "user" and sys.argv[2] in ("add", "password"): +elif sys.argv[1] == "user" and sys.argv[2] in {"add", "password"}: if len(sys.argv) < 5: if len(sys.argv) < 4: email = input("email: ") @@ -109,7 +109,7 @@ elif sys.argv[1] == "user" and sys.argv[2] in ("add", "password"): elif sys.argv[1] == "user" and sys.argv[2] == "remove" and len(sys.argv) == 4: print(mgmt("/mail/users/remove", { "email": sys.argv[3] })) -elif sys.argv[1] == "user" and sys.argv[2] in ("make-admin", "remove-admin") and len(sys.argv) == 4: +elif sys.argv[1] == "user" and sys.argv[2] in {"make-admin", "remove-admin"} and len(sys.argv) == 4: if sys.argv[2] == "make-admin": action = "add" else: @@ -132,7 +132,7 @@ elif sys.argv[1] == "user" and len(sys.argv) == 5 and sys.argv[2:4] == ["mfa", " for mfa in status["enabled_mfa"]: W.writerow([mfa["id"], mfa["type"], mfa["label"]]) -elif sys.argv[1] == "user" and len(sys.argv) in (5, 6) and sys.argv[2:4] == ["mfa", "disable"]: +elif sys.argv[1] == "user" and len(sys.argv) in {5, 6} and sys.argv[2:4] == ["mfa", "disable"]: # Disable MFA (all or a particular device) for a user. print(mgmt("/mfa/disable", { "user": sys.argv[4], "mfa-id": sys.argv[5] if len(sys.argv) == 6 else None })) diff --git a/management/daemon.py b/management/daemon.py index 5081aea3..1a35fba7 100755 --- a/management/daemon.py +++ b/management/daemon.py @@ -90,7 +90,7 @@ def authorized_personnel_only(viewfunc): status = 403 headers = None - if request.headers.get('Accept') in (None, "", "*/*"): + if request.headers.get('Accept') in {None, "", "*/*"}: # Return plain text output. return Response(error+"\n", status=status, mimetype='text/plain', headers=headers) else: @@ -355,9 +355,9 @@ def dns_set_record(qname, rtype="A"): # Get the existing records matching the qname and rtype. return dns_get_records(qname, rtype) - elif request.method in ("POST", "PUT"): + elif request.method in {"POST", "PUT"}: # There is a default value for A/AAAA records. - if rtype in ("A", "AAAA") and value == "": + if rtype in {"A", "AAAA"} and value == "": value = request.environ.get("HTTP_X_FORWARDED_FOR") # normally REMOTE_ADDR but we're behind nginx as a reverse proxy # Cannot add empty records. diff --git a/management/dns_update.py b/management/dns_update.py index 0869e77d..23aee2a5 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -364,7 +364,7 @@ def build_zone(domain, domain_properties, additional_records, env, is_zone=True) # non-mail domain and also may include qnames from custom DNS records. # Do this once at the end of generating a zone. if is_zone: - qnames_with_a = set(qname for (qname, rtype, value, explanation) in records if rtype in ("A", "AAAA")) + qnames_with_a = set(qname for (qname, rtype, value, explanation) in records if rtype in {"A", "AAAA"}) qnames_with_mx = set(qname for (qname, rtype, value, explanation) in records if rtype == "MX") for qname in qnames_with_a - qnames_with_mx: # Mark this domain as not sending mail with hard-fail SPF and DMARC records. @@ -921,12 +921,12 @@ def set_custom_dns_record(qname, rtype, value, action, env): if not re.search(DOMAIN_RE, qname): raise ValueError("Invalid name.") - if rtype in ("A", "AAAA"): + if rtype in {"A", "AAAA"}: if value != "local": # "local" is a special flag for us v = ipaddress.ip_address(value) # raises a ValueError if there's a problem if rtype == "A" and not isinstance(v, ipaddress.IPv4Address): raise ValueError("That's an IPv6 address.") if rtype == "AAAA" and not isinstance(v, ipaddress.IPv6Address): raise ValueError("That's an IPv4 address.") - elif rtype in ("CNAME", "NS"): + elif rtype in {"CNAME", "NS"}: if rtype == "NS" and qname == zone: raise ValueError("NS records can only be set for subdomains.") @@ -936,7 +936,7 @@ def set_custom_dns_record(qname, rtype, value, action, env): if not re.search(DOMAIN_RE, value): raise ValueError("Invalid value.") - elif rtype in ("CNAME", "TXT", "SRV", "MX", "SSHFP", "CAA"): + elif rtype in {"CNAME", "TXT", "SRV", "MX", "SSHFP", "CAA"}: # anything goes pass else: @@ -979,7 +979,7 @@ def set_custom_dns_record(qname, rtype, value, action, env): # Preserve this record. newconfig.append((_qname, _rtype, _value)) - if action in ("add", "set") and needs_add and value is not None: + if action in {"add", "set"} and needs_add and value is not None: newconfig.append((qname, rtype, value)) made_change = True diff --git a/management/mail_log.py b/management/mail_log.py index 1a963966..38ae1ec6 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -376,9 +376,9 @@ def scan_mail_log_line(line, collector): elif service == "postfix/smtpd": if SCAN_BLOCKED: scan_postfix_smtpd_line(date, log, collector) - elif service in ("postfix/qmgr", "postfix/pickup", "postfix/cleanup", "postfix/scache", + elif service in {"postfix/qmgr", "postfix/pickup", "postfix/cleanup", "postfix/scache", "spampd", "postfix/anvil", "postfix/master", "opendkim", "postfix/lmtp", - "postfix/tlsmgr", "anvil"): + "postfix/tlsmgr", "anvil"}: # nothing to look at return True else: @@ -500,7 +500,7 @@ def add_login(user, date, protocol_name, host, collector): data["totals_by_protocol"][protocol_name] += 1 data["totals_by_protocol_and_host"][(protocol_name, host)] += 1 - if host not in ("127.0.0.1", "::1") or True: + if host not in {"127.0.0.1", "::1"} or True: data["activity-by-hour"][protocol_name][date.hour] += 1 collector["logins"][user] = data @@ -684,7 +684,7 @@ def print_user_table(users, data=None, sub_data=None, activity=None, latest=None data_accum[col] += d[row] try: - if None not in [latest, earliest]: + if None not in {latest, earliest}: vert_pos = len(line) e = earliest[row] l = latest[row] @@ -740,7 +740,7 @@ def print_user_table(users, data=None, sub_data=None, activity=None, latest=None else: header += l.rjust(max(5, len(l) + 1, col_widths[col])) - if None not in (latest, earliest): + if None not in {latest, earliest}: header += " │ timespan " lines.insert(0, header.rstrip()) @@ -765,7 +765,7 @@ def print_user_table(users, data=None, sub_data=None, activity=None, latest=None footer += temp.format(data_accum[row]) try: - if None not in [latest, earliest]: + if None not in {latest, earliest}: max_l = max(latest) min_e = min(earliest) timespan = relativedelta(max_l, min_e) diff --git a/management/mailconfig.py b/management/mailconfig.py index 4068b393..3bfc8dc0 100755 --- a/management/mailconfig.py +++ b/management/mailconfig.py @@ -588,7 +588,7 @@ def kick(env, mail_result=None): # They are now stored in the auto_aliases table. for address, forwards_to, permitted_senders, auto in get_mail_aliases(env): user, domain = address.split("@") - if user in ("postmaster", "admin", "abuse") \ + if user in {"postmaster", "admin", "abuse"} \ and address not in required_aliases \ and forwards_to == get_system_administrator(env) \ and not auto: diff --git a/management/ssl_certificates.py b/management/ssl_certificates.py index f3e83a15..77a411b6 100755 --- a/management/ssl_certificates.py +++ b/management/ssl_certificates.py @@ -637,7 +637,7 @@ def load_pem(pem): if pem_type is None: raise ValueError("File is not a valid PEM-formatted file.") pem_type = pem_type.group(1) - if pem_type in (b"RSA PRIVATE KEY", b"PRIVATE KEY"): + if pem_type in {b"RSA PRIVATE KEY", b"PRIVATE KEY"}: return serialization.load_pem_private_key(pem, password=None, backend=default_backend()) if pem_type == b"CERTIFICATE": return load_pem_x509_certificate(pem, default_backend()) diff --git a/management/status_checks.py b/management/status_checks.py index 81038300..7cdfffc6 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -151,7 +151,7 @@ def check_service(i, service, env): output.print_error("%s is not running (port %d)." % (service['name'], service['port'])) # Why is nginx not running? - if not running and service["port"] in (80, 443): + if not running and service["port"] in {80, 443}: output.print_line(shell('check_output', ['nginx', '-t'], capture_stderr=True, trap=True)[1].strip()) else: @@ -340,7 +340,7 @@ def run_domain_checks(rounded_time, env, output, pool, domains_to_check=None): domains_to_check = [ d for d in domains_to_check if not ( - d.split(".", 1)[0] in ("www", "autoconfig", "autodiscover", "mta-sts") + d.split(".", 1)[0] in {"www", "autoconfig", "autodiscover", "mta-sts"} and len(d.split(".", 1)) == 2 and d.split(".", 1)[1] in domains_to_check ) @@ -467,7 +467,7 @@ def check_primary_hostname_dns(domain, env, output, dns_domains, dns_zonefiles): # a DNS zone if it is a subdomain of another domain we have a zone for. existing_rdns_v4 = query_dns(dns.reversename.from_address(env['PUBLIC_IP']), "PTR") existing_rdns_v6 = query_dns(dns.reversename.from_address(env['PUBLIC_IPV6']), "PTR") if env.get("PUBLIC_IPV6") else None - if existing_rdns_v4 == domain and existing_rdns_v6 in (None, domain): + if existing_rdns_v4 == domain and existing_rdns_v6 in {None, domain}: output.print_ok("Reverse DNS is set correctly at ISP. [%s ↦ %s]" % (my_ips, env['PRIMARY_HOSTNAME'])) elif existing_rdns_v4 == existing_rdns_v6 or existing_rdns_v6 is None: output.print_error("""Your box's reverse DNS is currently %s, but it should be %s. Your ISP or cloud provider will have instructions @@ -636,7 +636,7 @@ def check_dnssec(domain, env, output, dns_zonefiles, is_checking_primary=False): if set(r[1] for r in matched_ds) == { '13' } and set(r[2] for r in matched_ds) <= { '2', '4' }: # all are alg 13 and digest type 2 or 4 output.print_ok("DNSSEC 'DS' record is set correctly at registrar.") return - elif len([r for r in matched_ds if r[1] == '13' and r[2] in ( '2', '4' )]) > 0: # some but not all are alg 13 + elif len([r for r in matched_ds if r[1] == '13' and r[2] in { '2', '4' }]) > 0: # some but not all are alg 13 output.print_ok("DNSSEC 'DS' record is set correctly at registrar. (Records using algorithm other than ECDSAP256SHA256 and digest types other than SHA-256/384 should be removed.)") return else: # no record uses alg 13 @@ -825,7 +825,7 @@ def query_dns(qname, rtype, nxdomain='[Not Set]', at=None, as_list=False): # be expressed in equivalent string forms. Canonicalize the form before # returning them. The caller should normalize any IP addresses the result # of this method is compared with. - if rtype in ("A", "AAAA"): + if rtype in {"A", "AAAA"}: response = [normalize_ip(str(r)) for r in response] if as_list: @@ -841,7 +841,7 @@ def check_ssl_cert(domain, rounded_time, ssl_certificates, env, output): # Check that TLS certificate is signed. # Skip the check if the A record is not pointed here. - if query_dns(domain, "A", None) not in (env['PUBLIC_IP'], None): return + if query_dns(domain, "A", None) not in {env['PUBLIC_IP'], None}: return # Where is the certificate file stored? tls_cert = get_domain_ssl_files(domain, ssl_certificates, env, allow_missing_cert=True) @@ -1002,14 +1002,14 @@ def run_and_output_changes(env, pool): out.add_heading(category + " -- Previously:") elif op == "delete": out.add_heading(category + " -- Removed") - if op in ("replace", "delete"): + if op in {"replace", "delete"}: BufferedOutput(with_lines=prev_lines[i1:i2]).playback(out) if op == "replace": out.add_heading(category + " -- Currently:") elif op == "insert": out.add_heading(category + " -- Added") - if op in ("replace", "insert"): + if op in {"replace", "insert"}: BufferedOutput(with_lines=cur_lines[j1:j2]).playback(out) for category, prev_lines in prev_status.items(): @@ -1095,7 +1095,7 @@ class BufferedOutput: def __init__(self, with_lines=None): self.buf = [] if not with_lines else with_lines def __getattr__(self, attr): - if attr not in ("add_heading", "print_ok", "print_error", "print_warning", "print_block", "print_line"): + if attr not in {"add_heading", "print_ok", "print_error", "print_warning", "print_block", "print_line"}: raise AttributeError # Return a function that just records the call & arguments to our buffer. def w(*args, **kwargs): diff --git a/management/web_update.py b/management/web_update.py index 46a6f2a0..14ad1379 100644 --- a/management/web_update.py +++ b/management/web_update.py @@ -53,7 +53,7 @@ def get_domains_with_a_records(env): domains = set() dns = get_custom_dns_config(env) for domain, rtype, value in dns: - if rtype == "CNAME" or (rtype in ("A", "AAAA") and value not in ("local", env['PUBLIC_IP'])): + if rtype == "CNAME" or (rtype in {"A", "AAAA"} and value not in {"local", env['PUBLIC_IP']}): domains.add(domain) return domains diff --git a/tools/editconf.py b/tools/editconf.py index ea4d9020..9e397846 100755 --- a/tools/editconf.py +++ b/tools/editconf.py @@ -84,7 +84,7 @@ while len(input_lines) > 0: # If this configuration file uses folded lines, append any folded lines # into our input buffer. - if folded_lines and line[0] not in (comment_char, " ", ""): + if folded_lines and line[0] not in {comment_char, " ", ""}: while len(input_lines) > 0 and input_lines[0][0] in " \t": line += input_lines.pop(0) From dd61844cedc9edc150466bf063320d6180685c65 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:10:48 -0800 Subject: [PATCH 04/55] Fixed EM101 (raw-string-in-exception): Exception must not use a string literal, assign to variable first --- management/auth.py | 12 ++++++++---- management/backup.py | 6 ++++-- management/dns_update.py | 9 ++++++--- management/mailconfig.py | 6 ++++-- management/mfa.py | 12 ++++++++---- management/ssl_certificates.py | 6 ++++-- tests/fail2ban.py | 12 ++++++++---- 7 files changed, 42 insertions(+), 21 deletions(-) diff --git a/management/auth.py b/management/auth.py index 2d5b8a36..36112572 100644 --- a/management/auth.py +++ b/management/auth.py @@ -49,10 +49,12 @@ class AuthService: username, password = parse_http_authorization_basic(request.headers.get('Authorization', '')) if username in {None, ""}: - raise ValueError("Authorization header invalid.") + msg = "Authorization header invalid." + raise ValueError(msg) if username.strip() == "" and password.strip() == "": - raise ValueError("No email address, password, session key, or API key provided.") + msg = "No email address, password, session key, or API key provided." + raise ValueError(msg) # If user passed the system API key, grant administrative privs. This key # is not associated with a user. @@ -72,7 +74,8 @@ class AuthService: # If no password was given, but a username was given, we're missing some information. elif password.strip() == "": - raise ValueError("Enter a password.") + msg = "Enter a password." + raise ValueError(msg) else: # The user is trying to log in with a username and a password @@ -114,7 +117,8 @@ class AuthService: ]) except: # Login failed. - raise ValueError("Incorrect email address or password.") + msg = "Incorrect email address or password." + raise ValueError(msg) # If MFA is enabled, check that MFA passes. status, hints = validate_auth_mfa(email, request, env) diff --git a/management/backup.py b/management/backup.py index bb6dff9a..75046971 100755 --- a/management/backup.py +++ b/management/backup.py @@ -507,7 +507,8 @@ def list_target_files(config): path = '' if bucket == "": - raise ValueError("Enter an S3 bucket name.") + msg = "Enter an S3 bucket name." + raise ValueError(msg) # connect to the region & bucket try: @@ -535,7 +536,8 @@ def list_target_files(config): b2_api.authorize_account("production", b2_application_keyid, b2_application_key) bucket = b2_api.get_bucket_by_name(b2_bucket) except NonExistentBucket as e: - raise ValueError("B2 Bucket does not exist. Please double check your information!") + msg = "B2 Bucket does not exist. Please double check your information!" + raise ValueError(msg) return [(key.file_name, key.size) for key, _ in bucket.ls()] else: diff --git a/management/dns_update.py b/management/dns_update.py index 23aee2a5..c9e4d359 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -919,7 +919,8 @@ def set_custom_dns_record(qname, rtype, value, action, env): rtype = rtype.upper() if value is not None and qname != "_secondary_nameserver": if not re.search(DOMAIN_RE, qname): - raise ValueError("Invalid name.") + msg = "Invalid name." + raise ValueError(msg) if rtype in {"A", "AAAA"}: if value != "local": # "local" is a special flag for us @@ -928,14 +929,16 @@ def set_custom_dns_record(qname, rtype, value, action, env): if rtype == "AAAA" and not isinstance(v, ipaddress.IPv6Address): raise ValueError("That's an IPv4 address.") elif rtype in {"CNAME", "NS"}: if rtype == "NS" and qname == zone: - raise ValueError("NS records can only be set for subdomains.") + msg = "NS records can only be set for subdomains." + raise ValueError(msg) # ensure value has a trailing dot if not value.endswith("."): value = value + "." if not re.search(DOMAIN_RE, value): - raise ValueError("Invalid value.") + msg = "Invalid value." + raise ValueError(msg) elif rtype in {"CNAME", "TXT", "SRV", "MX", "SSHFP", "CAA"}: # anything goes pass diff --git a/management/mailconfig.py b/management/mailconfig.py index 3bfc8dc0..cfdc3239 100755 --- a/management/mailconfig.py +++ b/management/mailconfig.py @@ -608,9 +608,11 @@ def kick(env, mail_result=None): def validate_password(pw): # validate password if pw.strip() == "": - raise ValueError("No password provided.") + msg = "No password provided." + raise ValueError(msg) if len(pw) < 8: - raise ValueError("Passwords must be at least eight characters.") + msg = "Passwords must be at least eight characters." + raise ValueError(msg) if __name__ == "__main__": import sys diff --git a/management/mfa.py b/management/mfa.py index 32eb5183..148fd912 100644 --- a/management/mfa.py +++ b/management/mfa.py @@ -41,9 +41,11 @@ def enable_mfa(email, type, secret, token, label, env): # Sanity check with the provide current token. totp = pyotp.TOTP(secret) if not totp.verify(token, valid_window=1): - raise ValueError("Invalid token.") + msg = "Invalid token." + raise ValueError(msg) else: - raise ValueError("Invalid MFA type.") + msg = "Invalid MFA type." + raise ValueError(msg) conn, c = open_database(env, with_connection=True) c.execute('INSERT INTO mfa (user_id, type, secret, label) VALUES (?, ?, ?, ?)', (get_user_id(email, c), type, secret, label)) @@ -67,9 +69,11 @@ def disable_mfa(email, mfa_id, env): def validate_totp_secret(secret): if type(secret) != str or secret.strip() == "": - raise ValueError("No secret provided.") + msg = "No secret provided." + raise ValueError(msg) if len(secret) != 32: - raise ValueError("Secret should be a 32 characters base32 string") + msg = "Secret should be a 32 characters base32 string" + raise ValueError(msg) def provision_totp(email, env): # Make a new secret. diff --git a/management/ssl_certificates.py b/management/ssl_certificates.py index 77a411b6..8ff43b11 100755 --- a/management/ssl_certificates.py +++ b/management/ssl_certificates.py @@ -624,7 +624,8 @@ def load_cert_chain(pemfile): pem = f.read() + b"\n" # ensure trailing newline pemblocks = re.findall(re_pem, pem) if len(pemblocks) == 0: - raise ValueError("File does not contain valid PEM data.") + msg = "File does not contain valid PEM data." + raise ValueError(msg) return pemblocks def load_pem(pem): @@ -635,7 +636,8 @@ def load_pem(pem): from cryptography.hazmat.backends import default_backend pem_type = re.match(b"-+BEGIN (.*?)-+[\r\n]", pem) if pem_type is None: - raise ValueError("File is not a valid PEM-formatted file.") + msg = "File is not a valid PEM-formatted file." + raise ValueError(msg) pem_type = pem_type.group(1) if pem_type in {b"RSA PRIVATE KEY", b"PRIVATE KEY"}: return serialization.load_pem_private_key(pem, password=None, backend=default_backend()) diff --git a/tests/fail2ban.py b/tests/fail2ban.py index 9979507f..254232c4 100644 --- a/tests/fail2ban.py +++ b/tests/fail2ban.py @@ -39,7 +39,8 @@ def smtp_test(): try: server.login("fakeuser", "fakepassword") - raise Exception("authentication didn't fail") + msg = "authentication didn't fail" + raise Exception(msg) except smtplib.SMTPAuthenticationError: # athentication should fail pass @@ -61,7 +62,8 @@ def imap_test(): try: M.login("fakeuser", "fakepassword") - raise Exception("authentication didn't fail") + msg = "authentication didn't fail" + raise Exception(msg) except imaplib.IMAP4.error: # authentication should fail pass @@ -85,7 +87,8 @@ def pop_test(): M = None # don't .quit() return M.list() - raise Exception("authentication didn't fail") + msg = "authentication didn't fail" + raise Exception(msg) finally: if M: M.quit() @@ -103,7 +106,8 @@ def managesieve_test(): try: M.login("fakeuser", "fakepassword") - raise Exception("authentication didn't fail") + msg = "authentication didn't fail" + raise Exception(msg) except imaplib.IMAP4.error: # authentication should fail pass From 555ecc1ebb31915721cd26b9f0b0c19485119c33 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:12:34 -0800 Subject: [PATCH 05/55] Fixed PIE810 (multiple-starts-ends-with): Call `startswith` once with a `tuple` --- management/backup.py | 2 +- management/dns_update.py | 2 +- management/mailconfig.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/management/backup.py b/management/backup.py index 75046971..bd8e9f00 100755 --- a/management/backup.py +++ b/management/backup.py @@ -69,7 +69,7 @@ def backup_status(env): # destination for the backups or the last backup job terminated unexpectedly. raise Exception("Something is wrong with the backup: " + collection_status) for line in collection_status.split('\n'): - if line.startswith(" full") or line.startswith(" inc"): + if line.startswith((" full", " inc")): backup = parse_line(line) backups[backup["date"]] = backup diff --git a/management/dns_update.py b/management/dns_update.py index c9e4d359..8afbd376 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -262,7 +262,7 @@ def build_zone(domain, domain_properties, additional_records, env, is_zone=True) has_rec_base = list(records) a_expl = "Required. May have a different value. Sets the IP address that %s resolves to for web hosting and other services besides mail. The A record must be present but its value does not affect mail delivery." % domain if domain_properties[domain]["auto"]: - if domain.startswith("ns1.") or domain.startswith("ns2."): a_expl = False # omit from 'External DNS' page since this only applies if box is its own DNS server + if domain.startswith(("ns1.", "ns2.")): a_expl = False # omit from 'External DNS' page since this only applies if box is its own DNS server if domain.startswith("www."): a_expl = "Optional. Sets the IP address that %s resolves to so that the box can provide a redirect to the parent domain." % domain if domain.startswith("mta-sts."): a_expl = "Optional. MTA-STS Policy Host serving /.well-known/mta-sts.txt." if domain.startswith("autoconfig."): a_expl = "Provides email configuration autodiscovery support for Thunderbird Autoconfig." diff --git a/management/mailconfig.py b/management/mailconfig.py index cfdc3239..3971f825 100755 --- a/management/mailconfig.py +++ b/management/mailconfig.py @@ -87,7 +87,7 @@ def prettify_idn_email_address(email): def is_dcv_address(email): email = email.lower() for localpart in ("admin", "administrator", "postmaster", "hostmaster", "webmaster", "abuse"): - if email.startswith(localpart+"@") or email.startswith(localpart+"+"): + if email.startswith((localpart + "@", localpart + "+")): return True return False From 6bfd1e5140eab643c0e9461fc552c2c504fec473 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:12:50 -0800 Subject: [PATCH 06/55] Fixed W293 (blank-line-with-whitespace): Blank line contains whitespace --- management/backup.py | 6 +++--- management/dns_update.py | 2 +- management/mail_log.py | 2 +- management/utils.py | 2 +- tools/editconf.py | 12 ++++++------ 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/management/backup.py b/management/backup.py index bd8e9f00..c8b33d2f 100755 --- a/management/backup.py +++ b/management/backup.py @@ -226,7 +226,7 @@ def get_duplicity_additional_args(env): port = 22 if port is None: port = 22 - + return [ f"--ssh-options='-i /root/.ssh/id_rsa_miab -p {port}'", f"--rsync-options='-e \"/usr/bin/ssh -oStrictHostKeyChecking=no -oBatchMode=yes -p {port} -i /root/.ssh/id_rsa_miab\"'", @@ -497,7 +497,7 @@ def list_target_files(config): elif target.scheme == "s3": import boto3.s3 from botocore.exceptions import ClientError - + # separate bucket from path in target bucket = target.path[1:].split('/')[0] path = '/'.join(target.path[1:].split('/')[1:]) + '/' @@ -526,7 +526,7 @@ def list_target_files(config): from b2sdk.v1.exception import NonExistentBucket info = InMemoryAccountInfo() b2_api = B2Api(info) - + # Extract information from target b2_application_keyid = target.netloc[:target.netloc.index(':')] b2_application_key = urllib.parse.unquote(target.netloc[target.netloc.index(':')+1:target.netloc.index('@')]) diff --git a/management/dns_update.py b/management/dns_update.py index 8afbd376..74ea8a34 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -1044,7 +1044,7 @@ def set_secondary_dns(hostnames, env): resolver = dns.resolver.get_default_resolver() resolver.timeout = 5 resolver.lifetime = 5 - + for item in hostnames: if not item.startswith("xfr:"): # Resolve hostname. diff --git a/management/mail_log.py b/management/mail_log.py index 38ae1ec6..c54c2cb5 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -344,7 +344,7 @@ def scan_mail_log_line(line, collector): # Replaced the dateutil parser for a less clever way of parser that is roughly 4 times faster. # date = dateutil.parser.parse(date) - + # strptime fails on Feb 29 with ValueError: day is out of range for month if correct year is not provided. # See https://bugs.python.org/issue26460 date = datetime.datetime.strptime(str(NOW.year) + ' ' + date, '%Y %b %d %H:%M:%S') diff --git a/management/utils.py b/management/utils.py index baf99143..0c475d5c 100644 --- a/management/utils.py +++ b/management/utils.py @@ -95,7 +95,7 @@ def sort_domains(domain_names, env): # Then in right-to-left lexicographic order of the .-separated parts of the name. list(reversed(d.split("."))), )) - + return domain_names def sort_email_addresses(email_addresses, env): diff --git a/tools/editconf.py b/tools/editconf.py index 9e397846..898e49b6 100755 --- a/tools/editconf.py +++ b/tools/editconf.py @@ -110,30 +110,30 @@ while len(input_lines) > 0: buf += line found.add(i) break - + # comment-out the existing line (also comment any folded lines) if is_comment is None: buf += comment_char + line.rstrip().replace("\n", "\n" + comment_char) + "\n" else: # the line is already commented, pass it through buf += line - + # if this option already is set don't add the setting again, # or if we're clearing the setting with -e, don't add it if (i in found) or (not val and erase_setting): break - + # add the new setting buf += indent + name + delimiter + val + "\n" - + # note that we've applied this option found.add(i) - + break else: # If did not match any setting names, pass this line through. buf += line - + # Put any settings we didn't see at the end of the file, # except settings being cleared. for i in range(len(settings)): From b7f70b17acf2b8abfca31de9badda9ea5580172b Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:13:36 -0800 Subject: [PATCH 07/55] Fixed RET504 (unnecessary-assign) --- management/backup.py | 3 +-- management/mailconfig.py | 3 +-- management/status_checks.py | 3 +-- management/utils.py | 3 +-- management/web_update.py | 6 ++---- 5 files changed, 6 insertions(+), 12 deletions(-) diff --git a/management/backup.py b/management/backup.py index c8b33d2f..0f7674db 100755 --- a/management/backup.py +++ b/management/backup.py @@ -257,8 +257,7 @@ def get_duplicity_env_vars(env): return env def get_target_type(config): - protocol = config["target"].split(":")[0] - return protocol + return config["target"].split(":")[0] def perform_backup(full_backup): env = load_environment() diff --git a/management/mailconfig.py b/management/mailconfig.py index 3971f825..ed06c242 100755 --- a/management/mailconfig.py +++ b/management/mailconfig.py @@ -192,8 +192,7 @@ def get_mail_aliases(env): aliases = { row[0]: row for row in c.fetchall() } # make dict # put in a canonical order: sort by domain, then by email address lexicographically - aliases = [ aliases[address] for address in utils.sort_email_addresses(aliases.keys(), env) ] - return aliases + return [ aliases[address] for address in utils.sort_email_addresses(aliases.keys(), env) ] def get_mail_aliases_ex(env): # Returns a complex data structure of all mail aliases, similar diff --git a/management/status_checks.py b/management/status_checks.py index 7cdfffc6..3964e2df 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -915,8 +915,7 @@ def what_version_is_this(env): # Git may not be installed and Mail-in-a-Box may not have been cloned from github, # so this function may raise all sorts of exceptions. miab_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) - tag = shell("check_output", ["/usr/bin/git", "describe", "--always", "--abbrev=0"], env={"GIT_DIR": os.path.join(miab_dir, '.git')}).strip() - return tag + return shell("check_output", ["/usr/bin/git", "describe", "--always", "--abbrev=0"], env={"GIT_DIR": os.path.join(miab_dir, '.git')}).strip() def get_latest_miab_version(): # This pings https://mailinabox.email/setup.sh and extracts the tag named in diff --git a/management/utils.py b/management/utils.py index 0c475d5c..4ee100f1 100644 --- a/management/utils.py +++ b/management/utils.py @@ -81,7 +81,7 @@ def sort_domains(domain_names, env): )) # Now sort the domain names that fall within each zone. - domain_names = sorted(domain_names, + return sorted(domain_names, key = lambda d : ( # First by zone. zone_domains.index(zones[d]), @@ -96,7 +96,6 @@ def sort_domains(domain_names, env): list(reversed(d.split("."))), )) - return domain_names def sort_email_addresses(email_addresses, env): email_addresses = set(email_addresses) diff --git a/management/web_update.py b/management/web_update.py index 14ad1379..08b272e2 100644 --- a/management/web_update.py +++ b/management/web_update.py @@ -45,9 +45,8 @@ def get_web_domains(env, include_www_redirects=True, include_auto=True, exclude_ domains.add(env['PRIMARY_HOSTNAME']) # Sort the list so the nginx conf gets written in a stable order. - domains = sort_domains(domains, env) + return sort_domains(domains, env) - return domains def get_domains_with_a_records(env): domains = set() @@ -225,9 +224,8 @@ def make_domain_config(domain, templates, ssl_certificates, env): nginx_conf = nginx_conf.replace("$ROOT", root) nginx_conf = nginx_conf.replace("$SSL_KEY", tls_cert["private-key"]) nginx_conf = nginx_conf.replace("$SSL_CERTIFICATE", tls_cert["certificate"]) - nginx_conf = nginx_conf.replace("$REDIRECT_DOMAIN", re.sub(r"^www\.", "", domain)) # for default www redirects to parent domain + return nginx_conf.replace("$REDIRECT_DOMAIN", re.sub(r"^www\.", "", domain)) # for default www redirects to parent domain - return nginx_conf def get_web_root(domain, env, test_exists=True): # Try STORAGE_ROOT/web/domain_name if it exists, but fall back to STORAGE_ROOT/web/default. From 2b426851f9f256bb2d4a12e64c6712e6c1a6178e Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:14:00 -0800 Subject: [PATCH 08/55] Fixed UP032 (f-string): Use f-string instead of `format` call --- management/backup.py | 10 +++++----- management/daemon.py | 6 +++--- management/dns_update.py | 6 +++--- management/email_administrator.py | 2 +- management/mail_log.py | 9 ++++----- management/status_checks.py | 11 +++++------ tests/test_mail.py | 10 +++------- 7 files changed, 24 insertions(+), 30 deletions(-) diff --git a/management/backup.py b/management/backup.py index 0f7674db..81bd0f82 100755 --- a/management/backup.py +++ b/management/backup.py @@ -482,16 +482,16 @@ def list_target_files(config): if 'Permission denied (publickey).' in listing: reason = "Invalid user or check you correctly copied the SSH key." elif 'No such file or directory' in listing: - reason = "Provided path {} is invalid.".format(target_path) + reason = f"Provided path {target_path} is invalid." elif 'Network is unreachable' in listing: - reason = "The IP address {} is unreachable.".format(target.hostname) + reason = f"The IP address {target.hostname} is unreachable." elif 'Could not resolve hostname' in listing: - reason = "The hostname {} cannot be resolved.".format(target.hostname) + reason = f"The hostname {target.hostname} cannot be resolved." else: reason = "Unknown error." \ "Please check running 'management/backup.py --verify'" \ "from mailinabox sources to debug the issue." - raise ValueError("Connection to rsync host failed: {}".format(reason)) + raise ValueError(f"Connection to rsync host failed: {reason}") elif target.scheme == "s3": import boto3.s3 @@ -625,7 +625,7 @@ if __name__ == "__main__": elif sys.argv[-1] == "--list": # List the saved backup files. for fn, size in list_target_files(get_backup_config(load_environment())): - print("{}\t{}".format(fn, size)) + print(f"{fn}\t{size}") elif sys.argv[-1] == "--status": # Show backup status. diff --git a/management/daemon.py b/management/daemon.py index 1a35fba7..e53db4e6 100755 --- a/management/daemon.py +++ b/management/daemon.py @@ -80,7 +80,7 @@ def authorized_personnel_only(viewfunc): # Not authorized. Return a 401 (send auth) and a prompt to authorize by default. status = 401 headers = { - 'WWW-Authenticate': 'Basic realm="{0}"'.format(auth_service.auth_realm), + 'WWW-Authenticate': f'Basic realm="{auth_service.auth_realm}"', 'X-Reason': error, } @@ -164,7 +164,7 @@ def login(): "api_key": auth_service.create_session_key(email, env, type='login'), } - app.logger.info("New login session created for {}".format(email)) + app.logger.info(f"New login session created for {email}") # Return. return json_response(resp) @@ -173,7 +173,7 @@ def login(): def logout(): try: email, _ = auth_service.authenticate(request, env, logout=True) - app.logger.info("{} logged out".format(email)) + app.logger.info(f"{email} logged out") except ValueError as e: pass finally: diff --git a/management/dns_update.py b/management/dns_update.py index 74ea8a34..90523d33 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -534,7 +534,7 @@ $TTL 86400 ; default time to live zone += value + "\n" # Append a stable hash of DNSSEC signing keys in a comment. - zone += "\n; DNSSEC signing keys hash: {}\n".format(hash_dnssec_keys(domain, env)) + zone += f"\n; DNSSEC signing keys hash: {hash_dnssec_keys(domain, env)}\n" # DNSSEC requires re-signing a zone periodically. That requires # bumping the serial number even if no other records have changed. @@ -780,7 +780,7 @@ def write_opendkim_tables(domains, env): # So we must have a separate KeyTable entry for each domain. "SigningTable": "".join( - "*@{domain} {domain}\n".format(domain=domain) + f"*@{domain} {domain}\n" for domain in domains ), @@ -789,7 +789,7 @@ def write_opendkim_tables(domains, env): # signing domain must match the sender's From: domain. "KeyTable": "".join( - "{domain} {domain}:mail:{key_file}\n".format(domain=domain, key_file=opendkim_key_file) + f"{domain} {domain}:mail:{opendkim_key_file}\n" for domain in domains ), } diff --git a/management/email_administrator.py b/management/email_administrator.py index c87eda40..f2d7cac1 100755 --- a/management/email_administrator.py +++ b/management/email_administrator.py @@ -41,7 +41,7 @@ msg['From'] = "\"%s\" <%s>" % (env['PRIMARY_HOSTNAME'], admin_addr) msg['To'] = admin_addr msg['Subject'] = "[%s] %s" % (env['PRIMARY_HOSTNAME'], subject) -content_html = '
{}
'.format(html.escape(content)) +content_html = f'
{html.escape(content)}
' msg.attach(MIMEText(content, 'plain')) msg.attach(MIMEText(content_html, 'html')) diff --git a/management/mail_log.py b/management/mail_log.py index c54c2cb5..e52705b9 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -120,8 +120,7 @@ def scan_mail_log(env): except ImportError: pass - print("Scanning logs from {:%Y-%m-%d %H:%M:%S} to {:%Y-%m-%d %H:%M:%S}".format( - START_DATE, END_DATE) + print(f"Scanning logs from {START_DATE:%Y-%m-%d %H:%M:%S} to {END_DATE:%Y-%m-%d %H:%M:%S}" ) # Scan the lines in the log files until the date goes out of range @@ -227,7 +226,7 @@ def scan_mail_log(env): ], sub_data=[ ("Protocol and Source", [[ - "{} {}: {} times".format(protocol_name, host, count) + f"{protocol_name} {host}: {count} times" for (protocol_name, host), count in sorted(u["totals_by_protocol_and_host"].items(), key=lambda kv:-kv[1]) ] for u in data.values()]) @@ -672,7 +671,7 @@ def print_user_table(users, data=None, sub_data=None, activity=None, latest=None col_str = str_temp.format(d[row][:31] + "…" if len(d[row]) > 32 else d[row]) col_left[col] = True elif isinstance(d[row], datetime.datetime): - col_str = "{:<20}".format(str(d[row])) + col_str = f"{str(d[row]):<20}" col_left[col] = True else: temp = "{:>%s}" % max(5, len(l) + 1, len(str(d[row])) + 1) @@ -844,7 +843,7 @@ if __name__ == "__main__": END_DATE = args.enddate if args.timespan == 'today': args.timespan = 'day' - print("Setting end date to {}".format(END_DATE)) + print(f"Setting end date to {END_DATE}") START_DATE = END_DATE - TIME_DELTAS[args.timespan] diff --git a/management/status_checks.py b/management/status_checks.py index 3964e2df..3ab2f890 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -268,8 +268,7 @@ def check_free_disk_space(rounded_values, env, output): except: backup_cache_count = 0 if backup_cache_count > 1: - output.print_warning("The backup cache directory {} has more than one backup target cache. Consider clearing this directory to save disk space." - .format(backup_cache_path)) + output.print_warning(f"The backup cache directory {backup_cache_path} has more than one backup target cache. Consider clearing this directory to save disk space.") def check_free_memory(rounded_values, env, output): # Check free memory. @@ -731,9 +730,9 @@ def check_mail_domain(domain, env, output): if policy[1].get("mx") == [env['PRIMARY_HOSTNAME']] and policy[1].get("mode") == "enforce": # policy[0] is the policyid output.print_ok("MTA-STS policy is present.") else: - output.print_error("MTA-STS policy is present but has unexpected settings. [{}]".format(policy[1])) + output.print_error(f"MTA-STS policy is present but has unexpected settings. [{policy[1]}]") else: - output.print_error("MTA-STS policy is missing: {}".format(valid)) + output.print_error(f"MTA-STS policy is missing: {valid}") else: output.print_error("""This domain's DNS MX record is incorrect. It is currently set to '%s' but should be '%s'. Mail will not @@ -752,9 +751,9 @@ def check_mail_domain(domain, env, output): if dbl is None: output.print_ok("Domain is not blacklisted by dbl.spamhaus.org.") elif dbl == "[timeout]": - output.print_warning("Connection to dbl.spamhaus.org timed out. We could not determine whether the domain {} is blacklisted. Please try again later.".format(domain)) + output.print_warning(f"Connection to dbl.spamhaus.org timed out. We could not determine whether the domain {domain} is blacklisted. Please try again later.") elif dbl == "[Not Set]": - output.print_warning("Could not connect to dbl.spamhaus.org. We could not determine whether the domain {} is blacklisted. Please try again later.".format(domain)) + output.print_warning(f"Could not connect to dbl.spamhaus.org. We could not determine whether the domain {domain} is blacklisted. Please try again later.") else: output.print_error("""This domain is listed in the Spamhaus Domain Block List (code %s), which may prevent recipients from receiving your mail. diff --git a/tests/test_mail.py b/tests/test_mail.py index 312f3332..64ed3679 100755 --- a/tests/test_mail.py +++ b/tests/test_mail.py @@ -30,15 +30,11 @@ print("IMAP login is OK.") # Attempt to send a mail to ourself. mailsubject = "Mail-in-a-Box Automated Test Message " + uuid.uuid4().hex emailto = emailaddress -msg = """From: {emailaddress} +msg = f"""From: {emailaddress} To: {emailto} -Subject: {subject} +Subject: {mailsubject} -This is a test message. It should be automatically deleted by the test script.""".format( - emailaddress=emailaddress, - emailto=emailto, - subject=mailsubject, - ) +This is a test message. It should be automatically deleted by the test script.""" # Connect to the server on the SMTP submission TLS port. server = smtplib.SMTP_SSL(host) From 13b38cc04db83719ec967b49b47d7abdca4b4bf6 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:15:14 -0800 Subject: [PATCH 09/55] Fixed F841 (unused-variable) --- management/backup.py | 2 +- management/daemon.py | 2 +- management/dns_update.py | 10 +++++----- management/mail_log.py | 4 ++-- management/ssl_certificates.py | 4 ++-- management/status_checks.py | 4 ++-- tests/fail2ban.py | 4 ++-- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/management/backup.py b/management/backup.py index 81bd0f82..3b6b75cf 100755 --- a/management/backup.py +++ b/management/backup.py @@ -534,7 +534,7 @@ def list_target_files(config): try: b2_api.authorize_account("production", b2_application_keyid, b2_application_key) bucket = b2_api.get_bucket_by_name(b2_bucket) - except NonExistentBucket as e: + except NonExistentBucket: msg = "B2 Bucket does not exist. Please double check your information!" raise ValueError(msg) return [(key.file_name, key.size) for key, _ in bucket.ls()] diff --git a/management/daemon.py b/management/daemon.py index e53db4e6..026c2d6b 100755 --- a/management/daemon.py +++ b/management/daemon.py @@ -174,7 +174,7 @@ def logout(): try: email, _ = auth_service.authenticate(request, env, logout=True) app.logger.info(f"{email} logged out") - except ValueError as e: + except ValueError: pass finally: return json_response({ "status": "ok" }) diff --git a/management/dns_update.py b/management/dns_update.py index 90523d33..16a589c2 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -471,7 +471,7 @@ def build_sshfp_records(): for key in keys: if key.strip() == "" or key[0] == "#": continue try: - host, keytype, pubkey = key.split(" ") + _host, keytype, pubkey = key.split(" ") yield "%d %d ( %s )" % ( algorithm_number[keytype], 2, # specifies we are using SHA-256 on next line @@ -1049,19 +1049,19 @@ def set_secondary_dns(hostnames, env): if not item.startswith("xfr:"): # Resolve hostname. try: - response = resolver.resolve(item, "A") + resolver.resolve(item, "A") except (dns.resolver.NoNameservers, dns.resolver.NXDOMAIN, dns.resolver.NoAnswer, dns.resolver.Timeout): try: - response = resolver.resolve(item, "AAAA") + resolver.resolve(item, "AAAA") except (dns.resolver.NoNameservers, dns.resolver.NXDOMAIN, dns.resolver.NoAnswer, dns.resolver.Timeout): raise ValueError("Could not resolve the IP address of %s." % item) else: # Validate IP address. try: if "/" in item[4:]: - v = ipaddress.ip_network(item[4:]) # raises a ValueError if there's a problem + ipaddress.ip_network(item[4:]) # raises a ValueError if there's a problem else: - v = ipaddress.ip_address(item[4:]) # raises a ValueError if there's a problem + ipaddress.ip_address(item[4:]) # raises a ValueError if there's a problem except ValueError: raise ValueError("'%s' is not an IPv4 or IPv6 address or subnet." % item[4:]) diff --git a/management/mail_log.py b/management/mail_log.py index e52705b9..965fa74e 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -332,7 +332,7 @@ def scan_mail_log_line(line, collector): if not m: return True - date, system, service, log = m.groups() + date, _system, service, log = m.groups() collector["scan_count"] += 1 # print() @@ -554,7 +554,7 @@ def scan_postfix_submission_line(date, log, collector): m = re.match("([A-Z0-9]+): client=(\S+), sasl_method=(PLAIN|LOGIN), sasl_username=(\S+)(? Date: Fri, 22 Dec 2023 07:16:15 -0800 Subject: [PATCH 10/55] Fixed RSE102 (unnecessary-paren-on-raise-exception): Unnecessary parentheses on raised exception --- management/backup.py | 2 +- management/dns_update.py | 6 +++--- management/utils.py | 2 +- tests/fail2ban.py | 12 ++++++------ 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/management/backup.py b/management/backup.py index 3b6b75cf..93744d7d 100755 --- a/management/backup.py +++ b/management/backup.py @@ -581,7 +581,7 @@ def get_backup_config(env, for_save=False, for_ui=False): try: with open(os.path.join(backup_root, 'custom.yaml')) as f: custom_config = rtyaml.load(f) - if not isinstance(custom_config, dict): raise ValueError() # caught below + if not isinstance(custom_config, dict): raise ValueError # caught below config.update(custom_config) except: pass diff --git a/management/dns_update.py b/management/dns_update.py index 16a589c2..e911135f 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -817,7 +817,7 @@ def get_custom_dns_config(env, only_real_records=False): try: with open(os.path.join(env['STORAGE_ROOT'], 'dns/custom.yaml')) as f: custom_dns = rtyaml.load(f) - if not isinstance(custom_dns, dict): raise ValueError() # caught below + if not isinstance(custom_dns, dict): raise ValueError # caught below except: return [ ] @@ -835,7 +835,7 @@ def get_custom_dns_config(env, only_real_records=False): # No other type of data is allowed. else: - raise ValueError() + raise ValueError for rtype, value2 in values: if isinstance(value2, str): @@ -845,7 +845,7 @@ def get_custom_dns_config(env, only_real_records=False): yield (qname, rtype, value3) # No other type of data is allowed. else: - raise ValueError() + raise ValueError def filter_custom_records(domain, custom_dns_iter): for qname, rtype, value in custom_dns_iter: diff --git a/management/utils.py b/management/utils.py index 4ee100f1..f126f66b 100644 --- a/management/utils.py +++ b/management/utils.py @@ -38,7 +38,7 @@ def load_settings(env): try: with open(fn) as f: config = rtyaml.load(f) - if not isinstance(config, dict): raise ValueError() # caught below + if not isinstance(config, dict): raise ValueError # caught below return config except: return { } diff --git a/tests/fail2ban.py b/tests/fail2ban.py index bf002160..239eea23 100644 --- a/tests/fail2ban.py +++ b/tests/fail2ban.py @@ -33,7 +33,7 @@ def smtp_test(): server = smtplib.SMTP(hostname, 587) except ConnectionRefusedError: # looks like fail2ban worked - raise IsBlocked() + raise IsBlocked server.starttls() server.ehlo_or_helo_if_needed() @@ -58,7 +58,7 @@ def imap_test(): M = imaplib.IMAP4_SSL(hostname) except ConnectionRefusedError: # looks like fail2ban worked - raise IsBlocked() + raise IsBlocked try: M.login("fakeuser", "fakepassword") @@ -77,7 +77,7 @@ def pop_test(): M = poplib.POP3_SSL(hostname) except ConnectionRefusedError: # looks like fail2ban worked - raise IsBlocked() + raise IsBlocked try: M.user('fakeuser') try: @@ -102,7 +102,7 @@ def managesieve_test(): M = imaplib.IMAP4(hostname, 4190) except ConnectionRefusedError: # looks like fail2ban worked - raise IsBlocked() + raise IsBlocked try: M.login("fakeuser", "fakepassword") @@ -134,10 +134,10 @@ def http_test(url, expected_status, postdata=None, qsargs=None, auth=None): timeout=8, verify=False) # don't bother with HTTPS validation, it may not be configured yet except requests.exceptions.ConnectTimeout: - raise IsBlocked() + raise IsBlocked except requests.exceptions.ConnectionError as e: if "Connection refused" in str(e): - raise IsBlocked() + raise IsBlocked raise # some other unexpected condition # return response status code From fba92de0516d58d35217ebd70e02ef33d133ef90 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:17:23 -0800 Subject: [PATCH 11/55] Fixed SIM108 (if-else-block-instead-of-if-exp) --- management/cli.py | 10 ++-------- management/daemon.py | 5 +---- management/dns_update.py | 10 ++-------- management/mailconfig.py | 5 +---- 4 files changed, 6 insertions(+), 24 deletions(-) diff --git a/management/cli.py b/management/cli.py index 115ed9f2..f3394cc0 100755 --- a/management/cli.py +++ b/management/cli.py @@ -93,10 +93,7 @@ elif sys.argv[1] == "user" and len(sys.argv) == 2: elif sys.argv[1] == "user" and sys.argv[2] in {"add", "password"}: if len(sys.argv) < 5: - if len(sys.argv) < 4: - email = input("email: ") - else: - email = sys.argv[3] + email = input('email: ') if len(sys.argv) < 4 else sys.argv[3] pw = read_password() else: email, pw = sys.argv[3:5] @@ -110,10 +107,7 @@ elif sys.argv[1] == "user" and sys.argv[2] == "remove" and len(sys.argv) == 4: print(mgmt("/mail/users/remove", { "email": sys.argv[3] })) elif sys.argv[1] == "user" and sys.argv[2] in {"make-admin", "remove-admin"} and len(sys.argv) == 4: - if sys.argv[2] == "make-admin": - action = "add" - else: - action = "remove" + action = 'add' if sys.argv[2] == 'make-admin' else 'remove' print(mgmt("/mail/users/privileges/" + action, { "email": sys.argv[3], "privilege": "admin" })) elif sys.argv[1] == "user" and sys.argv[2] == "admins": diff --git a/management/daemon.py b/management/daemon.py index 026c2d6b..0e5b672b 100755 --- a/management/daemon.py +++ b/management/daemon.py @@ -751,10 +751,7 @@ def log_failed_login(request): # During setup we call the management interface directly to determine the user # status. So we can't always use X-Forwarded-For because during setup that header # will not be present. - if request.headers.getlist("X-Forwarded-For"): - ip = request.headers.getlist("X-Forwarded-For")[0] - else: - ip = request.remote_addr + ip = request.headers.getlist("X-Forwarded-For")[0] if request.headers.getlist("X-Forwarded-For") else request.remote_addr # We need to add a timestamp to the log message, otherwise /dev/log will eat the "duplicate" # message. diff --git a/management/dns_update.py b/management/dns_update.py index e911135f..3b623302 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -861,10 +861,7 @@ def filter_custom_records(domain, custom_dns_iter): # our short form (None => domain, or a relative QNAME) if # domain is not None. if domain is not None: - if qname == domain: - qname = None - else: - qname = qname[0:len(qname)-len("." + domain)] + qname = None if qname == domain else qname[0:len(qname) - len("." + domain)] yield (qname, rtype, value) @@ -1094,10 +1091,7 @@ def build_recommended_dns(env): # expand qnames for i in range(len(records)): - if records[i][0] == None: - qname = domain - else: - qname = records[i][0] + "." + domain + qname = domain if records[i][0] == None else records[i][0] + "." + domain records[i] = { "qname": qname, diff --git a/management/mailconfig.py b/management/mailconfig.py index ed06c242..787acd64 100755 --- a/management/mailconfig.py +++ b/management/mailconfig.py @@ -476,10 +476,7 @@ def add_mail_alias(address, forwards_to, permitted_senders, env, update_if_exist forwards_to = ",".join(validated_forwards_to) - if len(validated_permitted_senders) == 0: - permitted_senders = None - else: - permitted_senders = ",".join(validated_permitted_senders) + permitted_senders = None if len(validated_permitted_senders) == 0 else ",".join(validated_permitted_senders) conn, c = open_database(env, with_connection=True) try: From eefc0514b24adc5d05e021bf3466fa4a3c504a6e Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:17:45 -0800 Subject: [PATCH 12/55] Fixed UP030 (format-literals): Use implicit references for positional format fields --- management/status_checks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/status_checks.py b/management/status_checks.py index 693be554..09cb4f0c 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -679,7 +679,7 @@ def check_dnssec(domain, env, output, dns_zonefiles, is_checking_primary=False): output.print_line("") output.print_line("The DS record is currently set to:") for rr in sorted(ds): - output.print_line("Key Tag: {0}, Algorithm: {1}, Digest Type: {2}, Digest: {3}".format(*rr)) + output.print_line("Key Tag: {}, Algorithm: {}, Digest Type: {}, Digest: {}".format(*rr)) def check_mail_domain(domain, env, output): # Check the MX record. From 64540fbb4459cb568fc844394cf28c2884084fb7 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:18:38 -0800 Subject: [PATCH 13/55] Fixed UP034 (extraneous-parentheses): Avoid extraneous parentheses --- management/daemon.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/daemon.py b/management/daemon.py index 0e5b672b..ccde2e07 100755 --- a/management/daemon.py +++ b/management/daemon.py @@ -419,7 +419,7 @@ def ssl_get_status(): { "domain": d["domain"], "status": d["ssl_certificate"][0], - "text": d["ssl_certificate"][1] + ((" " + cant_provision[d["domain"]] if d["domain"] in cant_provision else "")) + "text": d["ssl_certificate"][1] + (" " + cant_provision[d["domain"]] if d["domain"] in cant_provision else "") } for d in domains_status ] # Warn the user about domain names not hosted here because of other settings. From 14a5613dc8cb9fe5a3d30d98b07ff628c42ff0e9 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:19:06 -0800 Subject: [PATCH 14/55] Fixed UP031 (printf-string-formatting): Use format specifiers instead of percent format --- management/daemon.py | 5 +- management/dns_update.py | 6 +- management/email_administrator.py | 4 +- management/mail_log.py | 2 +- management/mailconfig.py | 2 +- management/ssl_certificates.py | 10 ++-- management/status_checks.py | 96 ++++++++++++++----------------- management/utils.py | 2 +- management/web_update.py | 4 +- setup/migrate.py | 2 +- tests/fail2ban.py | 2 +- tests/test_dns.py | 2 +- tests/test_smtp_server.py | 6 +- 13 files changed, 67 insertions(+), 76 deletions(-) diff --git a/management/daemon.py b/management/daemon.py index ccde2e07..1d4cc2cd 100755 --- a/management/daemon.py +++ b/management/daemon.py @@ -580,8 +580,7 @@ def system_status(): def show_updates(): from status_checks import list_apt_updates return "".join( - "%s (%s)\n" - % (p["package"], p["version"]) + "{} ({})\n".format(p["package"], p["version"]) for p in list_apt_updates()) @app.route('/system/update-packages', methods=["POST"]) @@ -755,7 +754,7 @@ def log_failed_login(request): # We need to add a timestamp to the log message, otherwise /dev/log will eat the "duplicate" # message. - app.logger.warning( "Mail-in-a-Box Management Daemon: Failed login attempt from ip %s - timestamp %s" % (ip, time.time())) + app.logger.warning( "Mail-in-a-Box Management Daemon: Failed login attempt from ip {} - timestamp {}".format(ip, time.time())) # APP diff --git a/management/dns_update.py b/management/dns_update.py index 3b623302..13d03989 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -620,9 +620,9 @@ def write_nsd_conf(zonefiles, additional_records, env): for domain, zonefile in zonefiles: nsdconf += """ zone: - name: %s - zonefile: %s -""" % (domain, zonefile) + name: {} + zonefile: {} +""".format(domain, zonefile) # If custom secondary nameservers have been set, allow zone transfers # and, if not a subnet, notifies to them. diff --git a/management/email_administrator.py b/management/email_administrator.py index f2d7cac1..bd4d62d5 100755 --- a/management/email_administrator.py +++ b/management/email_administrator.py @@ -37,9 +37,9 @@ msg = MIMEMultipart('alternative') # In Python 3.6: #msg = Message() -msg['From'] = "\"%s\" <%s>" % (env['PRIMARY_HOSTNAME'], admin_addr) +msg['From'] = "\"{}\" <{}>".format(env['PRIMARY_HOSTNAME'], admin_addr) msg['To'] = admin_addr -msg['Subject'] = "[%s] %s" % (env['PRIMARY_HOSTNAME'], subject) +msg['Subject'] = "[{}] {}".format(env['PRIMARY_HOSTNAME'], subject) content_html = f'
{html.escape(content)}
' diff --git a/management/mail_log.py b/management/mail_log.py index 965fa74e..153470b3 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -302,7 +302,7 @@ def scan_mail_log(env): for date, sender, message in user_data["blocked"]: if len(sender) > 64: sender = sender[:32] + "…" + sender[-32:] - user_rejects.append("%s - %s " % (date, sender)) + user_rejects.append("{} - {} ".format(date, sender)) user_rejects.append(" %s" % message) rejects.append(user_rejects) diff --git a/management/mailconfig.py b/management/mailconfig.py index 787acd64..8b98de6e 100755 --- a/management/mailconfig.py +++ b/management/mailconfig.py @@ -589,7 +589,7 @@ def kick(env, mail_result=None): and forwards_to == get_system_administrator(env) \ and not auto: remove_mail_alias(address, env, do_kick=False) - results.append("removed alias %s (was to %s; domain no longer used for email)\n" % (address, forwards_to)) + results.append("removed alias {} (was to {}; domain no longer used for email)\n".format(address, forwards_to)) # Update DNS and nginx in case any domains are added/removed. diff --git a/management/ssl_certificates.py b/management/ssl_certificates.py index 9c263599..9ae7a0fa 100755 --- a/management/ssl_certificates.py +++ b/management/ssl_certificates.py @@ -211,7 +211,7 @@ def get_certificates_to_provision(env, limit_domains=None, show_valid_certs=True if not value: continue # IPv6 is not configured response = query_dns(domain, rtype) if response != normalize_ip(value): - bad_dns.append("%s (%s)" % (response, rtype)) + bad_dns.append("{} ({})".format(response, rtype)) if bad_dns: domains_cant_provision[domain] = "The domain name does not resolve to this machine: " \ @@ -413,7 +413,7 @@ def create_csr(domain, ssl_key, country_code, env): "openssl", "req", "-new", "-key", ssl_key, "-sha256", - "-subj", "/C=%s/CN=%s" % (country_code, domain)]) + "-subj", "/C={}/CN={}".format(country_code, domain)]) def install_cert(domain, ssl_cert, ssl_chain, env, raw=False): # Write the combined cert+chain to a temporary path and validate that it is OK. @@ -450,7 +450,7 @@ def install_cert_copy_file(fn, env): from binascii import hexlify cert = load_pem(load_cert_chain(fn)[0]) _all_domains, cn = get_certificate_domains(cert) - path = "%s-%s-%s.pem" % ( + path = "{}-{}-{}.pem".format( safe_domain_name(cn), # common name, which should be filename safe because it is IDNA-encoded, but in case of a malformed cert make sure it's ok to use as a filename cert.not_valid_after.date().isoformat().replace("-", ""), # expiration date hexlify(cert.fingerprint(hashes.SHA256())).decode("ascii")[0:8], # fingerprint prefix @@ -537,7 +537,7 @@ def check_certificate(domain, ssl_certificate, ssl_private_key, warn_if_expiring with open(ssl_private_key, 'rb') as f: priv_key = load_pem(f.read()) except ValueError as e: - return ("The private key file %s is not a private key file: %s" % (ssl_private_key, str(e)), None) + return ("The private key file {} is not a private key file: {}".format(ssl_private_key, str(e)), None) if not isinstance(priv_key, RSAPrivateKey): return ("The private key file %s is not a private key file." % ssl_private_key, None) @@ -565,7 +565,7 @@ def check_certificate(domain, ssl_certificate, ssl_private_key, warn_if_expiring import datetime now = datetime.datetime.utcnow() if not(cert.not_valid_before <= now <= cert.not_valid_after): - return ("The certificate has expired or is not yet valid. It is valid from %s to %s." % (cert.not_valid_before, cert.not_valid_after), None) + return ("The certificate has expired or is not yet valid. It is valid from {} to {}.".format(cert.not_valid_before, cert.not_valid_after), None) # Next validate that the certificate is valid. This checks whether the certificate # is self-signed, that the chain of trust makes sense, that it is signed by a CA diff --git a/management/status_checks.py b/management/status_checks.py index 09cb4f0c..c4422ea8 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -194,7 +194,7 @@ def check_ufw(env, output): for service in get_services(): if service["public"] and not is_port_allowed(ufw, service["port"]): not_allowed_ports += 1 - output.print_error("Port %s (%s) should be allowed in the firewall, please re-run the setup." % (service["port"], service["name"])) + output.print_error("Port {} ({}) should be allowed in the firewall, please re-run the setup.".format(service["port"], service["name"])) if not_allowed_ports == 0: output.print_ok("Firewall is active.") @@ -236,7 +236,7 @@ def check_software_updates(env, output): else: output.print_error("There are %d software packages that can be updated." % len(pkgs)) for p in pkgs: - output.print_line("%s (%s)" % (p["package"], p["version"])) + output.print_line("{} ({})".format(p["package"], p["version"])) def check_system_aliases(env, output): # Check that the administrator alias exists since that's where all @@ -316,9 +316,8 @@ def run_network_checks(env, output): elif zen == "[Not Set]": output.print_warning("Could not connect to zen.spamhaus.org. We could not determine whether your server's IP address is blacklisted. Please try again later.") else: - output.print_error("""The IP address of this machine %s is listed in the Spamhaus Block List (code %s), - which may prevent recipients from receiving your email. See http://www.spamhaus.org/query/ip/%s.""" - % (env['PUBLIC_IP'], zen, env['PUBLIC_IP'])) + output.print_error("""The IP address of this machine {} is listed in the Spamhaus Block List (code {}), + which may prevent recipients from receiving your email. See http://www.spamhaus.org/query/ip/{}.""".format(env['PUBLIC_IP'], zen, env['PUBLIC_IP'])) def run_domain_checks(rounded_time, env, output, pool, domains_to_check=None): # Get the list of domains we handle mail for. @@ -436,30 +435,27 @@ def check_primary_hostname_dns(domain, env, output, dns_domains, dns_zonefiles): # the nameserver, are reporting the right info --- but if the glue is incorrect this # will probably fail. if ns_ips == env['PUBLIC_IP'] + '/' + env['PUBLIC_IP']: - output.print_ok("Nameserver glue records are correct at registrar. [ns1/ns2.%s ↦ %s]" % (env['PRIMARY_HOSTNAME'], env['PUBLIC_IP'])) + output.print_ok("Nameserver glue records are correct at registrar. [ns1/ns2.{} ↦ {}]".format(env['PRIMARY_HOSTNAME'], env['PUBLIC_IP'])) elif ip == env['PUBLIC_IP']: # The NS records are not what we expect, but the domain resolves correctly, so # the user may have set up external DNS. List this discrepancy as a warning. - output.print_warning("""Nameserver glue records (ns1.%s and ns2.%s) should be configured at your domain name - registrar as having the IP address of this box (%s). They currently report addresses of %s. If you have set up External DNS, this may be OK.""" - % (env['PRIMARY_HOSTNAME'], env['PRIMARY_HOSTNAME'], env['PUBLIC_IP'], ns_ips)) + output.print_warning("""Nameserver glue records (ns1.{} and ns2.{}) should be configured at your domain name + registrar as having the IP address of this box ({}). They currently report addresses of {}. If you have set up External DNS, this may be OK.""".format(env['PRIMARY_HOSTNAME'], env['PRIMARY_HOSTNAME'], env['PUBLIC_IP'], ns_ips)) else: - output.print_error("""Nameserver glue records are incorrect. The ns1.%s and ns2.%s nameservers must be configured at your domain name - registrar as having the IP address %s. They currently report addresses of %s. It may take several hours for - public DNS to update after a change.""" - % (env['PRIMARY_HOSTNAME'], env['PRIMARY_HOSTNAME'], env['PUBLIC_IP'], ns_ips)) + output.print_error("""Nameserver glue records are incorrect. The ns1.{} and ns2.{} nameservers must be configured at your domain name + registrar as having the IP address {}. They currently report addresses of {}. It may take several hours for + public DNS to update after a change.""".format(env['PRIMARY_HOSTNAME'], env['PRIMARY_HOSTNAME'], env['PUBLIC_IP'], ns_ips)) # Check that PRIMARY_HOSTNAME resolves to PUBLIC_IP[V6] in public DNS. ipv6 = query_dns(domain, "AAAA") if env.get("PUBLIC_IPV6") else None if ip == env['PUBLIC_IP'] and not (ipv6 and env['PUBLIC_IPV6'] and ipv6 != normalize_ip(env['PUBLIC_IPV6'])): - output.print_ok("Domain resolves to box's IP address. [%s ↦ %s]" % (env['PRIMARY_HOSTNAME'], my_ips)) + output.print_ok("Domain resolves to box's IP address. [{} ↦ {}]".format(env['PRIMARY_HOSTNAME'], my_ips)) else: - output.print_error("""This domain must resolve to your box's IP address (%s) in public DNS but it currently resolves - to %s. It may take several hours for public DNS to update after a change. This problem may result from other - issues listed above.""" - % (my_ips, ip + ((" / " + ipv6) if ipv6 is not None else ""))) + output.print_error("""This domain must resolve to your box's IP address ({}) in public DNS but it currently resolves + to {}. It may take several hours for public DNS to update after a change. This problem may result from other + issues listed above.""".format(my_ips, ip + ((" / " + ipv6) if ipv6 is not None else ""))) # Check reverse DNS matches the PRIMARY_HOSTNAME. Note that it might not be @@ -467,13 +463,13 @@ def check_primary_hostname_dns(domain, env, output, dns_domains, dns_zonefiles): existing_rdns_v4 = query_dns(dns.reversename.from_address(env['PUBLIC_IP']), "PTR") existing_rdns_v6 = query_dns(dns.reversename.from_address(env['PUBLIC_IPV6']), "PTR") if env.get("PUBLIC_IPV6") else None if existing_rdns_v4 == domain and existing_rdns_v6 in {None, domain}: - output.print_ok("Reverse DNS is set correctly at ISP. [%s ↦ %s]" % (my_ips, env['PRIMARY_HOSTNAME'])) + output.print_ok("Reverse DNS is set correctly at ISP. [{} ↦ {}]".format(my_ips, env['PRIMARY_HOSTNAME'])) elif existing_rdns_v4 == existing_rdns_v6 or existing_rdns_v6 is None: - output.print_error("""Your box's reverse DNS is currently %s, but it should be %s. Your ISP or cloud provider will have instructions - on setting up reverse DNS for your box.""" % (existing_rdns_v4, domain) ) + output.print_error("""Your box's reverse DNS is currently {}, but it should be {}. Your ISP or cloud provider will have instructions + on setting up reverse DNS for your box.""".format(existing_rdns_v4, domain) ) else: - output.print_error("""Your box's reverse DNS is currently %s (IPv4) and %s (IPv6), but it should be %s. Your ISP or cloud provider will have instructions - on setting up reverse DNS for your box.""" % (existing_rdns_v4, existing_rdns_v6, domain) ) + output.print_error("""Your box's reverse DNS is currently {} (IPv4) and {} (IPv6), but it should be {}. Your ISP or cloud provider will have instructions + on setting up reverse DNS for your box.""".format(existing_rdns_v4, existing_rdns_v6, domain) ) # Check the TLSA record. tlsa_qname = "_25._tcp." + domain @@ -487,9 +483,8 @@ def check_primary_hostname_dns(domain, env, output, dns_domains, dns_zonefiles): # since TLSA shouldn't be used without DNSSEC. output.print_warning("""The DANE TLSA record for incoming mail is not set. This is optional.""") else: - output.print_error("""The DANE TLSA record for incoming mail (%s) is not correct. It is '%s' but it should be '%s'. - It may take several hours for public DNS to update after a change.""" - % (tlsa_qname, tlsa25, tlsa25_expected)) + output.print_error("""The DANE TLSA record for incoming mail ({}) is not correct. It is '{}' but it should be '{}'. + It may take several hours for public DNS to update after a change.""".format(tlsa_qname, tlsa25, tlsa25_expected)) # Check that the hostmaster@ email address exists. check_alias_exists("Hostmaster contact address", "hostmaster@" + domain, env, output) @@ -498,7 +493,7 @@ def check_alias_exists(alias_name, alias, env, output): mail_aliases = dict([(address, receivers) for address, receivers, *_ in get_mail_aliases(env)]) if alias in mail_aliases: if mail_aliases[alias]: - output.print_ok("%s exists as a mail alias. [%s ↦ %s]" % (alias_name, alias, mail_aliases[alias])) + output.print_ok("{} exists as a mail alias. [{} ↦ {}]".format(alias_name, alias, mail_aliases[alias])) else: output.print_error("""You must set the destination of the mail alias for %s to direct email to you or another administrator.""" % alias) else: @@ -533,14 +528,12 @@ def check_dns_zone(domain, env, output, dns_zonefiles): output.print_ok("Nameservers are set correctly at registrar. [%s]" % correct_ns) elif ip == correct_ip: # The domain resolves correctly, so maybe the user is using External DNS. - output.print_warning("""The nameservers set on this domain at your domain name registrar should be %s. They are currently %s. - If you are using External DNS, this may be OK.""" - % (correct_ns, existing_ns) ) + output.print_warning("""The nameservers set on this domain at your domain name registrar should be {}. They are currently {}. + If you are using External DNS, this may be OK.""".format(correct_ns, existing_ns) ) probably_external_dns = True else: - output.print_error("""The nameservers set on this domain are incorrect. They are currently %s. Use your domain name registrar's - control panel to set the nameservers to %s.""" - % (existing_ns, correct_ns) ) + output.print_error("""The nameservers set on this domain are incorrect. They are currently {}. Use your domain name registrar's + control panel to set the nameservers to {}.""".format(existing_ns, correct_ns) ) # Check that each custom secondary nameserver resolves the IP address. @@ -561,7 +554,7 @@ def check_dns_zone(domain, env, output, dns_zonefiles): elif ip is None: output.print_error("Secondary nameserver %s is not configured to resolve this domain." % ns) else: - output.print_error("Secondary nameserver %s is not configured correctly. (It resolved this domain as %s. It should be %s.)" % (ns, ip, correct_ip)) + output.print_error("Secondary nameserver {} is not configured correctly. (It resolved this domain as {}. It should be {}.)".format(ns, ip, correct_ip)) def check_dns_zone_suggestions(domain, env, output, dns_zonefiles, domains_with_a_records): # Warn if a custom DNS record is preventing this or the automatic www redirect from @@ -667,8 +660,8 @@ def check_dnssec(domain, env, output, dns_zonefiles, is_checking_primary=False): output.print_line("----------") output.print_line("Key Tag: " + ds_suggestion['keytag']) output.print_line("Key Flags: KSK / 257") - output.print_line("Algorithm: %s / %s" % (ds_suggestion['alg'], ds_suggestion['alg_name'])) - output.print_line("Digest Type: %s / %s" % (ds_suggestion['digalg'], ds_suggestion['digalg_name'])) + output.print_line("Algorithm: {} / {}".format(ds_suggestion['alg'], ds_suggestion['alg_name'])) + output.print_line("Digest Type: {} / {}".format(ds_suggestion['digalg'], ds_suggestion['digalg_name'])) output.print_line("Digest: " + ds_suggestion['digest']) output.print_line("Public Key: ") output.print_line(ds_suggestion['pubkey'], monospace=True) @@ -701,7 +694,7 @@ def check_mail_domain(domain, env, output): # the primary hostname's A record (the MX fallback) is... itself, # which is what we want the MX to be. if domain == env['PRIMARY_HOSTNAME']: - output.print_ok("Domain's email is directed to this domain. [%s has no MX record, which is ok]" % (domain,)) + output.print_ok("Domain's email is directed to this domain. [{} has no MX record, which is ok]".format(domain)) # And a missing MX record is okay on other domains if the A record # matches the A record of the PRIMARY_HOSTNAME. Actually this will @@ -710,16 +703,16 @@ def check_mail_domain(domain, env, output): domain_a = query_dns(domain, "A", nxdomain=None) primary_a = query_dns(env['PRIMARY_HOSTNAME'], "A", nxdomain=None) if domain_a != None and domain_a == primary_a: - output.print_ok("Domain's email is directed to this domain. [%s has no MX record but its A record is OK]" % (domain,)) + output.print_ok("Domain's email is directed to this domain. [{} has no MX record but its A record is OK]".format(domain)) else: - output.print_error("""This domain's DNS MX record is not set. It should be '%s'. Mail will not + output.print_error("""This domain's DNS MX record is not set. It should be '{}'. Mail will not be delivered to this box. It may take several hours for public DNS to update after a - change. This problem may result from other issues listed here.""" % (recommended_mx,)) + change. This problem may result from other issues listed here.""".format(recommended_mx)) elif mxhost == env['PRIMARY_HOSTNAME']: - good_news = "Domain's email is directed to this domain. [%s ↦ %s]" % (domain, mx) + good_news = "Domain's email is directed to this domain. [{} ↦ {}]".format(domain, mx) if mx != recommended_mx: - good_news += " This configuration is non-standard. The recommended configuration is '%s'." % (recommended_mx,) + good_news += " This configuration is non-standard. The recommended configuration is '{}'.".format(recommended_mx) output.print_ok(good_news) # Check MTA-STS policy. @@ -735,9 +728,9 @@ def check_mail_domain(domain, env, output): output.print_error(f"MTA-STS policy is missing: {valid}") else: - output.print_error("""This domain's DNS MX record is incorrect. It is currently set to '%s' but should be '%s'. Mail will not + output.print_error("""This domain's DNS MX record is incorrect. It is currently set to '{}' but should be '{}'. Mail will not be delivered to this box. It may take several hours for public DNS to update after a change. This problem may result from - other issues listed here.""" % (mx, recommended_mx)) + other issues listed here.""".format(mx, recommended_mx)) # Check that the postmaster@ email address exists. Not required if the domain has a # catch-all address or domain alias. @@ -755,9 +748,9 @@ def check_mail_domain(domain, env, output): elif dbl == "[Not Set]": output.print_warning(f"Could not connect to dbl.spamhaus.org. We could not determine whether the domain {domain} is blacklisted. Please try again later.") else: - output.print_error("""This domain is listed in the Spamhaus Domain Block List (code %s), + output.print_error("""This domain is listed in the Spamhaus Domain Block List (code {}), which may prevent recipients from receiving your mail. - See http://www.spamhaus.org/dbl/ and http://www.spamhaus.org/query/domain/%s.""" % (dbl, domain)) + See http://www.spamhaus.org/dbl/ and http://www.spamhaus.org/query/domain/{}.""".format(dbl, domain)) def check_web_domain(domain, rounded_time, ssl_certificates, env, output): # See if the domain's A record resolves to our PUBLIC_IP. This is already checked @@ -771,13 +764,13 @@ def check_web_domain(domain, rounded_time, ssl_certificates, env, output): if value == normalize_ip(expected): ok_values.append(value) else: - output.print_error("""This domain should resolve to your box's IP address (%s %s) if you would like the box to serve - webmail or a website on this domain. The domain currently resolves to %s in public DNS. It may take several hours for - public DNS to update after a change. This problem may result from other issues listed here.""" % (rtype, expected, value)) + output.print_error("""This domain should resolve to your box's IP address ({} {}) if you would like the box to serve + webmail or a website on this domain. The domain currently resolves to {} in public DNS. It may take several hours for + public DNS to update after a change. This problem may result from other issues listed here.""".format(rtype, expected, value)) return # If both A and AAAA are correct... - output.print_ok("Domain resolves to this box's IP address. [%s ↦ %s]" % (domain, '; '.join(ok_values))) + output.print_ok("Domain resolves to this box's IP address. [{} ↦ {}]".format(domain, '; '.join(ok_values))) # We need a TLS certificate for PRIMARY_HOSTNAME because that's where the @@ -945,8 +938,7 @@ def check_miab_version(env, output): elif latest_ver is None: output.print_error("Latest Mail-in-a-Box version could not be determined. You are running version %s." % this_ver) else: - output.print_error("A new version of Mail-in-a-Box is available. You are running version %s. The latest version is %s. For upgrade instructions, see https://mailinabox.email. " - % (this_ver, latest_ver)) + output.print_error("A new version of Mail-in-a-Box is available. You are running version {}. The latest version is {}. For upgrade instructions, see https://mailinabox.email. ".format(this_ver, latest_ver)) def run_and_output_changes(env, pool): import json diff --git a/management/utils.py b/management/utils.py index f126f66b..d1c09924 100644 --- a/management/utils.py +++ b/management/utils.py @@ -22,7 +22,7 @@ def load_env_vars_from_file(fn): def save_environment(env): with open("/etc/mailinabox.conf", "w") as f: for k, v in env.items(): - f.write("%s=%s\n" % (k, v)) + f.write("{}={}\n".format(k, v)) # THE SETTINGS FILE AT STORAGE_ROOT/settings.yaml. diff --git a/management/web_update.py b/management/web_update.py index 08b272e2..498f87c5 100644 --- a/management/web_update.py +++ b/management/web_update.py @@ -149,7 +149,7 @@ def make_domain_config(domain, templates, ssl_certificates, env): with open(filepath, 'rb') as f: sha1.update(f.read()) return sha1.hexdigest() - nginx_conf_extra += "\t# ssl files sha1: %s / %s\n" % (hashfile(tls_cert["private-key"]), hashfile(tls_cert["certificate"])) + nginx_conf_extra += "\t# ssl files sha1: {} / {}\n".format(hashfile(tls_cert["private-key"]), hashfile(tls_cert["certificate"])) # Add in any user customizations in YAML format. hsts = "yes" @@ -195,7 +195,7 @@ def make_domain_config(domain, templates, ssl_certificates, env): nginx_conf_extra += "\n\t\talias %s;" % alias nginx_conf_extra += "\n\t}\n" for path, url in yaml.get("redirects", {}).items(): - nginx_conf_extra += "\trewrite %s %s permanent;\n" % (path, url) + nginx_conf_extra += "\trewrite {} {} permanent;\n".format(path, url) # override the HSTS directive type hsts = yaml.get("hsts", hsts) diff --git a/setup/migrate.py b/setup/migrate.py index 9065cf40..78899f20 100755 --- a/setup/migrate.py +++ b/setup/migrate.py @@ -222,7 +222,7 @@ def run_migrations(): if migration_id is None: print() - print("%s file doesn't exists. Skipping migration..." % (migration_id_file,)) + print("{} file doesn't exists. Skipping migration...".format(migration_id_file)) return ourver = int(migration_id) diff --git a/tests/fail2ban.py b/tests/fail2ban.py index 239eea23..23f9c298 100644 --- a/tests/fail2ban.py +++ b/tests/fail2ban.py @@ -153,7 +153,7 @@ def restart_fail2ban_service(final=False): if not final: # Stop recidive jails during testing. command += " && sudo fail2ban-client stop recidive" - os.system("%s \"%s\"" % (ssh_command, command)) + os.system("{} \"{}\"".format(ssh_command, command)) def testfunc_runner(i, testfunc, *args): print(i+1, end=" ", flush=True) diff --git a/tests/test_dns.py b/tests/test_dns.py index 62cf5e78..11f6d2ed 100755 --- a/tests/test_dns.py +++ b/tests/test_dns.py @@ -98,7 +98,7 @@ else: # And if that's OK, also check reverse DNS (the PTR record). if not test_ptr("8.8.8.8", "Google Public DNS (Reverse DNS)"): print () - print ("The reverse DNS for %s is not correct. Consult your ISP for how to set the reverse DNS (also called the PTR record) for %s to %s." % (hostname, hostname, ipaddr)) + print ("The reverse DNS for {} is not correct. Consult your ISP for how to set the reverse DNS (also called the PTR record) for {} to {}.".format(hostname, hostname, ipaddr)) sys.exit(1) else: print ("And the reverse DNS for the domain is correct.") diff --git a/tests/test_smtp_server.py b/tests/test_smtp_server.py index 914c94b2..ca7e15ed 100755 --- a/tests/test_smtp_server.py +++ b/tests/test_smtp_server.py @@ -6,11 +6,11 @@ if len(sys.argv) < 3: sys.exit(1) host, toaddr, fromaddr = sys.argv[1:4] -msg = """From: %s -To: %s +msg = """From: {} +To: {} Subject: SMTP server test -This is a test message.""" % (fromaddr, toaddr) +This is a test message.""".format(fromaddr, toaddr) server = smtplib.SMTP(host, 25) server.set_debuglevel(1) From 3d72c32b1d2347e0bf4da884b5a5572fa6f6e567 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:19:40 -0800 Subject: [PATCH 15/55] Fixed W605 (invalid-escape-sequence) --- management/dns_update.py | 2 +- management/mail_log.py | 6 +++--- management/ssl_certificates.py | 4 ++-- management/status_checks.py | 8 ++++---- tests/tls.py | 8 ++++---- tools/editconf.py | 6 +++--- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/management/dns_update.py b/management/dns_update.py index 13d03989..ff3ec45d 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -16,7 +16,7 @@ from ssl_certificates import get_ssl_certificates, check_certificate # This regular expression matches domain names according to RFCs, it also accepts fqdn with an leading dot, # underscores, as well as asteriks which are allowed in domain names but not hostnames (i.e. allowed in # DNS but not in URLs), which are common in certain record types like for DKIM. -DOMAIN_RE = "^(?!\-)(?:[*][.])?(?:[a-zA-Z\d\-_]{0,62}[a-zA-Z\d_]\.){1,126}(?!\d+)[a-zA-Z\d_]{1,63}(\.?)$" +DOMAIN_RE = r"^(?!\-)(?:[*][.])?(?:[a-zA-Z\d\-_]{0,62}[a-zA-Z\d_]\.){1,126}(?!\d+)[a-zA-Z\d_]{1,63}(\.?)$" def get_dns_domains(env): # Add all domain names in use by email users and mail aliases, any diff --git a/management/mail_log.py b/management/mail_log.py index 153470b3..1386bebf 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -391,7 +391,7 @@ def scan_mail_log_line(line, collector): def scan_postgrey_line(date, log, collector): """ Scan a postgrey log line and extract interesting data """ - m = re.match("action=(greylist|pass), reason=(.*?), (?:delay=\d+, )?client_name=(.*), " + m = re.match(r"action=(greylist|pass), reason=(.*?), (?:delay=\d+, )?client_name=(.*), " "client_address=(.*), sender=(.*), recipient=(.*)", log) @@ -513,7 +513,7 @@ def scan_postfix_lmtp_line(date, log, collector): """ - m = re.match("([A-Z0-9]+): to=<(\S+)>, .* Saved", log) + m = re.match(r"([A-Z0-9]+): to=<(\S+)>, .* Saved", log) if m: _, user = m.groups() @@ -551,7 +551,7 @@ def scan_postfix_submission_line(date, log, collector): # Match both the 'plain' and 'login' sasl methods, since both authentication methods are # allowed by Dovecot. Exclude trailing comma after the username when additional fields # follow after. - m = re.match("([A-Z0-9]+): client=(\S+), sasl_method=(PLAIN|LOGIN), sasl_username=(\S+)(? self.width-1-len(first_line)): diff --git a/tests/tls.py b/tests/tls.py index e06ddcc9..ac51262d 100644 --- a/tests/tls.py +++ b/tests/tls.py @@ -94,8 +94,8 @@ def sslyze(opts, port, ok_ciphers): # Trim output to make better for storing in git. if "SCAN RESULTS FOR" not in out: # Failed. Just output the error. - out = re.sub("[\w\W]*CHECKING HOST\(S\) AVAILABILITY\n\s*-+\n", "", out) # chop off header that shows the host we queried - out = re.sub("[\w\W]*SCAN RESULTS FOR.*\n\s*-+\n", "", out) # chop off header that shows the host we queried + out = re.sub("[\\w\\W]*CHECKING HOST\\(S\\) AVAILABILITY\n\\s*-+\n", "", out) # chop off header that shows the host we queried + out = re.sub("[\\w\\W]*SCAN RESULTS FOR.*\n\\s*-+\n", "", out) # chop off header that shows the host we queried out = re.sub("SCAN COMPLETED IN .*", "", out) out = out.rstrip(" \n-") + "\n" @@ -105,8 +105,8 @@ def sslyze(opts, port, ok_ciphers): # Pull out the accepted ciphers list for each SSL/TLS protocol # version outputted. accepted_ciphers = set() - for ciphers in re.findall(" Accepted:([\w\W]*?)\n *\n", out): - accepted_ciphers |= set(re.findall("\n\s*(\S*)", ciphers)) + for ciphers in re.findall(" Accepted:([\\w\\W]*?)\n *\n", out): + accepted_ciphers |= set(re.findall("\n\\s*(\\S*)", ciphers)) # Compare to what Mozilla recommends, for a given modernness-level. print(" Should Not Offer: " + (", ".join(sorted(accepted_ciphers-set(ok_ciphers))) or "(none -- good)")) diff --git a/tools/editconf.py b/tools/editconf.py index 898e49b6..d9d05412 100755 --- a/tools/editconf.py +++ b/tools/editconf.py @@ -93,9 +93,9 @@ while len(input_lines) > 0: # Check if this line contain this setting from the command-line arguments. name, val = settings[i].split("=", 1) m = re.match( - "(\s*)" - + "(" + re.escape(comment_char) + "\s*)?" - + re.escape(name) + delimiter_re + "(.*?)\s*$", + r"(\s*)" + + "(" + re.escape(comment_char) + r"\s*)?" + + re.escape(name) + delimiter_re + r"(.*?)\s*$", line, re.S) if not m: continue indent, is_comment, existing_val = m.groups() From 67b9d0b2798c1d05cf4ded26d504fe6d664e73bb Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:19:47 -0800 Subject: [PATCH 16/55] Fixed PLW0108 (unnecessary-lambda): Lambda may be unnecessary; consider inlining inner function --- management/dns_update.py | 2 +- management/utils.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/management/dns_update.py b/management/dns_update.py index ff3ec45d..d28db626 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -38,7 +38,7 @@ def get_dns_zones(env): # Exclude domains that are subdomains of other domains we know. Proceed # by looking at shorter domains first. zone_domains = set() - for domain in sorted(domains, key=lambda d : len(d)): + for domain in sorted(domains, key=len): for d in zone_domains: if domain.endswith("." + d): # We found a parent domain already in the list. diff --git a/management/utils.py b/management/utils.py index d1c09924..0cfce602 100644 --- a/management/utils.py +++ b/management/utils.py @@ -59,7 +59,7 @@ def sort_domains(domain_names, env): # from shortest to longest since zones are always shorter than their # subdomains. zones = { } - for domain in sorted(domain_names, key=lambda d : len(d)): + for domain in sorted(domain_names, key=len): for z in zones.values(): if domain.endswith("." + z): # We found a parent domain already in the list. From e8d1c037cbfb7213486f2765f55ce566705954eb Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:20:00 -0800 Subject: [PATCH 17/55] Fixed SIM102 (collapsible-if): Use a single `if` statement instead of nested `if` statements --- management/mail_log.py | 49 +++++++++++++++++----------------- management/ssl_certificates.py | 14 +++++----- management/status_checks.py | 7 +++-- 3 files changed, 33 insertions(+), 37 deletions(-) diff --git a/management/mail_log.py b/management/mail_log.py index 1386bebf..b8494fbe 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -434,36 +434,35 @@ def scan_postfix_smtpd_line(date, log, collector): return # only log mail to known recipients - if user_match(user): - if collector["known_addresses"] is None or user in collector["known_addresses"]: - data = collector["rejected"].get( - user, - { - "blocked": [], - "earliest": None, - "latest": None, - } - ) - # simplify this one + if user_match(user) and (collector["known_addresses"] is None or user in collector["known_addresses"]): + data = collector["rejected"].get( + user, + { + "blocked": [], + "earliest": None, + "latest": None, + } + ) + # simplify this one + m = re.search( + r"Client host \[(.*?)\] blocked using zen.spamhaus.org; (.*)", message + ) + if m: + message = "ip blocked: " + m.group(2) + else: + # simplify this one too m = re.search( - r"Client host \[(.*?)\] blocked using zen.spamhaus.org; (.*)", message + r"Sender address \[.*@(.*)\] blocked using dbl.spamhaus.org; (.*)", message ) if m: - message = "ip blocked: " + m.group(2) - else: - # simplify this one too - m = re.search( - r"Sender address \[.*@(.*)\] blocked using dbl.spamhaus.org; (.*)", message - ) - if m: - message = "domain blocked: " + m.group(2) + message = "domain blocked: " + m.group(2) - if data["earliest"] is None: - data["earliest"] = date - data["latest"] = date - data["blocked"].append((date, sender, message)) + if data["earliest"] is None: + data["earliest"] = date + data["latest"] = date + data["blocked"].append((date, sender, message)) - collector["rejected"][user] = data + collector["rejected"][user] = data def scan_dovecot_login_line(date, log, collector, protocol_name): diff --git a/management/ssl_certificates.py b/management/ssl_certificates.py index 24797c68..d9c5ac51 100755 --- a/management/ssl_certificates.py +++ b/management/ssl_certificates.py @@ -82,9 +82,8 @@ def get_ssl_certificates(env): for domain in cert_domains: # The primary hostname can only use a certificate mapped # to the system private key. - if domain == env['PRIMARY_HOSTNAME']: - if cert["private_key"]["filename"] != os.path.join(env['STORAGE_ROOT'], 'ssl', 'ssl_private_key.pem'): - continue + if domain == env['PRIMARY_HOSTNAME'] and cert["private_key"]["filename"] != os.path.join(env['STORAGE_ROOT'], 'ssl', 'ssl_private_key.pem'): + continue domains.setdefault(domain, []).append(cert) @@ -149,11 +148,10 @@ def get_domain_ssl_files(domain, ssl_certificates, env, allow_missing_cert=False "certificate_object": load_pem(load_cert_chain(ssl_certificate)[0]), } - if use_main_cert: - if domain == env['PRIMARY_HOSTNAME']: - # The primary domain must use the server certificate because - # it is hard-coded in some service configuration files. - return system_certificate + if use_main_cert and domain == env['PRIMARY_HOSTNAME']: + # The primary domain must use the server certificate because + # it is hard-coded in some service configuration files. + return system_certificate wildcard_domain = re.sub(r"^[^\.]+", "*", domain) if domain in ssl_certificates: diff --git a/management/status_checks.py b/management/status_checks.py index dce2718c..1f96c23b 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -420,10 +420,9 @@ def check_primary_hostname_dns(domain, env, output, dns_domains, dns_zonefiles): # If a DS record is set on the zone containing this domain, check DNSSEC now. has_dnssec = False for zone in dns_domains: - if zone == domain or domain.endswith("." + zone): - if query_dns(zone, "DS", nxdomain=None) is not None: - has_dnssec = True - check_dnssec(zone, env, output, dns_zonefiles, is_checking_primary=True) + if (zone == domain or domain.endswith("." + zone)) and query_dns(zone, "DS", nxdomain=None) is not None: + has_dnssec = True + check_dnssec(zone, env, output, dns_zonefiles, is_checking_primary=True) ip = query_dns(domain, "A") ns_ips = query_dns("ns1." + domain, "A") + '/' + query_dns("ns2." + domain, "A") From 541f31b1bae8087ce21537772c222f74969bf4af Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:20:10 -0800 Subject: [PATCH 18/55] Fixed FURB113 (repeated-append) --- management/dns_update.py | 3 +-- management/mail_log.py | 8 ++------ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/management/dns_update.py b/management/dns_update.py index d28db626..515e2195 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -674,8 +674,7 @@ def hash_dnssec_keys(domain, env): keydata = [] for keytype, keyfn in sorted(find_dnssec_signing_keys(domain, env)): oldkeyfn = os.path.join(env['STORAGE_ROOT'], 'dns/dnssec', keyfn + ".private") - keydata.append(keytype) - keydata.append(keyfn) + keydata.extend((keytype, keyfn)) with open(oldkeyfn) as fr: keydata.append( fr.read() ) keydata = "".join(keydata).encode("utf8") diff --git a/management/mail_log.py b/management/mail_log.py index b8494fbe..bb862395 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -302,8 +302,7 @@ def scan_mail_log(env): for date, sender, message in user_data["blocked"]: if len(sender) > 64: sender = sender[:32] + "…" + sender[-32:] - user_rejects.append("{} - {} ".format(date, sender)) - user_rejects.append(" %s" % message) + user_rejects.extend(('{} - {} '.format(date, sender), ' %s' % message)) rejects.append(user_rejects) print_user_table( @@ -710,10 +709,7 @@ def print_user_table(users, data=None, sub_data=None, activity=None, latest=None if sub_data is not None: for l, d in sub_data: if d[row]: - lines.append("┬") - lines.append("│ %s" % l) - lines.append("├─%s─" % (len(l) * "─")) - lines.append("│") + lines.extend(('┬', '│ %s' % l, '├─%s─' % (len(l) * '─'), '│')) max_len = 0 for v in list(d[row]): lines.append("│ %s" % v) From 99d3929f9918885e49b5411f5049dda27d3f325b Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:20:28 -0800 Subject: [PATCH 19/55] Fixed E711 (none-comparison) --- management/dns_update.py | 8 ++++---- management/status_checks.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/management/dns_update.py b/management/dns_update.py index 515e2195..dc678a25 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -223,7 +223,7 @@ def build_zone(domain, domain_properties, additional_records, env, is_zone=True) subdomain_qname = subdomain[0:-len("." + domain)] subzone = build_zone(subdomain, domain_properties, additional_records, env, is_zone=False) for child_qname, child_rtype, child_value, child_explanation in subzone: - if child_qname == None: + if child_qname is None: child_qname = subdomain_qname else: child_qname += "." + subdomain_qname @@ -968,7 +968,7 @@ def set_custom_dns_record(qname, rtype, value, action, env): # Drop this record. made_change = True continue - if value == None and (_qname, _rtype) == (qname, rtype): + if value is None and (_qname, _rtype) == (qname, rtype): # Drop all qname-rtype records. made_change = True continue @@ -999,7 +999,7 @@ def get_secondary_dns(custom_dns, mode=None): if qname != '_secondary_nameserver': continue for hostname in value.split(" "): hostname = hostname.strip() - if mode == None: + if mode is None: # Just return the setting. values.append(hostname) continue @@ -1090,7 +1090,7 @@ def build_recommended_dns(env): # expand qnames for i in range(len(records)): - qname = domain if records[i][0] == None else records[i][0] + "." + domain + qname = domain if records[i][0] is None else records[i][0] + "." + domain records[i] = { "qname": qname, diff --git a/management/status_checks.py b/management/status_checks.py index 1f96c23b..cc9fd7d4 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -688,7 +688,7 @@ def check_mail_domain(domain, env, output): # of priority-host pairs. mxhost = mx.split('; ')[0].split(' ')[1] - if mxhost == None: + if mxhost is None: # A missing MX record is okay on the primary hostname because # the primary hostname's A record (the MX fallback) is... itself, # which is what we want the MX to be. @@ -701,7 +701,7 @@ def check_mail_domain(domain, env, output): else: domain_a = query_dns(domain, "A", nxdomain=None) primary_a = query_dns(env['PRIMARY_HOSTNAME'], "A", nxdomain=None) - if domain_a != None and domain_a == primary_a: + if domain_a is not None and domain_a == primary_a: output.print_ok("Domain's email is directed to this domain. [{} has no MX record but its A record is OK]".format(domain)) else: output.print_error("""This domain's DNS MX record is not set. It should be '{}'. Mail will not From 81a4da0181a62d81170e2ba414d08ce68f428f47 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:20:48 -0800 Subject: [PATCH 20/55] Fixed SIM110 (reimplemented-builtin) --- management/dns_update.py | 5 +---- management/mailconfig.py | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/management/dns_update.py b/management/dns_update.py index dc678a25..eee99dc3 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -231,10 +231,7 @@ def build_zone(domain, domain_properties, additional_records, env, is_zone=True) has_rec_base = list(records) # clone current state def has_rec(qname, rtype, prefix=None): - for rec in has_rec_base: - if rec[0] == qname and rec[1] == rtype and (prefix is None or rec[2].startswith(prefix)): - return True - return False + return any(rec[0] == qname and rec[1] == rtype and (prefix is None or rec[2].startswith(prefix)) for rec in has_rec_base) # The user may set other records that don't conflict with our settings. # Don't put any TXT records above this line, or it'll prevent any custom TXT records. diff --git a/management/mailconfig.py b/management/mailconfig.py index 8b98de6e..8705d56a 100755 --- a/management/mailconfig.py +++ b/management/mailconfig.py @@ -86,10 +86,7 @@ def prettify_idn_email_address(email): def is_dcv_address(email): email = email.lower() - for localpart in ("admin", "administrator", "postmaster", "hostmaster", "webmaster", "abuse"): - if email.startswith((localpart + "@", localpart + "+")): - return True - return False + return any(email.startswith((localpart + "@", localpart + "+")) for localpart in ("admin", "administrator", "postmaster", "hostmaster", "webmaster", "abuse")) def open_database(env, with_connection=False): conn = sqlite3.connect(env["STORAGE_ROOT"] + "/mail/users.sqlite") From c953e5784d018dcca21a3d80595ac80e61b27fad Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:21:04 -0800 Subject: [PATCH 21/55] Fixed C401 (unnecessary-generator-set): Unnecessary generator (rewrite as a `set` comprehension) --- management/dns_update.py | 4 ++-- management/mail_log.py | 2 +- management/status_checks.py | 2 +- management/utils.py | 4 ++-- management/web_update.py | 8 ++++---- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/management/dns_update.py b/management/dns_update.py index eee99dc3..96cffbae 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -361,8 +361,8 @@ def build_zone(domain, domain_properties, additional_records, env, is_zone=True) # non-mail domain and also may include qnames from custom DNS records. # Do this once at the end of generating a zone. if is_zone: - qnames_with_a = set(qname for (qname, rtype, value, explanation) in records if rtype in {"A", "AAAA"}) - qnames_with_mx = set(qname for (qname, rtype, value, explanation) in records if rtype == "MX") + qnames_with_a = {qname for (qname, rtype, value, explanation) in records if rtype in {"A", "AAAA"}} + qnames_with_mx = {qname for (qname, rtype, value, explanation) in records if rtype == "MX"} for qname in qnames_with_a - qnames_with_mx: # Mark this domain as not sending mail with hard-fail SPF and DMARC records. d = (qname+"." if qname else "") + domain diff --git a/management/mail_log.py b/management/mail_log.py index bb862395..ae718e55 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -116,7 +116,7 @@ def scan_mail_log(env): try: import mailconfig collector["known_addresses"] = (set(mailconfig.get_mail_users(env)) | - set(alias[0] for alias in mailconfig.get_mail_aliases(env))) + {alias[0] for alias in mailconfig.get_mail_aliases(env)}) except ImportError: pass diff --git a/management/status_checks.py b/management/status_checks.py index cc9fd7d4..ac4f3021 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -624,7 +624,7 @@ def check_dnssec(domain, env, output, dns_zonefiles, is_checking_primary=False): # # But it may not be preferred. Only algorithm 13 is preferred. Warn if any of the # matched zones uses a different algorithm. - if set(r[1] for r in matched_ds) == { '13' } and set(r[2] for r in matched_ds) <= { '2', '4' }: # all are alg 13 and digest type 2 or 4 + if {r[1] for r in matched_ds} == { '13' } and {r[2] for r in matched_ds} <= { '2', '4' }: # all are alg 13 and digest type 2 or 4 output.print_ok("DNSSEC 'DS' record is set correctly at registrar.") return elif len([r for r in matched_ds if r[1] == '13' and r[2] in { '2', '4' }]) > 0: # some but not all are alg 13 diff --git a/management/utils.py b/management/utils.py index 0cfce602..982c8c8a 100644 --- a/management/utils.py +++ b/management/utils.py @@ -99,10 +99,10 @@ def sort_domains(domain_names, env): def sort_email_addresses(email_addresses, env): email_addresses = set(email_addresses) - domains = set(email.split("@", 1)[1] for email in email_addresses if "@" in email) + domains = {email.split("@", 1)[1] for email in email_addresses if "@" in email} ret = [] for domain in sort_domains(domains, env): - domain_emails = set(email for email in email_addresses if email.endswith("@" + domain)) + domain_emails = {email for email in email_addresses if email.endswith("@" + domain)} ret.extend(sorted(domain_emails)) email_addresses -= domain_emails ret.extend(sorted(email_addresses)) # whatever is left diff --git a/management/web_update.py b/management/web_update.py index 498f87c5..6747a5b8 100644 --- a/management/web_update.py +++ b/management/web_update.py @@ -22,17 +22,17 @@ def get_web_domains(env, include_www_redirects=True, include_auto=True, exclude_ # Add 'www.' subdomains that we want to provide default redirects # to the main domain for. We'll add 'www.' to any DNS zones, i.e. # the topmost of each domain we serve. - domains |= set('www.' + zone for zone, zonefile in get_dns_zones(env)) + domains |= {'www.' + zone for zone, zonefile in get_dns_zones(env)} if include_auto: # Add Autoconfiguration domains for domains that there are user accounts at: # 'autoconfig.' for Mozilla Thunderbird auto setup. # 'autodiscover.' for ActiveSync autodiscovery (Z-Push). - domains |= set('autoconfig.' + maildomain for maildomain in get_mail_domains(env, users_only=True)) - domains |= set('autodiscover.' + maildomain for maildomain in get_mail_domains(env, users_only=True)) + domains |= {'autoconfig.' + maildomain for maildomain in get_mail_domains(env, users_only=True)} + domains |= {'autodiscover.' + maildomain for maildomain in get_mail_domains(env, users_only=True)} # 'mta-sts.' for MTA-STS support for all domains that have email addresses. - domains |= set('mta-sts.' + maildomain for maildomain in get_mail_domains(env)) + domains |= {'mta-sts.' + maildomain for maildomain in get_mail_domains(env)} if exclude_dns_elsewhere: # ...Unless the domain has an A/AAAA record that maps it to a different From 57d05c1ab26ef350689ee6c2551629254de04d0c Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:21:10 -0800 Subject: [PATCH 22/55] Fixed B007 (unused-loop-control-variable) --- management/dns_update.py | 10 +++++----- management/mailconfig.py | 2 +- management/ssl_certificates.py | 2 +- management/status_checks.py | 2 +- management/utils.py | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/management/dns_update.py b/management/dns_update.py index 96cffbae..753a4115 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -513,7 +513,7 @@ $TTL 86400 ; default time to live zone = zone.format(domain=domain, primary_domain=env["PRIMARY_HOSTNAME"]) # Add records. - for subdomain, querytype, value, explanation in records: + for subdomain, querytype, value, _explanation in records: if subdomain: zone += subdomain zone += "\tIN\t" + querytype + "\t" @@ -898,7 +898,7 @@ def write_custom_dns_config(config, env): def set_custom_dns_record(qname, rtype, value, action, env): # validate qname - for zone, fn in get_dns_zones(env): + for zone, _fn in get_dns_zones(env): # It must match a zone apex or be a subdomain of a zone # that we are otherwise hosting. if qname == zone or qname.endswith("."+zone): @@ -992,7 +992,7 @@ def get_secondary_dns(custom_dns, mode=None): resolver.lifetime = 10 values = [] - for qname, rtype, value in custom_dns: + for qname, _rtype, value in custom_dns: if qname != '_secondary_nameserver': continue for hostname in value.split(" "): hostname = hostname.strip() @@ -1078,7 +1078,7 @@ def get_custom_dns_records(custom_dns, qname, rtype): def build_recommended_dns(env): ret = [] - for (domain, zonefile, records) in build_zones(env): + for (domain, _zonefile, records) in build_zones(env): # remove records that we don't display records = [r for r in records if r[3] is not False] @@ -1106,7 +1106,7 @@ if __name__ == "__main__": if sys.argv[-1] == "--lint": write_custom_dns_config(get_custom_dns_config(env), env) else: - for zone, records in build_recommended_dns(env): + for _zone, records in build_recommended_dns(env): for record in records: print("; " + record['explanation']) print(record['qname'], record['rtype'], record['value'], sep="\t") diff --git a/management/mailconfig.py b/management/mailconfig.py index 8705d56a..b6c45c24 100755 --- a/management/mailconfig.py +++ b/management/mailconfig.py @@ -579,7 +579,7 @@ def kick(env, mail_result=None): # Remove auto-generated postmaster/admin/abuse alises from the main aliases table. # They are now stored in the auto_aliases table. - for address, forwards_to, permitted_senders, auto in get_mail_aliases(env): + for address, forwards_to, _permitted_senders, auto in get_mail_aliases(env): user, domain = address.split("@") if user in {"postmaster", "admin", "abuse"} \ and address not in required_aliases \ diff --git a/management/ssl_certificates.py b/management/ssl_certificates.py index d9c5ac51..4e84fc07 100755 --- a/management/ssl_certificates.py +++ b/management/ssl_certificates.py @@ -262,7 +262,7 @@ def provision_certificates(env, limit_domains): # primary domain listed in each certificate. from dns_update import get_dns_zones certs = { } - for zone, zonefile in get_dns_zones(env): + for zone, _zonefile in get_dns_zones(env): certs[zone] = [[]] for domain in sort_domains(domains, env): # Does the domain end with any domain we've seen so far. diff --git a/management/status_checks.py b/management/status_checks.py index ac4f3021..f73e5121 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -88,7 +88,7 @@ def run_services_checks(env, output, pool): all_running = True fatal = False ret = pool.starmap(check_service, ((i, service, env) for i, service in enumerate(get_services())), chunksize=1) - for i, running, fatal2, output2 in sorted(ret): + for _i, running, fatal2, output2 in sorted(ret): if output2 is None: continue # skip check (e.g. no port was set, e.g. no sshd) all_running = all_running and running fatal = fatal or fatal2 diff --git a/management/utils.py b/management/utils.py index 982c8c8a..a8fb5c2f 100644 --- a/management/utils.py +++ b/management/utils.py @@ -148,7 +148,7 @@ def du(path): # soft and hard links. total_size = 0 seen = set() - for dirpath, dirnames, filenames in os.walk(path): + for dirpath, _dirnames, filenames in os.walk(path): for f in filenames: fp = os.path.join(dirpath, f) try: From ca8f06d59093c0bb30221c8e56997145235a2f64 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:21:28 -0800 Subject: [PATCH 23/55] Fixed PLR1711 (useless-return): Useless `return` statement at end of function --- management/dns_update.py | 1 - 1 file changed, 1 deletion(-) diff --git a/management/dns_update.py b/management/dns_update.py index 753a4115..43f801d3 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -1072,7 +1072,6 @@ def get_custom_dns_records(custom_dns, qname, rtype): for qname1, rtype1, value in custom_dns: if qname1 == qname and rtype1 == rtype: yield value - return None ######################################################################## From 4999ed7b1cb631ba64f9b0b31892dacdbf6f2f25 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:21:37 -0800 Subject: [PATCH 24/55] Fixed Q003 (avoidable-escaped-quote): Change outer quotes to avoid escaping inner quotes --- management/email_administrator.py | 2 +- management/web_update.py | 4 ++-- tests/fail2ban.py | 4 ++-- tests/test_dns.py | 8 ++++---- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/management/email_administrator.py b/management/email_administrator.py index bd4d62d5..e5307e32 100755 --- a/management/email_administrator.py +++ b/management/email_administrator.py @@ -37,7 +37,7 @@ msg = MIMEMultipart('alternative') # In Python 3.6: #msg = Message() -msg['From'] = "\"{}\" <{}>".format(env['PRIMARY_HOSTNAME'], admin_addr) +msg['From'] = '"{}" <{}>'.format(env['PRIMARY_HOSTNAME'], admin_addr) msg['To'] = admin_addr msg['Subject'] = "[{}] {}".format(env['PRIMARY_HOSTNAME'], subject) diff --git a/management/web_update.py b/management/web_update.py index 6747a5b8..0a652b22 100644 --- a/management/web_update.py +++ b/management/web_update.py @@ -202,9 +202,9 @@ def make_domain_config(domain, templates, ssl_certificates, env): # Add the HSTS header. if hsts == "yes": - nginx_conf_extra += "\tadd_header Strict-Transport-Security \"max-age=15768000\" always;\n" + nginx_conf_extra += '\tadd_header Strict-Transport-Security "max-age=15768000" always;\n' elif hsts == "preload": - nginx_conf_extra += "\tadd_header Strict-Transport-Security \"max-age=15768000; includeSubDomains; preload\" always;\n" + nginx_conf_extra += '\tadd_header Strict-Transport-Security "max-age=15768000; includeSubDomains; preload" always;\n' # Add in any user customizations in the includes/ folder. nginx_conf_custom_include = os.path.join(env["STORAGE_ROOT"], "www", safe_domain_name(domain) + ".conf") diff --git a/tests/fail2ban.py b/tests/fail2ban.py index 23f9c298..04eabacc 100644 --- a/tests/fail2ban.py +++ b/tests/fail2ban.py @@ -11,7 +11,7 @@ import sys, os, time # parse command line if len(sys.argv) != 4: - print("Usage: tests/fail2ban.py \"ssh user@hostname\" hostname owncloud_user") + print('Usage: tests/fail2ban.py "ssh user@hostname" hostname owncloud_user') sys.exit(1) ssh_command, hostname, owncloud_user = sys.argv[1:4] @@ -153,7 +153,7 @@ def restart_fail2ban_service(final=False): if not final: # Stop recidive jails during testing. command += " && sudo fail2ban-client stop recidive" - os.system("{} \"{}\"".format(ssh_command, command)) + os.system('{} "{}"'.format(ssh_command, command)) def testfunc_runner(i, testfunc, *args): print(i+1, end=" ", flush=True) diff --git a/tests/test_dns.py b/tests/test_dns.py index 11f6d2ed..eac0ce1b 100755 --- a/tests/test_dns.py +++ b/tests/test_dns.py @@ -27,10 +27,10 @@ def test(server, description): ("ns2." + primary_hostname, "A", ipaddr), ("www." + hostname, "A", ipaddr), (hostname, "MX", "10 " + primary_hostname + "."), - (hostname, "TXT", "\"v=spf1 mx -all\""), - ("mail._domainkey." + hostname, "TXT", "\"v=DKIM1; k=rsa; s=email; \" \"p=__KEY__\""), + (hostname, "TXT", '"v=spf1 mx -all"'), + ("mail._domainkey." + hostname, "TXT", '"v=DKIM1; k=rsa; s=email; " "p=__KEY__"'), #("_adsp._domainkey." + hostname, "TXT", "\"dkim=all\""), - ("_dmarc." + hostname, "TXT", "\"v=DMARC1; p=quarantine;\""), + ("_dmarc." + hostname, "TXT", '"v=DMARC1; p=quarantine;"'), ] return test2(tests, server, description) @@ -59,7 +59,7 @@ def test2(tests, server, description): response = ["[no value]"] response = ";".join(str(r) for r in response) response = re.sub(r"(\"p=).*(\")", r"\1__KEY__\2", response) # normalize DKIM key - response = response.replace("\"\" ", "") # normalize TXT records (DNSSEC signing inserts empty text string components) + response = response.replace('"" ', "") # normalize TXT records (DNSSEC signing inserts empty text string components) # is it right? if response == expected_answer: From e0e6f1081b180e189ff7913868a65dcc84bcc33e Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:22:00 -0800 Subject: [PATCH 25/55] Fixed C414 (unnecessary-double-cast-or-process): Unnecessary `list` call within `sorted()` --- management/mail_log.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/mail_log.py b/management/mail_log.py index ae718e55..a8133218 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -320,7 +320,7 @@ def scan_mail_log(env): if collector["other-services"] and VERBOSE and False: print_header("Other services") print("The following unkown services were found in the log file.") - print(" ", *sorted(list(collector["other-services"])), sep='\n│ ') + print(" ", *sorted(collector["other-services"]), sep='\n│ ') def scan_mail_log_line(line, collector): From c585c1ecf66f739c2ba373ff0a7148423126623a Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:22:21 -0800 Subject: [PATCH 26/55] Fixed W291 (trailing-whitespace): Trailing whitespace --- management/mail_log.py | 2 +- tools/editconf.py | 2 +- tools/readable_bash.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/management/mail_log.py b/management/mail_log.py index a8133218..b654dcbc 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -547,7 +547,7 @@ def scan_postfix_submission_line(date, log, collector): """ # Match both the 'plain' and 'login' sasl methods, since both authentication methods are - # allowed by Dovecot. Exclude trailing comma after the username when additional fields + # allowed by Dovecot. Exclude trailing comma after the username when additional fields # follow after. m = re.match(r"([A-Z0-9]+): client=(\S+), sasl_method=(PLAIN|LOGIN), sasl_username=(\S+)(? Date: Fri, 22 Dec 2023 07:22:35 -0800 Subject: [PATCH 27/55] Fixed RET503 (implicit-return): Missing explicit `return` at the end of function able to return non-`None` value --- management/mail_log.py | 1 + management/mailconfig.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/management/mail_log.py b/management/mail_log.py index b654dcbc..0e1b6c5f 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -636,6 +636,7 @@ def print_time_table(labels, data, do_print=True): if do_print: print("\n".join(lines)) + return None else: return lines diff --git a/management/mailconfig.py b/management/mailconfig.py index b6c45c24..e5697929 100755 --- a/management/mailconfig.py +++ b/management/mailconfig.py @@ -491,6 +491,7 @@ def add_mail_alias(address, forwards_to, permitted_senders, env, update_if_exist if do_kick: # Update things in case any new domains are added. return kick(env, return_status) + return None def remove_mail_alias(address, env, do_kick=True): # convert Unicode domain to IDNA @@ -506,6 +507,7 @@ def remove_mail_alias(address, env, do_kick=True): if do_kick: # Update things in case any domains are removed. return kick(env, "alias removed") + return None def add_auto_aliases(aliases, env): conn, c = open_database(env, with_connection=True) From 57dcd4bb517d79200874e863f9f14ad6e917e9cf Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:22:55 -0800 Subject: [PATCH 28/55] Fixed E713 (not-in-test): Test for membership should be `not in` --- management/mailconfig.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/mailconfig.py b/management/mailconfig.py index e5697929..4e83fc72 100755 --- a/management/mailconfig.py +++ b/management/mailconfig.py @@ -221,7 +221,7 @@ def get_mail_aliases_ex(env): domain = get_domain(address) # add to list - if not domain in domains: + if domain not in domains: domains[domain] = { "domain": domain, "aliases": [], From ec32e1d57865456cabf10fcd8b36df728283dddc Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:23:08 -0800 Subject: [PATCH 29/55] Fixed E703 (useless-semicolon): Statement ends with an unnecessary semicolon --- management/mailconfig.py | 2 +- setup/migrate.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/management/mailconfig.py b/management/mailconfig.py index 4e83fc72..9eb40849 100755 --- a/management/mailconfig.py +++ b/management/mailconfig.py @@ -511,7 +511,7 @@ def remove_mail_alias(address, env, do_kick=True): def add_auto_aliases(aliases, env): conn, c = open_database(env, with_connection=True) - c.execute("DELETE FROM auto_aliases"); + c.execute("DELETE FROM auto_aliases") for source, destination in aliases.items(): c.execute("INSERT INTO auto_aliases (source, destination) VALUES (?, ?)", (source, destination)) conn.commit() diff --git a/setup/migrate.py b/setup/migrate.py index 78899f20..bf5685ff 100755 --- a/setup/migrate.py +++ b/setup/migrate.py @@ -213,7 +213,7 @@ def run_migrations(): migration_id = None if os.path.exists(migration_id_file): with open(migration_id_file) as f: - migration_id = f.read().strip(); + migration_id = f.read().strip() if migration_id is None: # Load the legacy location of the migration ID. We'll drop support From f62178929800e2a25ab3a3a61ef7ea3e411b1429 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:23:15 -0800 Subject: [PATCH 30/55] Fixed SIM118 (in-dict-keys): Use `key in dict` instead of `key in dict.keys()` --- management/ssl_certificates.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/ssl_certificates.py b/management/ssl_certificates.py index 4e84fc07..3e4bbd2a 100755 --- a/management/ssl_certificates.py +++ b/management/ssl_certificates.py @@ -266,7 +266,7 @@ def provision_certificates(env, limit_domains): certs[zone] = [[]] for domain in sort_domains(domains, env): # Does the domain end with any domain we've seen so far. - for parent in certs.keys(): + for parent in certs: if domain.endswith("." + parent): # Add this to the parent's list of domains. # Start a new group if the list already has From d661d623dc3e4bfbc4d1e1059dc1bfa1cd707041 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:23:24 -0800 Subject: [PATCH 31/55] Fixed RUF017 (quadratic-list-summation): Avoid quadratic list summation --- management/ssl_certificates.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/management/ssl_certificates.py b/management/ssl_certificates.py index 3e4bbd2a..3c1df5aa 100755 --- a/management/ssl_certificates.py +++ b/management/ssl_certificates.py @@ -4,6 +4,8 @@ import os, os.path, re, shutil, subprocess, tempfile from utils import shell, safe_domain_name, sort_domains +import functools +import operator # SELECTING SSL CERTIFICATES FOR USE IN WEB @@ -283,7 +285,7 @@ def provision_certificates(env, limit_domains): # Flatten to a list of lists of domains (from a mapping). Remove empty # lists (zones with no domains that need certs). - certs = sum(certs.values(), []) + certs = functools.reduce(operator.iadd, certs.values(), []) certs = [_ for _ in certs if len(_) > 0] # Prepare to provision. From fd4fcdaf53510ff9b79512a92f602d2128eca53f Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:23:57 -0800 Subject: [PATCH 32/55] Fixed E712 (true-false-comparison): Comparison to `False` should be `cond is False` or `if not cond:` --- management/status_checks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/status_checks.py b/management/status_checks.py index f73e5121..39149011 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -162,7 +162,7 @@ def check_service(i, service, env): output.print_error("%s is not running (port %d)." % (service['name'], service['port'])) # Flag if local DNS is not running. - if not running and service["port"] == 53 and service["public"] == False: + if not running and service["port"] == 53 and service["public"] is False: fatal = True return (i, running, fatal, output) From 54af4725f9ad8807374cc2ad2ee806f7ff49817b Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:24:11 -0800 Subject: [PATCH 33/55] Fixed C404 (unnecessary-list-comprehension-dict): Unnecessary `list` comprehension (rewrite as a `dict` comprehension) --- management/status_checks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/status_checks.py b/management/status_checks.py index 39149011..5424f0db 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -489,7 +489,7 @@ def check_primary_hostname_dns(domain, env, output, dns_domains, dns_zonefiles): check_alias_exists("Hostmaster contact address", "hostmaster@" + domain, env, output) def check_alias_exists(alias_name, alias, env, output): - mail_aliases = dict([(address, receivers) for address, receivers, *_ in get_mail_aliases(env)]) + mail_aliases = {address: receivers for address, receivers, *_ in get_mail_aliases(env)} if alias in mail_aliases: if mail_aliases[alias]: output.print_ok("{} exists as a mail alias. [{} ↦ {}]".format(alias_name, alias, mail_aliases[alias])) From 20a99c0ab81f10134bd4ce35b33423b18938ad5f Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:24:29 -0800 Subject: [PATCH 34/55] Fixed UP041 (timeout-error-alias): Replace aliased errors with `TimeoutError` --- management/status_checks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/status_checks.py b/management/status_checks.py index 5424f0db..3bc5faa5 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -916,7 +916,7 @@ def get_latest_miab_version(): try: return re.search(b'TAG=(.*)', urlopen("https://mailinabox.email/setup.sh?ping=1", timeout=5).read()).group(1).decode("utf8") - except (HTTPError, URLError, timeout): + except (TimeoutError, HTTPError, URLError): return None def check_miab_version(env, output): From 922c59ddafa5b604f4d45b9989248b64493e957c Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:24:58 -0800 Subject: [PATCH 35/55] Fixed SIM212 (if-expr-with-twisted-arms): Use `with_lines if with_lines else []` instead of `[] if not with_lines else with_lines` --- management/status_checks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/status_checks.py b/management/status_checks.py index 3bc5faa5..044a1187 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -1082,7 +1082,7 @@ class ConsoleOutput(FileOutput): class BufferedOutput: # Record all of the instance method calls so we can play them back later. def __init__(self, with_lines=None): - self.buf = [] if not with_lines else with_lines + self.buf = with_lines if with_lines else [] def __getattr__(self, attr): if attr not in {"add_heading", "print_ok", "print_error", "print_warning", "print_block", "print_line"}: raise AttributeError From d1d3d08d70ac126c7a0c0e619398318e8e96fc51 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:25:13 -0800 Subject: [PATCH 36/55] Fixed B006 (mutable-argument-default): Do not use mutable data structures for argument defaults --- management/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/management/utils.py b/management/utils.py index a8fb5c2f..178a03bc 100644 --- a/management/utils.py +++ b/management/utils.py @@ -108,11 +108,13 @@ def sort_email_addresses(email_addresses, env): ret.extend(sorted(email_addresses)) # whatever is left return ret -def shell(method, cmd_args, env={}, capture_stderr=False, return_bytes=False, trap=False, input=None): +def shell(method, cmd_args, env=None, capture_stderr=False, return_bytes=False, trap=False, input=None): # A safe way to execute processes. # Some processes like apt-get require being given a sane PATH. import subprocess + if env is None: + env = {} env.update({ "PATH": "/sbin:/bin:/usr/sbin:/usr/bin" }) kwargs = { 'env': env, From 8b9d3ec094e679a54d04a6dc259322ef29c79eb7 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:25:22 -0800 Subject: [PATCH 37/55] Fixed W292 (missing-newline-at-end-of-file): No newline at end of file --- management/wsgi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/wsgi.py b/management/wsgi.py index 9cf21c4b..1d5a37b8 100644 --- a/management/wsgi.py +++ b/management/wsgi.py @@ -4,4 +4,4 @@ import utils app.logger.addHandler(utils.create_syslog_handler()) if __name__ == "__main__": - app.run(port=10222) \ No newline at end of file + app.run(port=10222) From b13cef9b1dce9770235de38a954d32c5a8eb0a95 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:25:50 -0800 Subject: [PATCH 38/55] Fixed PIE790 (unnecessary-placeholder): Unnecessary `pass` statement --- tests/fail2ban.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/fail2ban.py b/tests/fail2ban.py index 04eabacc..b9447260 100644 --- a/tests/fail2ban.py +++ b/tests/fail2ban.py @@ -24,7 +24,6 @@ socket.setdefaulttimeout(10) class IsBlocked(Exception): """Tests raise this exception when it appears that a fail2ban jail is in effect, i.e. on a connection refused error.""" - pass def smtp_test(): import smtplib From 9b961b7ba028e4a30556914466707e55028a2912 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:25:57 -0800 Subject: [PATCH 39/55] Fixed UP024 (os-error-alias): Replace aliased errors with `OSError` --- tests/fail2ban.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fail2ban.py b/tests/fail2ban.py index b9447260..cf5bec88 100644 --- a/tests/fail2ban.py +++ b/tests/fail2ban.py @@ -142,7 +142,7 @@ def http_test(url, expected_status, postdata=None, qsargs=None, auth=None): # return response status code if r.status_code != expected_status: r.raise_for_status() # anything but 200 - raise IOError("Got unexpected status code %s." % r.status_code) + raise OSError("Got unexpected status code %s." % r.status_code) # define how to run a test From 6508d47da149ae55a068befe49193840bae097e5 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:26:06 -0800 Subject: [PATCH 40/55] Fixed C405 (unnecessary-literal-set): Unnecessary `list` literal (rewrite as a `set` literal) --- tests/tls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tls.py b/tests/tls.py index ac51262d..10782a9b 100644 --- a/tests/tls.py +++ b/tests/tls.py @@ -142,7 +142,7 @@ for cipher in csv.DictReader(io.StringIO(urllib.request.urlopen("https://raw.git client_compatibility = json.loads(urllib.request.urlopen("https://raw.githubusercontent.com/mail-in-a-box/user-agent-tls-capabilities/master/clients.json").read().decode("utf8")) cipher_clients = { } for client in client_compatibility: - if len(set(client['protocols']) & set(["TLS 1.0", "TLS 1.1", "TLS 1.2"])) == 0: continue # does not support TLS + if len(set(client['protocols']) & {"TLS 1.0", "TLS 1.1", "TLS 1.2"}) == 0: continue # does not support TLS for cipher in client['ciphers']: cipher_clients.setdefault(cipher_names.get(cipher), set()).add("/".join(x for x in [client['client']['name'], client['client']['version'], client['client']['platform']] if x)) From 3111cf56de6100932f299c879fce6dad92650c4e Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:40:31 -0800 Subject: [PATCH 41/55] Fixed EM102 (f-string-in-exception): Exception must not use an f-string literal, assign to variable first --- management/backup.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/management/backup.py b/management/backup.py index 93744d7d..13ff4a44 100755 --- a/management/backup.py +++ b/management/backup.py @@ -491,7 +491,8 @@ def list_target_files(config): reason = "Unknown error." \ "Please check running 'management/backup.py --verify'" \ "from mailinabox sources to debug the issue." - raise ValueError(f"Connection to rsync host failed: {reason}") + msg = f"Connection to rsync host failed: {reason}" + raise ValueError(msg) elif target.scheme == "s3": import boto3.s3 From c719fce40a33256ec25d25b8b8a4fa3c44fff904 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:41:21 -0800 Subject: [PATCH 42/55] Fixed UP032 (f-string): Use f-string instead of `format` call --- management/daemon.py | 2 +- management/dns_update.py | 8 +++--- management/mail_log.py | 2 +- management/mailconfig.py | 2 +- management/ssl_certificates.py | 8 +++--- management/status_checks.py | 52 +++++++++++++++++----------------- management/utils.py | 2 +- management/web_update.py | 2 +- setup/migrate.py | 2 +- tests/fail2ban.py | 2 +- tests/test_dns.py | 2 +- tests/test_smtp_server.py | 6 ++-- 12 files changed, 45 insertions(+), 45 deletions(-) diff --git a/management/daemon.py b/management/daemon.py index 1d4cc2cd..0c7531e6 100755 --- a/management/daemon.py +++ b/management/daemon.py @@ -754,7 +754,7 @@ def log_failed_login(request): # We need to add a timestamp to the log message, otherwise /dev/log will eat the "duplicate" # message. - app.logger.warning( "Mail-in-a-Box Management Daemon: Failed login attempt from ip {} - timestamp {}".format(ip, time.time())) + app.logger.warning( f"Mail-in-a-Box Management Daemon: Failed login attempt from ip {ip} - timestamp {time.time()}") # APP diff --git a/management/dns_update.py b/management/dns_update.py index 43f801d3..a4caf3b0 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -615,11 +615,11 @@ def write_nsd_conf(zonefiles, additional_records, env): # Append the zones. for domain, zonefile in zonefiles: - nsdconf += """ + nsdconf += f""" zone: - name: {} - zonefile: {} -""".format(domain, zonefile) + name: {domain} + zonefile: {zonefile} +""" # If custom secondary nameservers have been set, allow zone transfers # and, if not a subnet, notifies to them. diff --git a/management/mail_log.py b/management/mail_log.py index 0e1b6c5f..0e440422 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -302,7 +302,7 @@ def scan_mail_log(env): for date, sender, message in user_data["blocked"]: if len(sender) > 64: sender = sender[:32] + "…" + sender[-32:] - user_rejects.extend(('{} - {} '.format(date, sender), ' %s' % message)) + user_rejects.extend((f'{date} - {sender} ', ' %s' % message)) rejects.append(user_rejects) print_user_table( diff --git a/management/mailconfig.py b/management/mailconfig.py index 9eb40849..39ca1b6e 100755 --- a/management/mailconfig.py +++ b/management/mailconfig.py @@ -588,7 +588,7 @@ def kick(env, mail_result=None): and forwards_to == get_system_administrator(env) \ and not auto: remove_mail_alias(address, env, do_kick=False) - results.append("removed alias {} (was to {}; domain no longer used for email)\n".format(address, forwards_to)) + results.append(f"removed alias {address} (was to {forwards_to}; domain no longer used for email)\n") # Update DNS and nginx in case any domains are added/removed. diff --git a/management/ssl_certificates.py b/management/ssl_certificates.py index 3c1df5aa..9d866e6b 100755 --- a/management/ssl_certificates.py +++ b/management/ssl_certificates.py @@ -211,7 +211,7 @@ def get_certificates_to_provision(env, limit_domains=None, show_valid_certs=True if not value: continue # IPv6 is not configured response = query_dns(domain, rtype) if response != normalize_ip(value): - bad_dns.append("{} ({})".format(response, rtype)) + bad_dns.append(f"{response} ({rtype})") if bad_dns: domains_cant_provision[domain] = "The domain name does not resolve to this machine: " \ @@ -413,7 +413,7 @@ def create_csr(domain, ssl_key, country_code, env): "openssl", "req", "-new", "-key", ssl_key, "-sha256", - "-subj", "/C={}/CN={}".format(country_code, domain)]) + "-subj", f"/C={country_code}/CN={domain}"]) def install_cert(domain, ssl_cert, ssl_chain, env, raw=False): # Write the combined cert+chain to a temporary path and validate that it is OK. @@ -537,7 +537,7 @@ def check_certificate(domain, ssl_certificate, ssl_private_key, warn_if_expiring with open(ssl_private_key, 'rb') as f: priv_key = load_pem(f.read()) except ValueError as e: - return ("The private key file {} is not a private key file: {}".format(ssl_private_key, str(e)), None) + return (f"The private key file {ssl_private_key} is not a private key file: {str(e)}", None) if not isinstance(priv_key, RSAPrivateKey): return ("The private key file %s is not a private key file." % ssl_private_key, None) @@ -565,7 +565,7 @@ def check_certificate(domain, ssl_certificate, ssl_private_key, warn_if_expiring import datetime now = datetime.datetime.utcnow() if not(cert.not_valid_before <= now <= cert.not_valid_after): - return ("The certificate has expired or is not yet valid. It is valid from {} to {}.".format(cert.not_valid_before, cert.not_valid_after), None) + return (f"The certificate has expired or is not yet valid. It is valid from {cert.not_valid_before} to {cert.not_valid_after}.", None) # Next validate that the certificate is valid. This checks whether the certificate # is self-signed, that the chain of trust makes sense, that it is signed by a CA diff --git a/management/status_checks.py b/management/status_checks.py index 044a1187..a753cb59 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -464,11 +464,11 @@ def check_primary_hostname_dns(domain, env, output, dns_domains, dns_zonefiles): if existing_rdns_v4 == domain and existing_rdns_v6 in {None, domain}: output.print_ok("Reverse DNS is set correctly at ISP. [{} ↦ {}]".format(my_ips, env['PRIMARY_HOSTNAME'])) elif existing_rdns_v4 == existing_rdns_v6 or existing_rdns_v6 is None: - output.print_error("""Your box's reverse DNS is currently {}, but it should be {}. Your ISP or cloud provider will have instructions - on setting up reverse DNS for your box.""".format(existing_rdns_v4, domain) ) + output.print_error(f"""Your box's reverse DNS is currently {existing_rdns_v4}, but it should be {domain}. Your ISP or cloud provider will have instructions + on setting up reverse DNS for your box.""" ) else: - output.print_error("""Your box's reverse DNS is currently {} (IPv4) and {} (IPv6), but it should be {}. Your ISP or cloud provider will have instructions - on setting up reverse DNS for your box.""".format(existing_rdns_v4, existing_rdns_v6, domain) ) + output.print_error(f"""Your box's reverse DNS is currently {existing_rdns_v4} (IPv4) and {existing_rdns_v6} (IPv6), but it should be {domain}. Your ISP or cloud provider will have instructions + on setting up reverse DNS for your box.""" ) # Check the TLSA record. tlsa_qname = "_25._tcp." + domain @@ -482,8 +482,8 @@ def check_primary_hostname_dns(domain, env, output, dns_domains, dns_zonefiles): # since TLSA shouldn't be used without DNSSEC. output.print_warning("""The DANE TLSA record for incoming mail is not set. This is optional.""") else: - output.print_error("""The DANE TLSA record for incoming mail ({}) is not correct. It is '{}' but it should be '{}'. - It may take several hours for public DNS to update after a change.""".format(tlsa_qname, tlsa25, tlsa25_expected)) + output.print_error(f"""The DANE TLSA record for incoming mail ({tlsa_qname}) is not correct. It is '{tlsa25}' but it should be '{tlsa25_expected}'. + It may take several hours for public DNS to update after a change.""") # Check that the hostmaster@ email address exists. check_alias_exists("Hostmaster contact address", "hostmaster@" + domain, env, output) @@ -492,7 +492,7 @@ def check_alias_exists(alias_name, alias, env, output): mail_aliases = {address: receivers for address, receivers, *_ in get_mail_aliases(env)} if alias in mail_aliases: if mail_aliases[alias]: - output.print_ok("{} exists as a mail alias. [{} ↦ {}]".format(alias_name, alias, mail_aliases[alias])) + output.print_ok(f"{alias_name} exists as a mail alias. [{alias} ↦ {mail_aliases[alias]}]") else: output.print_error("""You must set the destination of the mail alias for %s to direct email to you or another administrator.""" % alias) else: @@ -527,12 +527,12 @@ def check_dns_zone(domain, env, output, dns_zonefiles): output.print_ok("Nameservers are set correctly at registrar. [%s]" % correct_ns) elif ip == correct_ip: # The domain resolves correctly, so maybe the user is using External DNS. - output.print_warning("""The nameservers set on this domain at your domain name registrar should be {}. They are currently {}. - If you are using External DNS, this may be OK.""".format(correct_ns, existing_ns) ) + output.print_warning(f"""The nameservers set on this domain at your domain name registrar should be {correct_ns}. They are currently {existing_ns}. + If you are using External DNS, this may be OK.""" ) probably_external_dns = True else: - output.print_error("""The nameservers set on this domain are incorrect. They are currently {}. Use your domain name registrar's - control panel to set the nameservers to {}.""".format(existing_ns, correct_ns) ) + output.print_error(f"""The nameservers set on this domain are incorrect. They are currently {existing_ns}. Use your domain name registrar's + control panel to set the nameservers to {correct_ns}.""" ) # Check that each custom secondary nameserver resolves the IP address. @@ -553,7 +553,7 @@ def check_dns_zone(domain, env, output, dns_zonefiles): elif ip is None: output.print_error("Secondary nameserver %s is not configured to resolve this domain." % ns) else: - output.print_error("Secondary nameserver {} is not configured correctly. (It resolved this domain as {}. It should be {}.)".format(ns, ip, correct_ip)) + output.print_error(f"Secondary nameserver {ns} is not configured correctly. (It resolved this domain as {ip}. It should be {correct_ip}.)") def check_dns_zone_suggestions(domain, env, output, dns_zonefiles, domains_with_a_records): # Warn if a custom DNS record is preventing this or the automatic www redirect from @@ -693,7 +693,7 @@ def check_mail_domain(domain, env, output): # the primary hostname's A record (the MX fallback) is... itself, # which is what we want the MX to be. if domain == env['PRIMARY_HOSTNAME']: - output.print_ok("Domain's email is directed to this domain. [{} has no MX record, which is ok]".format(domain)) + output.print_ok(f"Domain's email is directed to this domain. [{domain} has no MX record, which is ok]") # And a missing MX record is okay on other domains if the A record # matches the A record of the PRIMARY_HOSTNAME. Actually this will @@ -702,16 +702,16 @@ def check_mail_domain(domain, env, output): domain_a = query_dns(domain, "A", nxdomain=None) primary_a = query_dns(env['PRIMARY_HOSTNAME'], "A", nxdomain=None) if domain_a is not None and domain_a == primary_a: - output.print_ok("Domain's email is directed to this domain. [{} has no MX record but its A record is OK]".format(domain)) + output.print_ok(f"Domain's email is directed to this domain. [{domain} has no MX record but its A record is OK]") else: - output.print_error("""This domain's DNS MX record is not set. It should be '{}'. Mail will not + output.print_error(f"""This domain's DNS MX record is not set. It should be '{recommended_mx}'. Mail will not be delivered to this box. It may take several hours for public DNS to update after a - change. This problem may result from other issues listed here.""".format(recommended_mx)) + change. This problem may result from other issues listed here.""") elif mxhost == env['PRIMARY_HOSTNAME']: - good_news = "Domain's email is directed to this domain. [{} ↦ {}]".format(domain, mx) + good_news = f"Domain's email is directed to this domain. [{domain} ↦ {mx}]" if mx != recommended_mx: - good_news += " This configuration is non-standard. The recommended configuration is '{}'.".format(recommended_mx) + good_news += f" This configuration is non-standard. The recommended configuration is '{recommended_mx}'." output.print_ok(good_news) # Check MTA-STS policy. @@ -727,9 +727,9 @@ def check_mail_domain(domain, env, output): output.print_error(f"MTA-STS policy is missing: {valid}") else: - output.print_error("""This domain's DNS MX record is incorrect. It is currently set to '{}' but should be '{}'. Mail will not + output.print_error(f"""This domain's DNS MX record is incorrect. It is currently set to '{mx}' but should be '{recommended_mx}'. Mail will not be delivered to this box. It may take several hours for public DNS to update after a change. This problem may result from - other issues listed here.""".format(mx, recommended_mx)) + other issues listed here.""") # Check that the postmaster@ email address exists. Not required if the domain has a # catch-all address or domain alias. @@ -747,9 +747,9 @@ def check_mail_domain(domain, env, output): elif dbl == "[Not Set]": output.print_warning(f"Could not connect to dbl.spamhaus.org. We could not determine whether the domain {domain} is blacklisted. Please try again later.") else: - output.print_error("""This domain is listed in the Spamhaus Domain Block List (code {}), + output.print_error(f"""This domain is listed in the Spamhaus Domain Block List (code {dbl}), which may prevent recipients from receiving your mail. - See http://www.spamhaus.org/dbl/ and http://www.spamhaus.org/query/domain/{}.""".format(dbl, domain)) + See http://www.spamhaus.org/dbl/ and http://www.spamhaus.org/query/domain/{domain}.""") def check_web_domain(domain, rounded_time, ssl_certificates, env, output): # See if the domain's A record resolves to our PUBLIC_IP. This is already checked @@ -763,9 +763,9 @@ def check_web_domain(domain, rounded_time, ssl_certificates, env, output): if value == normalize_ip(expected): ok_values.append(value) else: - output.print_error("""This domain should resolve to your box's IP address ({} {}) if you would like the box to serve - webmail or a website on this domain. The domain currently resolves to {} in public DNS. It may take several hours for - public DNS to update after a change. This problem may result from other issues listed here.""".format(rtype, expected, value)) + output.print_error(f"""This domain should resolve to your box's IP address ({rtype} {expected}) if you would like the box to serve + webmail or a website on this domain. The domain currently resolves to {value} in public DNS. It may take several hours for + public DNS to update after a change. This problem may result from other issues listed here.""") return # If both A and AAAA are correct... @@ -937,7 +937,7 @@ def check_miab_version(env, output): elif latest_ver is None: output.print_error("Latest Mail-in-a-Box version could not be determined. You are running version %s." % this_ver) else: - output.print_error("A new version of Mail-in-a-Box is available. You are running version {}. The latest version is {}. For upgrade instructions, see https://mailinabox.email. ".format(this_ver, latest_ver)) + output.print_error(f"A new version of Mail-in-a-Box is available. You are running version {this_ver}. The latest version is {latest_ver}. For upgrade instructions, see https://mailinabox.email. ") def run_and_output_changes(env, pool): import json diff --git a/management/utils.py b/management/utils.py index 178a03bc..54b5bb86 100644 --- a/management/utils.py +++ b/management/utils.py @@ -22,7 +22,7 @@ def load_env_vars_from_file(fn): def save_environment(env): with open("/etc/mailinabox.conf", "w") as f: for k, v in env.items(): - f.write("{}={}\n".format(k, v)) + f.write(f"{k}={v}\n") # THE SETTINGS FILE AT STORAGE_ROOT/settings.yaml. diff --git a/management/web_update.py b/management/web_update.py index 0a652b22..0aeae2ec 100644 --- a/management/web_update.py +++ b/management/web_update.py @@ -195,7 +195,7 @@ def make_domain_config(domain, templates, ssl_certificates, env): nginx_conf_extra += "\n\t\talias %s;" % alias nginx_conf_extra += "\n\t}\n" for path, url in yaml.get("redirects", {}).items(): - nginx_conf_extra += "\trewrite {} {} permanent;\n".format(path, url) + nginx_conf_extra += f"\trewrite {path} {url} permanent;\n" # override the HSTS directive type hsts = yaml.get("hsts", hsts) diff --git a/setup/migrate.py b/setup/migrate.py index bf5685ff..4b7591ba 100755 --- a/setup/migrate.py +++ b/setup/migrate.py @@ -222,7 +222,7 @@ def run_migrations(): if migration_id is None: print() - print("{} file doesn't exists. Skipping migration...".format(migration_id_file)) + print(f"{migration_id_file} file doesn't exists. Skipping migration...") return ourver = int(migration_id) diff --git a/tests/fail2ban.py b/tests/fail2ban.py index cf5bec88..2f2ac352 100644 --- a/tests/fail2ban.py +++ b/tests/fail2ban.py @@ -152,7 +152,7 @@ def restart_fail2ban_service(final=False): if not final: # Stop recidive jails during testing. command += " && sudo fail2ban-client stop recidive" - os.system('{} "{}"'.format(ssh_command, command)) + os.system(f'{ssh_command} "{command}"') def testfunc_runner(i, testfunc, *args): print(i+1, end=" ", flush=True) diff --git a/tests/test_dns.py b/tests/test_dns.py index eac0ce1b..53fdb545 100755 --- a/tests/test_dns.py +++ b/tests/test_dns.py @@ -98,7 +98,7 @@ else: # And if that's OK, also check reverse DNS (the PTR record). if not test_ptr("8.8.8.8", "Google Public DNS (Reverse DNS)"): print () - print ("The reverse DNS for {} is not correct. Consult your ISP for how to set the reverse DNS (also called the PTR record) for {} to {}.".format(hostname, hostname, ipaddr)) + print (f"The reverse DNS for {hostname} is not correct. Consult your ISP for how to set the reverse DNS (also called the PTR record) for {hostname} to {ipaddr}.") sys.exit(1) else: print ("And the reverse DNS for the domain is correct.") diff --git a/tests/test_smtp_server.py b/tests/test_smtp_server.py index ca7e15ed..246f4ecc 100755 --- a/tests/test_smtp_server.py +++ b/tests/test_smtp_server.py @@ -6,11 +6,11 @@ if len(sys.argv) < 3: sys.exit(1) host, toaddr, fromaddr = sys.argv[1:4] -msg = """From: {} -To: {} +msg = f"""From: {fromaddr} +To: {toaddr} Subject: SMTP server test -This is a test message.""".format(fromaddr, toaddr) +This is a test message.""" server = smtplib.SMTP(host, 25) server.set_debuglevel(1) From 15bddcbc397da9849f88b8040b0903657da00ac4 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:42:13 -0800 Subject: [PATCH 43/55] Fixed RUF010 (explicit-f-string-type-conversion): Use explicit conversion flag --- management/mail_log.py | 2 +- management/ssl_certificates.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/management/mail_log.py b/management/mail_log.py index 0e440422..52dd62d3 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -670,7 +670,7 @@ def print_user_table(users, data=None, sub_data=None, activity=None, latest=None col_str = str_temp.format(d[row][:31] + "…" if len(d[row]) > 32 else d[row]) col_left[col] = True elif isinstance(d[row], datetime.datetime): - col_str = f"{str(d[row]):<20}" + col_str = f"{d[row]!s:<20}" col_left[col] = True else: temp = "{:>%s}" % max(5, len(l) + 1, len(str(d[row])) + 1) diff --git a/management/ssl_certificates.py b/management/ssl_certificates.py index 9d866e6b..ab0cea4d 100755 --- a/management/ssl_certificates.py +++ b/management/ssl_certificates.py @@ -537,7 +537,7 @@ def check_certificate(domain, ssl_certificate, ssl_private_key, warn_if_expiring with open(ssl_private_key, 'rb') as f: priv_key = load_pem(f.read()) except ValueError as e: - return (f"The private key file {ssl_private_key} is not a private key file: {str(e)}", None) + return (f"The private key file {ssl_private_key} is not a private key file: {e!s}", None) if not isinstance(priv_key, RSAPrivateKey): return ("The private key file %s is not a private key file." % ssl_private_key, None) From a02b59d4e4af8f15405557a4597c28ab061d1159 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Fri, 22 Dec 2023 07:42:38 -0800 Subject: [PATCH 44/55] Fixed F401 (unused-import): `socket.timeout` imported but unused --- management/status_checks.py | 1 - 1 file changed, 1 deletion(-) diff --git a/management/status_checks.py b/management/status_checks.py index a753cb59..9f0a1474 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -912,7 +912,6 @@ def get_latest_miab_version(): # This pings https://mailinabox.email/setup.sh and extracts the tag named in # the script to determine the current product version. from urllib.request import urlopen, HTTPError, URLError - from socket import timeout try: return re.search(b'TAG=(.*)', urlopen("https://mailinabox.email/setup.sh?ping=1", timeout=5).read()).group(1).decode("utf8") From 0e9193651d3f9f23e08f9dd7823b4b9642ac120d Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Sat, 23 Dec 2023 05:07:25 -0800 Subject: [PATCH 45/55] Fixed PLW1514 (unspecified-encoding): `open` in text mode without explicit `encoding` argument --- management/auth.py | 2 +- management/backup.py | 8 +++--- management/cli.py | 2 +- management/daemon.py | 2 +- management/dns_update.py | 32 ++++++++++----------- management/mail_log.py | 2 +- management/status_checks.py | 10 +++---- management/utils.py | 8 +++--- management/web_update.py | 10 +++---- setup/migrate.py | 4 +-- tools/editconf.py | 4 +-- tools/parse-nginx-log-bootstrap-accesses.py | 4 +-- 12 files changed, 44 insertions(+), 44 deletions(-) diff --git a/management/auth.py b/management/auth.py index 36112572..873047d5 100644 --- a/management/auth.py +++ b/management/auth.py @@ -22,7 +22,7 @@ class AuthService: def init_system_api_key(self): """Write an API key to a local file so local processes can use the API""" - with open(self.key_path) as file: + with open(self.key_path, encoding='utf-8') as file: self.key = file.read() def authenticate(self, request, env, login_only=False, logout=False): diff --git a/management/backup.py b/management/backup.py index 13ff4a44..183ff607 100755 --- a/management/backup.py +++ b/management/backup.py @@ -185,7 +185,7 @@ def get_passphrase(env): # only needs to be 43 base64-characters to match AES256's key # length of 32 bytes. backup_root = os.path.join(env["STORAGE_ROOT"], 'backup') - with open(os.path.join(backup_root, 'secret_key.txt')) as f: + with open(os.path.join(backup_root, 'secret_key.txt'), encoding="utf-8") as f: passphrase = f.readline().strip() if len(passphrase) < 43: raise Exception("secret_key.txt's first line is too short!") @@ -580,7 +580,7 @@ def get_backup_config(env, for_save=False, for_ui=False): # Merge in anything written to custom.yaml. try: - with open(os.path.join(backup_root, 'custom.yaml')) as f: + with open(os.path.join(backup_root, 'custom.yaml'), encoding="utf-8") as f: custom_config = rtyaml.load(f) if not isinstance(custom_config, dict): raise ValueError # caught below config.update(custom_config) @@ -606,14 +606,14 @@ def get_backup_config(env, for_save=False, for_ui=False): config["target"] = "file://" + config["file_target_directory"] ssh_pub_key = os.path.join('/root', '.ssh', 'id_rsa_miab.pub') if os.path.exists(ssh_pub_key): - with open(ssh_pub_key) as f: + with open(ssh_pub_key, encoding="utf-8") as f: config["ssh_pub_key"] = f.read() return config def write_backup_config(env, newconfig): backup_root = os.path.join(env["STORAGE_ROOT"], 'backup') - with open(os.path.join(backup_root, 'custom.yaml'), "w") as f: + with open(os.path.join(backup_root, 'custom.yaml'), "w", encoding="utf-8") as f: f.write(rtyaml.dump(newconfig)) if __name__ == "__main__": diff --git a/management/cli.py b/management/cli.py index f3394cc0..ea060d3f 100755 --- a/management/cli.py +++ b/management/cli.py @@ -47,7 +47,7 @@ def read_password(): return first def setup_key_auth(mgmt_uri): - with open('/var/lib/mailinabox/api.key') as f: + with open('/var/lib/mailinabox/api.key', encoding='utf-8') as f: key = f.read().strip() auth_handler = urllib.request.HTTPBasicAuthHandler() diff --git a/management/daemon.py b/management/daemon.py index 0c7531e6..074e9fa7 100755 --- a/management/daemon.py +++ b/management/daemon.py @@ -36,7 +36,7 @@ except OSError: # for generating CSRs we need a list of country codes csr_country_codes = [] -with open(os.path.join(os.path.dirname(me), "csr_country_codes.tsv")) as f: +with open(os.path.join(os.path.dirname(me), "csr_country_codes.tsv"), encoding="utf-8") as f: for line in f: if line.strip() == "" or line.startswith("#"): continue code, name = line.strip().split("\t")[0:2] diff --git a/management/dns_update.py b/management/dns_update.py index a4caf3b0..c06ef352 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -295,7 +295,7 @@ def build_zone(domain, domain_properties, additional_records, env, is_zone=True) # Append the DKIM TXT record to the zone as generated by OpenDKIM. # Skip if the user has set a DKIM record already. opendkim_record_file = os.path.join(env['STORAGE_ROOT'], 'mail/dkim/mail.txt') - with open(opendkim_record_file) as orf: + with open(opendkim_record_file, encoding="utf-8") as orf: m = re.match(r'(\S+)\s+IN\s+TXT\s+\( ((?:"[^"]+"\s+)+)\)', orf.read(), re.S) val = "".join(re.findall(r'"([^"]+)"', m.group(2))) if not has_rec(m.group(1), "TXT", prefix="v=DKIM1; "): @@ -452,7 +452,7 @@ def build_sshfp_records(): # specify that port to sshkeyscan. port = 22 - with open('/etc/ssh/sshd_config') as f: + with open('/etc/ssh/sshd_config', encoding="utf-8") as f: for line in f: s = line.rstrip().split() if len(s) == 2 and s[0] == 'Port': @@ -547,7 +547,7 @@ $TTL 86400 ; default time to live # We've signed the domain. Check if we are close to the expiration # time of the signature. If so, we'll force a bump of the serial # number so we can re-sign it. - with open(zonefile + ".signed") as f: + with open(zonefile + ".signed", encoding="utf-8") as f: signed_zone = f.read() expiration_times = re.findall(r"\sRRSIG\s+SOA\s+\d+\s+\d+\s\d+\s+(\d{14})", signed_zone) if len(expiration_times) == 0: @@ -566,7 +566,7 @@ $TTL 86400 ; default time to live if os.path.exists(zonefile): # If the zone already exists, is different, and has a later serial number, # increment the number. - with open(zonefile) as f: + with open(zonefile, encoding="utf-8") as f: existing_zone = f.read() m = re.search(r"(\d+)\s*;\s*serial number", existing_zone) if m: @@ -590,7 +590,7 @@ $TTL 86400 ; default time to live zone = zone.replace("__SERIAL__", serial) # Write the zone file. - with open(zonefile, "w") as f: + with open(zonefile, "w", encoding="utf-8") as f: f.write(zone) return True # file is updated @@ -603,7 +603,7 @@ def get_dns_zonefile(zone, env): raise ValueError("%s is not a domain name that corresponds to a zone." % zone) nsd_zonefile = "/etc/nsd/zones/" + fn - with open(nsd_zonefile) as f: + with open(nsd_zonefile, encoding="utf-8") as f: return f.read() ######################################################################## @@ -631,13 +631,13 @@ zone: # Check if the file is changing. If it isn't changing, # return False to flag that no change was made. if os.path.exists(nsd_conf_file): - with open(nsd_conf_file) as f: + with open(nsd_conf_file, encoding="utf-8") as f: if f.read() == nsdconf: return False # Write out new contents and return True to signal that # configuration changed. - with open(nsd_conf_file, "w") as f: + with open(nsd_conf_file, "w", encoding="utf-8") as f: f.write(nsdconf) return True @@ -672,7 +672,7 @@ def hash_dnssec_keys(domain, env): for keytype, keyfn in sorted(find_dnssec_signing_keys(domain, env)): oldkeyfn = os.path.join(env['STORAGE_ROOT'], 'dns/dnssec', keyfn + ".private") keydata.extend((keytype, keyfn)) - with open(oldkeyfn) as fr: + with open(oldkeyfn, encoding="utf-8") as fr: keydata.append( fr.read() ) keydata = "".join(keydata).encode("utf8") return hashlib.sha1(keydata).hexdigest() @@ -700,12 +700,12 @@ def sign_zone(domain, zonefile, env): # Use os.umask and open().write() to securely create a copy that only # we (root) can read. oldkeyfn = os.path.join(env['STORAGE_ROOT'], 'dns/dnssec', keyfn + ext) - with open(oldkeyfn) as fr: + with open(oldkeyfn, encoding="utf-8") as fr: keydata = fr.read() keydata = keydata.replace("_domain_", domain) prev_umask = os.umask(0o77) # ensure written file is not world-readable try: - with open(newkeyfn + ext, "w") as fw: + with open(newkeyfn + ext, "w", encoding="utf-8") as fw: fw.write(keydata) finally: os.umask(prev_umask) # other files we write should be world-readable @@ -739,7 +739,7 @@ def sign_zone(domain, zonefile, env): # be used, so we'll pre-generate all for each key. One DS record per line. Only one # needs to actually be deployed at the registrar. We'll select the preferred one # in the status checks. - with open("/etc/nsd/zones/" + zonefile + ".ds", "w") as f: + with open("/etc/nsd/zones/" + zonefile + ".ds", "w", encoding="utf-8") as f: for key in ksk_keys: for digest_type in ('1', '2', '4'): rr_ds = shell('check_output', ["/usr/bin/ldns-key2ds", @@ -794,12 +794,12 @@ def write_opendkim_tables(domains, env): for filename, content in config.items(): # Don't write the file if it doesn't need an update. if os.path.exists("/etc/opendkim/" + filename): - with open("/etc/opendkim/" + filename) as f: + with open("/etc/opendkim/" + filename, encoding="utf-8") as f: if f.read() == content: continue # The contents needs to change. - with open("/etc/opendkim/" + filename, "w") as f: + with open("/etc/opendkim/" + filename, "w", encoding="utf-8") as f: f.write(content) did_update = True @@ -811,7 +811,7 @@ def write_opendkim_tables(domains, env): def get_custom_dns_config(env, only_real_records=False): try: - with open(os.path.join(env['STORAGE_ROOT'], 'dns/custom.yaml')) as f: + with open(os.path.join(env['STORAGE_ROOT'], 'dns/custom.yaml'), encoding="utf-8") as f: custom_dns = rtyaml.load(f) if not isinstance(custom_dns, dict): raise ValueError # caught below except: @@ -893,7 +893,7 @@ def write_custom_dns_config(config, env): # Write. config_yaml = rtyaml.dump(dns) - with open(os.path.join(env['STORAGE_ROOT'], 'dns/custom.yaml'), "w") as f: + with open(os.path.join(env['STORAGE_ROOT'], 'dns/custom.yaml'), "w", encoding="utf-8") as f: f.write(config_yaml) def set_custom_dns_record(qname, rtype, value, action, env): diff --git a/management/mail_log.py b/management/mail_log.py index 52dd62d3..0c91365b 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -585,7 +585,7 @@ def scan_postfix_submission_line(date, log, collector): def readline(filename): """ A generator that returns the lines of a file """ - with open(filename, errors='replace') as file: + with open(filename, errors='replace', encoding='utf-8') as file: while True: line = file.readline() if not line: diff --git a/management/status_checks.py b/management/status_checks.py index 9f0a1474..fa27396d 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -212,7 +212,7 @@ def check_ssh_password(env, output): # the configuration file. if not os.path.exists("/etc/ssh/sshd_config"): return - with open("/etc/ssh/sshd_config") as f: + with open("/etc/ssh/sshd_config", encoding="utf-8") as f: sshd = f.read() if re.search("\nPasswordAuthentication\\s+yes", sshd) \ or not re.search("\nPasswordAuthentication\\s+no", sshd): @@ -582,7 +582,7 @@ def check_dnssec(domain, env, output, dns_zonefiles, is_checking_primary=False): expected_ds_records = { } ds_file = '/etc/nsd/zones/' + dns_zonefiles[domain] + '.ds' if not os.path.exists(ds_file): return # Domain is in our database but DNS has not yet been updated. - with open(ds_file) as f: + with open(ds_file, encoding="utf-8") as f: for rr_ds in f: rr_ds = rr_ds.rstrip() ds_keytag, ds_alg, ds_digalg, ds_digest = rr_ds.split("\t")[4].split(" ") @@ -591,7 +591,7 @@ def check_dnssec(domain, env, output, dns_zonefiles, is_checking_primary=False): # record that we suggest using is for the KSK (and that's how the DS records were generated). # We'll also give the nice name for the key algorithm. dnssec_keys = load_env_vars_from_file(os.path.join(env['STORAGE_ROOT'], 'dns/dnssec/%s.conf' % alg_name_map[ds_alg])) - with open(os.path.join(env['STORAGE_ROOT'], 'dns/dnssec/' + dnssec_keys['KSK'] + '.key')) as f: + with open(os.path.join(env['STORAGE_ROOT'], 'dns/dnssec/' + dnssec_keys['KSK'] + '.key'), encoding="utf-8") as f: dnsssec_pubkey = f.read().split("\t")[3].split(" ")[3] expected_ds_records[ (ds_keytag, ds_alg, ds_digalg, ds_digest) ] = { @@ -951,7 +951,7 @@ def run_and_output_changes(env, pool): # Load previously saved status checks. cache_fn = "/var/cache/mailinabox/status_checks.json" if os.path.exists(cache_fn): - with open(cache_fn) as f: + with open(cache_fn, encoding="utf-8") as f: try: prev = json.load(f) except json.JSONDecodeError: @@ -1007,7 +1007,7 @@ def run_and_output_changes(env, pool): # Store the current status checks output for next time. os.makedirs(os.path.dirname(cache_fn), exist_ok=True) - with open(cache_fn, "w") as f: + with open(cache_fn, "w", encoding="utf-8") as f: json.dump(cur.buf, f, indent=True) def normalize_ip(ip): diff --git a/management/utils.py b/management/utils.py index 54b5bb86..929544d1 100644 --- a/management/utils.py +++ b/management/utils.py @@ -14,13 +14,13 @@ def load_env_vars_from_file(fn): # Load settings from a KEY=VALUE file. import collections env = collections.OrderedDict() - with open(fn) as f: + with open(fn, encoding="utf-8") as f: for line in f: env.setdefault(*line.strip().split("=", 1)) return env def save_environment(env): - with open("/etc/mailinabox.conf", "w") as f: + with open("/etc/mailinabox.conf", "w", encoding="utf-8") as f: for k, v in env.items(): f.write(f"{k}={v}\n") @@ -29,14 +29,14 @@ def save_environment(env): def write_settings(config, env): import rtyaml fn = os.path.join(env['STORAGE_ROOT'], 'settings.yaml') - with open(fn, "w") as f: + with open(fn, "w", encoding="utf-8") as f: f.write(rtyaml.dump(config)) def load_settings(env): import rtyaml fn = os.path.join(env['STORAGE_ROOT'], 'settings.yaml') try: - with open(fn) as f: + with open(fn, encoding="utf-8") as f: config = rtyaml.load(f) if not isinstance(config, dict): raise ValueError # caught below return config diff --git a/management/web_update.py b/management/web_update.py index 0aeae2ec..722c8883 100644 --- a/management/web_update.py +++ b/management/web_update.py @@ -62,7 +62,7 @@ def get_web_domains_with_root_overrides(env): root_overrides = { } nginx_conf_custom_fn = os.path.join(env["STORAGE_ROOT"], "www/custom.yaml") if os.path.exists(nginx_conf_custom_fn): - with open(nginx_conf_custom_fn) as f: + with open(nginx_conf_custom_fn, encoding='utf-8') as f: custom_settings = rtyaml.load(f) for domain, settings in custom_settings.items(): for type, value in [('redirect', settings.get('redirects', {}).get('/')), @@ -77,7 +77,7 @@ def do_web_update(env): # Helper for reading config files and templates def read_conf(conf_fn): - with open(os.path.join(os.path.dirname(__file__), "../conf", conf_fn)) as f: + with open(os.path.join(os.path.dirname(__file__), "../conf", conf_fn), encoding='utf-8') as f: return f.read() # Build an nginx configuration file. @@ -112,12 +112,12 @@ def do_web_update(env): # Did the file change? If not, don't bother writing & restarting nginx. nginx_conf_fn = "/etc/nginx/conf.d/local.conf" if os.path.exists(nginx_conf_fn): - with open(nginx_conf_fn) as f: + with open(nginx_conf_fn, encoding='utf-8') as f: if f.read() == nginx_conf: return "" # Save the file. - with open(nginx_conf_fn, "w") as f: + with open(nginx_conf_fn, "w", encoding='utf-8') as f: f.write(nginx_conf) # Kick nginx. Since this might be called from the web admin @@ -155,7 +155,7 @@ def make_domain_config(domain, templates, ssl_certificates, env): hsts = "yes" nginx_conf_custom_fn = os.path.join(env["STORAGE_ROOT"], "www/custom.yaml") if os.path.exists(nginx_conf_custom_fn): - with open(nginx_conf_custom_fn) as f: + with open(nginx_conf_custom_fn, encoding='utf-8') as f: yaml = rtyaml.load(f) if domain in yaml: yaml = yaml[domain] diff --git a/setup/migrate.py b/setup/migrate.py index 4b7591ba..3364b55d 100755 --- a/setup/migrate.py +++ b/setup/migrate.py @@ -212,7 +212,7 @@ def run_migrations(): migration_id_file = os.path.join(env['STORAGE_ROOT'], 'mailinabox.version') migration_id = None if os.path.exists(migration_id_file): - with open(migration_id_file) as f: + with open(migration_id_file, encoding='utf-8') as f: migration_id = f.read().strip() if migration_id is None: @@ -253,7 +253,7 @@ def run_migrations(): # Write out our current version now. Do this sooner rather than later # in case of any problems. - with open(migration_id_file, "w") as f: + with open(migration_id_file, "w", encoding='utf-8') as f: f.write(str(ourver) + "\n") # Delete the legacy location of this field. diff --git a/tools/editconf.py b/tools/editconf.py index 4724bbe8..8b36c6bb 100755 --- a/tools/editconf.py +++ b/tools/editconf.py @@ -76,7 +76,7 @@ for setting in settings: found = set() buf = "" -with open(filename) as f: +with open(filename, encoding="utf-8") as f: input_lines = list(f) while len(input_lines) > 0: @@ -144,7 +144,7 @@ for i in range(len(settings)): if not testing: # Write out the new file. - with open(filename, "w") as f: + with open(filename, "w", encoding="utf-8") as f: f.write(buf) else: # Just print the new file to stdout. diff --git a/tools/parse-nginx-log-bootstrap-accesses.py b/tools/parse-nginx-log-bootstrap-accesses.py index 6207b2e9..8eb74dec 100755 --- a/tools/parse-nginx-log-bootstrap-accesses.py +++ b/tools/parse-nginx-log-bootstrap-accesses.py @@ -38,7 +38,7 @@ for date, ip in accesses: # Since logs are rotated, store the statistics permanently in a JSON file. # Load in the stats from an existing file. if os.path.exists(outfn): - with open(outfn) as f: + with open(outfn, encoding="utf-8") as f: existing_data = json.load(f) for date, count in existing_data: if date not in by_date: @@ -51,5 +51,5 @@ by_date = sorted(by_date.items()) by_date.pop(-1) # Write out. -with open(outfn, "w") as f: +with open(outfn, "w", encoding="utf-8") as f: json.dump(by_date, f, sort_keys=True, indent=True) From e466b9bb5387a83cdd54a177cc49fcc9a85f1210 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Sat, 23 Dec 2023 05:20:45 -0800 Subject: [PATCH 46/55] Fixed RUF005 (collection-literal-concatenation) --- management/backup.py | 20 ++++++++++---------- management/status_checks.py | 2 +- management/web_update.py | 2 +- tests/fail2ban.py | 2 +- tests/tls.py | 2 +- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/management/backup.py b/management/backup.py index 183ff607..55320049 100755 --- a/management/backup.py +++ b/management/backup.py @@ -59,7 +59,7 @@ def backup_status(env): "--archive-dir", backup_cache_dir, "--gpg-options", "'--cipher-algo=AES256'", "--log-fd", "1", - ] + get_duplicity_additional_args(env) + [ + *get_duplicity_additional_args(env), get_duplicity_target_url(config) ], get_duplicity_env_vars(env), @@ -322,8 +322,8 @@ def perform_backup(full_backup): "--exclude", backup_root, "--volsize", "250", "--gpg-options", "'--cipher-algo=AES256'", - "--allow-source-mismatch" - ] + get_duplicity_additional_args(env) + [ + "--allow-source-mismatch", + *get_duplicity_additional_args(env), env["STORAGE_ROOT"], get_duplicity_target_url(config), ], @@ -344,7 +344,7 @@ def perform_backup(full_backup): "--verbosity", "error", "--archive-dir", backup_cache_dir, "--force", - ] + get_duplicity_additional_args(env) + [ + *get_duplicity_additional_args(env), get_duplicity_target_url(config) ], get_duplicity_env_vars(env)) @@ -360,7 +360,7 @@ def perform_backup(full_backup): "--verbosity", "error", "--archive-dir", backup_cache_dir, "--force", - ] + get_duplicity_additional_args(env) + [ + *get_duplicity_additional_args(env), get_duplicity_target_url(config) ], get_duplicity_env_vars(env)) @@ -399,7 +399,7 @@ def run_duplicity_verification(): "--compare-data", "--archive-dir", backup_cache_dir, "--exclude", backup_root, - ] + get_duplicity_additional_args(env) + [ + *get_duplicity_additional_args(env), get_duplicity_target_url(config), env["STORAGE_ROOT"], ], get_duplicity_env_vars(env)) @@ -412,9 +412,9 @@ def run_duplicity_restore(args): "/usr/bin/duplicity", "restore", "--archive-dir", backup_cache_dir, - ] + get_duplicity_additional_args(env) + [ - get_duplicity_target_url(config) - ] + args, + *get_duplicity_additional_args(env), + get_duplicity_target_url(config), + *args], get_duplicity_env_vars(env)) def print_duplicity_command(): @@ -426,7 +426,7 @@ def print_duplicity_command(): print(f"export {k}={shlex.quote(v)}") print("duplicity", "{command}", shlex.join([ "--archive-dir", backup_cache_dir, - ] + get_duplicity_additional_args(env) + [ + *get_duplicity_additional_args(env), get_duplicity_target_url(config) ])) diff --git a/management/status_checks.py b/management/status_checks.py index fa27396d..831a7510 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -518,7 +518,7 @@ def check_dns_zone(domain, env, output, dns_zonefiles): secondary_ns = custom_secondary_ns or ["ns2." + env['PRIMARY_HOSTNAME']] existing_ns = query_dns(domain, "NS") - correct_ns = "; ".join(sorted(["ns1." + env['PRIMARY_HOSTNAME']] + secondary_ns)) + correct_ns = "; ".join(sorted(["ns1." + env["PRIMARY_HOSTNAME"], *secondary_ns])) ip = query_dns(domain, "A") probably_external_dns = False diff --git a/management/web_update.py b/management/web_update.py index 722c8883..a0f0f799 100644 --- a/management/web_update.py +++ b/management/web_update.py @@ -215,7 +215,7 @@ def make_domain_config(domain, templates, ssl_certificates, env): # Combine the pieces. Iteratively place each template into the "# ADDITIONAL DIRECTIVES HERE" placeholder # of the previous template. nginx_conf = "# ADDITIONAL DIRECTIVES HERE\n" - for t in templates + [nginx_conf_extra]: + for t in [*templates, nginx_conf_extra]: nginx_conf = re.sub("[ \t]*# ADDITIONAL DIRECTIVES HERE *\n", t, nginx_conf) # Replace substitution strings in the template & return. diff --git a/tests/fail2ban.py b/tests/fail2ban.py index 2f2ac352..dbab874a 100644 --- a/tests/fail2ban.py +++ b/tests/fail2ban.py @@ -181,7 +181,7 @@ def run_test(testfunc, args, count, within_seconds, parallel): # Distribute the requests across the pool. asyncresults = [] for i in range(count): - ar = p.apply_async(testfunc_runner, [i, testfunc] + list(args)) + ar = p.apply_async(testfunc_runner, [i, testfunc, *list(args)]) asyncresults.append(ar) # Wait for all runs to finish. diff --git a/tests/tls.py b/tests/tls.py index 10782a9b..8795f36a 100644 --- a/tests/tls.py +++ b/tests/tls.py @@ -88,7 +88,7 @@ def sslyze(opts, port, ok_ciphers): try: # Execute SSLyze. - out = subprocess.check_output([SSLYZE] + common_opts + opts + [connection_string]) + out = subprocess.check_output([SSLYZE, *common_opts, *opts, connection_string]) out = out.decode("utf8") # Trim output to make better for storing in git. From 7f456d8e8ba14e50c2f60cfb78cbb959c93cf10a Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Sat, 23 Dec 2023 05:23:58 -0800 Subject: [PATCH 47/55] Fixed ISC002 (multi-line-implicit-string-concatenation): Implicitly concatenated string literals over multiple lines --- management/backup.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/management/backup.py b/management/backup.py index 55320049..3031662b 100755 --- a/management/backup.py +++ b/management/backup.py @@ -488,9 +488,9 @@ def list_target_files(config): elif 'Could not resolve hostname' in listing: reason = f"The hostname {target.hostname} cannot be resolved." else: - reason = "Unknown error." \ - "Please check running 'management/backup.py --verify'" \ - "from mailinabox sources to debug the issue." + reason = ("Unknown error." + "Please check running 'management/backup.py --verify'" + "from mailinabox sources to debug the issue.") msg = f"Connection to rsync host failed: {reason}" raise ValueError(msg) From 6a47133e3fe5616e83a65ac87006d35d54e0cf70 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Sat, 23 Dec 2023 05:24:59 -0800 Subject: [PATCH 48/55] Fixed F811 (redefined-while-unused): Redefinition of unused `sys` from line 10 --- management/backup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/management/backup.py b/management/backup.py index 3031662b..f352fb41 100755 --- a/management/backup.py +++ b/management/backup.py @@ -617,7 +617,6 @@ def write_backup_config(env, newconfig): f.write(rtyaml.dump(newconfig)) if __name__ == "__main__": - import sys if sys.argv[-1] == "--verify": # Run duplicity's verification command to check a) the backup files # are readable, and b) report if they are up to date. From f0377dd59ee10891edac84443e4a4018708d016b Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Sat, 23 Dec 2023 05:26:32 -0800 Subject: [PATCH 49/55] Fixed SIM105 (suppressible-exception) --- management/cli.py | 5 ++--- management/daemon.py | 5 ++--- management/dns_update.py | 5 ++--- setup/migrate.py | 5 ++--- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/management/cli.py b/management/cli.py index ea060d3f..70dbe894 100755 --- a/management/cli.py +++ b/management/cli.py @@ -7,6 +7,7 @@ # tool can only be used as root. import sys, getpass, urllib.request, urllib.error, json, csv +import contextlib def mgmt(cmd, data=None, is_json=False): # The base URL for the management daemon. (Listens on IPv4 only.) @@ -19,10 +20,8 @@ def mgmt(cmd, data=None, is_json=False): response = urllib.request.urlopen(req) except urllib.error.HTTPError as e: if e.code == 401: - try: + with contextlib.suppress(Exception): print(e.read().decode("utf8")) - except: - pass print("The management daemon refused access. The API key file may be out of sync. Try 'service mailinabox restart'.", file=sys.stderr) elif hasattr(e, 'read'): print(e.read().decode('utf8'), file=sys.stderr) diff --git a/management/daemon.py b/management/daemon.py index 074e9fa7..bb4cd15d 100755 --- a/management/daemon.py +++ b/management/daemon.py @@ -22,6 +22,7 @@ from mailconfig import get_mail_users, get_mail_users_ex, get_admins, add_mail_u from mailconfig import get_mail_user_privileges, add_remove_mail_user_privilege from mailconfig import get_mail_aliases, get_mail_aliases_ex, get_mail_domains, add_mail_alias, remove_mail_alias from mfa import get_public_mfa_state, provision_totp, validate_totp_secret, enable_mfa, disable_mfa +import contextlib env = utils.load_environment() @@ -29,10 +30,8 @@ auth_service = auth.AuthService() # We may deploy via a symbolic link, which confuses flask's template finding. me = __file__ -try: +with contextlib.suppress(OSError): me = os.readlink(__file__) -except OSError: - pass # for generating CSRs we need a list of country codes csr_country_codes = [] diff --git a/management/dns_update.py b/management/dns_update.py index c06ef352..5e24f118 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -11,6 +11,7 @@ import dns.resolver from utils import shell, load_env_vars_from_file, safe_domain_name, sort_domains from ssl_certificates import get_ssl_certificates, check_certificate +import contextlib # From https://stackoverflow.com/questions/3026957/how-to-validate-a-domain-name-using-regex-php/16491074#16491074 # This regular expression matches domain names according to RFCs, it also accepts fqdn with an leading dot, @@ -456,10 +457,8 @@ def build_sshfp_records(): for line in f: s = line.rstrip().split() if len(s) == 2 and s[0] == 'Port': - try: + with contextlib.suppress(ValueError): port = int(s[1]) - except ValueError: - pass break keys = shell("check_output", ["ssh-keyscan", "-4", "-t", "rsa,dsa,ecdsa,ed25519", "-p", str(port), "localhost"]) diff --git a/setup/migrate.py b/setup/migrate.py index 3364b55d..17fbd5cf 100755 --- a/setup/migrate.py +++ b/setup/migrate.py @@ -9,6 +9,7 @@ import sys, os, os.path, glob, re, shutil sys.path.insert(0, 'management') from utils import load_environment, save_environment, shell +import contextlib def migration_1(env): # Re-arrange where we store SSL certificates. There was a typo also. @@ -31,10 +32,8 @@ def migration_1(env): move_file(sslfn, domain_name, file_type) # Move the old domains directory if it is now empty. - try: + with contextlib.suppress(Exception): os.rmdir(os.path.join( env["STORAGE_ROOT"], 'ssl/domains')) - except: - pass def migration_2(env): # Delete the .dovecot_sieve script everywhere. This was formerly a copy of our spam -> Spam From cacf6d2006db3bf4c7b8785296da31529fe5be9e Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Sat, 23 Dec 2023 05:32:34 -0800 Subject: [PATCH 50/55] Fixed E721 (type-comparison): Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks --- management/daemon.py | 2 +- management/mfa.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/management/daemon.py b/management/daemon.py index bb4cd15d..3aa6eed2 100755 --- a/management/daemon.py +++ b/management/daemon.py @@ -490,7 +490,7 @@ def totp_post_enable(): secret = request.form.get('secret') token = request.form.get('token') label = request.form.get('label') - if type(token) != str: + if not isinstance(token, str): return ("Bad Input", 400) try: validate_totp_secret(secret) diff --git a/management/mfa.py b/management/mfa.py index 148fd912..6b56ad86 100644 --- a/management/mfa.py +++ b/management/mfa.py @@ -68,7 +68,7 @@ def disable_mfa(email, mfa_id, env): return c.rowcount > 0 def validate_totp_secret(secret): - if type(secret) != str or secret.strip() == "": + if not isinstance(secret, str) or secret.strip() == "": msg = "No secret provided." raise ValueError(msg) if len(secret) != 32: From 1d79f9bb2b0ba29e979af60c0bcbdfd3225ba6b1 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Sat, 23 Dec 2023 05:40:37 -0800 Subject: [PATCH 51/55] Fixed PERF401 (manual-list-comprehension): Use a list comprehension to create a transformed list --- management/dns_update.py | 10 +++------- management/mail_log.py | 5 +---- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/management/dns_update.py b/management/dns_update.py index 5e24f118..597df243 100755 --- a/management/dns_update.py +++ b/management/dns_update.py @@ -49,9 +49,7 @@ def get_dns_zones(env): zone_domains.add(domain) # Make a nice and safe filename for each domain. - zonefiles = [] - for domain in zone_domains: - zonefiles.append([domain, safe_domain_name(domain) + ".txt"]) + zonefiles = [[domain, safe_domain_name(domain) + ".txt"] for domain in zone_domains] # Sort the list so that the order is nice and so that nsd.conf has a # stable order so we don't rewrite the file & restart the service @@ -195,8 +193,7 @@ def build_zone(domain, domain_properties, additional_records, env, is_zone=True) # User may provide one or more additional nameservers secondary_ns_list = get_secondary_dns(additional_records, mode="NS") \ or ["ns2." + env["PRIMARY_HOSTNAME"]] - for secondary_ns in secondary_ns_list: - records.append((None, "NS", secondary_ns+'.', False)) + records.extend((None, "NS", secondary_ns+'.', False) for secondary_ns in secondary_ns_list) # In PRIMARY_HOSTNAME... @@ -213,8 +210,7 @@ def build_zone(domain, domain_properties, additional_records, env, is_zone=True) records.append(("_443._tcp", "TLSA", build_tlsa_record(env), "Optional. When DNSSEC is enabled, provides out-of-band HTTPS certificate validation for a few web clients that support it.")) # Add a SSHFP records to help SSH key validation. One per available SSH key on this system. - for value in build_sshfp_records(): - records.append((None, "SSHFP", value, "Optional. Provides an out-of-band method for verifying an SSH key before connecting. Use 'VerifyHostKeyDNS yes' (or 'VerifyHostKeyDNS ask') when connecting with ssh.")) + records.extend((None, "SSHFP", value, "Optional. Provides an out-of-band method for verifying an SSH key before connecting. Use 'VerifyHostKeyDNS yes' (or 'VerifyHostKeyDNS ask') when connecting with ssh.") for value in build_sshfp_records()) # Add DNS records for any subdomains of this domain. We should not have a zone for # both a domain and one of its subdomains. diff --git a/management/mail_log.py b/management/mail_log.py index 0c91365b..e127af3b 100755 --- a/management/mail_log.py +++ b/management/mail_log.py @@ -619,10 +619,7 @@ def print_time_table(labels, data, do_print=True): data.insert(0, [str(h) for h in range(24)]) temp = "│ {:<%d} " % max(len(l) for l in labels) - lines = [] - - for label in labels: - lines.append(temp.format(label)) + lines = [temp.format(label) for label in labels] for h in range(24): max_len = max(len(str(d[h])) for d in data) From a32354fd91551bd0d10ee14cb1bfad7d70ebae66 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Sat, 23 Dec 2023 05:51:45 -0800 Subject: [PATCH 52/55] Fixed PLR5501 (collapsible-else-if): Use `elif` instead of `else` then `if`, to reduce indentation --- management/status_checks.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/management/status_checks.py b/management/status_checks.py index 831a7510..0f060100 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -154,12 +154,11 @@ def check_service(i, service, env): if not running and service["port"] in {80, 443}: output.print_line(shell('check_output', ['nginx', '-t'], capture_stderr=True, trap=True)[1].strip()) + # Service should be running locally. + elif try_connect("127.0.0.1"): + running = True else: - # Service should be running locally. - if try_connect("127.0.0.1"): - running = True - else: - output.print_error("%s is not running (port %d)." % (service['name'], service['port'])) + output.print_error("%s is not running (port %d)." % (service['name'], service['port'])) # Flag if local DNS is not running. if not running and service["port"] == 53 and service["public"] is False: From 618c466b84d6cf00fa4bf5d3c36c94b7e5f0f950 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Sat, 23 Dec 2023 05:52:53 -0800 Subject: [PATCH 53/55] Fixed SIM114 (if-with-same-arms): Combine `if` branches using logical `or` operator --- management/status_checks.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/management/status_checks.py b/management/status_checks.py index 0f060100..cd3e9f28 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -678,9 +678,7 @@ def check_mail_domain(domain, env, output): recommended_mx = "10 " + env['PRIMARY_HOSTNAME'] mx = query_dns(domain, "MX", nxdomain=None) - if mx is None: - mxhost = None - elif mx == "[timeout]": + if mx is None or mx == "[timeout]": mxhost = None else: # query_dns returns a semicolon-delimited list From 775a4223de343e596b30d6dd3f2872d941c26200 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Sat, 23 Dec 2023 05:56:07 -0800 Subject: [PATCH 54/55] Fixed F821 (undefined-name): Undefined name `e` --- setup/migrate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup/migrate.py b/setup/migrate.py index 17fbd5cf..066e0e03 100755 --- a/setup/migrate.py +++ b/setup/migrate.py @@ -167,7 +167,7 @@ def migration_12(env): dropcmd = "DROP TABLE %s" % table c.execute(dropcmd) except: - print("Failed to drop table", table, e) + print("Failed to drop table", table) # Save. conn.commit() conn.close() From dbc2b5eee03745f9d565e17c8b262d7c9c45c412 Mon Sep 17 00:00:00 2001 From: Teal Dulcet Date: Sat, 23 Dec 2023 06:00:04 -0800 Subject: [PATCH 55/55] Fixed ISC003 (explicit-string-concatenation): Explicitly concatenated string should be implicitly concatenated --- tools/editconf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/editconf.py b/tools/editconf.py index 8b36c6bb..0438695b 100755 --- a/tools/editconf.py +++ b/tools/editconf.py @@ -94,7 +94,7 @@ while len(input_lines) > 0: name, val = settings[i].split("=", 1) m = re.match( r"(\s*)" - + "(" + re.escape(comment_char) + r"\s*)?" + "(" + re.escape(comment_char) + r"\s*)?" + re.escape(name) + delimiter_re + r"(.*?)\s*$", line, re.S) if not m: continue