fix(web): use fully-qualified .service unit names for privileged systemctl (#360)

The web interface runs headless, so every privileged systemctl call must be
covered by a NOPASSWD rule in /etc/sudoers.d/ledmatrix_web. The sudo command
matches the command line exactly, but the code called 'systemctl start
ledmatrix' while configure_web_sudo.sh grants 'systemctl start
ledmatrix.service'. The rule never matched, so start/stop/enable/disable/
restart fell back to a password prompt and failed with 'a terminal is
required to read the password'.

Align all privileged systemctl calls on the fully-qualified unit names the
sudoers grants use. Add a regression test that cross-checks api_v3.py calls
against the grants in configure_web_sudo.sh.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Ron Pierce
2026-06-04 12:17:00 -07:00
committed by GitHub
parent d488e8a2ad
commit 122e6d6863
2 changed files with 85 additions and 9 deletions

View File

@@ -0,0 +1,76 @@
"""Guards that every privileged systemctl call the web interface makes is
covered by a passwordless-sudo grant in configure_web_sudo.sh.
The web interface runs headless (no TTY), so any `sudo` call that is not
matched by a NOPASSWD rule in /etc/sudoers.d/ledmatrix_web falls back to a
password prompt and fails with:
sudo: a terminal is required to read the password
sudo matches the command line by exact string, so `systemctl start ledmatrix`
and `systemctl start ledmatrix.service` are NOT interchangeable. This test
parses both the production blueprint and the sudoers-generator script and
asserts the (verb, unit) pairs line up, catching the suffix-mismatch class of
bug before it ships.
"""
import ast
import re
from pathlib import Path
PROJECT_ROOT = Path(__file__).resolve().parents[2]
API_V3 = PROJECT_ROOT / "web_interface" / "blueprints" / "api_v3.py"
SUDOERS_SCRIPT = PROJECT_ROOT / "scripts" / "install" / "configure_web_sudo.sh"
def _sudo_systemctl_calls(source: str) -> set[tuple[str, str]]:
"""Return (verb, unit) for every list literal beginning with
['sudo', 'systemctl', ...] passed to a subprocess call in the source."""
calls: set[tuple[str, str]] = set()
for node in ast.walk(ast.parse(source)):
if not isinstance(node, ast.List):
continue
elts = node.elts
if len(elts) < 4:
continue
if not all(isinstance(e, ast.Constant) and isinstance(e.value, str) for e in elts[:4]):
continue
if elts[0].value == "sudo" and elts[1].value == "systemctl":
calls.add((elts[2].value, elts[3].value))
return calls
def _granted_systemctl_rules(script: str) -> set[tuple[str, str]]:
"""Return (verb, unit) for each `$SYSTEMCTL_PATH <verb> <unit>` NOPASSWD
grant emitted by the sudoers-generator script."""
rules: set[tuple[str, str]] = set()
for match in re.finditer(r"\$SYSTEMCTL_PATH\s+(\S+)\s+(\S+)", script):
verb, unit = match.group(1), match.group(2).rstrip('"')
rules.add((verb, unit))
return rules
def test_every_sudo_systemctl_call_is_granted() -> None:
calls = _sudo_systemctl_calls(API_V3.read_text())
rules = _granted_systemctl_rules(SUDOERS_SCRIPT.read_text())
assert calls, "expected to find sudo systemctl calls in api_v3.py"
uncovered = {c for c in calls if c not in rules}
assert not uncovered, (
"These sudo systemctl calls have no matching NOPASSWD grant in "
"configure_web_sudo.sh; they will fail headless with "
"'sudo: a terminal is required to read the password': "
+ ", ".join(f"systemctl {v} {u}" for v, u in sorted(uncovered))
)
def test_units_are_fully_qualified() -> None:
"""Privileged systemctl calls must name the unit as <name>.service so they
match the sudoers grants, which use the fully-qualified unit name."""
calls = _sudo_systemctl_calls(API_V3.read_text())
unqualified = {(v, u) for v, u in calls if not u.endswith(".service")}
assert not unqualified, (
"sudo systemctl calls must use fully-qualified .service unit names: "
+ ", ".join(f"systemctl {v} {u}" for v, u in sorted(unqualified))
)

View File

@@ -190,7 +190,7 @@ def _ensure_display_service_running():
if status.get('active'): if status.get('active'):
status['started'] = False status['started'] = False
return status return status
result = _run_systemctl_command(['sudo', 'systemctl', 'start', 'ledmatrix']) result = _run_systemctl_command(['sudo', 'systemctl', 'start', 'ledmatrix.service'])
service_status = _get_display_service_status() service_status = _get_display_service_status()
result['started'] = result.get('returncode') == 0 result['started'] = result.get('returncode') == 0
result['active'] = service_status.get('active') result['active'] = service_status.get('active')
@@ -199,7 +199,7 @@ def _ensure_display_service_running():
def _stop_display_service(): def _stop_display_service():
"""Stop the ledmatrix display service.""" """Stop the ledmatrix display service."""
result = _run_systemctl_command(['sudo', 'systemctl', 'stop', 'ledmatrix']) result = _run_systemctl_command(['sudo', 'systemctl', 'stop', 'ledmatrix.service'])
status = _get_display_service_status() status = _get_display_service_status()
result['active'] = status.get('active') result['active'] = status.get('active')
result['status'] = status result['status'] = status
@@ -1461,7 +1461,7 @@ def execute_system_action():
# For on-demand modes, we would need to integrate with the display controller # For on-demand modes, we would need to integrate with the display controller
# For now, just start the display service # For now, just start the display service
try: try:
result = subprocess.run(['sudo', 'systemctl', 'start', 'ledmatrix'], result = subprocess.run(['sudo', 'systemctl', 'start', 'ledmatrix.service'],
capture_output=True, text=True, timeout=10) capture_output=True, text=True, timeout=10)
except subprocess.TimeoutExpired as e: except subprocess.TimeoutExpired as e:
logger.error("start_display (%s) timed out: %s", mode, e) logger.error("start_display (%s) timed out: %s", mode, e)
@@ -1478,16 +1478,16 @@ def execute_system_action():
resp['stderr'] = result.stderr.strip() resp['stderr'] = result.stderr.strip()
return jsonify(resp) return jsonify(resp)
else: else:
result = subprocess.run(['sudo', 'systemctl', 'start', 'ledmatrix'], result = subprocess.run(['sudo', 'systemctl', 'start', 'ledmatrix.service'],
capture_output=True, text=True, timeout=10) capture_output=True, text=True, timeout=10)
elif action == 'stop_display': elif action == 'stop_display':
result = subprocess.run(['sudo', 'systemctl', 'stop', 'ledmatrix'], result = subprocess.run(['sudo', 'systemctl', 'stop', 'ledmatrix.service'],
capture_output=True, text=True, timeout=10) capture_output=True, text=True, timeout=10)
elif action == 'enable_autostart': elif action == 'enable_autostart':
result = subprocess.run(['sudo', 'systemctl', 'enable', 'ledmatrix'], result = subprocess.run(['sudo', 'systemctl', 'enable', 'ledmatrix.service'],
capture_output=True, text=True, timeout=10) capture_output=True, text=True, timeout=10)
elif action == 'disable_autostart': elif action == 'disable_autostart':
result = subprocess.run(['sudo', 'systemctl', 'disable', 'ledmatrix'], result = subprocess.run(['sudo', 'systemctl', 'disable', 'ledmatrix.service'],
capture_output=True, text=True, timeout=10) capture_output=True, text=True, timeout=10)
elif action == 'reboot_system': elif action == 'reboot_system':
result = subprocess.run(['sudo', 'reboot'], result = subprocess.run(['sudo', 'reboot'],
@@ -1568,11 +1568,11 @@ def execute_system_action():
'message': pull_message, 'message': pull_message,
}) })
elif action == 'restart_display_service': elif action == 'restart_display_service':
result = subprocess.run(['sudo', 'systemctl', 'restart', 'ledmatrix'], result = subprocess.run(['sudo', 'systemctl', 'restart', 'ledmatrix.service'],
capture_output=True, text=True, timeout=10) capture_output=True, text=True, timeout=10)
elif action == 'restart_web_service': elif action == 'restart_web_service':
# Try to restart the web service (assuming it's ledmatrix-web.service) # Try to restart the web service (assuming it's ledmatrix-web.service)
result = subprocess.run(['sudo', 'systemctl', 'restart', 'ledmatrix-web'], result = subprocess.run(['sudo', 'systemctl', 'restart', 'ledmatrix-web.service'],
capture_output=True, text=True, timeout=10) capture_output=True, text=True, timeout=10)
else: else:
return jsonify({'status': 'error', 'message': 'Unknown action'}), 400 return jsonify({'status': 'error', 'message': 'Unknown action'}), 400