fix(security): re-harden root sudo helpers; installer fixes; ARCHITECTURE and PERMISSIONS docs (#640)

* docs: add ARCHITECTURE and PERMISSIONS guides ARCHITECTURE.md maps the processes, the state the display and web services share through the cache, the display loop, the plugin system, the web UI and the update path, with links into the code and a where-to-start table. PERMISSIONS.md lists who owns what after install, both sudoers files (and why iptables is not granted), the polkit rule, and which scripts/fix_perms script to run as which user. Both are linked from the docs index, along with the MQTT bridge README and src/common/README.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: correct stale setup, service and troubleshooting claims - README: quick actions run systemctl on ledmatrix.service (run.py), not display_controller.py; use_short_date_format has no effect; the installer uses system pip with --break-system-packages, not a venv. - CONFIG_DEBUGGING: LEDMATRIX_DEBUG must be "true"; logs are in journald. - GETTING_STARTED, WEB_INTERFACE_GUIDE, TROUBLESHOOTING: enabling a plugin, plugin settings, brightness and Vegas settings apply without a restart; matrix hardware settings still need one. - TROUBLESHOOTING: install dependencies with sudo so the root service sees them; point permission problems at PERMISSIONS.md instead of a project-wide chown. - ADVANCED_FEATURES: real BackgroundDataService stats keys; Vegas hooks return VegasDisplayMode and None falls back to capture; cache files are 0660; fix_web_permissions.sh runs as the web user and does not touch sudoers. - STARLARK_APPS_GUIDE: only the linux-arm64 pixlet binary is downloaded. - HOW_TO_RUN_TESTS: test class examples that exist. - CLAUDE.md: PluginStoreManager, plugin_dirs.py, monorepo installs via the Trees API with ZIP fallback, requirements.txt is optional. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: mark deprecated plugin APIs and state manifest fields once Methods @deprecated("3.7.0") (the set pinned in test_deprecation.py) were shown as current API in the quick reference, API reference, advanced guide, development guide and FONT_MANAGER. Each is now marked deprecated with its replacement. FONT_MANAGER is rewritten around the current API; the override editor is gone and override methods are deprecated. Required manifest fields were stated three different ways. The API reference now has one section: the 7 schema-required fields, the 4 the store refuses without, class_name for the loader, and the 8 to set. The other guides link to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: document every src/common module and every widget - src/common/README.md covered 7 of 17 modules. It now has a table of all of them (purpose, whether plugins import it, release to floor on), a short entry each, and logging advice that matches the code. - SPORTS_UNIFICATION listed two shared modules and called sports_helpers the first; it now lists all six. - The widgets README lists all 28 registered widgets plus the support files, and absorbs the parts that only docs/widget-guide.md had (x-options.labels, x-advanced, x-display hidden, plugin-file-manager). docs/widget-guide.md is now a pointer to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(security): fix_web_permissions.sh re-hardens the root sudo helpers The script chowns the whole project to the web user. That included scripts/fix_perms/safe_plugin_rm.sh and safe_pip_install.sh -- the two helpers /etc/sudoers.d/ledmatrix_web lets the web user run as root -- so running it turned both into a root shell for whoever can edit them. It also re-grouped config_secrets.json away from ledmatrix. After the chown it now does what first_time_install.sh's Steps 11 and 11.1 do: helpers back to root:root 755, and config_secrets.json back to the web unit's User=:ledmatrix 640. Each step is non-fatal and prints the manual command if it fails. Also fixes what the script and its docs claimed: it never configured sudoers, its closing hint pointed at ./configure_web_sudo.sh (wrong path), and the README and ADVANCED_FEATURES.md said to run it with sudo, which it refuses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(security): validate and harden every sudoers drop-in the scripts write configure_wifi_permissions.sh copied its rules into /etc/sudoers.d/ledmatrix_wifi without `visudo -c`. A malformed drop-in makes sudo refuse every command for every user, which on a headless Pi leaves no way back in. It now checks first and leaves the installed file alone when the rules do not parse, as the other two writers do. (It already used mktemp, so that part of the review did not apply.) It also grants the two literal commands wifi_manager.py runs for NetworkManager's shared-mode dnsmasq drop-in -- `cp /tmp/ledmatrix-nm-dnsmasq.conf .../dnsmasq-shared.d/ledmatrix-captive.conf` and `rm -f` of that file. The directory's mkdir was granted, the file was not. Both are pinned in test_sudo_allowlist_covers_calls.py. configure_web_sudo.sh wrote its rules to /tmp/ledmatrix_web_sudoers_$$, a predictable name in a world-writable directory; it now uses mktemp with an EXIT trap, as first_time_install.sh does. It sets mode 440 on the installed file instead of leaving the temp file's mode, and finds visudo in /usr/sbin when that is not on the user's PATH, which skipped the check silently. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(install): escape the project path in the DNS-fix and MQTT unit renderers install_dns_fix.sh and install_mqtt_bridge.sh substituted __PROJECT_ROOT_DIR__ with the raw path, while the other three renderers go through sed_escape_replacement from lib_systemd_render.sh. A checkout under a path containing `&`, `\` or `|` rendered a corrupted unit from these two only. Both now source the helper and use it, and a test checks that every placeholder substitution in scripts/install uses an escaped value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(install): stop the installer scripts reporting things that are not true - first_time_install.sh printed "Password: ledmatrix123" for the setup access point. wifi_manager creates it as an open network ("No password" on the panel), so it now says so. - Step 10.1 printed "✓ WiFi management permissions configured" straight after its own failure message; install_wifi_monitor.sh printed "✓ Package installation completed" after a failed apt install. The tick now only follows success. - Step 7 printed "Web dependencies already installed ... in Step 5" in the one branch that runs because Step 5 did not install them, then created .web_deps_installed on that basis. It now warns and leaves the marker off so the next run retries, as the comment below it intends. - check_system_compatibility.sh called Debian 12 Bookworm "full compatibility confirmed" while first_time_install.sh refuses anything but Debian 13. Bookworm, older Debian and non-Debian systems are now errors. Its counters used ((X++)), which under `set -e` exits the script at the first warning or error (the expression is 0), so the check never reached its summary on any system with one. - configure_web_sudo.sh and configure_wifi_permissions.sh finished by testing `sudo -n test -f ...` and `sudo -n nmcli device status`, neither of which is granted, so they always reported a failure. They now ask `sudo -n -l` about commands the new rules do grant, which checks the rule without running anything. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(install): print the completion summary before rebooting With -y -- and so for every one-shot `curl | bash` install, which always passes -y -- first_time_install.sh ran `reboot` about 180 lines before its "Installation Complete / Web UI Access" summary. reboot returns at once, so the summary printed while the Pi was going down and the SSH session usually dropped before the web UI address could be read. The reboot block moves, unchanged, to the very end of the script. The interactive prompt now also follows the summary. Because the summary now runs before the -y reboot, its one command that could fail under `set -Eeuo pipefail` (the SSID lookup, when nmcli reports a connected device but no active network line) gets `|| true`; a missing SSID was already handled as "SSID unknown". one-shot-install.sh prints its "Next steps" after the installer returns, by which time the reboot is under way, so it now says so, and README's Quick Install mentions the automatic reboot. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(scripts): correct wrong comments and messages, drop dead code No behaviour change except the output text noted below. - 2775 is setgid, not the sticky bit (first_time_install.sh Step 3.1, fix_plugin_permissions.sh), and root needs no "PWM hardware access" to plugin files. - The 777 comments in first_time_install.sh Step 3's fallback and fix_assets_permissions.sh said root needs it to write. Root ignores mode bits; the comments now say what 777 actually opens. The 777 itself is unchanged. - apt_remove ends in `|| true`, so Step 12's "Some packages could not be removed" branch could never run; it is gone and the helper stays non-fatal. - detect_web_service_user's comment named Step 8 for the web unit (install_service.sh installs it in Step 7.5) and now says which branch actually runs. - Step 5 described an "already installed" check that does not exist; the ACTUAL_USER comment described the re-exec backwards. - on_error printed a literal "\n" before "Common fixes:". - Dead code: one-shot-install.sh's uncalled fix_tmp_permissions, LEDMATRIX_ELEVATED=1 (never read) on the sudo re-exec, and configure_web_sudo.sh's unused PYTHON_PATH, which also made a missing python3 fatal for rules that never mention it. - start_display.sh / stop_display.sh said "for user: <you>"; the service runs as root. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(fix_perms): fix_cache_permissions.sh uses setup_cache.sh's model There were two models for /var/cache/ledmatrix. setup_cache.sh (the installer's Step 2) and install_web_service.sh share it through the ledmatrix group: root:ledmatrix, 2775, files 660, which is also what DiskCache relies on to give files the directory's group. fix_cache_permissions.sh instead made it 777 and re-grouped it to the invoking user's group, undoing that. It now runs setup_cache.sh for /var/cache/ledmatrix and keeps its own handling of ~/.ledmatrix_cache. Dropped: /var/cache/ledmatrix/ placeholder_logos (nothing reads it) and the checks against the `daemon` user (no service runs as daemon). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: pin actions/checkout in the Claude workflows, drop template comments claude.yml and claude-code-review.yml used actions/checkout@v4 while test.yml and release-version-check.yml pin the v4.2.2 commit SHA; they now pin the same SHA. The commented-out starter-template settings (prompt, claude_args, paths, author filter) are removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(scripts): index every script and list removal candidates New scripts/README.md gives one line per top-level script and scripts directory, marked keep, dev-only or diagnostic, and lists the eight scripts nothing in the repo refers to as candidates for removal (kept for now). The install, utils and dev READMEs now list the files they were missing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: tighten two checks that mutation testing showed were too loose - The wifi sudoers check matched `visudo -c -f "$TEMP_SUDOERS"` in the error report too, so replacing the check with `if false` still passed. It now requires the command as the condition. - The summary test never had the setup access point up, so reinstating the bogus "Password: ledmatrix123" line went unnoticed. A case with hostapd active now checks the AP is described as open. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(permissions): describe the repaired fix_perms scripts and new WiFi grants Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(changelog): docs-scripts Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-04 06:15:09 +00:00 · 2026-09-24 17:31:41 -04:00
parent 4e61d7248a
commit 3967a6cffc
52 changed files with 1957 additions and 2004 deletions
@@ -0,0 +1,109 @@
+"""scripts/fix_perms/fix_web_permissions.sh must not undo the installer's hardening.
+
+The script chowns the whole project to the web user. That used to include the
+two helpers /etc/sudoers.d/ledmatrix_web lets the web user run as root
+(safe_plugin_rm.sh, safe_pip_install.sh) -- a helper the web user owns is a
+root shell for anyone who can edit it -- and config_secrets.json, which lost
+the ledmatrix group first_time_install.sh gives it. After the chown the script
+now puts both back the way the installer's Steps 11 and 11.1 leave them.
+
+The behavioural test runs the real script against a scratch copy of the
+project with `sudo`, `getent` and `journalctl` stubbed, and checks the order
+of what it asked sudo to do.
+"""
+import os
+import re
+import shutil
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+ROOT = Path(__file__).resolve().parent.parent
+SCRIPT = ROOT / "scripts" / "fix_perms" / "fix_web_permissions.sh"
+LIB = ROOT / "scripts" / "install" / "lib_sudoers.sh"
+
+
+def _text(path):
+    return path.read_text(encoding="utf-8", errors="replace").replace("\r\n", "\n")
+
+
+def _granted_helpers():
+    helpers = set(re.findall(r"scripts/fix_perms/([\w.-]+\.sh) \*", _text(LIB)))
+    assert helpers, "no fix_perms helper grant found in lib_sudoers.sh"
+    return helpers
+
+
+def test_every_granted_helper_is_rehardened_after_the_chown():
+    text = _text(SCRIPT)
+    chown = text.index('sudo chown -R "$WEB_USER:$WEB_USER" "$PROJECT_DIR"')
+    loop = re.search(r"for helper in ([^;]+); do\n(.*?)\ndone", text, re.S)
+    assert loop, "no helper-hardening loop in fix_web_permissions.sh"
+    assert "sudo chown root:root" in loop.group(2) and "sudo chmod 755" in loop.group(2)
+    assert loop.start() > chown, "helpers are hardened before the chown that undoes it"
+    assert _granted_helpers() <= set(loop.group(1).split())
+
+
+def test_no_longer_claims_to_configure_sudoers():
+    text = _text(SCRIPT)
+    assert "Configure sudoers for passwordless access" not in text
+    assert "./configure_web_sudo.sh" not in text.replace("scripts/install/configure_web_sudo.sh", "")
+
+
+_STUB_SUDO = """#!/bin/bash
+printf '%s\\n' "$*" >> "$SUDO_LOG"
+# `sudo -n ...` probes and `sudo -u ...` tests: report failure, run nothing.
+case "$1" in -n|-u) exit 1 ;; esac
+exit 0
+"""
+
+
+@pytest.mark.skipif(sys.platform == "win32" or shutil.which("bash") is None,
+                    reason="needs a POSIX bash")
+def test_script_rehardens_helpers_and_secrets(tmp_path):
+    project = tmp_path / "LED Matrix"
+    (project / "scripts" / "fix_perms").mkdir(parents=True)
+    (project / "config").mkdir()
+    script = project / "scripts" / "fix_perms" / "fix_web_permissions.sh"
+    script.write_text(_text(SCRIPT), encoding="utf-8")
+    for helper in ("safe_plugin_rm.sh", "safe_pip_install.sh"):
+        (project / "scripts" / "fix_perms" / helper).write_text("#!/bin/bash\n")
+    (project / "config" / "config_secrets.json").write_text("{}\n")
+
+    stubs = tmp_path / "stubs"
+    stubs.mkdir()
+    for name, body in (("sudo", _STUB_SUDO),
+                       ("getent", "#!/bin/sh\nexit 0\n"),
+                       ("journalctl", "#!/bin/sh\nexit 1\n")):
+        (stubs / name).write_text(body)
+        (stubs / name).chmod(0o755)
+    log = tmp_path / "sudo.log"
+    env = dict(os.environ, SUDO_LOG=str(log),
+               PATH=os.pathsep.join([str(stubs), os.environ.get("PATH", "")]))
+
+    result = subprocess.run(["bash", str(script)], input="y", env=env,
+                            capture_output=True, text=True)
+    if os.geteuid() == 0:
+        # The script refuses to run as root; that refusal is the whole test.
+        assert result.returncode == 1 and "should not be run as root" in result.stdout
+        return
+    assert result.returncode == 0, result.stdout + result.stderr
+
+    calls = log.read_text().splitlines()
+    user = subprocess.run(["whoami"], capture_output=True, text=True).stdout.strip()
+    chown_all = calls.index(f"chown -R {user}:{user} {project}")
+    for helper in ("safe_plugin_rm.sh", "safe_pip_install.sh"):
+        path = project / "scripts" / "fix_perms" / helper
+        assert calls.index(f"chown root:root {path}") > chown_all, calls
+        assert calls.index(f"chmod 755 {path}") > chown_all, calls
+    secrets = project / "config" / "config_secrets.json"
+    # The owner is the installed web unit's User= when there is one.
+    owner = user
+    unit = Path("/etc/systemd/system/ledmatrix-web.service")
+    if unit.is_file():
+        m = re.search(r"^User=(.*)$", unit.read_text(), re.M)
+        if m and m.group(1):
+            owner = m.group(1)
+    assert calls.index(f"chown {owner}:ledmatrix {secrets}") > chown_all, calls
+    assert calls.index(f"chmod 640 {secrets}") > chown_all, calls
@@ -0,0 +1,134 @@
+"""first_time_install.sh prints its completion summary before it reboots.
+
+With -y (and so with the one-shot `curl | bash` installer, which always
+passes -y) the reboot used to be issued ~180 lines before the "Installation
+Complete / Web UI Access" summary. `reboot` returns at once and the script
+carried on printing while the system went down, so the SSH session usually
+dropped before the user saw the web UI address.
+
+first_time_install.sh exits on anything but Raspberry Pi OS Trixie before it
+parses its arguments, so the behavioural test runs only the tail of the
+script -- from the summary to the end -- with systemctl, nmcli, hostname, ip
+and reboot stubbed.
+"""
+import os
+import shutil
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+ROOT = Path(__file__).resolve().parent.parent
+FIRST_TIME = ROOT / "first_time_install.sh"
+SUMMARY_START = 'echo "Installation Complete!"'
+
+
+def _text():
+    return FIRST_TIME.read_text(encoding="utf-8").replace("\r\n", "\n")
+
+
+def test_every_reboot_comes_after_the_summary():
+    text = _text()
+    summary = text.index(SUMMARY_START)
+    lines = text.splitlines()
+    reboots = [i for i, line in enumerate(lines) if line.strip() == "reboot"]
+    assert reboots, "no reboot call found"
+    summary_line = text[:summary].count("\n")
+    enjoy_line = text[:text.index('echo "Enjoy your LED Matrix display!"')].count("\n")
+    assert all(i > enjoy_line > summary_line for i in reboots), (
+        f"reboot at line(s) {[i + 1 for i in reboots]} runs before the summary "
+        f"(line {summary_line + 1}) has finished printing")
+
+
+def _tail():
+    """The script from the summary header to the end, header rule included."""
+    text = _text()
+    start = text.rindex('echo "=========================================="', 0,
+                        text.index(SUMMARY_START))
+    return text[start:]
+
+
+_POSIX = pytest.mark.skipif(sys.platform == "win32" or shutil.which("bash") is None,
+                            reason="needs a POSIX bash")
+
+
+def _run(tmp_path, env_extra, nmcli_active_line=True, stdin="", hostapd_active=False):
+    stubs = tmp_path / "stubs"
+    stubs.mkdir()
+    log = tmp_path / "calls.log"
+    active = 'echo "yes:HomeNet"' if nmcli_active_line else ":"
+    hostapd = 'case "$*" in *"is-active --quiet hostapd"*) exit 0 ;; esac\n' if hostapd_active else ""
+    bodies = {
+        "reboot": f'#!/bin/sh\necho REBOOT-CALLED\necho reboot >> "{log}"\n',
+        "systemctl": f"#!/bin/sh\n{hostapd}exit 3\n",
+        "hostname": '#!/bin/sh\necho "192.168.1.50 fe80::1"\n',
+        "ip": "#!/bin/sh\nexit 1\n",
+        # device status -> one connected wifi device; device wifi -> active line
+        "nmcli": ('#!/bin/sh\ncase "$*" in\n'
+                  '  *"device status"*) echo "wlan0:wifi:connected" ;;\n'
+                  f'  *"device wifi"*) {active} ;;\n'
+                  "esac\n"),
+    }
+    for name, body in bodies.items():
+        (stubs / name).write_text(body)
+        (stubs / name).chmod(0o755)
+    script = "\n".join([
+        "set -Eeuo pipefail",
+        "on_error() { echo \"ERR-TRAP line $1\" >&2; exit 1; }",
+        "trap 'on_error $LINENO' ERR",
+        "PROJECT_ROOT_DIR=/home/pi/LEDMatrix",
+        "ASSUME_YES=${ASSUME_YES:-0}",
+        "SKIP_REBOOT_PROMPT=${SKIP_REBOOT_PROMPT:-0}",
+        _tail(),
+    ])
+    env = dict(os.environ, PATH=os.pathsep.join([str(stubs), "/usr/bin", "/bin"]), **env_extra)
+    result = subprocess.run(["bash", "-c", script], env=env, input=stdin,
+                            capture_output=True, text=True)
+    calls = log.read_text().splitlines() if log.exists() else []
+    return result, calls
+
+
+@_POSIX
+@pytest.mark.parametrize("nmcli_active_line", [True, False], ids=["ssid", "no-ssid"])
+def test_assume_yes_prints_the_summary_then_reboots(tmp_path, nmcli_active_line):
+    result, calls = _run(tmp_path, {"ASSUME_YES": "1"}, nmcli_active_line)
+    out = result.stdout
+    assert result.returncode == 0, out + result.stderr
+    assert calls == ["reboot"]
+    for text in ("Installation Complete!", "Web UI Access:", "http://192.168.1.50:5000",
+                 "Enjoy your LED Matrix display!"):
+        assert out.index(text) < out.index("REBOOT-CALLED"), text
+    assert "Password: ledmatrix123" not in out
+
+
+@_POSIX
+def test_setup_access_point_is_described_as_open(tmp_path):
+    """wifi_manager creates the setup AP with no security ("No password" on
+    the panel); the summary used to print a password it does not have."""
+    result, _ = _run(tmp_path, {"ASSUME_YES": "1"}, hostapd_active=True)
+    assert result.returncode == 0, result.stdout + result.stderr
+    assert "AP Mode is ACTIVE" in result.stdout
+    assert "Open network, no password" in result.stdout
+    assert "Password:" not in result.stdout
+
+
+@_POSIX
+def test_no_reboot_prompt_prints_the_summary_and_does_not_reboot(tmp_path):
+    result, calls = _run(tmp_path, {"ASSUME_YES": "1", "SKIP_REBOOT_PROMPT": "1"})
+    assert result.returncode == 0, result.stdout + result.stderr
+    assert calls == []
+    assert "Enjoy your LED Matrix display!" in result.stdout
+    assert "Skipping reboot prompt" in result.stdout
+
+
+@_POSIX
+@pytest.mark.parametrize("answer,expected", [("y", ["reboot"]), ("n", [])])
+def test_interactive_prompt_comes_after_the_summary(tmp_path, answer, expected):
+    result, calls = _run(tmp_path, {}, stdin=answer)
+    assert result.returncode == 0, result.stdout + result.stderr
+    assert calls == expected
+    out = result.stdout
+    assert "Enjoy your LED Matrix display!" in out
+    if expected:
+        assert out.index("Enjoy your LED Matrix display!") < out.index("REBOOT-CALLED")
@@ -11,12 +11,16 @@ Four such calls were ungranted, all of them captive-portal teardown/setup:
    rfkill unblock wifi                   wifi_manager.py:1811
    mkdir -p .../dnsmasq-shared.d         wifi_manager.py:922

+The drop-in written into that directory was missing too: the literal
+`cp /tmp/ledmatrix-nm-dnsmasq.conf .../dnsmasq-shared.d/ledmatrix-captive.conf`
+and `rm -f` of the same file, so the directory was granted but not the file.
+
 It goes unnoticed because a stock Raspberry Pi image ships
 /etc/sudoers.d/010_pi-nopasswd granting the default user
 `ALL=(ALL) NOPASSWD: ALL`, which satisfies every gap in both files. It only
 bites once that blanket rule is removed or the service runs as another user.

-Scope, deliberately narrow: this pins the four commands above, each of which
+Scope, deliberately narrow: this pins the commands above, each of which
 can be written out literally. The portal makes further sudo calls whose
 arguments are built at runtime -- iptables and nft rules carrying an interface
 name and a port, `ip addr`, `ip link` -- and those cannot be granted safely
@@ -50,6 +54,11 @@ REQUIRED = (
    ("nft", "delete", "table", "ip", "ledmatrix"),
    ("rfkill", "unblock", "wifi"),
    ("mkdir", "-p", "/etc/NetworkManager/dnsmasq-shared.d"),
+    # The drop-in that directory exists for, written and removed by
+    # _write_nm_dnsmasq_captive_conf / _remove_nm_dnsmasq_captive_conf.
+    ("cp", "/tmp/ledmatrix-nm-dnsmasq.conf",
+     "/etc/NetworkManager/dnsmasq-shared.d/ledmatrix-captive.conf"),
+    ("rm", "-f", "/etc/NetworkManager/dnsmasq-shared.d/ledmatrix-captive.conf"),
 )

 #: Tools with an option that executes a program of the caller's choosing.
@@ -19,6 +19,7 @@ import pytest
 REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 FIRST_TIME = os.path.join(REPO_ROOT, "first_time_install.sh")
 CONFIGURE = os.path.join(REPO_ROOT, "scripts", "install", "configure_web_sudo.sh")
+WIFI = os.path.join(REPO_ROOT, "scripts", "install", "configure_wifi_permissions.sh")

 VISUDO = shutil.which("visudo") or (
    "/usr/sbin/visudo" if os.path.exists("/usr/sbin/visudo") else None
@@ -62,6 +63,54 @@ def test_configure_web_sudo_validates_before_installing():
    assert validate < install, "the rules must be checked before they are installed"


+def test_configure_web_sudo_does_not_use_a_predictable_temp_file():
+    body = _read(CONFIGURE)
+    assert 'TEMP_SUDOERS=$(mktemp' in body
+    assert "/tmp/ledmatrix_web_sudoers_$$" not in body
+    assert "trap 'rm -f \"$TEMP_SUDOERS\"' EXIT" in body
+
+
+def test_configure_web_sudo_installs_mode_440():
+    body = _read(CONFIGURE)
+    install = body.index('cp "$TEMP_SUDOERS" /etc/sudoers.d/ledmatrix_web')
+    assert body.index("chmod 440 /etc/sudoers.d/ledmatrix_web") > install
+
+
+def test_configure_wifi_permissions_validates_before_installing():
+    """The third sudoers writer. It installed its rules unchecked."""
+    body = _read(WIFI)
+    # The check itself, as a condition -- not merely the command appearing in
+    # the error report that follows it.
+    validate = body.index('if ! visudo -c -f "$TEMP_SUDOERS"')
+    install = body.index('sudo cp "$TEMP_SUDOERS" "$SUDOERS_FILE"')
+    assert validate < install, "the rules must be checked before they are installed"
+    # ...and a failed check stops the script before the copy.
+    assert "exit 1" in body[validate:install]
+    assert "TEMP_SUDOERS=$(mktemp" in body
+
+
+@pytest.mark.skipif(sys.platform == "win32", reason="visudo is POSIX only")
+@pytest.mark.skipif(VISUDO is None, reason="visudo not installed")
+def test_the_wifi_rules_actually_parse(tmp_path):
+    """Render configure_wifi_permissions.sh's heredoc with realistic paths."""
+    body = _read(WIFI)
+    opener = 'cat > "$TEMP_SUDOERS" << EOF\n'
+    start = body.index(opener) + len(opener)
+    end = body.index("\nEOF\n", start)
+    out = tmp_path / "wifi"
+    script = "\n".join([
+        "WEB_USER=ledmatrix", "NMCLI_PATH=/usr/bin/nmcli",
+        "SYSTEMCTL_PATH=/usr/bin/systemctl", "SYSCTL_PATH=/usr/sbin/sysctl",
+        "NFT_PATH=/usr/sbin/nft", "RFKILL_PATH=/usr/sbin/rfkill",
+        "MKDIR_PATH=/usr/bin/mkdir",
+        f"cat > '{out}' << EOF", body[start:end], "EOF",
+    ])
+    subprocess.run(["bash", "-c", script], check=True)
+    os.chmod(out, 0o440)
+    result = subprocess.run([VISUDO, "-c", "-f", str(out)], capture_output=True, text=True)
+    assert result.returncode == 0, result.stdout + result.stderr
+
+
 def test_a_missing_rules_library_installs_nothing():
    """If lib_sudoers.sh is missing, nothing is generated -- and an empty file
    would pass `visudo -c` -- so that branch must set the flag the install is
@@ -17,6 +17,7 @@ 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
+import re
 import shlex
 import subprocess
 from pathlib import Path
@@ -253,6 +254,24 @@ def test_sed_escape_replacement_preserves_special_characters():
        "a sed-special character in the replacement was not preserved literally")


+def test_every_unit_renderer_escapes_its_replacement():
+    """Each `sed s|__PLACEHOLDER__|$VALUE|` in an install script uses an escaped value.
+
+    install_dns_fix.sh and install_mqtt_bridge.sh interpolated the raw project
+    path while the other three renderers went through sed_escape_replacement,
+    so a checkout under a path containing `&` rendered a broken unit from
+    those two only.
+    """
+    project_root = Path("src/startup_validator.py").resolve().parent.parent
+    offenders = []
+    for script in sorted((project_root / "scripts" / "install").glob("*.sh")):
+        text = script.read_text(encoding="utf-8")
+        for m in re.finditer(r"s\|__[A-Z_]+__\|\$\{?([A-Za-z_][A-Za-z0-9_]*)\}?\|", text):
+            if not m.group(1).startswith("ESCAPED_") and m.group(1) != "root":
+                offenders.append(f"{script.name}: ${m.group(1)}")
+    assert not offenders, "unescaped sed replacement(s): " + ", ".join(offenders)
+
+
 def test_no_installer_carries_its_own_copy_of_a_unit():
    """The regression guard.