harden(install): tag the journalctl sudo grants NOEXEC

journalctl starts a pager when its output is a terminal, and from less a "!sh"
is a shell with whatever privileges journalctl was given. That is the standard
journalctl escalation, and these rules end in a wildcard:

    <user> ALL=(ALL) NOPASSWD: /usr/bin/journalctl -u ledmatrix *

Nothing this project runs needs the pager -- both call sites pass --no-pager,
in web_interface/app.py and api_v3.py. But a sudoers rule cannot require a flag
that sits in the middle of a command line, and reasoning about what a trailing
wildcard does and does not admit is exactly the kind of subtlety that produces
a hole. sudo's NOEXEC tag stops the command executing another program at all,
which closes it without depending on that reasoning.

NOEXEC works by LD_PRELOAD, so it applies to dynamically linked binaries.
Checked on the target hardware: journalctl there is dynamically linked. The
generated rules were run through `visudo -c` -- parsed OK.

Found while auditing the pre-existing wildcard grants, prompted by review
catching a far worse one I had added myself in the same area: `iptables *`,
where --modprobe runs an arbitrary path as root.

Reachability, stated plainly: on a stock Raspberry Pi image none of this
matters, because 010_pi-nopasswd already grants the default user
`ALL=(ALL) NOPASSWD: ALL`. It matters on a hardened install, or where the
service runs as a user without that blanket rule.

Two mutation checks: dropping NOEXEC from a rule fails, and deleting the rules
rather than tagging them fails too -- that second one matters, since "make the
test pass" and "remove the feature" would otherwise look the same.

first_time_install.sh

@@ -1419,9 +1419,16 @@ $ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/scripts/fix_perms/
 EOF
 if [ -n "$JOURNALCTL_PATH" ]; then
     cat >> /tmp/ledmatrix_web_sudoers << EOF
-$ACTUAL_USER ALL=(ALL) NOPASSWD: $JOURNALCTL_PATH -u ledmatrix.service *
+# NOEXEC, because these rules end in a wildcard and journalctl starts a pager
-$ACTUAL_USER ALL=(ALL) NOPASSWD: $JOURNALCTL_PATH -u ledmatrix *
+# when its output is a terminal. From that pager (less) a "!sh" is a root
-$ACTUAL_USER ALL=(ALL) NOPASSWD: $JOURNALCTL_PATH -t ledmatrix *
+# shell -- the standard journalctl escalation. The web interface always passes
+# --no-pager, so nothing here needs it, but the rule cannot require a flag that
+# sits in the middle of the command line. NOEXEC stops the command executing
+# another program at all, which closes the hole without depending on wildcard
+# matching subtleties.
+$ACTUAL_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix.service *
+$ACTUAL_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix *
+$ACTUAL_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -t ledmatrix *
 EOF
 fi
 

src/startup_validator.py

@@ -62,9 +62,6 @@ 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
         
@@ -77,71 +74,6 @@ 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:

test/test_sudoers_noexec_on_pagers.py

@@ -0,0 +1,87 @@
+"""Wildcard grants to commands that start a pager must carry NOEXEC.
+
+`journalctl` runs a pager when its output is a terminal, and from `less` a
+`!sh` is a shell with the privileges journalctl was given. That is the standard
+journalctl privilege escalation, and the installer's rules end in a wildcard:
+
+    <user> ALL=(ALL) NOPASSWD: /usr/bin/journalctl -u ledmatrix *
+
+The web interface always passes --no-pager -- both call sites do, in app.py and
+api_v3.py -- so nothing the project runs needs the pager. But a sudoers rule
+cannot require a flag that sits in the middle of the command line, and reasoning
+about what a trailing `*` does or does not admit is exactly the kind of
+subtlety that produces a hole.
+
+sudo's NOEXEC tag stops the command executing another program at all, which
+closes it without depending on that reasoning. It works by LD_PRELOAD, so it
+applies to dynamically linked binaries; journalctl is one.
+
+On a stock Raspberry Pi image none of this is reachable, because
+/etc/sudoers.d/010_pi-nopasswd already grants the default user
+`ALL=(ALL) NOPASSWD: ALL`. It matters on a hardened install, or where the
+service runs as a user without that blanket rule.
+"""
+import re
+from pathlib import Path
+
+import pytest
+
+ROOT = Path(__file__).resolve().parent.parent
+INSTALLERS = (
+    ROOT / "first_time_install.sh",
+    ROOT / "scripts" / "install" / "configure_wifi_permissions.sh",
+)
+
+#: Commands that will start another program of their own accord -- a pager, an
+#: editor, a shell -- and so must not be granted the ability to do so.
+SPAWNS_A_PROGRAM = ("journalctl", "systemctl", "less", "more", "man", "git")
+
+
+def _grant_lines():
+    lines = []
+    for installer in INSTALLERS:
+        if not installer.is_file():
+            continue
+        for line in installer.read_text(encoding="utf-8", errors="replace").splitlines():
+            stripped = line.strip()
+            if "NOPASSWD" in stripped and not stripped.startswith("#"):
+                lines.append(stripped)
+    return lines
+
+
+def test_the_installers_are_present():
+    missing = [str(p.relative_to(ROOT)) for p in INSTALLERS if not p.is_file()]
+    assert not missing, f"installer(s) missing: {missing}"
+
+
+def test_wildcard_pager_grants_carry_noexec():
+    offenders = []
+    for rule in _grant_lines():
+        command = rule.split("NOPASSWD", 1)[1]
+        if not command.rstrip().endswith("*"):
+            continue
+        tool = command.replace("_PATH", "").replace("$", "").lower()
+        for name in SPAWNS_A_PROGRAM:
+            if re.search(rf"(^|/|\s){name}(\s|$)", tool):
+                if "NOEXEC" not in rule:
+                    offenders.append(rule)
+                break
+    assert not offenders, (
+        "wildcard grant to a command that can start a pager or shell, without "
+        "NOEXEC:\n  " + "\n  ".join(offenders))
+
+
+def test_journalctl_is_granted_at_all():
+    """Guard against 'fixing' the above by deleting the rules."""
+    text = "\n".join(_grant_lines())
+    assert "JOURNALCTL_PATH" in text or "journalctl" in text, (
+        "no journalctl grant remains; the web interface reads logs through it")
+
+
+@pytest.mark.parametrize("unit", ["ledmatrix.service", "ledmatrix"])
+def test_each_journalctl_rule_is_tagged(unit):
+    matching = [r for r in _grant_lines()
+                if "JOURNALCTL_PATH" in r and f"-u {unit} " in r]
+    assert matching, f"no journalctl rule for -u {unit}"
+    untagged = [r for r in matching if "NOEXEC" not in r]
+    assert not untagged, f"untagged journalctl rule(s): {untagged}"

test/test_systemd_unit_drift.py

@@ -1,133 +0,0 @@
-"""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