diff --git a/plinth/action_utils.py b/plinth/action_utils.py index 0b7800cef..119c842de 100644 --- a/plinth/action_utils.py +++ b/plinth/action_utils.py @@ -33,20 +33,19 @@ def is_systemd_running(): def systemd_get_default() -> str: """Return the default target that systemd will boot into.""" - process = subprocess.run(['systemctl', 'get-default'], - stdout=subprocess.PIPE, check=True) + process = run(['systemctl', 'get-default'], stdout=subprocess.PIPE, + check=True) return process.stdout.decode().strip() def systemd_set_default(target: str): """Set the default target that systemd will boot into.""" - subprocess.run(['systemctl', 'set-default', target], check=True) + run(['systemctl', 'set-default', target], check=True) def service_daemon_reload(): """Reload systemd to ensure that newer unit files are read.""" - subprocess.run(['systemctl', 'daemon-reload'], check=True, - stdout=subprocess.DEVNULL) + run(['systemctl', 'daemon-reload'], check=True, stdout=subprocess.DEVNULL) def service_is_running(servicename): @@ -55,8 +54,8 @@ def service_is_running(servicename): Does not need to run as root. """ try: - subprocess.run(['systemctl', 'status', servicename], check=True, - stdout=subprocess.DEVNULL) + run(['systemctl', 'status', servicename], check=True, + stdout=subprocess.DEVNULL) return True except subprocess.CalledProcessError: # If a service is not running we get a status code != 0 and @@ -102,9 +101,8 @@ def service_is_enabled(service_name, strict_check=False): """ try: - process = subprocess.run(['systemctl', 'is-enabled', service_name], - check=True, stdout=subprocess.PIPE, - stderr=subprocess.DEVNULL) + process = run(['systemctl', 'is-enabled', service_name], check=True, + stdout=subprocess.PIPE, stderr=subprocess.DEVNULL) if not strict_check: return True @@ -115,13 +113,13 @@ def service_is_enabled(service_name, strict_check=False): def service_enable(service_name: str, check: bool = False): """Enable and start a service in systemd.""" - subprocess.run(['systemctl', 'enable', service_name], check=check) + run(['systemctl', 'enable', service_name], check=check) service_start(service_name, check=check) def service_disable(service_name: str, check: bool = False): """Disable and stop service in systemd.""" - subprocess.run(['systemctl', 'disable', service_name], check=check) + run(['systemctl', 'disable', service_name], check=check) try: service_stop(service_name, check=check) except subprocess.CalledProcessError: @@ -130,12 +128,12 @@ def service_disable(service_name: str, check: bool = False): def service_mask(service_name: str, check: bool = False): """Mask a service""" - subprocess.run(['systemctl', 'mask', service_name], check=check) + run(['systemctl', 'mask', service_name], check=check) def service_unmask(service_name: str, check: bool = False): """Unmask a service""" - subprocess.run(['systemctl', 'unmask', service_name], check=check) + run(['systemctl', 'unmask', service_name], check=check) def service_start(service_name: str, check: bool = False): @@ -181,14 +179,14 @@ def service_get_logs(service_name: str) -> str: command = [ 'journalctl', '--no-pager', '--lines=200', '--unit', service_name ] - process = subprocess.run(command, check=False, stdout=subprocess.PIPE) + process = run(command, check=False, stdout=subprocess.PIPE) return process.stdout.decode() def service_show(service_name: str) -> dict[str, str]: """Return the status of the service in dictionary format.""" command = ['systemctl', 'show', service_name] - process = subprocess.run(command, check=False, stdout=subprocess.PIPE) + process = run(command, check=False, stdout=subprocess.PIPE) status = {} for line in process.stdout.decode().splitlines(): parts = line.partition('=') @@ -199,8 +197,8 @@ def service_show(service_name: str) -> dict[str, str]: def service_action(service_name: str, action: str, check: bool = False): """Perform the given action on the service_name.""" - subprocess.run(['systemctl', action, service_name], - stdout=subprocess.DEVNULL, check=check) + run(['systemctl', action, service_name], stdout=subprocess.DEVNULL, + check=check) def webserver_is_enabled(name, kind='config'): @@ -440,7 +438,7 @@ Owners: {package} env['DEBCONF_DB_OVERRIDE'] = 'File{' + override_file.name + \ ' readonly:true}' env['DEBIAN_FRONTEND'] = 'noninteractive' - subprocess.run(['dpkg-reconfigure', package], env=env, check=False) + run(['dpkg-reconfigure', package], env=env, check=False) try: os.remove(override_file.name) @@ -454,7 +452,7 @@ def debconf_set_selections(presets): # Workaround Debian Bug #487300. In some situations, debconf complains # it can't find the question being answered even though it is supposed # to create a dummy question for it. - subprocess.run(['/usr/share/debconf/fix_db.pl'], check=True) + run(['/usr/share/debconf/fix_db.pl'], check=True) except (FileNotFoundError, PermissionError): pass @@ -481,8 +479,8 @@ def run_apt_command(arguments, stdout=subprocess.DEVNULL, env['DEBIAN_FRONTEND'] = 'noninteractive' if not enable_triggers: env['FREEDOMBOX_INVOKED'] = 'true' - process = subprocess.run(command, stdin=subprocess.DEVNULL, stdout=stdout, - env=env, check=False) + process = run(command, stdin=subprocess.DEVNULL, stdout=stdout, env=env, + check=False) return process.returncode @@ -506,8 +504,7 @@ def apt_hold(packages): current_hold = subprocess.check_output( ['apt-mark', 'showhold', package]) if not current_hold: - process = subprocess.run(['apt-mark', 'hold', package], - check=False) + process = run(['apt-mark', 'hold', package], check=False) if process.returncode == 0: # success held_packages.append(package) @@ -539,9 +536,8 @@ def apt_hold_freedombox(): def apt_unhold_freedombox(): """Remove any hold on freedombox package, and clear flag.""" - subprocess.run(['apt-mark', 'unhold', 'freedombox'], - stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, - check=False) + run(['apt-mark', 'unhold', 'freedombox'], stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, check=False) if apt_hold_flag.exists(): apt_hold_flag.unlink() @@ -568,15 +564,14 @@ def podman_create(container_name: str, image_name: str, volume_name: str, service_stop(container_name) # Data is kept - subprocess.run(['podman', 'volume', 'rm', '--force', volume_name], - check=False) + run(['podman', 'volume', 'rm', '--force', volume_name], check=False) directory = pathlib.Path('/etc/containers/systemd') directory.mkdir(parents=True, exist_ok=True) # Fetch the image before creating the container. The systemd service for # the container won't timeout due to slow internet connectivity. - subprocess.run(['podman', 'image', 'pull', image_name], check=True) + run(['podman', 'image', 'pull', image_name], check=True) pathlib.Path(volume_path).mkdir(parents=True, exist_ok=True) # Create storage volume @@ -735,10 +730,8 @@ def podman_disable(container_name: str): def podman_uninstall(container_name: str, volume_name: str, image_name: str, volume_path: str): """Remove a podman container's components and systemd unit.""" - subprocess.run(['podman', 'volume', 'rm', '--force', volume_name], - check=True) - subprocess.run(['podman', 'image', 'rm', '--ignore', image_name], - check=True) + run(['podman', 'volume', 'rm', '--force', volume_name], check=True) + run(['podman', 'image', 'rm', '--ignore', image_name], check=True) volume_file = pathlib.Path( '/etc/containers/systemd/') / f'{volume_name}.volume' volume_file.unlink(missing_ok=True) diff --git a/plinth/db/mariadb.py b/plinth/db/mariadb.py index d7680c34e..c5f52433e 100644 --- a/plinth/db/mariadb.py +++ b/plinth/db/mariadb.py @@ -6,12 +6,14 @@ Uses utilities from 'mysql-client' package such as 'mysql' and 'mysqldump'. import subprocess +from .. import action_utils + def run_query(database_name: str, query: str) -> subprocess.CompletedProcess: """Run a database query using 'root' user. Does not ensure that the database server is running. """ - return subprocess.run( + return action_utils.run( ['mysql', '--user=root', '--database', database_name], input=query.encode('utf-8'), check=True) diff --git a/plinth/db/postgres.py b/plinth/db/postgres.py index 1170c4608..1ddf618d6 100644 --- a/plinth/db/postgres.py +++ b/plinth/db/postgres.py @@ -6,7 +6,6 @@ Uses utilities from 'postgres' package such as 'psql' and 'pg_dump'. import os import pathlib -import subprocess from plinth import action_utils @@ -14,7 +13,7 @@ from plinth import action_utils def _run_as(command, **kwargs): """Run a command as 'postgres' user.""" command = ['sudo', '--user', 'postgres'] + command - return subprocess.run(command, check=True, **kwargs) + return action_utils.run(command, check=True, **kwargs) def run_query(query): diff --git a/plinth/modules/apache/privileged.py b/plinth/modules/apache/privileged.py index 2568aa82e..f58a0e791 100644 --- a/plinth/modules/apache/privileged.py +++ b/plinth/modules/apache/privileged.py @@ -5,7 +5,6 @@ import glob import os import pathlib import re -import subprocess from plinth import action_utils from plinth.actions import privileged @@ -62,14 +61,14 @@ def setup(old_version: int): # version of Apache FreedomBox app and setting up for the first time don't # regenerate. if action_utils.is_disk_image() and old_version == 0: - subprocess.run([ + action_utils.run([ 'make-ssl-cert', 'generate-default-snakeoil', '--force-overwrite' ], check=True) # In case the certificate has been removed after ssl-cert is installed # on a fresh Debian machine. elif not os.path.exists('/etc/ssl/certs/ssl-cert-snakeoil.pem'): - subprocess.run(['make-ssl-cert', 'generate-default-snakeoil'], - check=True) + action_utils.run(['make-ssl-cert', 'generate-default-snakeoil'], + check=True) with action_utils.WebserverChange() as webserver: # Disable mod_php as we have switched to mod_fcgi + php-fpm. Disable diff --git a/plinth/modules/backups/privileged.py b/plinth/modules/backups/privileged.py index f7fbdb64f..132cd4f27 100644 --- a/plinth/modules/backups/privileged.py +++ b/plinth/modules/backups/privileged.py @@ -194,7 +194,7 @@ def _is_mounted(mountpoint): cmd = ['mountpoint', '-q', mountpoint] # mountpoint exits with status non-zero if it didn't find a mountpoint try: - subprocess.run(cmd, check=True) + action_utils.run(cmd, check=True) return True except subprocess.CalledProcessError: return False diff --git a/plinth/modules/bepasty/privileged.py b/plinth/modules/bepasty/privileged.py index 7e6a186b2..11a09c5e6 100644 --- a/plinth/modules/bepasty/privileged.py +++ b/plinth/modules/bepasty/privileged.py @@ -10,7 +10,6 @@ import pwd import secrets import shutil import string -import subprocess import augeas @@ -71,13 +70,13 @@ def setup(domain_name: str): try: grp.getgrnam('bepasty') except KeyError: - subprocess.run(['addgroup', '--system', 'bepasty'], check=True) + action_utils.run(['addgroup', '--system', 'bepasty'], check=True) # Create bepasty user if needed. try: pwd.getpwnam('bepasty') except KeyError: - subprocess.run([ + action_utils.run([ 'adduser', '--system', '--ingroup', 'bepasty', '--home', '/var/lib/bepasty', '--gecos', 'bepasty file sharing', 'bepasty' ], check=True) @@ -174,5 +173,5 @@ def uninstall(): """Remove bepasty user, group and data.""" shutil.rmtree(DATA_DIR, ignore_errors=True) CONF_FILE.unlink(missing_ok=True) - subprocess.run(['deluser', 'bepasty'], check=False) - subprocess.run(['delgroup', 'bepasty'], check=False) + action_utils.run(['deluser', 'bepasty'], check=False) + action_utils.run(['delgroup', 'bepasty'], check=False) diff --git a/plinth/modules/datetime/privileged.py b/plinth/modules/datetime/privileged.py index 45790405e..8e435bfcd 100644 --- a/plinth/modules/datetime/privileged.py +++ b/plinth/modules/datetime/privileged.py @@ -3,6 +3,7 @@ import subprocess +from plinth import action_utils from plinth.actions import privileged @@ -10,4 +11,4 @@ from plinth.actions import privileged def set_timezone(timezone: str): """Set time zone with timedatectl.""" command = ['timedatectl', 'set-timezone', timezone] - subprocess.run(command, stdout=subprocess.DEVNULL, check=True) + action_utils.run(command, stdout=subprocess.DEVNULL, check=True) diff --git a/plinth/modules/email/postfix.py b/plinth/modules/email/postfix.py index 1a04731cb..2f6b4a57c 100644 --- a/plinth/modules/email/postfix.py +++ b/plinth/modules/email/postfix.py @@ -11,6 +11,8 @@ import re import subprocess from dataclasses import dataclass +from plinth import action_utils + @dataclass class Service: # NOQA, pylint: disable=too-many-instance-attributes @@ -109,7 +111,7 @@ def _run(args): Raise a RuntimeError on non-zero exit codes. """ try: - result = subprocess.run(args, check=True, capture_output=True) + result = action_utils.run(args, check=True, capture_output=True) return result.stdout.decode() except subprocess.SubprocessError as subprocess_error: raise RuntimeError('Subprocess failed') from subprocess_error diff --git a/plinth/modules/email/privileged/dkim.py b/plinth/modules/email/privileged/dkim.py index d3b9273c6..62195a5d9 100644 --- a/plinth/modules/email/privileged/dkim.py +++ b/plinth/modules/email/privileged/dkim.py @@ -9,6 +9,7 @@ import re import shutil import subprocess +from plinth import action_utils from plinth.actions import privileged from plinth.privileged import service as service_privileged @@ -54,7 +55,7 @@ def setup_dkim(domain: str): # Ed25519 is widely *not* accepted as of 2022-01. See: # https://serverfault.com/questions/1023674 - subprocess.run([ + action_utils.run([ 'rspamadm', 'dkim_keygen', '-t', 'rsa', '-b', '2048', '-s', 'dkim', '-d', domain, '-k', (str(key_file)) ], check=True) diff --git a/plinth/modules/email/privileged/home.py b/plinth/modules/email/privileged/home.py index 544643dc3..05a9f974e 100644 --- a/plinth/modules/email/privileged/home.py +++ b/plinth/modules/email/privileged/home.py @@ -6,8 +6,7 @@ See: https://doc.dovecot.org/configuration_manual/authentication/user_databases_userdb/ """ -import subprocess - +from plinth import action_utils from plinth.actions import privileged @@ -19,4 +18,4 @@ def setup_home(): Dovecot creates new directories with the same permissions as the parent directory. Ensure that 'others' can't access /var/mail/. """ - subprocess.run(['chmod', 'o-rwx', '/var/mail'], check=True) + action_utils.run(['chmod', 'o-rwx', '/var/mail'], check=True) diff --git a/plinth/modules/email/privileged/spam.py b/plinth/modules/email/privileged/spam.py index c42331393..6205f2d7f 100644 --- a/plinth/modules/email/privileged/spam.py +++ b/plinth/modules/email/privileged/spam.py @@ -10,8 +10,8 @@ For testing DKIM signatures: https://www.mail-tester.com/ import pathlib import re -import subprocess +from plinth import action_utils from plinth.actions import privileged from plinth.modules.email import postfix @@ -31,10 +31,11 @@ def setup_spam(): def _compile_sieve(): """Compile all .sieve script to binary format for performance.""" - sieve_dirs = ['/etc/dovecot/freedombox-sieve-after/', - '/etc/dovecot/freedombox-sieve'] + sieve_dirs = [ + '/etc/dovecot/freedombox-sieve-after/', '/etc/dovecot/freedombox-sieve' + ] for sieve_dir in sieve_dirs: - subprocess.run(['sievec', sieve_dir], check=True) + action_utils.run(['sievec', sieve_dir], check=True) def _setup_rspamd(): diff --git a/plinth/modules/firewall/privileged.py b/plinth/modules/firewall/privileged.py index 605c7e948..33f0dd7bf 100644 --- a/plinth/modules/firewall/privileged.py +++ b/plinth/modules/firewall/privileged.py @@ -36,10 +36,10 @@ def _flush_iptables_rules(): iptables_rules += rule_template.format(table=table) ip6tables_rules += rule_template.format(table=table) - subprocess.run(['iptables-restore'], input=iptables_rules.encode(), - check=True) - subprocess.run(['ip6tables-restore'], input=iptables_rules.encode(), - check=True) + action_utils.run(['iptables-restore'], input=iptables_rules.encode(), + check=True) + action_utils.run(['ip6tables-restore'], input=iptables_rules.encode(), + check=True) def set_firewall_backend(backend): @@ -67,8 +67,8 @@ def set_firewall_backend(backend): def _run_firewall_cmd(args): """Run firewall-cmd command, discard output and check return value.""" - subprocess.run(['firewall-cmd'] + args, stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, check=True) + action_utils.run(['firewall-cmd'] + args, stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, check=True) def _setup_local_service_protection(): @@ -160,9 +160,9 @@ def _setup_inter_zone_forwarding(): def setup(): """Perform basic firewalld setup.""" action_utils.service_enable('firewalld') - subprocess.run(['firewall-cmd', '--set-default-zone=external'], - stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, - check=True) + action_utils.run(['firewall-cmd', '--set-default-zone=external'], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=True) set_firewall_backend('nftables') _setup_local_service_protection() diff --git a/plinth/modules/ikiwiki/privileged.py b/plinth/modules/ikiwiki/privileged.py index fda1eca9b..798c8581b 100644 --- a/plinth/modules/ikiwiki/privileged.py +++ b/plinth/modules/ikiwiki/privileged.py @@ -7,6 +7,7 @@ import re import shutil import subprocess +from plinth import action_utils from plinth.actions import privileged, secret_str SETUP_WIKI = '/etc/ikiwiki/plinth-wiki.setup' @@ -61,10 +62,10 @@ def create_wiki(wiki_name: str, admin_name: str, admin_password: secret_str): """Create a wiki.""" pw_bytes = admin_password.encode() input_ = pw_bytes + b'\n' + pw_bytes - subprocess.run(['ikiwiki', '-setup', SETUP_WIKI, wiki_name, admin_name], - stdout=subprocess.PIPE, input=input_, - stderr=subprocess.PIPE, - env=dict(os.environ, PERL_UNICODE='AS'), check=True) + action_utils.run(['ikiwiki', '-setup', SETUP_WIKI, wiki_name, admin_name], + stdout=subprocess.PIPE, input=input_, + stderr=subprocess.PIPE, + env=dict(os.environ, PERL_UNICODE='AS'), check=True) @privileged @@ -72,17 +73,17 @@ def create_blog(blog_name: str, admin_name: str, admin_password: secret_str): """Create a blog.""" pw_bytes = admin_password.encode() input_ = pw_bytes + b'\n' + pw_bytes - subprocess.run(['ikiwiki', '-setup', SETUP_BLOG, blog_name, admin_name], - stdout=subprocess.PIPE, input=input_, - stderr=subprocess.PIPE, env=dict(os.environ, - PERL_UNICODE='AS')) + action_utils.run(['ikiwiki', '-setup', SETUP_BLOG, blog_name, admin_name], + stdout=subprocess.PIPE, input=input_, + stderr=subprocess.PIPE, env=dict(os.environ, + PERL_UNICODE='AS')) @privileged def setup_site(site_name: str): """Run setup for a site.""" setup_path = os.path.join(WIKI_PATH, site_name + '.setup') - subprocess.run(['ikiwiki', '-setup', setup_path], check=True) + action_utils.run(['ikiwiki', '-setup', setup_path], check=True) @privileged diff --git a/plinth/modules/infinoted/privileged.py b/plinth/modules/infinoted/privileged.py index c37f4680e..e79c3cef7 100644 --- a/plinth/modules/infinoted/privileged.py +++ b/plinth/modules/infinoted/privileged.py @@ -9,6 +9,7 @@ import shutil import subprocess import time +from plinth import action_utils from plinth.actions import privileged DATA_DIR = '/var/lib/infinoted' @@ -105,7 +106,7 @@ def _kill_daemon(): end_time = time.time() + 300 while time.time() < end_time: try: - subprocess.run(['infinoted', '--kill-daemon'], check=True) + action_utils.run(['infinoted', '--kill-daemon'], check=True) break except subprocess.CalledProcessError: pass @@ -129,13 +130,13 @@ def setup(): try: grp.getgrnam('infinoted') except KeyError: - subprocess.run(['addgroup', '--system', 'infinoted'], check=True) + action_utils.run(['addgroup', '--system', 'infinoted'], check=True) # Create infinoted user if needed. try: pwd.getpwnam('infinoted') except KeyError: - subprocess.run([ + action_utils.run([ 'adduser', '--system', '--ingroup', 'infinoted', '--home', DATA_DIR, '--gecos', 'Infinoted collaborative editing server', 'infinoted' @@ -151,7 +152,7 @@ def setup(): try: # infinoted doesn't have a "create key and exit" mode. Run as # daemon so we can stop after. - subprocess.run([ + action_utils.run([ 'infinoted', '--create-key', '--create-certificate', '--daemonize' ], check=True) diff --git a/plinth/modules/letsencrypt/privileged.py b/plinth/modules/letsencrypt/privileged.py index 0993c02d4..3002a0ef8 100644 --- a/plinth/modules/letsencrypt/privileged.py +++ b/plinth/modules/letsencrypt/privileged.py @@ -115,7 +115,7 @@ def revoke(domain: str): if TEST_MODE: command.append('--staging') - subprocess.run(command, check=True) + action_utils.run(command, check=True) action_utils.webserver_disable(domain, kind='site') @@ -132,7 +132,7 @@ def obtain(domain: str): if TEST_MODE: command.append('--staging') - subprocess.run(command, check=True) + action_utils.run(command, check=True) @privileged @@ -249,5 +249,5 @@ def _assert_managed_path(module, path): def delete(domain: str): """Disable a domain and delete the certificate.""" command = ['certbot', 'delete', '--non-interactive', '--cert-name', domain] - subprocess.run(command, check=True) + action_utils.run(command, check=True) action_utils.webserver_disable(domain, kind='site') diff --git a/plinth/modules/mediawiki/privileged.py b/plinth/modules/mediawiki/privileged.py index fa01c2b58..1035e6328 100644 --- a/plinth/modules/mediawiki/privileged.py +++ b/plinth/modules/mediawiki/privileged.py @@ -7,6 +7,7 @@ import shutil import subprocess import tempfile +from plinth import action_utils from plinth.actions import privileged, secret_str from plinth.utils import generate_password @@ -26,8 +27,8 @@ def get_php_command(): version = '' try: - process = subprocess.run(['dpkg', '-s', 'php'], stdout=subprocess.PIPE, - check=True) + process = action_utils.run(['dpkg', '-s', 'php'], + stdout=subprocess.PIPE, check=True) for line in process.stdout.decode().splitlines(): if line.startswith('Version:'): version = line.split(':')[-1].split('+')[0].strip() @@ -57,8 +58,9 @@ def setup(): '--scriptpath=/mediawiki', '--passfile', password_file_handle.name, 'Wiki', 'admin' ]) - subprocess.run(['chmod', '-R', 'o-rwx', data_dir], check=True) - subprocess.run(['chown', '-R', 'www-data:www-data', data_dir], check=True) + action_utils.run(['chmod', '-R', 'o-rwx', data_dir], check=True) + action_utils.run(['chown', '-R', 'www-data:www-data', data_dir], + check=True) conf_file = pathlib.Path(CONF_FILE) if not conf_file.exists(): diff --git a/plinth/modules/minidlna/privileged.py b/plinth/modules/minidlna/privileged.py index 2fba83493..e57058770 100644 --- a/plinth/modules/minidlna/privileged.py +++ b/plinth/modules/minidlna/privileged.py @@ -1,7 +1,6 @@ # SPDX-License-Identifier: AGPL-3.0-or-later """Configure minidlna server.""" -import subprocess from os import chmod, fdopen, remove, stat from shutil import move from tempfile import mkstemp @@ -51,7 +50,7 @@ def setup(): encoding='utf-8') as conf: conf.write(SYSCTL_CONF) - subprocess.run(['systemctl', 'restart', 'systemd-sysctl'], check=True) + action_utils.run(['systemctl', 'restart', 'systemd-sysctl'], check=True) @privileged diff --git a/plinth/modules/mumble/privileged.py b/plinth/modules/mumble/privileged.py index 759c0eab3..1469f6d55 100644 --- a/plinth/modules/mumble/privileged.py +++ b/plinth/modules/mumble/privileged.py @@ -37,8 +37,8 @@ def check_setup() -> bool: @privileged def set_super_user_password(password: secret_str): """Set the superuser password with murmurd command.""" - subprocess.run(['murmurd', '-readsupw'], input=password.encode(), - stdout=subprocess.DEVNULL, check=False) + action_utils.run(['murmurd', '-readsupw'], input=password.encode(), + stdout=subprocess.DEVNULL, check=False) @privileged diff --git a/plinth/modules/names/privileged.py b/plinth/modules/names/privileged.py index 37804eebc..f9f7fa248 100644 --- a/plinth/modules/names/privileged.py +++ b/plinth/modules/names/privileged.py @@ -2,7 +2,6 @@ """Configure Names App.""" import pathlib -import subprocess import augeas @@ -22,7 +21,7 @@ HOSTS_LOCAL_IP = '127.0.1.1' @privileged def set_hostname(hostname: str): """Set system hostname using hostnamectl.""" - subprocess.run( + action_utils.run( ['hostnamectl', 'set-hostname', '--transient', '--static', hostname], check=True) action_utils.service_restart('avahi-daemon') @@ -83,7 +82,7 @@ def domain_delete_all(): def install_resolved(): """Install systemd-resolved related packages.""" packages = ['systemd-resolved', 'libnss-resolve'] - subprocess.run(['dpkg', '--configure', '-a'], check=False) + action_utils.run(['dpkg', '--configure', '-a'], check=False) with action_utils.apt_hold_freedombox(): action_utils.run_apt_command(['--fix-broken', 'install']) returncode = action_utils.run_apt_command(['install'] + packages) diff --git a/plinth/modules/networks/privileged.py b/plinth/modules/networks/privileged.py index 2ea77518e..4f671a97b 100644 --- a/plinth/modules/networks/privileged.py +++ b/plinth/modules/networks/privileged.py @@ -52,7 +52,7 @@ def _add_connection(connection_name: str, interface: str, logging.info('Connection %s already exists for device %s, not adding.', connection_name, interface) else: - subprocess.run([ + action_utils.run([ 'nmcli', 'con', 'add', 'con-name', connection_name, 'ifname', interface ] + remaining_arguments, check=True) @@ -108,8 +108,9 @@ def _set_connection_properties(connection_name: str, properties: dict[str, str]): """Configure property key/values on a connection.""" for key, value in properties.items(): - subprocess.run(['nmcli', 'con', 'modify', connection_name, key, value], - check=True) + action_utils.run( + ['nmcli', 'con', 'modify', connection_name, key, value], + check=True) def _configure_wireless_interface(interface: str): diff --git a/plinth/modules/nextcloud/privileged.py b/plinth/modules/nextcloud/privileged.py index f7da36d57..51813bf43 100644 --- a/plinth/modules/nextcloud/privileged.py +++ b/plinth/modules/nextcloud/privileged.py @@ -79,7 +79,8 @@ def _run_in_container( env_args = [f'--env={key}={value}' for key, value in (env or {}).items()] command = ['podman', 'exec', '--user', WWW_DATA_UID ] + env_args + [CONTAINER_NAME] + list(args) - return subprocess.run(command, capture_output=capture_output, check=check) + return action_utils.run(command, capture_output=capture_output, + check=check) def _run_occ(*args, **kwargs) -> subprocess.CompletedProcess: @@ -174,7 +175,7 @@ def set_default_phone_region(region: str): def _database_query(query: str): """Run a database query.""" - subprocess.run(['mysql'], input=query.encode(), check=True) + action_utils.run(['mysql'], input=query.encode(), check=True) def _create_database(): @@ -239,7 +240,7 @@ def _nextcloud_wait_until_ready(): # obtaining. We are unable to obtain the lock for 5 minutes, fail and stop # the setup process. lock_file = _data_path / 'nextcloud-init-sync.lock' - subprocess.run( + action_utils.run( ['flock', '--exclusive', '--wait', '300', lock_file, 'echo'], check=True) @@ -362,7 +363,7 @@ def dump_database(): with _maintenance_mode(): with DB_BACKUP_FILE.open('w', encoding='utf-8') as file_handle: - subprocess.run([ + action_utils.run([ 'mysqldump', '--add-drop-database', '--add-drop-table', '--add-drop-trigger', '--single-transaction', '--default-character-set=utf8mb4', '--user', 'root', @@ -374,11 +375,11 @@ def dump_database(): def restore_database(): """Restore database from file.""" with DB_BACKUP_FILE.open('r', encoding='utf-8') as file_handle: - subprocess.run(['mysql', '--user', 'root'], stdin=file_handle, - check=True) + action_utils.run(['mysql', '--user', 'root'], stdin=file_handle, + check=True) - subprocess.run(['redis-cli', '-n', - str(REDIS_DB), 'FLUSHDB', 'SYNC'], check=False) + action_utils.run(['redis-cli', '-n', + str(REDIS_DB), 'FLUSHDB', 'SYNC'], check=False) _set_database_privileges(_get_database_password()) diff --git a/plinth/modules/openvpn/privileged.py b/plinth/modules/openvpn/privileged.py index 7ec0d98b1..50f64c722 100644 --- a/plinth/modules/openvpn/privileged.py +++ b/plinth/modules/openvpn/privileged.py @@ -110,7 +110,7 @@ def _setup_firewall(): def _is_tunplus_enabled(): """Return whether tun+ interface is already added.""" try: - process = subprocess.run( + process = action_utils.run( ['firewall-cmd', '--zone', 'internal', '--list-interfaces'], stdout=subprocess.PIPE, check=True) return 'tun+' in process.stdout.decode().strip().split() @@ -135,8 +135,8 @@ def _setup_firewall(): def _run_easy_rsa(args): """Execute easy-rsa command with some default arguments.""" - return subprocess.run(['/usr/share/easy-rsa/easyrsa'] + args, - cwd=KEYS_DIRECTORY, check=True) + return action_utils.run(['/usr/share/easy-rsa/easyrsa'] + args, + cwd=KEYS_DIRECTORY, check=True) def _write_easy_rsa_config(): @@ -162,7 +162,7 @@ def _is_renewable(cert_name): if not cert_path.exists(): return False - process = subprocess.run( + process = action_utils.run( ['openssl', 'x509', '-noout', '-enddate', '-in', str(cert_path)], check=True, stdout=subprocess.PIPE) date_string = process.stdout.decode().strip().partition('=')[2] diff --git a/plinth/modules/snapshot/privileged.py b/plinth/modules/snapshot/privileged.py index 4691003c4..ec3385b09 100644 --- a/plinth/modules/snapshot/privileged.py +++ b/plinth/modules/snapshot/privileged.py @@ -21,13 +21,13 @@ def setup(old_version: int): """Configure snapper.""" # Check if root config exists. command = ['snapper', 'list-configs'] - process = subprocess.run(command, stdout=subprocess.PIPE, check=True) + process = action_utils.run(command, stdout=subprocess.PIPE, check=True) output = process.stdout.decode() # Create root config if needed. if 'root' not in output: command = ['snapper', 'create-config', '/'] - subprocess.run(command, check=True) + action_utils.run(command, check=True) if old_version and old_version <= 4: _remove_fstab_entry('/') @@ -76,7 +76,7 @@ def _migrate_config_from_version_3(): 'EMPTY_PRE_POST_MIN_AGE=0', 'FREE_LIMIT=0.3', ] - subprocess.run(command, check=True) + action_utils.run(command, check=True) def _set_default_config(): @@ -98,7 +98,7 @@ def _set_default_config(): 'EMPTY_PRE_POST_MIN_AGE=0', 'FREE_LIMIT=0.3', ] - subprocess.run(command, check=True) + action_utils.run(command, check=True) def _remove_fstab_entry(mount_point): @@ -137,16 +137,17 @@ def _remove_fstab_entry(mount_point): def _systemd_path_escape(path): """Escape a string using systemd path rules.""" - process = subprocess.run(['systemd-escape', '--path', path], - stdout=subprocess.PIPE, check=True) + process = action_utils.run(['systemd-escape', '--path', path], + stdout=subprocess.PIPE, check=True) return process.stdout.decode().strip() def _get_subvolume_path(mount_point): """Return the subvolume path for .snapshots in a filesystem.""" # -o causes the list of subvolumes directly under the given mount point - process = subprocess.run(['btrfs', 'subvolume', 'list', '-o', mount_point], - stdout=subprocess.PIPE, check=True) + process = action_utils.run( + ['btrfs', 'subvolume', 'list', '-o', mount_point], + stdout=subprocess.PIPE, check=True) for line in process.stdout.decode().splitlines(): entry = line.split() @@ -223,8 +224,8 @@ def _parse_number(number): @privileged def list_() -> list[dict[str, str]]: """List snapshots.""" - process = subprocess.run(['snapper', 'list'], stdout=subprocess.PIPE, - check=True) + process = action_utils.run(['snapper', 'list'], stdout=subprocess.PIPE, + check=True) lines = process.stdout.decode().splitlines() keys = ('number', 'is_default', 'is_active', 'type', 'pre_number', 'date', @@ -246,7 +247,7 @@ def list_() -> list[dict[str, str]]: def _get_default_snapshot(): """Return the default snapshot by looking at default subvolume.""" command = ['btrfs', 'subvolume', 'get-default', '/'] - process = subprocess.run(command, stdout=subprocess.PIPE, check=True) + process = action_utils.run(command, stdout=subprocess.PIPE, check=True) output = process.stdout.decode() output_parts = output.split() @@ -277,26 +278,26 @@ def disable_apt_snapshot(state: str): def create(): """Create snapshot.""" command = ['snapper', 'create', '--description', 'manually created'] - subprocess.run(command, check=True) + action_utils.run(command, check=True) @privileged def delete(number: str): """Delete a snapshot by number.""" command = ['snapper', 'delete', number] - subprocess.run(command, check=True) + action_utils.run(command, check=True) @privileged def set_config(config: list[str]): """Set snapper configuration.""" command = ['snapper', 'set-config'] + config - subprocess.run(command, check=True) + action_utils.run(command, check=True) def _get_config(): command = ['snapper', 'get-config'] - process = subprocess.run(command, stdout=subprocess.PIPE, check=True) + process = action_utils.run(command, stdout=subprocess.PIPE, check=True) lines = process.stdout.decode().splitlines() config = {} for line in lines[2:]: @@ -345,4 +346,4 @@ def rollback(number: str): # behavior when a snapshot number to rollback to is provided is the # behavior that we desire. command = ['snapper', '--ambit', 'classic', 'rollback', number] - subprocess.run(command, check=True) + action_utils.run(command, check=True) diff --git a/plinth/modules/sogo/privileged.py b/plinth/modules/sogo/privileged.py index 39b6a198c..1767ee525 100644 --- a/plinth/modules/sogo/privileged.py +++ b/plinth/modules/sogo/privileged.py @@ -7,7 +7,7 @@ import shutil import subprocess import tempfile -from plinth import utils +from plinth import action_utils, utils from plinth.actions import privileged from plinth.db import postgres from plinth.modules.email.privileged.domain import \ @@ -144,8 +144,8 @@ def set_domain(domain: str): def _get_config_value(key: str) -> str: """Return the value of a property from the configuration file.""" - process = subprocess.run(['plget', key], input=CONFIG_FILE.read_bytes(), - stdout=subprocess.PIPE, check=True) + process = action_utils.run(['plget', key], input=CONFIG_FILE.read_bytes(), + stdout=subprocess.PIPE, check=True) return process.stdout.decode().strip() @@ -154,7 +154,7 @@ def _set_config_value(key: str, value: str): with tempfile.NamedTemporaryFile(delete=False) as temp_file: temp_file.write(f'{{\n{key} = "{value}";\n}}'.encode('utf-8')) temp_file.close() - subprocess.run(['plmerge', CONFIG_FILE, temp_file.name], check=True) + action_utils.run(['plmerge', CONFIG_FILE, temp_file.name], check=True) pathlib.Path(temp_file.name).unlink() diff --git a/plinth/modules/storage/privileged.py b/plinth/modules/storage/privileged.py index dc156fa3a..234739ba5 100644 --- a/plinth/modules/storage/privileged.py +++ b/plinth/modules/storage/privileged.py @@ -46,7 +46,7 @@ def _move_gpt_second_header(device): """ command = ['sgdisk', '--move-second-header', device] try: - subprocess.run(command, check=True) + action_utils.run(command, check=True) except subprocess.CalledProcessError: raise RuntimeError('Error moving GPT second header to the end') @@ -65,12 +65,12 @@ def _resize_partition(device, requested_partition, free_space): 'B', 'resizepart', requested_partition['number'] ] try: - subprocess.run(command, check=True) + action_utils.run(command, check=True) except subprocess.CalledProcessError: try: input_text = 'yes\n' + str(free_space['end']) - subprocess.run(fallback_command, check=True, - input=input_text.encode()) + action_utils.run(fallback_command, check=True, + input=input_text.encode()) except subprocess.CalledProcessError as exception: raise RuntimeError(f'Error expanding partition: {exception}') @@ -90,8 +90,8 @@ def _resize_ext4(device, requested_partition, _free_space, _mount_point): requested_partition['number']) try: command = ['resize2fs', partition_device] - subprocess.run(command, stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, check=True) + action_utils.run(command, stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, check=True) except subprocess.CalledProcessError as exception: raise RuntimeError(f'Error expanding filesystem: {exception}') @@ -100,7 +100,7 @@ def _resize_btrfs(_device, _requested_partition, _free_space, mount_point='/'): """Resize a btrfs file system inside a partition.""" try: command = ['btrfs', 'filesystem', 'resize', 'max', mount_point] - subprocess.run(command, stdout=subprocess.DEVNULL, check=True) + action_utils.run(command, stdout=subprocess.DEVNULL, check=True) except subprocess.CalledProcessError as exception: raise RuntimeError(f'Error expanding filesystem: {exception}') @@ -167,7 +167,7 @@ def _get_partitions_and_free_spaces(device, partition_number): command = [ 'parted', '--machine', '--script', device, 'unit', 'B', 'print', 'free' ] - process = subprocess.run(command, stdout=subprocess.PIPE, check=True) + process = action_utils.run(command, stdout=subprocess.PIPE, check=True) requested_partition = None free_spaces = [] @@ -215,7 +215,7 @@ def mount(block_device: str): UDISKS_FILESYSTEM_SHARED=1 by writing a udev rule. """ - subprocess.run([ + action_utils.run([ 'udisksctl', 'mount', '--block-device', block_device, '--no-user-interaction' ], check=True) diff --git a/plinth/modules/syncthing/privileged.py b/plinth/modules/syncthing/privileged.py index 6180c353d..5359a09d6 100644 --- a/plinth/modules/syncthing/privileged.py +++ b/plinth/modules/syncthing/privileged.py @@ -5,7 +5,6 @@ import grp import os import pwd import shutil -import subprocess import time import augeas @@ -37,13 +36,13 @@ def setup(): try: grp.getgrnam('syncthing') except KeyError: - subprocess.run(['addgroup', '--system', 'syncthing'], check=True) + action_utils.run(['addgroup', '--system', 'syncthing'], check=True) # Create syncthing user if needed. try: pwd.getpwnam('syncthing') except KeyError: - subprocess.run([ + action_utils.run([ 'adduser', '--system', '--ingroup', 'syncthing', '--home', DATA_DIR, '--gecos', 'Syncthing file synchronization server', 'syncthing' diff --git a/plinth/modules/tor/privileged.py b/plinth/modules/tor/privileged.py index b2b4847f2..17ca0bebe 100644 --- a/plinth/modules/tor/privileged.py +++ b/plinth/modules/tor/privileged.py @@ -8,7 +8,6 @@ import pathlib import re import shutil import socket -import subprocess import time from typing import Any @@ -54,7 +53,7 @@ def _first_time_setup(): """Setup Tor configuration for the first time setting defaults.""" logger.info('Performing first time setup for Tor') - subprocess.run(['tor-instance-create', INSTANCE_NAME], check=True) + action_utils.run(['tor-instance-create', INSTANCE_NAME], check=True) # Remove line starting with +SocksPort, since our augeas lens # doesn't handle it correctly. diff --git a/plinth/modules/torproxy/privileged.py b/plinth/modules/torproxy/privileged.py index 8ce68cc2a..883df4a79 100644 --- a/plinth/modules/torproxy/privileged.py +++ b/plinth/modules/torproxy/privileged.py @@ -4,7 +4,6 @@ import logging import os import shutil -import subprocess from typing import Any import augeas @@ -31,7 +30,7 @@ def setup(): # Mask the service to prevent re-enabling it by the Tor master service. action_utils.service_mask('tor@default') - subprocess.run(['tor-instance-create', INSTANCE_NAME], check=True) + action_utils.run(['tor-instance-create', INSTANCE_NAME], check=True) # Remove line starting with +SocksPort, since our augeas lens # doesn't handle it correctly. diff --git a/plinth/modules/upgrades/distupgrade.py b/plinth/modules/upgrades/distupgrade.py index ce95eff6f..0e978c6d6 100644 --- a/plinth/modules/upgrades/distupgrade.py +++ b/plinth/modules/upgrades/distupgrade.py @@ -5,7 +5,6 @@ import contextlib import datetime import logging import pathlib -import subprocess from datetime import timezone from typing import Generator @@ -218,11 +217,11 @@ def _snapshot_run_and_disable() -> Generator[None, None, None]: try: logger.info('Taking a snapshot before dist upgrade...') command = ['snapper', 'create', '--description', 'before dist-upgrade'] - subprocess.run(command, check=True) + action_utils.run(command, check=True) aug = snapshot_module.load_augeas() if snapshot_module.is_apt_snapshots_enabled(aug): logger.info('Disabling apt snapshots during dist upgrade...') - subprocess.run([ + action_utils.run([ '/usr/bin/freedombox-cmd', 'snapshot', 'disable_apt_snapshot', @@ -235,7 +234,7 @@ def _snapshot_run_and_disable() -> Generator[None, None, None]: finally: if reenable: logger.info('Re-enabling apt snapshots...') - subprocess.run([ + action_utils.run([ '/usr/bin/freedombox-cmd', 'snapshot', 'disable_apt_snapshot' ], input='{"args": ["no"], "kwargs": {}}'.encode(), check=True) else: @@ -303,7 +302,7 @@ def _apt_update(): def _apt_fix(): """Try to fix any problems with apt/dpkg before the upgrade.""" logger.info('Fixing any broken apt/dpkg states...') - subprocess.run(['dpkg', '--configure', '-a'], check=False) + action_utils.run(['dpkg', '--configure', '-a'], check=False) _apt_run(['--fix-broken', 'install']) @@ -341,7 +340,7 @@ def _unattended_upgrades_run(): To handle upgrading the freedombox package. """ logger.info('Running unattended-upgrade...') - subprocess.run(['unattended-upgrade', '--verbose'], check=False) + action_utils.run(['unattended-upgrade', '--verbose'], check=False) def _freedombox_restart(): @@ -360,7 +359,7 @@ def _trigger_on_complete(): # file will not be possible. For that, we need to launch a new process with # a different systemd service (which does not have the bind mounts). logger.info('Triggering on-complete to commit sources.lists') - subprocess.run([ + action_utils.run([ 'systemd-run', '--unit=freedombox-dist-upgrade-on-complete', '--description=Finish up upgrade to new stable Debian release', '/usr/bin/freedombox-cmd', 'upgrades', 'dist_upgrade_on_complete', @@ -417,7 +416,7 @@ def start_service(): '--property=KillMode=process', '--property=TimeoutSec=72hr', f'--property=BindPaths={temp_sources_list}:{sources_list}' ] - subprocess.run(['systemd-run'] + args + [ + action_utils.run(['systemd-run'] + args + [ 'systemd-inhibit', '/usr/bin/freedombox-cmd', 'upgrades', 'dist_upgrade', '--no-args' ], check=True) diff --git a/plinth/modules/upgrades/privileged.py b/plinth/modules/upgrades/privileged.py index ad107cdfa..8629f5345 100644 --- a/plinth/modules/upgrades/privileged.py +++ b/plinth/modules/upgrades/privileged.py @@ -7,6 +7,7 @@ import pathlib import re import subprocess +from plinth import action_utils from plinth.action_utils import (apt_hold_flag, apt_unhold_freedombox, is_package_manager_busy, run_apt_command, service_is_running) @@ -130,14 +131,14 @@ def release_held_packages(): output = subprocess.check_output(['apt-mark', 'showhold']).decode().strip() holds = output.split('\n') logger.info('Releasing package holds: %s', holds) - subprocess.run(['apt-mark', 'unhold', *holds], stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, check=True) + action_utils.run(['apt-mark', 'unhold', *holds], stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, check=True) @privileged def run(): """Run unattended-upgrades.""" - subprocess.run(['dpkg', '--configure', '-a'], check=False) + action_utils.run(['dpkg', '--configure', '-a'], check=False) run_apt_command(['--fix-broken', 'install']) _release_held_freedombox() diff --git a/plinth/modules/upgrades/tests/test_distupgrade.py b/plinth/modules/upgrades/tests/test_distupgrade.py index 8852de9f8..1d3fc1e52 100644 --- a/plinth/modules/upgrades/tests/test_distupgrade.py +++ b/plinth/modules/upgrades/tests/test_distupgrade.py @@ -219,7 +219,7 @@ def test_snapshot_run_and_disable(is_supported, is_apt_snapshots_enabled, run): with distupgrade._snapshot_run_and_disable(): assert run.call_args_list == [ call(['snapper', 'create', '--description', 'before dist-upgrade'], - check=True) + stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=True) ] run.reset_mock() @@ -230,16 +230,18 @@ def test_snapshot_run_and_disable(is_supported, is_apt_snapshots_enabled, run): with distupgrade._snapshot_run_and_disable(): assert run.call_args_list == [ call(['snapper', 'create', '--description', 'before dist-upgrade'], - check=True), + stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=True), call([ '/usr/bin/freedombox-cmd', 'snapshot', 'disable_apt_snapshot' - ], input=b'{"args": ["yes"], "kwargs": {}}', check=True) + ], input=b'{"args": ["yes"], "kwargs": {}}', + stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=True) ] run.reset_mock() assert run.call_args_list == [ call(['/usr/bin/freedombox-cmd', 'snapshot', 'disable_apt_snapshot'], - input=b'{"args": ["no"], "kwargs": {}}', check=True) + input=b'{"args": ["no"], "kwargs": {}}', stdout=subprocess.PIPE, + stderr=subprocess.PIPE, check=True) ] @@ -278,8 +280,10 @@ def test_apt_hold_packages(check_output, check_call, run, tmp_path): expected_call = [call(['apt-mark', 'hold', 'freedombox'])] assert check_call.call_args_list == expected_call expected_calls = [ - call(['apt-mark', 'hold', 'package1'], check=False), - call(['apt-mark', 'hold', 'package2'], check=False) + call(['apt-mark', 'hold', 'package1'], stdout=subprocess.PIPE, + stderr=subprocess.PIPE, check=False), + call(['apt-mark', 'hold', 'package2'], stdout=subprocess.PIPE, + stderr=subprocess.PIPE, check=False) ] assert run.call_args_list == expected_calls check_call.reset_mock() @@ -340,7 +344,8 @@ def test_apt_fix(run, apt_run): """Test that apt fixes work.""" distupgrade._apt_fix() assert run.call_args_list == [ - call(['dpkg', '--configure', '-a'], check=False) + call(['dpkg', '--configure', '-a'], stdout=subprocess.PIPE, + stderr=subprocess.PIPE, check=False) ] assert apt_run.call_args_list == [call(['--fix-broken', 'install'])] @@ -365,7 +370,9 @@ def test_apt_full_upgrade(apt_run): def test_unatteneded_upgrades_run(run): """Test that running unattended upgrades works.""" distupgrade._unattended_upgrades_run() - run.assert_called_with(['unattended-upgrade', '--verbose'], check=False) + run.assert_called_with(['unattended-upgrade', '--verbose'], + stdout=subprocess.PIPE, stderr=subprocess.PIPE, + check=False) @patch('plinth.action_utils.service_restart') @@ -384,7 +391,7 @@ def test_trigger_on_complete(run): '--description=Finish up upgrade to new stable Debian release', '/usr/bin/freedombox-cmd', 'upgrades', 'dist_upgrade_on_complete', '--no-args' - ], check=True) + ], stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=True) def test_on_complete(tmp_path): diff --git a/plinth/modules/users/privileged.py b/plinth/modules/users/privileged.py index 3d75043ca..3bb2670fc 100644 --- a/plinth/modules/users/privileged.py +++ b/plinth/modules/users/privileged.py @@ -68,7 +68,7 @@ def first_setup(): def setup(): """Setup LDAP.""" # Update pam config for mkhomedir. - subprocess.run(['pam-auth-update', '--package'], check=True) + action_utils.run(['pam-auth-update', '--package'], check=True) _configure_ldapscripts() @@ -145,7 +145,7 @@ def _create_organizational_unit(unit): """Create an organizational unit in LDAP.""" distinguished_name = 'ou={unit},dc=thisbox'.format(unit=unit) try: - subprocess.run([ + action_utils.run([ 'ldapsearch', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///', '-s', 'base', '-b', distinguished_name, '(objectclass=*)' ], stdout=subprocess.DEVNULL, check=True) @@ -156,14 +156,14 @@ dn: ou={unit},dc=thisbox objectClass: top objectClass: organizationalUnit ou: {unit}'''.format(unit=unit) - subprocess.run(['ldapadd', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///'], - input=input.encode(), stdout=subprocess.DEVNULL, - check=True) + action_utils.run( + ['ldapadd', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///'], + input=input.encode(), stdout=subprocess.DEVNULL, check=True) def _setup_admin(): """Remove LDAP admin password and Allow root to modify the users.""" - process = subprocess.run([ + process = action_utils.run([ 'ldapsearch', '-Q', '-L', '-L', '-L', '-Y', 'EXTERNAL', '-H', 'ldapi:///', '-s', 'base', '-b', 'olcDatabase={1}mdb,cn=config', '(objectclass=*)', 'olcRootDN', 'olcRootPW' @@ -175,7 +175,7 @@ def _setup_admin(): ldap_object[line[0]] = line[1] if 'olcRootPW' in ldap_object: - subprocess.run( + action_utils.run( ['ldapmodify', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///'], check=True, stdout=subprocess.DEVNULL, input=b''' dn: olcDatabase={1}mdb,cn=config @@ -184,7 +184,7 @@ delete: olcRootPW''') root_dn = 'gidNumber=0+uidNumber=0,cn=peercred,cn=external,cn=auth' if ldap_object['olcRootDN'] != root_dn: - subprocess.run( + action_utils.run( ['ldapmodify', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///'], check=True, stdout=subprocess.DEVNULL, input=b''' dn: olcDatabase={1}mdb,cn=config @@ -205,7 +205,7 @@ def _setup_ldap_ppolicy() -> bool: """ # Load ppolicy module try: - subprocess.run( + action_utils.run( ['ldapmodify', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///'], check=True, stdout=subprocess.DEVNULL, input=b''' dn: cn=module{0},cn=config @@ -218,7 +218,7 @@ olcModuleLoad: ppolicy''') # Add namedobject schema needed for 'objectClass: namedPolicy'. try: - subprocess.run([ + action_utils.run([ 'ldapadd', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///', '-f', '/etc/ldap/schema/namedobject.ldif' ], check=True, stdout=subprocess.DEVNULL) @@ -228,8 +228,9 @@ olcModuleLoad: ppolicy''') # Set up default password policy try: - subprocess.run(['ldapadd', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///'], - check=True, stdout=subprocess.DEVNULL, input=b''' + action_utils.run( + ['ldapadd', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///'], check=True, + stdout=subprocess.DEVNULL, input=b''' dn: cn=DefaultPPolicy,ou=policies,dc=thisbox cn: DefaultPPolicy objectClass: pwdPolicy @@ -243,8 +244,9 @@ pwdLockout: TRUE''') # Make DefaultPPolicy as a default ppolicy overlay try: - subprocess.run(['ldapadd', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///'], - check=True, stdout=subprocess.DEVNULL, input=b''' + action_utils.run( + ['ldapadd', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///'], check=True, + stdout=subprocess.DEVNULL, input=b''' dn: olcOverlay={0}ppolicy,olcDatabase={1}mdb,cn=config objectClass: olcOverlayConfig objectClass: olcPPolicyConfig @@ -463,9 +465,9 @@ def _set_samba_user(username, password): If a user already exists, update password. """ - proc = subprocess.run(['smbpasswd', '-a', '-s', username], - input='{0}\n{0}\n'.format(password).encode(), - stderr=subprocess.PIPE, check=False) + proc = action_utils.run(['smbpasswd', '-a', '-s', username], + input='{0}\n{0}\n'.format(password).encode(), + stderr=subprocess.PIPE, check=False) if proc.returncode != 0: raise RuntimeError('Unable to add Samba user: ', proc.stderr) @@ -684,7 +686,7 @@ def set_user_status(username: str, status: str, auth_user: str, if status == 'inactive': # Kill all user processes. This includes disconnectiong ssh, samba and # cockpit sessions. - subprocess.run(['pkill', '--signal', 'KILL', '--uid', username]) + action_utils.run(['pkill', '--signal', 'KILL', '--uid', username]) def _upgrade_inactivate_users(usernames: list[str]): @@ -695,7 +697,7 @@ def _upgrade_inactivate_users(usernames: list[str]): _flush_cache() for username in usernames: - subprocess.run(['pkill', '--signal', 'KILL', '--uid', username]) + action_utils.run(['pkill', '--signal', 'KILL', '--uid', username]) def _flush_cache(): @@ -708,4 +710,4 @@ def _run(arguments, check=True, **kwargs): env = dict(os.environ, LDAPSCRIPTS_CONF=LDAPSCRIPTS_CONF) kwargs['stdout'] = kwargs.get('stdout', subprocess.DEVNULL) kwargs['stderr'] = kwargs.get('stderr', subprocess.DEVNULL) - return subprocess.run(arguments, env=env, check=check, **kwargs) + return action_utils.run(arguments, env=env, check=check, **kwargs) diff --git a/plinth/modules/wordpress/privileged.py b/plinth/modules/wordpress/privileged.py index 0b5c92f72..23fd7b994 100644 --- a/plinth/modules/wordpress/privileged.py +++ b/plinth/modules/wordpress/privileged.py @@ -6,7 +6,6 @@ import pathlib import random import shutil import string -import subprocess import augeas @@ -90,8 +89,8 @@ def _create_database(db_name): # Wordpress' install.php creates the tables. # SQL injection is avoided due to known input. query = f'''CREATE DATABASE {db_name};''' - subprocess.run(['mysql', '--user', 'root'], input=query.encode(), - check=True) + action_utils.run(['mysql', '--user', 'root'], input=query.encode(), + check=True) def _set_privileges(db_host, db_name, db_user, db_password): @@ -103,8 +102,8 @@ def _set_privileges(db_host, db_name, db_user, db_password): IDENTIFIED BY '{db_password}'; FLUSH PRIVILEGES; ''' - subprocess.run(['mysql', '--user', 'root'], input=query.encode(), - check=True) + action_utils.run(['mysql', '--user', 'root'], input=query.encode(), + check=True) def _generate_secret_key(length=64, chars=None): @@ -146,7 +145,7 @@ def dump_database(): _db_backup_file.parent.mkdir(parents=True, exist_ok=True) with action_utils.service_ensure_running('mysql'): with _db_backup_file.open('w', encoding='utf-8') as file_handle: - subprocess.run([ + action_utils.run([ 'mysqldump', '--add-drop-database', '--add-drop-table', '--add-drop-trigger', '--user', 'root', '--databases', DB_NAME ], stdout=file_handle, check=True) @@ -157,8 +156,8 @@ def restore_database(): """Restore database from file.""" with action_utils.service_ensure_running('mysql'): with _db_backup_file.open('r', encoding='utf-8') as file_handle: - subprocess.run(['mysql', '--user', 'root'], stdin=file_handle, - check=True) + action_utils.run(['mysql', '--user', 'root'], stdin=file_handle, + check=True) _set_privileges(DB_HOST, DB_NAME, DB_USER, _read_db_password()) @@ -192,9 +191,9 @@ def _drop_database(db_host, db_name, db_user): """Drop the mysql database that was created during install.""" with action_utils.service_ensure_running('mysql'): query = f"DROP DATABASE {db_name};" - subprocess.run(['mysql', '--user', 'root'], input=query.encode(), - check=False) + action_utils.run(['mysql', '--user', 'root'], input=query.encode(), + check=False) query = f"DROP USER IF EXISTS {db_user}@{db_host};" - subprocess.run(['mysql', '--user', 'root'], input=query.encode(), - check=False) + action_utils.run(['mysql', '--user', 'root'], input=query.encode(), + check=False) diff --git a/plinth/modules/zoph/privileged.py b/plinth/modules/zoph/privileged.py index 6ed2eefe8..a65974402 100644 --- a/plinth/modules/zoph/privileged.py +++ b/plinth/modules/zoph/privileged.py @@ -33,15 +33,15 @@ def get_configuration() -> dict[str, str]: """Return the current configuration.""" configuration = {} try: - process = subprocess.run(['zoph', '--dump-config'], - stdout=subprocess.PIPE, check=True) + process = action_utils.run(['zoph', '--dump-config'], + stdout=subprocess.PIPE, check=True) except subprocess.CalledProcessError as exception: if exception.returncode != 96: raise _zoph_setup_cli_user() - process = subprocess.run(['zoph', '--dump-config'], - stdout=subprocess.PIPE, check=True) + process = action_utils.run(['zoph', '--dump-config'], + stdout=subprocess.PIPE, check=True) for line in process.stdout.decode().splitlines(): name, value = line.partition(':')[::2] @@ -75,13 +75,13 @@ WHERE def _zoph_configure(key, value): """Set a configure value in Zoph.""" try: - subprocess.run(['zoph', '--config', key, value], check=True) + action_utils.run(['zoph', '--config', key, value], check=True) except subprocess.CalledProcessError as exception: if exception.returncode != 96: raise _zoph_setup_cli_user() - subprocess.run(['zoph', '--config', key, value], check=True) + action_utils.run(['zoph', '--config', key, value], check=True) @privileged @@ -137,15 +137,15 @@ def set_configuration(enable_osm: bool | None = None, query = f"UPDATE zoph_users SET user_name='{admin_user}' \ WHERE user_name='admin';" - subprocess.run(['mysql', _get_db_config()['db_name']], - input=query.encode(), check=True) + action_utils.run(['mysql', _get_db_config()['db_name']], + input=query.encode(), check=True) @privileged def is_configured() -> bool | None: """Return whether zoph app is configured.""" try: - process = subprocess.run( + process = action_utils.run( ['zoph', '--get-config', 'interface.user.remote'], stdout=subprocess.PIPE, check=True) return process.stdout.decode().strip() == 'true' @@ -163,8 +163,8 @@ def dump_database(): db_name = _get_db_config()['db_name'] os.makedirs(os.path.dirname(DB_BACKUP_FILE), exist_ok=True) with open(DB_BACKUP_FILE, 'w', encoding='utf-8') as db_backup_file: - subprocess.run(['mysqldump', db_name], stdout=db_backup_file, - check=True) + action_utils.run(['mysqldump', db_name], stdout=db_backup_file, + check=True) @privileged @@ -178,15 +178,16 @@ def restore_database(): db_user = _get_db_config()['db_user'] db_host = _get_db_config()['db_host'] db_pass = _get_db_config()['db_pass'] - subprocess.run(['mysqladmin', '--force', 'drop', db_name], check=False) - subprocess.run(['mysqladmin', 'create', db_name], check=True) + action_utils.run(['mysqladmin', '--force', 'drop', db_name], + check=False) + action_utils.run(['mysqladmin', 'create', db_name], check=True) with open(DB_BACKUP_FILE, 'r', encoding='utf-8') as db_restore_file: - subprocess.run(['mysql', db_name], stdin=db_restore_file, - check=True) + action_utils.run(['mysql', db_name], stdin=db_restore_file, + check=True) # Set the password for user from restored configuration query = f'ALTER USER {db_user}@{db_host} IDENTIFIED BY "{db_pass}";' - subprocess.run(['mysql'], input=query.encode(), check=True) + action_utils.run(['mysql'], input=query.encode(), check=True) @privileged @@ -198,12 +199,12 @@ def uninstall(): with action_utils.service_ensure_running('mysql'): try: config = _get_db_config() - subprocess.run( + action_utils.run( ['mysqladmin', '--force', 'drop', config['db_name']], check=False) query = f'DROP USER IF EXISTS {config["db_user"]}@localhost;' - subprocess.run(['mysql'], input=query.encode(), check=False) + action_utils.run(['mysql'], input=query.encode(), check=False) except FileNotFoundError: # Database configuration not found pass diff --git a/plinth/privileged/packages.py b/plinth/privileged/packages.py index 90d40ccd1..68928fd04 100644 --- a/plinth/privileged/packages.py +++ b/plinth/privileged/packages.py @@ -3,7 +3,6 @@ import logging import os -import subprocess from collections import defaultdict from typing import Any @@ -14,7 +13,7 @@ import apt_pkg from plinth import action_utils from plinth import app as app_module from plinth import module_loader -from plinth.action_utils import run_apt_command +from plinth.action_utils import run, run_apt_command from plinth.actions import privileged logger = logging.getLogger(__name__) @@ -61,7 +60,7 @@ def install(app_id: str, packages: list[str], skip_recommends: bool = False, if force_missing_configuration: extra_arguments += ['-o', 'Dpkg::Options::=--force-confmiss'] - subprocess.run(['dpkg', '--configure', '-a'], check=False) + run(['dpkg', '--configure', '-a'], check=False) with action_utils.apt_hold_freedombox(): run_apt_command(['--fix-broken', 'install']) returncode = run_apt_command(['install'] + extra_arguments + packages) @@ -79,7 +78,7 @@ def remove(app_id: str, packages: list[str], purge: bool): except Exception: raise PermissionError(f'Packages are not managed: {packages}') - subprocess.run(['dpkg', '--configure', '-a'], check=False) + run(['dpkg', '--configure', '-a'], check=False) with action_utils.apt_hold_freedombox(): run_apt_command(['--fix-broken', 'install']) options = [] if not purge else ['--purge'] diff --git a/plinth/tests/test_daemon.py b/plinth/tests/test_daemon.py index 55a58d60f..bcc43c513 100644 --- a/plinth/tests/test_daemon.py +++ b/plinth/tests/test_daemon.py @@ -81,54 +81,59 @@ def test_is_enabled(service_is_enabled, daemon): @patch('subprocess.run') def test_enable(subprocess_run, apps_init, app_list, mock_privileged, daemon): """Test that enabling the daemon works.""" + common_args1 = dict(stdout=subprocess.PIPE, stderr=subprocess.PIPE, + check=False) + common_args2 = dict(stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, + check=False) daemon.enable() subprocess_run.assert_has_calls( - [call(['systemctl', 'enable', 'test-unit'], check=False)]) + [call(['systemctl', 'enable', 'test-unit'], **common_args1)]) subprocess_run.assert_any_call(['systemctl', 'start', 'test-unit'], - stdout=subprocess.DEVNULL, check=False) + **common_args2) subprocess_run.reset_mock() daemon.alias = 'test-unit-2' daemon.enable() + subprocess_run.assert_has_calls([ - call(['systemctl', 'enable', 'test-unit'], check=False), - call(['systemctl', 'start', 'test-unit'], stdout=subprocess.DEVNULL, - check=False), - call(['systemctl', 'enable', 'test-unit-2'], check=False), - call(['systemctl', 'start', 'test-unit-2'], stdout=subprocess.DEVNULL, - check=False), + call(['systemctl', 'enable', 'test-unit'], **common_args1), + call(['systemctl', 'start', 'test-unit'], **common_args2), + call(['systemctl', 'enable', 'test-unit-2'], **common_args1), + call(['systemctl', 'start', 'test-unit-2'], **common_args2), ]) subprocess_run.assert_any_call(['systemctl', 'start', 'test-unit'], - stdout=subprocess.DEVNULL, check=False) + **common_args2) subprocess_run.assert_any_call(['systemctl', 'start', 'test-unit-2'], - stdout=subprocess.DEVNULL, check=False) + **common_args2) @patch('plinth.app.apps_init') @patch('subprocess.run') def test_disable(subprocess_run, apps_init, app_list, mock_privileged, daemon): """Test that disabling the daemon works.""" + common_args1 = dict(stdout=subprocess.PIPE, stderr=subprocess.PIPE, + check=False) + common_args2 = dict(stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, + check=False) daemon.disable() subprocess_run.assert_has_calls( - [call(['systemctl', 'disable', 'test-unit'], check=False)]) + [call(['systemctl', 'disable', 'test-unit'], **common_args1)]) subprocess_run.assert_any_call(['systemctl', 'stop', 'test-unit'], - stdout=subprocess.DEVNULL, check=False) + **common_args2) subprocess_run.reset_mock() daemon.alias = 'test-unit-2' daemon.disable() subprocess_run.assert_has_calls([ - call(['systemctl', 'disable', 'test-unit'], check=False), - call(['systemctl', 'stop', 'test-unit'], stdout=subprocess.DEVNULL, - check=False), - call(['systemctl', 'disable', 'test-unit-2'], check=False), - call(['systemctl', 'stop', 'test-unit-2'], stdout=subprocess.DEVNULL, - check=False), + call(['systemctl', 'disable', 'test-unit'], **common_args1), + call(['systemctl', 'stop', 'test-unit'], **common_args2), + call(['systemctl', 'disable', 'test-unit-2'], **common_args1), + call(['systemctl', 'stop', 'test-unit-2'], **common_args2), ]) subprocess_run.assert_any_call(['systemctl', 'stop', 'test-unit'], - stdout=subprocess.DEVNULL, check=False) + **common_args2) subprocess_run.assert_any_call(['systemctl', 'stop', 'test-unit-2'], - stdout=subprocess.DEVNULL, check=False) + **common_args2) @patch('plinth.action_utils.service_is_running') @@ -148,6 +153,10 @@ def test_is_running(service_is_running, daemon): def test_ensure_running(subprocess_run, service_is_running, apps_init, app_list, mock_privileged, daemon): """Test that checking that the daemon is running works.""" + common_args1 = dict(stdout=subprocess.PIPE, stderr=subprocess.PIPE, + check=False) + common_args2 = dict(stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, + check=False) service_is_running.return_value = True with daemon.ensure_running() as starting_state: assert starting_state @@ -159,16 +168,14 @@ def test_ensure_running(subprocess_run, service_is_running, apps_init, with daemon.ensure_running() as starting_state: assert not starting_state assert subprocess_run.mock_calls == [ - call(['systemctl', 'enable', 'test-unit'], check=False), - call(['systemctl', 'start', 'test-unit'], - stdout=subprocess.DEVNULL, check=False), + call(['systemctl', 'enable', 'test-unit'], **common_args1), + call(['systemctl', 'start', 'test-unit'], **common_args2), ] subprocess_run.reset_mock() assert subprocess_run.mock_calls == [ - call(['systemctl', 'disable', 'test-unit'], check=False), - call(['systemctl', 'stop', 'test-unit'], stdout=subprocess.DEVNULL, - check=False), + call(['systemctl', 'disable', 'test-unit'], **common_args1), + call(['systemctl', 'stop', 'test-unit'], **common_args2), ]