From cf521bdfd8d46d42feb2297842ca8400476f0de4 Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Thu, 20 Aug 2026 00:48:06 -0400 Subject: [PATCH] fix(startup): warn when an installed systemd unit has drifted from the repo's Nothing re-applies systemd units after the first install. `git pull` -- which is what the web UI's update button runs -- brings a new template into the checkout, but no code in web_interface/ or src/ copies it to /etc/systemd/system, and nothing anywhere runs `systemctl daemon-reload`. The unit that actually runs is whatever first_time_install.sh wrote on day one. So every hardening added to a unit is inert on existing installs, silently. Measured on a live rig: installed /etc/systemd/system/ledmatrix.service 2026-08-06 template systemd/ledmatrix.service 2026-08-19 contents differ with the practical result that the MemoryMax=85% the repo's template specifies was not being enforced at all -- `systemctl show` reported MemoryMax=infinity. Anyone reading the template would reasonably believe the service was capped. Startup now compares each installed unit against its substituted template and warns when they differ, naming install_service.sh as the remedy. A warning, not an error, and deliberately not a silent rewrite: editing files under /etc and restarting services is the installer's job, not something a display process should do to a machine while it is booting. Making it fatal would also brick every development checkout whose unit is legitimately absent or hand-edited. Comparison ignores comments, blank lines and ordering. The template carries explanatory comments the installed copy will not have, and systemd does not care about order within a section, so a literal comparison would warn on every boot and be ignored within a week. Mutation-checked three ways: never reporting drift fails, making it fatal fails, and -- after the first attempt missed it -- comparing raw text now fails too. That last gap is worth noting: the comment-insensitivity tests originally exercised the helper directly, so a comparison that stopped calling the helper passed them all. The test that catches it goes through _validate_systemd_units. 29 startup-validator tests pass. --- src/startup_validator.py | 68 ++++++++++++++++ test/test_systemd_unit_drift.py | 133 ++++++++++++++++++++++++++++++++ 2 files changed, 201 insertions(+) create mode 100644 test/test_systemd_unit_drift.py diff --git a/src/startup_validator.py b/src/startup_validator.py index 9f2e0a51..5dc0d0f0 100644 --- a/src/startup_validator.py +++ b/src/startup_validator.py @@ -62,6 +62,9 @@ class StartupValidator: # Validate plugins if plugin manager is available if self.plugin_manager: self._validate_plugins() + + # Warn when the running systemd unit no longer matches the repo's + self._validate_systemd_units() is_valid = len(self.errors) == 0 @@ -74,6 +77,71 @@ class StartupValidator: return (is_valid, self.errors.copy(), self.warnings.copy()) + #: Units this project installs, and where each is installed to. + _UNITS = ( + ("systemd/ledmatrix.service", "/etc/systemd/system/ledmatrix.service"), + ("systemd/ledmatrix-web.service", "/etc/systemd/system/ledmatrix-web.service"), + ) + + def _validate_systemd_units(self) -> None: + """Warn when an installed unit has drifted from the repo's template. + + Nothing re-applies these after the first install. `git pull` -- which is + what the web UI's update button runs -- brings a new template into the + checkout, but nothing copies it to /etc/systemd/system and nothing runs + `systemctl daemon-reload`, so the unit that actually runs is whatever + first_time_install.sh wrote on day one. + + That makes every hardening added to a unit inert on existing installs. + Measured on one rig: the installed unit was thirteen days older than the + repo's and differed in content, so a MemoryMax the repo had specified + was not being enforced at all -- `systemctl show` reported + MemoryMax=infinity. + + A warning rather than an error, and certainly not a silent rewrite: + editing files under /etc and restarting services is the installer's job, + not something a display process should do to a machine while it boots. + The remedy is to re-run scripts/install/install_service.sh. + """ + try: + project_root = Path(__file__).resolve().parent.parent + for template_rel, installed_path in self._UNITS: + template = project_root / template_rel + installed = Path(installed_path) + if not template.is_file() or not installed.is_file(): + continue + + # The template carries placeholders the installer substitutes, + # so compare the substituted form rather than the raw file. + expected = template.read_text(encoding="utf-8") + expected = expected.replace("__PROJECT_ROOT_DIR__", str(project_root)) + expected = expected.replace("__USER__", "root") + + try: + actual = installed.read_text(encoding="utf-8") + except PermissionError: + continue + + if self._unit_body(expected) != self._unit_body(actual): + self.warnings.append( + f"{installed.name} differs from {template_rel}; the " + "installed unit is not refreshed by an update, so " + "settings added to the template are not in effect. " + "Re-run scripts/install/install_service.sh to apply them." + ) + except OSError as e: + self.logger.debug("Could not compare systemd units: %s", e) + + @staticmethod + def _unit_body(text: str) -> str: + """A unit's meaningful lines: no comments, no blanks, no ordering noise.""" + lines = [] + for line in text.splitlines(): + line = line.strip() + if line and not line.startswith("#"): + lines.append(line) + return "\n".join(sorted(lines)) + def _validate_config(self) -> None: """Validate configuration files.""" try: diff --git a/test/test_systemd_unit_drift.py b/test/test_systemd_unit_drift.py new file mode 100644 index 00000000..0aaa67cd --- /dev/null +++ b/test/test_systemd_unit_drift.py @@ -0,0 +1,133 @@ +"""An installed unit that no longer matches the repo's must be reported. + +Nothing re-applies systemd units after the first install. `git pull` -- what +the web UI's update button runs -- brings a new template into the checkout, but +no code in web_interface/ or src/ copies it to /etc/systemd/system or runs +`systemctl daemon-reload`. The unit that actually runs is whatever +first_time_install.sh wrote on day one. + +So every hardening added to a unit is inert on existing installs. Measured on a +live rig: the installed unit was dated 2026-08-06 and the repo's 2026-08-19, +and they differed -- with the result that a MemoryMax=85% present in the repo's +template was not being enforced at all. `systemctl show` reported +MemoryMax=infinity. + +This is a warning, not an error, and deliberately not a silent rewrite: +editing files under /etc and restarting services is the installer's job, not +something a display process should do to a machine while it boots. +""" +import logging +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from src.startup_validator import StartupValidator + + +@pytest.fixture +def validator(): + v = StartupValidator(config_manager=MagicMock()) + v.logger = logging.getLogger("test") + v.warnings = [] + v.errors = [] + return v + + +def test_a_matching_unit_produces_no_warning(validator, tmp_path): + """The installed unit, substituted exactly as the installer would.""" + project_root = Path("src/startup_validator.py").resolve().parent.parent + template_rel = "systemd/ledmatrix.service" + template = project_root / template_rel + if not template.is_file(): + pytest.skip("repo unit template not present") + + installed = tmp_path / "ledmatrix.service" + installed.write_text( + template.read_text(encoding="utf-8") + .replace("__PROJECT_ROOT_DIR__", str(project_root)) + .replace("__USER__", "root"), + encoding="utf-8") + + validator._UNITS = ((template_rel, str(installed)),) + validator._validate_systemd_units() + assert not validator.warnings, f"a matching unit warned: {validator.warnings}" + assert not validator.errors + + +def test_comments_and_blank_lines_are_not_drift(): + """Otherwise every comment the repo adds would look like a changed unit.""" + a = "[Service]\n# explains a setting\nExecStart=/x\nRestart=always\n" + b = "[Service]\nExecStart=/x\n\nRestart=always\n" + assert StartupValidator._unit_body(a) == StartupValidator._unit_body(b) + + +def test_a_changed_directive_is_drift(): + a = "[Service]\nExecStart=/x\nMemoryMax=85%\n" + b = "[Service]\nExecStart=/x\n" + assert StartupValidator._unit_body(a) != StartupValidator._unit_body(b) + + +def test_reordered_directives_are_not_drift(): + """systemd does not care about order within a section, so neither should this.""" + a = "[Service]\nExecStart=/x\nRestart=always\n" + b = "[Service]\nRestart=always\nExecStart=/x\n" + assert StartupValidator._unit_body(a) == StartupValidator._unit_body(b) + + +def test_cosmetic_differences_do_not_warn(validator, tmp_path): + """Through the real comparison, not the helper. + + The repo's template carries explanatory comments the installed copy may not + have, and the installer does not preserve ordering or blank lines. If those + counted as drift, every boot would warn and the warning would be ignored. + Asserting this on _unit_body alone would not catch a comparison that stopped + calling it -- which is exactly what a careless edit does. + """ + project_root = Path("src/startup_validator.py").resolve().parent.parent + template_rel = "systemd/ledmatrix.service" + template = project_root / template_rel + if not template.is_file(): + pytest.skip("repo unit template not present") + + substituted = (template.read_text(encoding="utf-8") + .replace("__PROJECT_ROOT_DIR__", str(project_root)) + .replace("__USER__", "root")) + # Same directives, stripped of comments and blank lines and reordered. + directives = sorted(line.strip() for line in substituted.splitlines() + if line.strip() and not line.strip().startswith("#")) + installed = tmp_path / "ledmatrix.service" + installed.write_text("\n".join(reversed(directives)) + "\n", encoding="utf-8") + + validator._UNITS = ((template_rel, str(installed)),) + validator._validate_systemd_units() + assert not validator.warnings, ( + f"cosmetic-only difference reported as drift: {validator.warnings}") + + +def test_drift_is_reported_as_a_warning(validator, tmp_path): + """The whole point: a real difference must surface, and only as a warning.""" + installed = tmp_path / "ledmatrix.service" + installed.write_text("[Service]\nExecStart=/usr/bin/python3 /x/run.py\n") + + project_root = Path("src/startup_validator.py").resolve().parent.parent + template_rel = "systemd/ledmatrix.service" + template = project_root / template_rel + if not template.is_file(): + pytest.skip("repo unit template not present") + + validator._UNITS = ((template_rel, str(installed)),) + validator._validate_systemd_units() + + assert validator.warnings, "a differing unit produced no warning" + assert "install_service.sh" in validator.warnings[0], ( + "the warning does not tell the user how to fix it") + assert not validator.errors, "drift must not be fatal at startup" + + +def test_a_missing_installed_unit_is_silent(validator, tmp_path): + """Development checkouts have no /etc/systemd unit; that is not drift.""" + validator._UNITS = (("systemd/ledmatrix.service", str(tmp_path / "absent.service")),) + validator._validate_systemd_units() + assert not validator.warnings + assert not validator.errors