*: Use action_utils.run instead of subprocess.run

- This is to capture stdout and stderr and transmit that from privileged daemon
back to the service to be displayed in HTML.

Tests:

- Unit tests and code checks pass.

- Some of the modified actions work as expected.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>

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'],
+    process = run(['systemctl', 'get-default'], stdout=subprocess.PIPE,
-                             stdout=subprocess.PIPE, check=True)
+                  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,
+    run(['systemctl', 'daemon-reload'], check=True, stdout=subprocess.DEVNULL)
-                   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,
+        run(['systemctl', 'status', servicename], check=True,
-                       stdout=subprocess.DEVNULL)
+            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],
+        process = run(['systemctl', 'is-enabled', service_name], check=True,
-                                 check=True, stdout=subprocess.PIPE,
+                      stdout=subprocess.PIPE, stderr=subprocess.DEVNULL)
-                                 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],
+    run(['systemctl', action, service_name], stdout=subprocess.DEVNULL,
-                   stdout=subprocess.DEVNULL, check=check)
+        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,
+    process = run(command, stdin=subprocess.DEVNULL, stdout=stdout, env=env,
-                             env=env, check=False)
+                  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],
+                process = run(['apt-mark', 'hold', package], check=False)
-                                         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'],
+    run(['apt-mark', 'unhold', 'freedombox'], stdout=subprocess.DEVNULL,
-                   stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
+        stderr=subprocess.DEVNULL, check=False)
-                   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],
+    run(['podman', 'volume', 'rm', '--force', volume_name], check=False)
-                   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],
+    run(['podman', 'volume', 'rm', '--force', volume_name], check=True)
-                   check=True)
+    run(['podman', 'image', 'rm', '--ignore', image_name], check=True)
-    subprocess.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)

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)

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

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'],
+        action_utils.run(['make-ssl-cert', 'generate-default-snakeoil'],
-                       check=True)
+                         check=True)
 
     with action_utils.WebserverChange() as webserver:
         # Disable mod_php as we have switched to mod_fcgi + php-fpm. Disable

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

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)
+    action_utils.run(['deluser', 'bepasty'], check=False)
-    subprocess.run(['delgroup', 'bepasty'], check=False)
+    action_utils.run(['delgroup', 'bepasty'], check=False)

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)

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

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)

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)

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/',
+    sieve_dirs = [
-                  '/etc/dovecot/freedombox-sieve']
+        '/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():

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(),
+    action_utils.run(['iptables-restore'], input=iptables_rules.encode(),
-                   check=True)
+                     check=True)
-    subprocess.run(['ip6tables-restore'], input=iptables_rules.encode(),
+    action_utils.run(['ip6tables-restore'], input=iptables_rules.encode(),
-                   check=True)
+                     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,
+    action_utils.run(['firewall-cmd'] + args, stdout=subprocess.DEVNULL,
-                   stderr=subprocess.DEVNULL, check=True)
+                     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'],
+    action_utils.run(['firewall-cmd', '--set-default-zone=external'],
-                   stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
+                     stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
-                   check=True)
+                     check=True)
     set_firewall_backend('nftables')
 
     _setup_local_service_protection()

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],
+    action_utils.run(['ikiwiki', '-setup', SETUP_WIKI, wiki_name, admin_name],
-                   stdout=subprocess.PIPE, input=input_,
+                     stdout=subprocess.PIPE, input=input_,
-                   stderr=subprocess.PIPE,
+                     stderr=subprocess.PIPE,
-                   env=dict(os.environ, PERL_UNICODE='AS'), check=True)
+                     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],
+    action_utils.run(['ikiwiki', '-setup', SETUP_BLOG, blog_name, admin_name],
-                   stdout=subprocess.PIPE, input=input_,
+                     stdout=subprocess.PIPE, input=input_,
-                   stderr=subprocess.PIPE, env=dict(os.environ,
+                     stderr=subprocess.PIPE, env=dict(os.environ,
-                                                    PERL_UNICODE='AS'))
+                                                      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

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)

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

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,
+        process = action_utils.run(['dpkg', '-s', 'php'],
-                                 check=True)
+                                   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)
+    action_utils.run(['chmod', '-R', 'o-rwx', data_dir], check=True)
-    subprocess.run(['chown', '-R', 'www-data:www-data', 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():

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

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(),
+    action_utils.run(['murmurd', '-readsupw'], input=password.encode(),
-                   stdout=subprocess.DEVNULL, check=False)
+                     stdout=subprocess.DEVNULL, check=False)
 
 
 @privileged

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)

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],
+        action_utils.run(
-                       check=True)
+            ['nmcli', 'con', 'modify', connection_name, key, value],
+            check=True)
 
 
 def _configure_wireless_interface(interface: str):

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,
+        action_utils.run(['mysql', '--user', 'root'], stdin=file_handle,
-                       check=True)
+                         check=True)
 
-    subprocess.run(['redis-cli', '-n',
+    action_utils.run(['redis-cli', '-n',
-                    str(REDIS_DB), 'FLUSHDB', 'SYNC'], check=False)
+                      str(REDIS_DB), 'FLUSHDB', 'SYNC'], check=False)
 
     _set_database_privileges(_get_database_password())
 

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,
+    return action_utils.run(['/usr/share/easy-rsa/easyrsa'] + args,
-                          cwd=KEYS_DIRECTORY, check=True)
+                            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]

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],
+    process = action_utils.run(['systemd-escape', '--path', path],
-                             stdout=subprocess.PIPE, check=True)
+                               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],
+    process = action_utils.run(
-                             stdout=subprocess.PIPE, check=True)
+        ['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,
+    process = action_utils.run(['snapper', 'list'], stdout=subprocess.PIPE,
-                             check=True)
+                               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)

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(),
+    process = action_utils.run(['plget', key], input=CONFIG_FILE.read_bytes(),
-                             stdout=subprocess.PIPE, check=True)
+                               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()
 
 

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,
+            action_utils.run(fallback_command, check=True,
-                           input=input_text.encode())
+                             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,
+        action_utils.run(command, stdout=subprocess.DEVNULL,
-                       stderr=subprocess.DEVNULL, check=True)
+                         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)

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'

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.

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.

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)

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,
+    action_utils.run(['apt-mark', 'unhold', *holds], stdout=subprocess.DEVNULL,
-                   stderr=subprocess.DEVNULL, check=True)
+                     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()
 

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', 'package1'], stdout=subprocess.PIPE,
-                call(['apt-mark', 'hold', 'package2'], check=False)
+                     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):

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:///'],
+        action_utils.run(
-                       input=input.encode(), stdout=subprocess.DEVNULL,
+            ['ldapadd', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///'],
-                       check=True)
+            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:///'],
+        action_utils.run(
-                       check=True, stdout=subprocess.DEVNULL, input=b'''
+            ['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:///'],
+        action_utils.run(
-                       check=True, stdout=subprocess.DEVNULL, input=b'''
+            ['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],
+    proc = action_utils.run(['smbpasswd', '-a', '-s', username],
-                          input='{0}\n{0}\n'.format(password).encode(),
+                            input='{0}\n{0}\n'.format(password).encode(),
-                          stderr=subprocess.PIPE, check=False)
+                            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)

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(),
+    action_utils.run(['mysql', '--user', 'root'], input=query.encode(),
-                   check=True)
+                     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(),
+    action_utils.run(['mysql', '--user', 'root'], input=query.encode(),
-                   check=True)
+                     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,
+            action_utils.run(['mysql', '--user', 'root'], stdin=file_handle,
-                           check=True)
+                             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(),
+        action_utils.run(['mysql', '--user', 'root'], input=query.encode(),
-                       check=False)
+                         check=False)
 
         query = f"DROP USER IF EXISTS {db_user}@{db_host};"
-        subprocess.run(['mysql', '--user', 'root'], input=query.encode(),
+        action_utils.run(['mysql', '--user', 'root'], input=query.encode(),
-                       check=False)
+                         check=False)

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'],
+        process = action_utils.run(['zoph', '--dump-config'],
-                                 stdout=subprocess.PIPE, check=True)
+                                   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'],
+        process = action_utils.run(['zoph', '--dump-config'],
-                                 stdout=subprocess.PIPE, check=True)
+                                   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']],
+        action_utils.run(['mysql', _get_db_config()['db_name']],
-                       input=query.encode(), check=True)
+                         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,
+            action_utils.run(['mysqldump', db_name], stdout=db_backup_file,
-                           check=True)
+                             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)
+        action_utils.run(['mysqladmin', '--force', 'drop', db_name],
-        subprocess.run(['mysqladmin', 'create', db_name], check=True)
+                         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,
+            action_utils.run(['mysql', db_name], stdin=db_restore_file,
-                           check=True)
+                             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
 

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']

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', 'enable', 'test-unit'], **common_args1),
-        call(['systemctl', 'start', 'test-unit'], stdout=subprocess.DEVNULL,
+        call(['systemctl', 'start', 'test-unit'], **common_args2),
-             check=False),
+        call(['systemctl', 'enable', 'test-unit-2'], **common_args1),
-        call(['systemctl', 'enable', 'test-unit-2'], check=False),
+        call(['systemctl', 'start', 'test-unit-2'], **common_args2),
-        call(['systemctl', 'start', 'test-unit-2'], stdout=subprocess.DEVNULL,
-             check=False),
     ])
     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', 'disable', 'test-unit'], **common_args1),
-        call(['systemctl', 'stop', 'test-unit'], stdout=subprocess.DEVNULL,
+        call(['systemctl', 'stop', 'test-unit'], **common_args2),
-             check=False),
+        call(['systemctl', 'disable', 'test-unit-2'], **common_args1),
-        call(['systemctl', 'disable', 'test-unit-2'], check=False),
+        call(['systemctl', 'stop', 'test-unit-2'], **common_args2),
-        call(['systemctl', 'stop', 'test-unit-2'], stdout=subprocess.DEVNULL,
-             check=False),
     ])
     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', 'enable', 'test-unit'], **common_args1),
-            call(['systemctl', 'start', 'test-unit'],
+            call(['systemctl', 'start', 'test-unit'], **common_args2),
-                 stdout=subprocess.DEVNULL, check=False),
         ]
         subprocess_run.reset_mock()
 
     assert subprocess_run.mock_calls == [
-        call(['systemctl', 'disable', 'test-unit'], check=False),
+        call(['systemctl', 'disable', 'test-unit'], **common_args1),
-        call(['systemctl', 'stop', 'test-unit'], stdout=subprocess.DEVNULL,
+        call(['systemctl', 'stop', 'test-unit'], **common_args2),
-             check=False),
     ]