test(install): cover sed_escape_replacement against sed-special characters

CodeRabbit asked for regression coverage using a project path containing an
ampersand; the earlier commits on this branch already fixed the escaping,
mktemp usage, and enable/start-on-fatal-render-failure findings, and the
l->line rename was already applied -- this closes the one remaining gap.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

scripts/install/install_service.sh

@@ -16,6 +16,9 @@ USER_HOME=$(eval echo ~$ACTUAL_USER)
 # Determine the Project Root Directory (parent of scripts/install/)
 PROJECT_ROOT_DIR=$(cd "$(dirname "$0")/../.." && pwd)
 
+# shellcheck source=scripts/install/lib_systemd_render.sh
+source "$PROJECT_ROOT_DIR/scripts/install/lib_systemd_render.sh"
+
 echo "Installing LED Matrix Display Service for user: $ACTUAL_USER"
 echo "Using home directory: $USER_HOME"
 echo "Project root directory: $PROJECT_ROOT_DIR"
@@ -23,15 +26,29 @@ echo "Project root directory: $PROJECT_ROOT_DIR"
 # Render the main display unit from its template. The display service runs as
 # root (it needs GPIO), so __USER__ is always root here -- unlike the web unit
 # below, which runs as whoever installed it.
+#
+# A missing template or a failed render is fatal: falling through would leave
+# whatever unit already sits at /etc/systemd/system/ledmatrix.service (from a
+# previous install) untouched, and the enable/start step below would then
+# silently reuse that stale unit instead of the one this run was asked to
+# install.
 if [ -f "$PROJECT_ROOT_DIR/systemd/ledmatrix.service" ]; then
-    sed "s|__PROJECT_ROOT_DIR__|$PROJECT_ROOT_DIR|g; s|__USER__|root|g" \
-        "$PROJECT_ROOT_DIR/systemd/ledmatrix.service" > /tmp/ledmatrix.service.tmp
+    ESCAPED_PROJECT_ROOT_DIR=$(sed_escape_replacement "$PROJECT_ROOT_DIR")
+    MAIN_UNIT_TMP=$(mktemp)
+    trap 'rm -f "$MAIN_UNIT_TMP"' EXIT
+    if ! sed "s|__PROJECT_ROOT_DIR__|$ESCAPED_PROJECT_ROOT_DIR|g; s|__USER__|root|g" \
+        "$PROJECT_ROOT_DIR/systemd/ledmatrix.service" > "$MAIN_UNIT_TMP"; then
+        echo "ERROR: failed to render ledmatrix.service from its template." >&2
+        exit 1
+    fi
     # Copy the service file to the systemd directory
-    sudo cp /tmp/ledmatrix.service.tmp /etc/systemd/system/ledmatrix.service
+    sudo cp "$MAIN_UNIT_TMP" /etc/systemd/system/ledmatrix.service
     # Clean up
-    rm /tmp/ledmatrix.service.tmp
+    rm -f "$MAIN_UNIT_TMP"
+    trap - EXIT
 else
-    echo "WARNING: ledmatrix.service template not found at $PROJECT_ROOT_DIR/systemd/ledmatrix.service. Main display service not configured."
+    echo "ERROR: ledmatrix.service template not found at $PROJECT_ROOT_DIR/systemd/ledmatrix.service." >&2
+    exit 1
 fi
 
 
@@ -57,12 +74,25 @@ echo "Installing LEDMatrix Web Interface service (ledmatrix-web.service)..."
 # src/startup_validator.py compares the installed unit against the template,
 # every boot warned "re-run install_service.sh" -- and doing so reinstalled the
 # same stale copy, so the warning could never clear.
+#
+# As with the main unit above, a missing template or a failed render is
+# fatal -- otherwise the enable/start check below would fall back to
+# whatever unit (possibly stale) already exists at the destination path.
 if [ -f "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" ]; then
-    sed "s|__PROJECT_ROOT_DIR__|$PROJECT_ROOT_DIR|g; s|__USER__|$ACTUAL_USER|g" \
-        "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" \
-        | sudo tee /etc/systemd/system/ledmatrix-web.service > /dev/null
+    ESCAPED_ACTUAL_USER=$(sed_escape_replacement "$ACTUAL_USER")
+    WEB_UNIT_TMP=$(mktemp)
+    trap 'rm -f "$WEB_UNIT_TMP"' EXIT
+    if ! sed "s|__PROJECT_ROOT_DIR__|$ESCAPED_PROJECT_ROOT_DIR|g; s|__USER__|$ESCAPED_ACTUAL_USER|g" \
+        "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" > "$WEB_UNIT_TMP"; then
+        echo "ERROR: failed to render ledmatrix-web.service from its template." >&2
+        exit 1
+    fi
+    sudo cp "$WEB_UNIT_TMP" /etc/systemd/system/ledmatrix-web.service
+    rm -f "$WEB_UNIT_TMP"
+    trap - EXIT
 else
-    echo "WARNING: ledmatrix-web.service template not found at $PROJECT_ROOT_DIR/systemd/ledmatrix-web.service. Web interface service not configured."
+    echo "ERROR: ledmatrix-web.service template not found at $PROJECT_ROOT_DIR/systemd/ledmatrix-web.service." >&2
+    exit 1
 fi
 
 echo "Reloading systemd daemon for web service..."

scripts/install/install_web_service.sh

@@ -17,6 +17,9 @@ fi
 # Determine the Project Root Directory (parent of scripts/install/)
 PROJECT_ROOT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)
 
+# shellcheck source=scripts/install/lib_systemd_render.sh
+source "$PROJECT_ROOT_DIR/scripts/install/lib_systemd_render.sh"
+
 echo "Installing for user: $ACTUAL_USER"
 echo "Project root directory: $PROJECT_ROOT_DIR"
 
@@ -38,7 +41,9 @@ if [ ! -f "$TEMPLATE" ]; then
 fi
 
 echo "Writing service file to /etc/systemd/system/ledmatrix-web.service"
-sed "s|__PROJECT_ROOT_DIR__|$PROJECT_ROOT_DIR|g; s|__USER__|$ACTUAL_USER|g" \
+ESCAPED_PROJECT_ROOT_DIR=$(sed_escape_replacement "$PROJECT_ROOT_DIR")
+ESCAPED_ACTUAL_USER=$(sed_escape_replacement "$ACTUAL_USER")
+sed "s|__PROJECT_ROOT_DIR__|$ESCAPED_PROJECT_ROOT_DIR|g; s|__USER__|$ESCAPED_ACTUAL_USER|g" \
     "$TEMPLATE" > /etc/systemd/system/ledmatrix-web.service
 
 # Ensure cache directory exists with proper permissions

scripts/install/install_wifi_monitor.sh

@@ -18,6 +18,9 @@ USER_HOME=$(eval echo ~$ACTUAL_USER)
 # Determine the Project Root Directory (parent of scripts/install/)
 PROJECT_ROOT_DIR=$(cd "$(dirname "$0")/../.." && pwd)
 
+# shellcheck source=scripts/install/lib_systemd_render.sh
+source "$PROJECT_ROOT_DIR/scripts/install/lib_systemd_render.sh"
+
 echo "Installing LED Matrix WiFi Monitor Service for user: $ACTUAL_USER"
 echo "Using home directory: $USER_HOME"
 echo "Project root directory: $PROJECT_ROOT_DIR"
@@ -75,7 +78,8 @@ if [ ! -f "$TEMPLATE" ]; then
     exit 1
 fi
 
-SERVICE_FILE_CONTENT=$(sed "s|__PROJECT_ROOT_DIR__|$PROJECT_ROOT_DIR|g; s|__USER__|root|g" "$TEMPLATE")
+ESCAPED_PROJECT_ROOT_DIR=$(sed_escape_replacement "$PROJECT_ROOT_DIR")
+SERVICE_FILE_CONTENT=$(sed "s|__PROJECT_ROOT_DIR__|$ESCAPED_PROJECT_ROOT_DIR|g; s|__USER__|root|g" "$TEMPLATE")
 
 if [ "$EUID" -eq 0 ]; then
     echo "$SERVICE_FILE_CONTENT" | tee /etc/systemd/system/ledmatrix-wifi-monitor.service > /dev/null

scripts/install/lib_systemd_render.sh

@@ -0,0 +1,27 @@
+#!/bin/bash
+#
+# Shared helper for rendering systemd unit templates via sed.
+#
+# Sourced by install_service.sh, install_web_service.sh and
+# install_wifi_monitor.sh so all three escape sed replacement text the same
+# way instead of carrying three copies of the same fix.
+
+# sed_escape_replacement VALUE
+#
+# Print VALUE escaped for safe use as the replacement side of `sed
+# s|pattern|replacement|`. Every one of these scripts builds its sed
+# expression by interpolating a shell variable (a path, a username, ...)
+# straight into the replacement text. sed gives three characters special
+# meaning there: backslash (escape character), & (whole match) and the
+# delimiter itself (here `|`). A value containing any of them -- e.g. a
+# username or path with an `&`, a literal backslash, or a `|` -- would
+# otherwise corrupt the rendered unit file instead of being substituted
+# literally. Escape the backslash first so the later escapes aren't
+# double-escaped.
+sed_escape_replacement() {
+    local value="$1"
+    value="${value//\\/\\\\}"
+    value="${value//&/\\&}"
+    value="${value//|/\\|}"
+    printf '%s' "$value"
+}

test/test_systemd_unit_drift.py

@@ -17,6 +17,8 @@ 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 shlex
+import subprocess
 from pathlib import Path
 from unittest.mock import MagicMock
 
@@ -204,7 +206,7 @@ def test_the_web_unit_still_reports_a_real_changed_directive(validator, tmp_path
 
     rendered = _render(template.read_text(encoding="utf-8"), project_root, "hdpi")
     # Drop RestartSec -- one of the directives the stale heredoc was missing.
-    stale = "\n".join(l for l in rendered.splitlines() if not l.startswith("RestartSec="))
+    stale = "\n".join(line for line in rendered.splitlines() if not line.startswith("RestartSec="))
     installed = tmp_path / "ledmatrix-web.service"
     installed.write_text(stale + "\n", encoding="utf-8")
 
@@ -221,6 +223,36 @@ def test_installed_user_falls_back_to_root():
     assert StartupValidator._installed_user("[Service]\n  User=hdpi  \n") == "hdpi"
 
 
+def test_sed_escape_replacement_preserves_special_characters():
+    """A project path or username containing sed-special characters must render literally.
+
+    The installers build their sed expression by interpolating a shell
+    variable into the replacement side of `sed s|pattern|replacement|`.
+    Unescaped, sed treats `&` as "insert the whole match" and `\\` as an
+    escape character, so a path like `/opt/led&matrix` would corrupt the
+    rendered unit instead of being substituted as-is. lib_systemd_render.sh's
+    sed_escape_replacement exists to prevent exactly that.
+    """
+    project_root = Path("src/startup_validator.py").resolve().parent.parent
+    helper = project_root / "scripts" / "install" / "lib_systemd_render.sh"
+    if not helper.is_file():
+        pytest.skip("install helper not present")
+
+    value = "/opt/led&matrix\\pi|two"
+    escape_cmd = f'source {shlex.quote(str(helper))}; sed_escape_replacement {shlex.quote(value)}'
+    escaped = subprocess.run(
+        ["bash", "-c", escape_cmd], capture_output=True, text=True, check=True
+    ).stdout
+
+    rendered = subprocess.run(
+        ["sed", f"s|__X__|{escaped}|g"],
+        input="path=__X__\n", capture_output=True, text=True, check=True,
+    ).stdout
+
+    assert rendered == f"path={value}\n", (
+        "a sed-special character in the replacement was not preserved literally")
+
+
 def test_no_installer_carries_its_own_copy_of_a_unit():
     """The regression guard.