fix(shell)!: run commands as argv lists, closing shell injection (GHSA-798h-hpph-m24j)

shell_exec() now requires the command as a list of arguments and always runs
with shell=False, so argument values are never interpreted by a shell. It no
longer accepts a command string, a shell= parameter or | pipelines, and
get_command_output() has been removed.

- shell: list-only argv, no shell, no shlex, no pipe split; raises TypeError on
  a string. Add safe_cli_value() to reject option-like values (leading "-") for
  positional or target arguments (ssh destination, ping target, ...).
- ssh: build_options()/target() return argv tokens; run()/scp()/rsync() build
  argv lists, drop use_shell, and reject an option-like host or username.
- distro, version: read /etc/os-release directly instead of sourcing via a shell.
- base: oao() and cu() normalize CRLF/CR to LF so Windows output is not rendered
  with doubled line breaks in pre-wrap web UIs.
- shell: decode Windows pipe output with the console/OEM code page instead of
  UTF-8, fixing UnicodeDecodeError and mangled umlauts
  (Linuxfabrik/monitoring-plugins#681).

BREAKING CHANGE: lib.shell.shell_exec() requires an argv list and drops the
shell= parameter, the | pipeline split and get_command_output(); lib.ssh
helpers return and accept argv lists and drop use_shell.

Author: markuslf

CHANGELOG.md

@@ -8,10 +8,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+* distro.py, version.py: read the OS name and version directly from `/etc/os-release` instead of sourcing it through a shell
+* shell.py: `shell_exec()` requires the command as a list of arguments (argv) and always runs with `shell=False`. It no longer accepts a command string, a `shell=` parameter, or `|` pipelines. This is a breaking change: pass `['df', '-h', path]` instead of `'df -h ' + path`
+* shell.py: new `safe_cli_value()` rejects a value that a called program could misread as an option (leading `-`), to guard positional or target arguments (such as an ssh destination or a `ping` target) against option injection
+* ssh.py: `build_options()` and `target()` return argument lists/tokens instead of pre-quoted strings; `run()`, `scp()` and `rsync()` build argument lists, drop the `use_shell` parameter, and reject an option-like host or username
+
+### Removed
+
+* shell.py: removed `get_command_output()` (it had no consumers; use `shell_exec()` directly)
+
 ### Fixed
 
+* base.py: `oao()` normalizes CRLF and stray CR in the plugin message to LF, so Windows command output is no longer rendered with doubled line breaks in web UIs that use `white-space: pre-wrap`
 * endoflifedate.py: the Apache httpd and Rocket.Chat offline data is keyed under their current endoflife.date URLs (`apache-http-server`, `rocket-chat`), so version checks still work when the endoflife.date API is unreachable
 * lftest.py: use the classic `with a, b:` form instead of parenthesized context managers (Python 3.10+ syntax), so the module parses under RHEL 8's default Python 3.6
+* shell.py: on Windows, a subprocess's piped output is decoded with the console / OEM code page instead of UTF-8, so non-ASCII characters such as umlauts in usernames are no longer mangled ([monitoring-plugins#681](https://github.com/Linuxfabrik/monitoring-plugins/issues/681))
 * url.py: use the classic `with a, b:` form instead of parenthesized context managers (Python 3.10+ syntax), so `import lib.url` no longer raises a SyntaxError under RHEL 8's default Python 3.6
 
 

base.py

@@ -11,7 +11,7 @@
 """Provides very common every-day functions."""
 
 __author__ = 'Linuxfabrik GmbH, Zurich/Switzerland'
-__version__ = '2026051002'
+__version__ = '2026061201'
 
 import html
 import numbers
@@ -119,6 +119,9 @@ def cu(msg=None):
     tb = format_exc() if has_traceback else None
 
     if msg is not None:
+        # Normalize line endings to LF (see oao); error output may also carry
+        # CRLF, for example when a Windows command prints its error to stdout.
+        msg = msg.replace('\r\n', '\n').replace('\r', '\n')
         msg = (
             txt.sanitize_sensitive_data(msg).strip().replace('<', "'").replace('>', "'")
         )
@@ -628,6 +631,10 @@ def oao(msg, state=STATE_OK, perfdata='', always_ok=False):
     (and exits with code 2)
 
     """
+    # Normalize line endings to LF. Output captured on Windows (or read from a
+    # file or HTTP response) can carry CRLF or stray CR, which a web UI showing
+    # the output with `white-space: pre-wrap` would render as an extra line break.
+    msg = msg.replace('\r\n', '\n').replace('\r', '\n')
     msg = html.escape(
         txt.sanitize_sensitive_data(msg.strip()),
         quote=False,

db_mysql.py

@@ -20,7 +20,7 @@
 warnings.filterwarnings('ignore', category=UserWarning, module='pymysql')
 
 __author__ = 'Linuxfabrik GmbH, Zurich/Switzerland'
-__version__ = '2026060201'
+__version__ = '2026061201'
 
 try:
     import pymysql.cursors
@@ -302,7 +302,9 @@ def _align_session_collation(conn):
         # Whitelist identifiers to keep the formatted `SET NAMES` safe from
         # any caller-controlled value (the row comes from a system view, but
         # defence in depth doesn't cost anything).
-        if not (re.match(r'^[A-Za-z0-9_]+$', cs) and re.match(r'^[A-Za-z0-9_]+$', coll)):
+        if not (
+            re.match(r'^[A-Za-z0-9_]+$', cs) and re.match(r'^[A-Za-z0-9_]+$', coll)
+        ):
             return
         with conn.cursor() as cursor:
             cursor.execute(f'set names {cs} collate {coll}')
@@ -541,10 +543,10 @@ def get_server_info(banner=None):
     from . import shell
 
     for command in (
-        'mysqld --version',
-        'mariadbd --version',
-        'mariadb --version',
-        'mysql --version',
+        ['mysqld', '--version'],
+        ['mariadbd', '--version'],
+        ['mariadb', '--version'],
+        ['mysql', '--version'],
     ):
         success, result = shell.shell_exec(command)
         if not success:

disk.py

@@ -13,7 +13,7 @@
 """
 
 __author__ = 'Linuxfabrik GmbH, Zurich/Switzerland'
-__version__ = '2026060801'
+__version__ = '2026061201'
 
 import csv
 import os
@@ -747,7 +747,14 @@ def udevadm(device, _property):
     >>> udevadm('/dev/linuxfabrik', 'DEVNAME')
     ''
     """
-    success, result = shell.shell_exec(f'udevadm info --query=property --name={device}')
+    # Only query real device nodes under /dev. Resolving the path first defends
+    # against a crafted `device` that points elsewhere or smuggles arguments.
+    device = os.path.realpath(device)
+    if not device.startswith('/dev/'):
+        return ''
+    success, result = shell.shell_exec(
+        ['udevadm', 'info', '--query=property', f'--name={device}']
+    )
     if not success:
         return ''
     stdout, _, _ = result

distro.py

@@ -17,15 +17,13 @@
 """
 
 __author__ = 'Linuxfabrik GmbH, Zurich/Switzerland'
-__version__ = '2025061001'
+__version__ = '2026061201'
 
 
 import os
 import platform
 import re
 
-from . import shell
-
 # --- Static mappings ---
 
 OSDIST_LIST = (
@@ -430,15 +428,22 @@ def get_distribution_facts():
     distro = facts.get('distribution_file_variety', 'distribution')
     facts['os_family'] = _map_os_family(distro)  # returns 'RedHat', for example
 
-    # hardcoded command, no user input; shell=True needed to source the file
-    cmd = '. /etc/os-release && echo "$NAME $VERSION"'
-    success, result = shell.shell_exec(cmd, shell=True)  # nosec B604
-    if not success:
+    # read NAME and VERSION from /etc/os-release
+    values = {}
+    try:
+        with open('/etc/os-release', encoding='utf-8') as os_release:
+            for line in os_release:
+                line = line.strip()
+                if not line or line.startswith('#') or '=' not in line:
+                    continue
+                key, value = line.split('=', 1)
+                values[key] = value.strip().strip('"').strip("'")
+    except OSError:
         return facts
 
-    stdout, _, _ = result
-    facts['os_info'] = (
-        stdout.strip()
-    )  # returns 'Fedora Linux 41 (Workstation Edition)', for example
+    # returns 'Fedora Linux 41 (Workstation Edition)', for example
+    facts['os_info'] = ' '.join(
+        part for part in (values.get('NAME'), values.get('VERSION')) if part
+    )
 
     return facts

dmidecode.py

@@ -14,7 +14,7 @@
 """
 
 __author__ = 'Linuxfabrik GmbH, Zurich/Switzerland'
-__version__ = '2026060201'
+__version__ = '2026061201'
 
 import re
 
@@ -511,7 +511,7 @@ def get_data():
         ...
     }
     """
-    success, result = shell.shell_exec('sudo dmidecode')
+    success, result = shell.shell_exec(['sudo', 'dmidecode'])
     if not success:
         return False
 

lftest.py

@@ -13,12 +13,13 @@
 import contextlib
 import os
 import re
+import shlex
 import tempfile
 
 from . import base, disk, shell
 
 __author__ = 'Linuxfabrik GmbH, Zurich/Switzerland'
-__version__ = '2026060202'
+__version__ = '2026061201'
 
 
 def run(test_instance, plugin, testcase):
@@ -73,8 +74,10 @@ def run(test_instance, plugin, testcase):
     ...             with self.subTest(id=t['id']):
     ...                 lib.lftest.run(self, self.check, t)
     """
+    # params come from the trusted testcase definition; tokenize them the way a
+    # shell would so quoted multi-word values stay intact, then build the argv.
     params = testcase.get('params', '')
-    cmd = f'{plugin} {params} --test={testcase["test"]}'.strip()
+    cmd = [plugin, *shlex.split(params), f'--test={testcase["test"]}']
     stdout, stderr, retc = base.coe(shell.shell_exec(cmd))
 
     test_instance.assertEqual(
@@ -133,7 +136,6 @@ def test(self):
     ### Example
     >>> class TestCheck(unittest.TestCase):
     ...     check = '../my-plugin'
-    ...
     >>> attach_tests(TestCheck, TESTS)
     >>>
     >>> if __name__ == '__main__':
@@ -157,6 +159,7 @@ def test(self):
         def _make(captured_testcase):
             def _method(self):
                 run(self, getattr(self, plugin_attr), captured_testcase)
+
             return _method
 
         setattr(test_class, method_name, _make(testcase))
@@ -222,6 +225,7 @@ def attach_each(test_class, items, action, id_func=str):
         def _make(captured_item):
             def _method(self):
                 action(self, captured_item)
+
             return _method
 
         setattr(test_class, method_name, _make(item))
@@ -339,8 +343,7 @@ def run_container(
         from testcontainers.core.wait_strategies import LogMessageWaitStrategy
     except ImportError as e:
         raise RuntimeError(
-            'testcontainers is not installed; run '
-            "`pip install testcontainers`"
+            'testcontainers is not installed; run `pip install testcontainers`'
         ) from e
 
     c = DockerContainer(image)
@@ -456,12 +459,15 @@ def _run_mysql_compatible_resolved(image_ref, command, seed):
             # in favour of `mariadb`; sclorg c10s ships both; MySQL
             # ships `mysql` only. Prefer `mariadb`, fall back to
             # `mysql` so all flavours work.
-            container.exec([
-                'sh', '-c',
-                'if command -v mariadb >/dev/null 2>&1; then CLIENT=mariadb; '
-                'else CLIENT=mysql; fi; '
-                f'"$CLIENT" -utest -ptest test -e "{seed}"',
-            ])
+            container.exec(
+                [
+                    'sh',
+                    '-c',
+                    'if command -v mariadb >/dev/null 2>&1; then CLIENT=mariadb; '
+                    'else CLIENT=mysql; fi; '
+                    f'"$CLIENT" -utest -ptest test -e "{seed}"',
+                ]
+            )
 
         host = container.get_container_host_ip()
         port = container.get_exposed_port(3306)
@@ -574,7 +580,8 @@ def run_mysql_compatible_from_containerfile(
     ... ) as (container, defaults_file):
     ...     result = subprocess.run(
     ...         ['python3', '../mysql-traffic', f'--defaults-file={defaults_file}'],
-    ...         capture_output=True, text=True,
+    ...         capture_output=True,
+    ...         text=True,
     ...     )
     """
     try:
@@ -594,14 +601,19 @@ def run_mysql_compatible_from_containerfile(
 
     # No parenthesized context managers: they are Python 3.10+ syntax and break
     # parsing on RHEL 8's default Python 3.6.
-    with DockerImage(
-        path=build_dir,
-        dockerfile_path=dockerfile_name,
-        tag=tag,
-        clean_up=False,
-    ) as image, _run_mysql_compatible_resolved(
-        str(image.tag), command, seed,
-    ) as result:
+    with (
+        DockerImage(
+            path=build_dir,
+            dockerfile_path=dockerfile_name,
+            tag=tag,
+            clean_up=False,
+        ) as image,
+        _run_mysql_compatible_resolved(
+            str(image.tag),
+            command,
+            seed,
+        ) as result,
+    ):
         yield result
 
 

nextcloud.py

@@ -11,10 +11,11 @@
 """This library collects some Nextcloud related functions."""
 
 __author__ = 'Linuxfabrik GmbH, Zurich/Switzerland'
-__version__ = '2026041401'
+__version__ = '2026061201'
 
 import json
 import os
+import shlex
 import shutil
 
 from . import disk, shell
@@ -80,16 +81,17 @@ def run_occ(path, cmd, _format='json'):
     # get the owner of config.php
     user = disk.get_owner(os.path.join(path, 'config/config.php'))
     occ = os.path.join(path, 'occ')
-    # When running a command as a UID, many shells require
-    # that the `#` be escaped with a backslash (`\`).
-    sudo_cmd = f'sudo -u \\#{user} {php} {occ} {cmd}'
+    # Run occ as the numeric UID of the config.php owner. `sudo -u '#<uid>'` selects
+    # the user by UID; the `#` only needs escaping for a shell, which we do not use.
+    sudo_cmd = ['sudo', '-u', f'#{user}', php, occ, *shlex.split(cmd)]
 
     success, result = shell.shell_exec(sudo_cmd)
     stdout, stderr, rc = result
 
     # Prefer the return code to decide success/failure, not stderr presence
     if not success or rc != 0:
-        return False, f'Error running `{sudo_cmd}`: rc={rc}\n{stderr or stdout}'
+        cmd_display = ' '.join(sudo_cmd)
+        return False, f'Error running `{cmd_display}`: rc={rc}\n{stderr or stdout}'
 
     # If we expect JSON, try to parse it; otherwise return text
     if str(_format).lower() == 'json':