Files
ServerKit/.github/workflows/test-system-utils.yml
T
Juan DenisandClaude Opus 5 89481b15c0 refactor(system): run_checked() and a ratchet on the raw subprocess surface
Plan 75 §G1 — the door and the ratchet. The 33-file migration is NOT
done; see below.

run_checked(cmd, *, privileged=False, timeout=60, input=None). Around
backend/app's raw subprocess calls sit 220 copies of capture_output/text,
56 hand-rolled TimeoutExpired handlers, 19 FileNotFoundError handlers and
1,164 literal {'success': False, 'error': ...} dicts. Each is a place an
exec failure becomes a false fact.

It keeps the distinction those handlers lose: returncode is None when the
command never ran. Exit 1 means the command answered "no"; no exit code
at all means nobody answered. A caller rendering "not installed" can
finally tell them apart — plan 74's outage in one field. Unprivileged
calls get the same argv[0] resolution as privileged ones, because the
sbin outage is not something that only happens when you ask for root.

The ratchet is tests/raw_subprocess_census.py + RAW_SUBPROCESS_CEILING,
wired into the existing "Audit Raw Subprocess Patterns" CI job and
enforced by test_raw_subprocess_ratchet.py. The grep steps already there
check eight hand-listed files for three known-bad patterns, so a new file
was invisible to the audit entirely; the census counts every call site
under backend/app. It is AST-based because a grep for subprocess.run also
matches comments, docstrings and `except subprocess.SubprocessError`, and
a miscounting ratchet reads green either way. test_the_ceiling_is_not_stale
fails if the ceiling drifts more than 10 above reality — a stale ceiling
silently re-authorises everything a migration removed.

First migration: cron_service, 13 raw calls -> 3. Its `crontab -l` read
and Popen-with-stdin `crontab -` install were written out five times each
and are now two helpers over run_checked. Census 267 -> 258.

The other 32 files are untouched: docker_service (49), database_service
(24), git_service (18), security_service (14), python_service (13) are
the bulk. The door plus the ratchet are what make that incremental
instead of one unreviewable diff.

test_utils_system.py loads system.py under its own importlib spec, so
@patch('app.utils.system.<name>') binds to whichever module object is in
sys.modules at patch time — not necessarily the one run_checked closes
over. shutil/subprocess are singletons so the file's existing patches are
safe; these are the first tests to patch a module-level function of
system.py, and they use patch.object(_module, ...) instead.

BASELINE_COUNT 3383 -> 4081. Most of that is pre-existing drift (the
ratchet only fails on a drop, so an un-bumped floor never complains), not
tests added here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-18 12:05:17 -04:00

168 lines
6.1 KiB
YAML

name: Test System Utilities
on:
push:
branches: [dev]
paths:
- 'backend/app/utils/system.py'
- 'backend/app/services/**'
- 'backend/tests/test_utils_system*.py'
- '.github/workflows/test-system-utils.yml'
pull_request:
branches: [main]
paths:
- 'backend/app/utils/system.py'
- 'backend/app/services/**'
- 'backend/tests/test_utils_system*.py'
# The mocked `unit-tests` job that used to lead this file was removed: it ran
# `pytest tests/test_utils_system.py` (44 tests), and Backend CI's bare
# `pytest -v` already collects that exact file — there is no pytest.ini,
# addopts, or collect_ignore narrowing it. What is left here is the part
# Backend CI genuinely cannot do: exercise the package-manager detection
# against real apt/dnf inside real distro images.
jobs:
# ──────────────────────────────────────────────────────────────────
# Job 1: Integration tests on real distros (no mocks)
# ──────────────────────────────────────────────────────────────────
integration-tests:
name: Integration (${{ matrix.distro }})
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
include:
- distro: ubuntu
image: ubuntu:24.04
expected_manager: apt
- distro: debian
image: debian:12
expected_manager: apt
- distro: fedora
image: fedora:41
expected_manager: dnf
- distro: rocky
image: rockylinux:9
expected_manager: dnf
container:
image: ${{ matrix.image }}
env:
DEBIAN_FRONTEND: noninteractive
steps:
- uses: actions/checkout@v4
- name: Install Python & pytest
shell: bash
run: |
if command -v apt-get >/dev/null 2>&1; then
rm -rf /var/lib/apt/lists/*
apt-get update
apt-get install -y python3 python3-pip
elif command -v dnf >/dev/null 2>&1; then
dnf install -y python3 python3-pip
elif command -v yum >/dev/null 2>&1; then
yum install -y python3 python3-pip
fi
pip3 install --break-system-packages pytest 2>/dev/null || pip3 install pytest
- name: Run integration smoke tests
working-directory: backend
env:
EXPECTED_MANAGER: ${{ matrix.expected_manager }}
run: python3 -m pytest tests/test_utils_system_integration.py -v
# ──────────────────────────────────────────────────────────────────
# Job 2: Audit — grep for raw subprocess patterns in services
# ──────────────────────────────────────────────────────────────────
audit-raw-patterns:
name: Audit Raw Subprocess Patterns
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.11'
# The ratchet (plan 75 §G1). The grep steps below check a hand-listed set
# of files for three known-bad patterns; this counts EVERY raw
# subprocess call site under backend/app and fails if the total grew.
# A grep cannot do this honestly — it also matches comments, docstrings
# and `except subprocess.SubprocessError` — so the census is AST-based.
- name: Raw subprocess census (ceiling may only go down)
working-directory: backend
run: python3 tests/raw_subprocess_census.py
- name: Check for raw 'sudo systemctl' calls in migrated services
run: |
MIGRATED_FILES=(
backend/app/services/firewall_service.py
backend/app/services/security_service.py
backend/app/services/python_service.py
backend/app/services/deployment_service.py
backend/app/services/ftp_service.py
backend/app/services/nginx_service.py
backend/app/services/php_service.py
backend/app/services/ssl_service.py
)
EXIT_CODE=0
echo "=== Checking for raw subprocess patterns ==="
echo ""
# Pattern 1: subprocess.run with 'sudo', 'systemctl'
echo "--- Pattern: 'sudo', 'systemctl' ---"
for f in "${MIGRATED_FILES[@]}"; do
if [ -f "$f" ]; then
MATCHES=$(grep -n "'sudo', 'systemctl'" "$f" || true)
if [ -n "$MATCHES" ]; then
echo "FAIL: $f"
echo "$MATCHES"
EXIT_CODE=1
fi
fi
done
# Pattern 2: subprocess.run with dpkg
echo ""
echo "--- Pattern: subprocess.run.*dpkg ---"
for f in "${MIGRATED_FILES[@]}"; do
if [ -f "$f" ]; then
MATCHES=$(grep -n "subprocess\.run.*dpkg" "$f" || true)
if [ -n "$MATCHES" ]; then
echo "FAIL: $f"
echo "$MATCHES"
EXIT_CODE=1
fi
fi
done
# Pattern 3: os.path.exists('/usr/bin/apt')
echo ""
echo "--- Pattern: os.path.exists('/usr/bin/apt') ---"
for f in "${MIGRATED_FILES[@]}"; do
if [ -f "$f" ]; then
MATCHES=$(grep -n "os\.path\.exists.*'/usr/bin/apt'" "$f" || true)
if [ -n "$MATCHES" ]; then
echo "FAIL: $f"
echo "$MATCHES"
EXIT_CODE=1
fi
fi
done
echo ""
if [ $EXIT_CODE -eq 0 ]; then
echo "All migrated files are clean."
else
echo "Raw subprocess patterns found — see failures above."
fi
exit $EXIT_CODE