ServerKit/backend/tests/test_sbin_command_guard.py

"""Guard: sbin-resident commands must never be exec'd by bare name.

The panel's systemd unit historically shipped

    Environment="PATH=<venv>/bin:/usr/local/bin:/usr/bin:/bin"

with no sbin directory, and it runs as root — so `_needs_sudo()` is False and
nothing prepends `sudo` to resolve the command through sudo's secure_path.
`subprocess` searches $PATH and nothing else, so every sbin-resident tool
raised FileNotFoundError. Callers wrapping the call in a broad `except` then
reported that as a *fact about the system*:

  - `ufw status`  -> "No Firewall Installed", on a host where ufw was installed
  - `nginx -t`    -> every config reported invalid

`fail2ban-client` and `certbot` live in /usr/bin, so they always worked — which
is exactly why the failure looked feature-specific instead of systemic.

Two layers keep it dead:

  1. `privileged_cmd` and `run_unprivileged` resolve argv[0] to an absolute path
     when $PATH cannot (verified below against a PATH with no sbin).
  2. this static scan, so a NEW raw `subprocess` call cannot reintroduce it by
     bypassing both helpers.

WHAT THIS FILE IS NOT: coverage. This scan only sees *literal* argv lists —
164 of the 288 raw `subprocess.*` sites in `backend/app` (57%), and only for
the hardcoded names in SBIN_COMMANDS. The other 43% (variables, f-strings,
concatenation) are invisible to AST, and a bare-name exec of a binary NOT in
SBIN_COMMANDS passes silently at any site. A green run means "no known sbin
command is exec'd by literal bare name", never "no bare sbin calls exist".
The runtime layer — `tests/popen_guard.py`, installed autouse by conftest —
watches the actual argv of every exec the suite performs and covers the sites
this scan cannot read (plan 75 §B2).
"""

import ast
import os
import subprocess
from pathlib import Path
from unittest.mock import patch

import pytest

from app.utils.system import privileged_cmd, resolve_command, run_unprivileged

APP_ROOT = Path(__file__).resolve().parent.parent / 'app'

#: Commands that live in /usr/sbin or /sbin on Debian/Ubuntu and RHEL. Bare-name
#: exec of any of these is a latent FileNotFoundError on a root install.
SBIN_COMMANDS = {
    'a2enmod', 'a2ensite', 'blkid', 'dovecot', 'e2fsck', 'fail2ban-server',
    'groupadd', 'ip', 'ip6tables', 'ipset', 'iptables', 'modprobe', 'nft',
    'nginx', 'postfix', 'service', 'sshd', 'swapon', 'sysctl', 'tune2fs',
    'ufw', 'useradd', 'userdel', 'usermod',
}

#: Direct subprocess entry points. These bypass both helpers, so they get no
#: argv[0] resolution and are the only way to reintroduce the bug.
RAW_SUBPROCESS_FUNCS = {'run', 'Popen', 'check_output', 'check_call', 'call'}


def _python_files():
    for path in APP_ROOT.rglob('*.py'):
        if '__pycache__' in path.parts:
            continue
        yield path


def _first_arg_command(node):
    """argv[0] of a subprocess call, when it is a plain literal list."""
    if not node.args:
        return None
    first = node.args[0]
    if not isinstance(first, (ast.List, ast.Tuple)) or not first.elts:
        return None
    head = first.elts[0]
    if isinstance(head, ast.Constant) and isinstance(head.value, str):
        return head.value
    return None


def _is_raw_subprocess(node):
    """`subprocess.<fn>(...)` — not run_privileged / run_unprivileged."""
    func = node.func
    return (isinstance(func, ast.Attribute)
            and func.attr in RAW_SUBPROCESS_FUNCS
            and isinstance(func.value, ast.Name)
            and func.value.id == 'subprocess')


def _scan(path):
    """[(lineno, command)] — bare sbin commands exec'd directly in *path*."""
    try:
        tree = ast.parse(path.read_text(encoding='utf-8'), filename=str(path))
    except (SyntaxError, UnicodeDecodeError):
        return []

    offenders = []
    for node in ast.walk(tree):
        if not isinstance(node, ast.Call) or not _is_raw_subprocess(node):
            continue
        command = _first_arg_command(node)
        # 'sudo' as argv[0] is safe: sudo resolves through its own secure_path,
        # which includes the sbin dirs on every distro we target.
        if command in SBIN_COMMANDS:
            offenders.append((node.lineno, command))
    return offenders


def test_no_bare_sbin_command_in_raw_subprocess_calls():
    """A new raw subprocess call must not reintroduce the FileNotFoundError."""
    found = []
    for path in _python_files():
        for lineno, command in _scan(path):
            rel = path.relative_to(APP_ROOT.parent.parent)
            found.append(f'{rel}:{lineno} execs {command!r} by bare name')

    assert not found, (
        'These commands live in /usr/sbin or /sbin, which the panel unit\'s '
        'PATH may omit — subprocess would raise FileNotFoundError:\n  '
        + '\n  '.join(found)
        + '\n\nUse run_privileged() (or run_unprivileged() when no root is needed); '
          'both resolve argv[0] to an absolute path when $PATH cannot.'
        + '\n\nReminder: this scan is a tripwire, not coverage — it reads only '
          'literal argv lists (~57% of raw subprocess sites) and only the names '
          'in SBIN_COMMANDS. A green run does not prove no bare sbin execs '
          'exist; the runtime guard (tests/popen_guard.py) watches actual argv.'
    )


# --------------------------------------------------------------------------- #
# The helpers actually resolve, under the PATH that caused the outage
# --------------------------------------------------------------------------- #

CONSTRAINED_PATH = '/opt/serverkit/venv/bin:/usr/local/bin:/usr/bin:/bin'


@pytest.mark.parametrize('command', sorted(SBIN_COMMANDS))
def test_helpers_resolve_every_sbin_command_without_sbin_on_path(command):
    """Simulates the real unit: sbin absent from $PATH, running as root."""
    sbin_path = f'/usr/sbin/{command}'

    with patch.dict(os.environ, {'PATH': CONSTRAINED_PATH}), \
         patch('app.utils.system.shutil.which', return_value=None), \
         patch('app.utils.system.os.path.exists',
               side_effect=lambda p: p == sbin_path), \
         patch('app.utils.system.os.name', 'posix'), \
         patch('app.utils.system.os.geteuid', return_value=0, create=True):

        assert resolve_command(command) == sbin_path
        assert privileged_cmd([command, '--version'])[0] == sbin_path


@patch('app.utils.system.subprocess.run')
@patch('app.utils.system.os.path.exists')
@patch('app.utils.system.shutil.which', return_value=None)
def test_run_unprivileged_resolves_too(_which, exists, mock_run):
    """The sibling helper had the same gap — `nginx -t` went through it."""
    exists.side_effect = lambda p: p == '/usr/sbin/nginx'
    mock_run.return_value = subprocess.CompletedProcess([], 0, stdout='', stderr='')

    run_unprivileged(['nginx', '-t'])

    assert mock_run.call_args[0][0] == ['/usr/sbin/nginx', '-t']


@patch('app.utils.system.subprocess.run')
@patch('app.utils.system.shutil.which', return_value='/usr/bin/git')
def test_run_unprivileged_leaves_resolvable_commands_alone(_which, mock_run):
    """A working $PATH must not have its argv rewritten underneath callers."""
    mock_run.return_value = subprocess.CompletedProcess([], 0, stdout='', stderr='')

    run_unprivileged(['git', 'status'])

    assert mock_run.call_args[0][0] == ['git', 'status']


def test_the_ambiguous_name_stays_gone():
    """`run_command` said nothing about privilege and sat one autocomplete from
    `run_privileged`. `nginx -t` went through it and reported every config
    invalid. Re-adding the name re-adds the second door this rename closed.

    (`PythonService.run_command` is unrelated and keeps its name — the collision
    is part of why the utils one had to change.)
    """
    import app.utils.system as system

    assert not hasattr(system, 'run_command'), (
        'app.utils.system.run_command is back. Use run_privileged() or the '
        'explicit run_unprivileged() — a name that does not say whether it '
        'escalates is how nginx -t ended up unprivileged.'
    )