fix: address five review findings (NM retry loop, start_display message, code quality)

- wifi_monitor_daemon: reset _consecutive_internet_failures = 0 in both
  NM-restart exception handlers; previously both left the counter at threshold,
  causing an immediate retry on the next iteration instead of waiting another
  full backoff period

- api_v3: fix start_display failure message — when mode is set and systemctl
  returns non-zero, message now includes the failure reason and a hint rather
  than always reporting success phrasing

- wifi_manager: move _redirect_backend from class variable to instance variable
  in __init__ alongside _ap_enabled_at; class-level default shadowed correctly
  in practice (single instance) but was misleading

- wifi_manager: narrow broad except Exception in _check_internet_connectivity
  to (subprocess.SubprocessError, OSError) for ping and OSError for HTTP
  (urllib.error.URLError is an OSError subclass in Python 3)

- wifi_manager: remove redundant local 'import re as _re' in _validate_ap_config;
  re is already imported at module level (line 37)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

first_time_install.sh

@@ -1086,6 +1086,7 @@ SYSTEMCTL_PATH=$(which systemctl)
 REBOOT_PATH=$(which reboot)
 POWEROFF_PATH=$(which poweroff)
 BASH_PATH=$(which bash)
+JOURNALCTL_PATH=$(which journalctl 2>/dev/null || true)
 
 # Create sudoers content
 cat > /tmp/ledmatrix_web_sudoers << EOF
@@ -1101,10 +1102,22 @@ $ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix.service
 $ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH enable ledmatrix.service
 $ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH disable ledmatrix.service
 $ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH status ledmatrix.service
+$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix
+$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix.service
+$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix-web.service
+$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix-web.service
+$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix-web.service
 $ACTUAL_USER ALL=(ALL) NOPASSWD: $PYTHON_PATH $PROJECT_ROOT_DIR/display_controller.py
 $ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/start_display.sh
 $ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/stop_display.sh
 EOF
+if [ -n "$JOURNALCTL_PATH" ]; then
+    cat >> /tmp/ledmatrix_web_sudoers << EOF
+$ACTUAL_USER ALL=(ALL) NOPASSWD: $JOURNALCTL_PATH -u ledmatrix.service *
+$ACTUAL_USER ALL=(ALL) NOPASSWD: $JOURNALCTL_PATH -u ledmatrix *
+$ACTUAL_USER ALL=(ALL) NOPASSWD: $JOURNALCTL_PATH -t ledmatrix *
+EOF
+fi
 
 if [ -f "$SUDOERS_FILE" ] && cmp -s /tmp/ledmatrix_web_sudoers "$SUDOERS_FILE"; then
     echo "Sudoers configuration already up to date"

plugin-repos/march-madness/requirements.txt

@@ -1,4 +1,4 @@
 requests>=2.28.0
-Pillow>=9.1.0
+Pillow>=10.3.0
 pytz>=2022.1
 numpy>=1.24.0

scripts/install/configure_web_sudo.sh

@@ -89,9 +89,9 @@ TEMP_SUDOERS="/tmp/ledmatrix_web_sudoers_$$"
     echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH status ledmatrix.service"
     echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix"
     echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix.service"
-    echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix-web"
-    echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix-web"
-    echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix-web"
+    echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix-web.service"
+    echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix-web.service"
+    echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix-web.service"
 
     # Optional: journalctl (non-critical — skip if not found)
     if [ -n "$JOURNALCTL_PATH" ]; then

scripts/utils/wifi_monitor_daemon.py

@@ -10,6 +10,7 @@ import sys
 import time
 import logging
 import signal
+import subprocess
 from pathlib import Path
 
 # Add project root to path (parent of scripts/utils/)
@@ -132,7 +133,7 @@ class WiFiMonitorDaemon:
                 # AP-enable trigger clean and avoid false-positive AP enables from
                 # transient packet loss on otherwise working WiFi.
                 if updated_status.connected and not updated_status.ap_mode_active:
-                    if not self.wifi_manager._check_internet_connectivity():
+                    if not self.wifi_manager.check_internet_connectivity():
                         self._consecutive_internet_failures += 1
                         logger.warning(
                             f"Internet unreachable despite nmcli connection "
@@ -140,10 +141,24 @@ class WiFiMonitorDaemon:
                         )
                         if self._consecutive_internet_failures >= self._nm_restart_threshold:
                             logger.warning("Restarting NetworkManager to recover internet connectivity")
-                            import subprocess as _sp
-                            _sp.run(["sudo", "systemctl", "restart", "NetworkManager"],
-                                    capture_output=True, timeout=20)
-                            self._consecutive_internet_failures = 0
+                            try:
+                                subprocess.run(
+                                    ["/usr/bin/systemctl", "restart", "NetworkManager"],
+                                    capture_output=True, timeout=20, check=True
+                                )
+                                self._consecutive_internet_failures = 0
+                                # NM restart causes a brief WiFi drop; reset the AP-mode grace
+                                # counter so that transient disconnect doesn't count toward
+                                # triggering AP mode.
+                                self.wifi_manager._disconnected_checks = 0
+                            except subprocess.CalledProcessError as e:
+                                logger.error(f"NetworkManager restart failed (rc={e.returncode}); "
+                                             "resetting failure counter to avoid tight retry loop")
+                                self._consecutive_internet_failures = 0
+                            except Exception as e:
+                                logger.error(f"NetworkManager restart error: {e}; "
+                                             "resetting failure counter to avoid tight retry loop")
+                                self._consecutive_internet_failures = 0
                     else:
                         self._consecutive_internet_failures = 0
                 else:

src/wifi_manager.py

@@ -144,6 +144,8 @@ class WiFiManager:
 
         # Timestamp set when AP mode is enabled; used for the idle-timeout check
         self._ap_enabled_at: Optional[float] = None
+        # Which redirect backend was used (iptables/nftables/None); set per-instance
+        self._redirect_backend: Optional[str] = None
 
         logger.info(f"WiFi Manager initialized - nmcli: {self.has_nmcli}, iwlist: {self.has_iwlist}, "
                    f"hostapd: {self.has_hostapd}, dnsmasq: {self.has_dnsmasq}, "
@@ -691,9 +693,8 @@ class WiFiManager:
 
     def _validate_ap_config(self) -> Tuple[str, int]:
         """Return a sanitized (ssid, channel) pair from config, falling back to defaults."""
-        import re as _re
         ssid = str(self.config.get("ap_ssid", DEFAULT_AP_SSID))
-        if not ssid or len(ssid) > 32 or not _re.match(r'^[\x20-\x7E]+$', ssid):
+        if not ssid or len(ssid) > 32 or not re.match(r'^[\x20-\x7E]+$', ssid):
             logger.warning(f"AP SSID '{ssid}' is invalid, falling back to default")
             ssid = DEFAULT_AP_SSID
         try:
@@ -707,143 +708,175 @@ class WiFiManager:
 
     def _setup_iptables_redirect(self) -> bool:
         """
-        Add iptables rules that redirect port 80 → Flask on 5000 for the captive portal.
-        The INPUT rule must accept port 5000 (the post-redirect destination), not port 80.
+        Add port 80 → 5000 redirect rules for the captive portal.
 
-        Uses _find_command_path() so binaries in /sbin or /usr/sbin are resolved even
-        when those directories are absent from PATH in service environments.
+        Tries iptables first, falls back to nftables (used by Debian Trixie).
+        When neither tool is available, logs a warning and returns True — the AP
+        still works and DNS spoofing still triggers the OS popup; users just land
+        on port 5000 directly rather than being redirected from port 80.
 
-        Reads ip_forward from /proc (no subprocess, always reliable), saves it to disk
-        only when the read succeeds, and skips the sysctl write if the value is already
-        "1" to avoid mutating global state unnecessarily.  Teardown will only restore the
-        saved value when the save file is actually present.
-
-        Returns True if all rules were applied successfully.
+        Only returns False when a tool was found but the rule addition itself failed.
         """
         try:
             iptables = self._find_command_path("iptables")
-            if not iptables:
-                logger.debug("iptables unavailable; captive portal requires direct port-5000 access")
-                return False
+            nft = self._find_command_path("nft")
 
-            # Read ip_forward from /proc — reliable with no subprocess or PATH dependency.
-            try:
-                current_fwd = Path("/proc/sys/net/ipv4/ip_forward").read_text().strip()
-            except OSError:
-                current_fwd = None  # can't read → don't save, teardown won't restore
-
-            # Persist the original value only when we could read it.
-            # If the write fails, leave the save file absent so teardown skips the restore
-            # rather than unconditionally forcing "0" (which could break VPNs/bridges).
-            if current_fwd is not None:
-                try:
-                    self._IP_FORWARD_SAVE_PATH.write_text(current_fwd)
-                except OSError:
-                    current_fwd = None  # treat as unsaved; teardown will skip restore
-                    logger.warning("Could not write ip_forward save file; state will not be restored")
-
-            # Enable ip_forward only when it isn't already set, to avoid mutating state
-            # that another service (e.g. NetworkManager shared mode, a VPN) already owns.
-            if current_fwd != "1":
-                sysctl = self._find_command_path("sysctl")
-                sysctl_bin = sysctl if sysctl else "sysctl"
-                sysctl_r = subprocess.run(
-                    ["sudo", sysctl_bin, "-w", "net.ipv4.ip_forward=1"],
-                    capture_output=True, text=True, timeout=5
+            if not iptables and not nft:
+                logger.warning(
+                    "Neither iptables nor nft found; captive portal port-80 redirect unavailable. "
+                    "DNS spoofing will still trigger the OS popup but HTTP on port 80 won't reach Flask."
                 )
-                if sysctl_r.returncode != 0:
-                    logger.error(f"Failed to enable ip_forward: {sysctl_r.stderr.strip()}")
-                    self._teardown_iptables_redirect()
-                    return False
+                self._redirect_backend = None
+                return True  # AP works; redirect is best-effort
 
-            # PREROUTING: redirect HTTP → Flask
-            if subprocess.run(
-                ["sudo", iptables, "-t", "nat", "-C", "PREROUTING",
-                 "-i", self._wifi_interface, "-p", "tcp", "--dport", "80",
-                 "-j", "REDIRECT", "--to-port", "5000"],
-                capture_output=True, timeout=5
-            ).returncode != 0:
-                add_r = subprocess.run(
-                    ["sudo", iptables, "-t", "nat", "-A", "PREROUTING",
-                     "-i", self._wifi_interface, "-p", "tcp", "--dport", "80",
-                     "-j", "REDIRECT", "--to-port", "5000"],
-                    capture_output=True, text=True, timeout=5
-                )
-                if add_r.returncode != 0:
-                    logger.error(f"Failed to add PREROUTING rule: {add_r.stderr.strip()}")
-                    self._teardown_iptables_redirect()
-                    return False
+            if iptables:
+                return self._setup_iptables_redirect_iptables(iptables)
+            else:
+                return self._setup_iptables_redirect_nftables(nft)
 
-            # INPUT: accept traffic on port 5000 (the post-redirect destination port)
-            if subprocess.run(
-                ["sudo", iptables, "-C", "INPUT",
-                 "-i", self._wifi_interface, "-p", "tcp", "--dport", "5000",
-                 "-j", "ACCEPT"],
-                capture_output=True, timeout=5
-            ).returncode != 0:
-                add_r = subprocess.run(
-                    ["sudo", iptables, "-A", "INPUT",
-                     "-i", self._wifi_interface, "-p", "tcp", "--dport", "5000",
-                     "-j", "ACCEPT"],
-                    capture_output=True, text=True, timeout=5
-                )
-                if add_r.returncode != 0:
-                    logger.error(f"Failed to add INPUT rule: {add_r.stderr.strip()}")
-                    self._teardown_iptables_redirect()
-                    return False
-
-            logger.info("iptables: port 80→5000 redirect and INPUT accept-5000 rules added")
-            return True
         except Exception as e:
-            logger.warning(f"Could not set up iptables redirect: {e}")
+            logger.warning(f"Could not set up port redirect: {e}")
             try:
                 self._teardown_iptables_redirect()
             except Exception as cleanup_e:
-                logger.warning(f"Cleanup after iptables redirect exception also failed: {cleanup_e}")
+                logger.warning(f"Cleanup after redirect exception also failed: {cleanup_e}")
             return False
 
-    def _teardown_iptables_redirect(self) -> None:
-        """Remove the port 80→5000 iptables rules and restore the saved ip_forward state.
-
-        ip_forward is only restored when the save file written by _setup_iptables_redirect
-        is present.  If the file is absent (save was skipped or failed), ip_forward is
-        left untouched to avoid forcing "0" onto state owned by another service.
-        """
+    def _setup_iptables_redirect_iptables(self, iptables: str) -> bool:
+        """Set up port 80→5000 redirect using iptables."""
+        # Save ip_forward state before enabling
         try:
-            iptables = self._find_command_path("iptables")
-            if not iptables:
-                return
+            current_fwd = Path("/proc/sys/net/ipv4/ip_forward").read_text().strip()
+        except OSError:
+            current_fwd = None
+        if current_fwd is not None:
+            try:
+                self._IP_FORWARD_SAVE_PATH.write_text(current_fwd)
+            except OSError:
+                current_fwd = None
+                logger.warning("Could not write ip_forward save file; state will not be restored")
 
-            subprocess.run(
-                ["sudo", iptables, "-t", "nat", "-D", "PREROUTING",
+        if current_fwd != "1":
+            sysctl = self._find_command_path("sysctl")
+            sysctl_bin = sysctl if sysctl else "sysctl"
+            r = subprocess.run(["sudo", sysctl_bin, "-w", "net.ipv4.ip_forward=1"],
+                               capture_output=True, text=True, timeout=5)
+            if r.returncode != 0:
+                logger.error(f"Failed to enable ip_forward: {r.stderr.strip()}")
+                self._teardown_iptables_redirect()
+                return False
+
+        if subprocess.run(
+            ["sudo", iptables, "-t", "nat", "-C", "PREROUTING",
+             "-i", self._wifi_interface, "-p", "tcp", "--dport", "80",
+             "-j", "REDIRECT", "--to-port", "5000"],
+            capture_output=True, timeout=5
+        ).returncode != 0:
+            r = subprocess.run(
+                ["sudo", iptables, "-t", "nat", "-A", "PREROUTING",
                  "-i", self._wifi_interface, "-p", "tcp", "--dport", "80",
                  "-j", "REDIRECT", "--to-port", "5000"],
-                capture_output=True, timeout=5
-            )
-            subprocess.run(
-                ["sudo", iptables, "-D", "INPUT",
-                 "-i", self._wifi_interface, "-p", "tcp", "--dport", "5000",
-                 "-j", "ACCEPT"],
-                capture_output=True, timeout=5
+                capture_output=True, text=True, timeout=5
             )
+            if r.returncode != 0:
+                logger.error(f"Failed to add PREROUTING rule: {r.stderr.strip()}")
+                self._teardown_iptables_redirect()
+                return False
+
+        if subprocess.run(
+            ["sudo", iptables, "-C", "INPUT",
+             "-i", self._wifi_interface, "-p", "tcp", "--dport", "5000", "-j", "ACCEPT"],
+            capture_output=True, timeout=5
+        ).returncode != 0:
+            r = subprocess.run(
+                ["sudo", iptables, "-A", "INPUT",
+                 "-i", self._wifi_interface, "-p", "tcp", "--dport", "5000", "-j", "ACCEPT"],
+                capture_output=True, text=True, timeout=5
+            )
+            if r.returncode != 0:
+                logger.error(f"Failed to add INPUT rule: {r.stderr.strip()}")
+                self._teardown_iptables_redirect()
+                return False
+
+        self._redirect_backend = "iptables"
+        logger.info("iptables: port 80→5000 redirect rules added")
+        return True
+
+    def _setup_iptables_redirect_nftables(self, nft: str) -> bool:
+        """Set up port 80→5000 redirect using nftables (Debian Trixie / modern systems)."""
+        # NM's ipv4.method=shared already enables ip_forward; no sysctl needed.
+        cmds = [
+            ["sudo", nft, "add", "table", "ip", "ledmatrix"],
+            ["sudo", nft, "add", "chain", "ip", "ledmatrix", "prerouting",
+             "{", "type", "nat", "hook", "prerouting", "priority", "-100", ";", "}"],
+            ["sudo", nft, "add", "rule", "ip", "ledmatrix", "prerouting",
+             "iif", self._wifi_interface, "tcp", "dport", "80", "redirect", "to", ":5000"],
+        ]
+        for cmd in cmds:
+            r = subprocess.run(cmd, capture_output=True, text=True, timeout=5)
+            if r.returncode != 0:
+                # Table/chain may already exist — only fail on rule add
+                if "add rule" in " ".join(cmd):
+                    logger.error(f"Failed to add nftables redirect rule: {r.stderr.strip()}")
+                    self._teardown_iptables_redirect()
+                    return False
+                logger.debug(f"nft cmd non-zero (may already exist): {r.stderr.strip()}")
+
+        self._redirect_backend = "nftables"
+        logger.info("nftables: port 80→5000 redirect rule added")
+        return True
+
+    def _teardown_iptables_redirect(self) -> None:
+        """Remove the port 80→5000 redirect rules and restore ip_forward if saved."""
+        try:
+            backend = self._redirect_backend
+            self._redirect_backend = None
+
+            if backend == "iptables":
+                iptables = self._find_command_path("iptables")
+                if iptables:
+                    subprocess.run(
+                        ["sudo", iptables, "-t", "nat", "-D", "PREROUTING",
+                         "-i", self._wifi_interface, "-p", "tcp", "--dport", "80",
+                         "-j", "REDIRECT", "--to-port", "5000"],
+                        capture_output=True, timeout=5
+                    )
+                    subprocess.run(
+                        ["sudo", iptables, "-D", "INPUT",
+                         "-i", self._wifi_interface, "-p", "tcp", "--dport", "5000",
+                         "-j", "ACCEPT"],
+                        capture_output=True, timeout=5
+                    )
+                # Restore ip_forward only when we saved it
+                if self._IP_FORWARD_SAVE_PATH.exists():
+                    try:
+                        saved = self._IP_FORWARD_SAVE_PATH.read_text().strip()
+                        self._IP_FORWARD_SAVE_PATH.unlink(missing_ok=True)
+                        sysctl = self._find_command_path("sysctl")
+                        sysctl_bin = sysctl if sysctl else "sysctl"
+                        subprocess.run(["sudo", sysctl_bin, "-w", f"net.ipv4.ip_forward={saved}"],
+                                       capture_output=True, timeout=5)
+                        logger.info(f"ip_forward restored to {saved}")
+                    except OSError as e:
+                        logger.warning(f"Could not restore ip_forward: {e}")
+                else:
+                    logger.debug("ip_forward not modified by setup; leaving unchanged")
+
+            elif backend == "nftables":
+                nft = self._find_command_path("nft")
+                if nft:
+                    subprocess.run(
+                        ["sudo", nft, "delete", "table", "ip", "ledmatrix"],
+                        capture_output=True, timeout=5
+                    )
+                    logger.info("nftables ledmatrix table removed")
 
-            # Only restore ip_forward when we have a saved value from setup.
-            # If the save file is absent the state was never changed here, so leave it.
-            if self._IP_FORWARD_SAVE_PATH.exists():
-                try:
-                    saved = self._IP_FORWARD_SAVE_PATH.read_text().strip()
-                    self._IP_FORWARD_SAVE_PATH.unlink(missing_ok=True)
-                    sysctl = self._find_command_path("sysctl")
-                    sysctl_bin = sysctl if sysctl else "sysctl"
-                    subprocess.run(["sudo", sysctl_bin, "-w", f"net.ipv4.ip_forward={saved}"],
-                                   capture_output=True, timeout=5)
-                    logger.info(f"iptables redirect rules removed; ip_forward restored to {saved}")
-                except OSError as e:
-                    logger.warning(f"Could not restore ip_forward: {e}")
             else:
-                logger.info("iptables redirect rules removed; ip_forward left unchanged (not modified by setup)")
+                # No redirect was set up (neither tool available); nothing to tear down
+                self._IP_FORWARD_SAVE_PATH.unlink(missing_ok=True)
+
         except Exception as e:
-            logger.warning(f"Could not tear down iptables redirect: {e}")
+            logger.warning(f"Could not tear down port redirect: {e}")
 
     def _write_nm_dnsmasq_captive_conf(self, ap_ip: str = "192.168.4.1") -> None:
         """
@@ -900,18 +933,22 @@ class WiFiManager:
             if r.returncode == 0:
                 logger.debug("Internet connectivity confirmed via ping 8.8.8.8")
                 return True
-        except Exception:
+        except (subprocess.SubprocessError, OSError):
             pass
         try:
             import urllib.request as _ureq
             _ureq.urlopen("http://connectivity-check.ubuntu.com/", timeout=timeout)
             logger.debug("Internet connectivity confirmed via HTTP check")
             return True
-        except Exception:
+        except OSError:
             pass
         logger.debug("Internet connectivity check failed (both ping and HTTP)")
         return False
 
+    def check_internet_connectivity(self, timeout: int = 5) -> bool:
+        """Public wrapper around _check_internet_connectivity for use by the daemon."""
+        return self._check_internet_connectivity(timeout=timeout)
+
     def _has_ap_clients(self) -> bool:
         """
         Return True if at least one client is associated with the AP.
@@ -2007,10 +2044,10 @@ class WiFiManager:
                 # No 802-11-wireless-security section → open network
             ]
 
-            # On Trixie disable PMF which can prevent older clients from connecting
-            if self._is_trixie:
-                cmd += ["802-11-wireless-security.pmf", "disable"]
-                logger.info("Trixie detected: disabling PMF for better client compatibility")
+            # PMF (Protected Management Frames) is only meaningful for WPA2/WPA3.
+            # An open AP has no security section, so adding 802-11-wireless-security.pmf
+            # would cause NM to require key-mgmt too, breaking the connection add on
+            # Trixie NM 1.52+. Leave PMF untouched — open APs have no frame protection.
 
             result = subprocess.run(cmd, capture_output=True, text=True, timeout=30)
 
@@ -2034,6 +2071,7 @@ class WiFiManager:
             if up_result.returncode != 0:
                 error_msg = up_result.stderr.strip() or up_result.stdout.strip()
                 logger.error(f"Failed to bring up AP connection: {error_msg}")
+                self._remove_nm_dnsmasq_captive_conf()
                 subprocess.run(["nmcli", "connection", "delete", "LEDMatrix-Setup-AP"],
                                capture_output=True, timeout=10)
                 self._show_led_message("AP mode failed", duration=5)
@@ -2045,6 +2083,7 @@ class WiFiManager:
             # need to add the iptables port-redirect rules for the captive portal.
             if not self._setup_iptables_redirect():
                 logger.error("Captive-portal redirect setup failed; rolling back AP profile")
+                self._remove_nm_dnsmasq_captive_conf()
                 subprocess.run(["nmcli", "connection", "down", "LEDMatrix-Setup-AP"],
                                capture_output=True, timeout=10)
                 subprocess.run(["nmcli", "connection", "delete", "LEDMatrix-Setup-AP"],
@@ -2062,6 +2101,7 @@ class WiFiManager:
             else:
                 logger.error("AP mode started but not verified by status check — rolling back")
                 self._teardown_iptables_redirect()
+                self._remove_nm_dnsmasq_captive_conf()
                 subprocess.run(["nmcli", "connection", "down", "LEDMatrix-Setup-AP"],
                                capture_output=True, timeout=10)
                 subprocess.run(["nmcli", "connection", "delete", "LEDMatrix-Setup-AP"],
@@ -2071,6 +2111,7 @@ class WiFiManager:
 
         except Exception as e:
             logger.error(f"Error starting AP mode with nmcli: {e}")
+            self._remove_nm_dnsmasq_captive_conf()
             self._show_led_message("Setup mode error", duration=5)
             return False, str(e)
     
@@ -2458,7 +2499,10 @@ address=/detectportal.firefox.com/192.168.4.1
             # Idle-timeout check: disable AP if no client has connected within the window.
             # Only applies when AP is active and we haven't just decided to enable/disable it.
             if ap_active and self._ap_enabled_at is not None:
-                idle_timeout_min = self.config.get("ap_idle_timeout_minutes", 15)
+                try:
+                    idle_timeout_min = max(1, min(1440, int(self.config.get("ap_idle_timeout_minutes", 15))))
+                except (TypeError, ValueError):
+                    idle_timeout_min = 15
                 elapsed = time.time() - self._ap_enabled_at
                 if elapsed > idle_timeout_min * 60 and not self._has_ap_clients():
                     logger.info(

test/test_wifi_manager_ap.py

@@ -129,7 +129,15 @@ def test_nmcli_ap_profile_has_no_security_params(manager: WiFiManager) -> None:
     assert "psk" not in add_str, "AP profile must not include a PSK/password"
     assert "wpa" not in add_str.lower(), "AP profile must not reference WPA"
     assert "802-11-wireless.mode" in add_str, "AP profile must declare wireless mode"
-    assert "ap" in add_calls[0], "Wireless mode value must be 'ap'"
+    # Verify the value for 802-11-wireless.mode is exactly "ap" — check the element
+    # that immediately follows the key in the command list, not a loose substring match.
+    cmd = add_calls[0]
+    try:
+        mode_idx = cmd.index("802-11-wireless.mode")
+        assert cmd[mode_idx + 1] == "ap", \
+            f"802-11-wireless.mode value must be exactly 'ap', got {cmd[mode_idx + 1]!r}"
+    except ValueError:
+        pytest.fail("802-11-wireless.mode not found as a list element in nmcli command")
 
 
 # ---------------------------------------------------------------------------
@@ -193,6 +201,8 @@ def test_iptables_rules_and_ip_forward_reverted_on_teardown(manager: WiFiManager
     """
     original_fwd = "0"
     manager._IP_FORWARD_SAVE_PATH.write_text(original_fwd)
+    # Teardown dispatches on the backend recorded during setup
+    manager._redirect_backend = "iptables"
 
     captured: list[list[str]] = []
 

web_interface/blueprints/api_v3.py

@@ -218,7 +218,7 @@ def _ensure_display_service_running():
     if status.get('active'):
         status['started'] = False
         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()
     result['started'] = result.get('returncode') == 0
     result['active'] = service_status.get('active')
@@ -227,7 +227,7 @@ def _ensure_display_service_running():
 
 def _stop_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()
     result['active'] = status.get('active')
     result['status'] = status
@@ -1716,33 +1716,34 @@ def execute_system_action():
             if mode:
                 # For on-demand modes, we would need to integrate with the display controller
                 # For now, just start the display service
-                result = subprocess.run(['sudo', 'systemctl', 'start', 'ledmatrix'],
-                                     capture_output=True, text=True)
+                result = subprocess.run(['sudo', 'systemctl', 'start', 'ledmatrix.service'],
+                                     capture_output=True, text=True, timeout=15)
                 return jsonify({
                     'status': 'success' if result.returncode == 0 else 'error',
-                    'message': f'Started display in {mode} mode',
+                    'message': f'Started display in {mode} mode' if result.returncode == 0
+                               else f'Failed to start display in {mode} mode: {result.stderr.strip() or "check sudo systemctl status ledmatrix.service"}',
                     'returncode': result.returncode,
                     'stdout': result.stdout,
                     'stderr': result.stderr
                 })
             else:
-                result = subprocess.run(['sudo', 'systemctl', 'start', 'ledmatrix'],
-                                     capture_output=True, text=True)
+                result = subprocess.run(['sudo', 'systemctl', 'start', 'ledmatrix.service'],
+                                     capture_output=True, text=True, timeout=15)
         elif action == 'stop_display':
-            result = subprocess.run(['sudo', 'systemctl', 'stop', 'ledmatrix'],
-                                 capture_output=True, text=True)
+            result = subprocess.run(['sudo', 'systemctl', 'stop', 'ledmatrix.service'],
+                                 capture_output=True, text=True, timeout=15)
         elif action == 'enable_autostart':
-            result = subprocess.run(['sudo', 'systemctl', 'enable', 'ledmatrix'],
-                                 capture_output=True, text=True)
+            result = subprocess.run(['sudo', 'systemctl', 'enable', 'ledmatrix.service'],
+                                 capture_output=True, text=True, timeout=15)
         elif action == 'disable_autostart':
-            result = subprocess.run(['sudo', 'systemctl', 'disable', 'ledmatrix'],
-                                 capture_output=True, text=True)
+            result = subprocess.run(['sudo', 'systemctl', 'disable', 'ledmatrix.service'],
+                                 capture_output=True, text=True, timeout=15)
         elif action == 'reboot_system':
             result = subprocess.run(['sudo', 'reboot'],
-                                 capture_output=True, text=True)
+                                 capture_output=True, text=True, timeout=10)
         elif action == 'shutdown_system':
             result = subprocess.run(['sudo', 'poweroff'],
-                                 capture_output=True, text=True)
+                                 capture_output=True, text=True, timeout=10)
         elif action == 'git_pull':
             # Use PROJECT_ROOT instead of hardcoded path
             project_dir = str(PROJECT_ROOT)
@@ -1823,12 +1824,11 @@ def execute_system_action():
                 'stderr': result.stderr
             })
         elif action == 'restart_display_service':
-            result = subprocess.run(['sudo', 'systemctl', 'restart', 'ledmatrix'],
-                                 capture_output=True, text=True)
+            result = subprocess.run(['sudo', 'systemctl', 'restart', 'ledmatrix.service'],
+                                 capture_output=True, text=True, timeout=15)
         elif action == 'restart_web_service':
-            # Try to restart the web service (assuming it's ledmatrix-web.service)
-            result = subprocess.run(['sudo', 'systemctl', 'restart', 'ledmatrix-web'],
-                                 capture_output=True, text=True)
+            result = subprocess.run(['sudo', 'systemctl', 'restart', 'ledmatrix-web.service'],
+                                 capture_output=True, text=True, timeout=15)
         else:
             return jsonify({'status': 'error', 'message': f'Unknown action: {action}'}), 400
 
@@ -7136,7 +7136,7 @@ def connect_wifi():
             # Propagate structured error type so the captive portal UI can show
             # "Wrong password — try again" instead of a generic failure message.
             error_type = "wrong_password" if (message or "").startswith("wrong_password:") else "connection_failed"
-            clean_message = (message or "").removeprefix("wrong_password: ") or "Failed to connect to network"
+            clean_message = (message or "").removeprefix("wrong_password:").lstrip() or "Failed to connect to network"
             return jsonify({
                 'status': 'error',
                 'message': clean_message,

web_interface/blueprints/pages_v3.py

@@ -1,6 +1,7 @@
 from flask import Blueprint, render_template, request, redirect, url_for, flash, jsonify
 import json
 import logging
+from html import escape as html_escape
 from pathlib import Path
 from src.web_interface.secret_helpers import mask_secret_fields
 
@@ -354,7 +355,7 @@ def _load_plugin_config_partial(plugin_id):
             plugin_info = pages_v3.plugin_manager.get_plugin_info(plugin_id)
         
         if not plugin_info:
-            return f'<div class="text-red-500 p-4">Plugin "{plugin_id}" not found</div>', 404
+            return f'<div class="text-red-500 p-4">Plugin "{html_escape(plugin_id)}" not found</div>', 404
         
         # Get plugin instance (may be None if not loaded)
         plugin_instance = pages_v3.plugin_manager.get_plugin(plugin_id)

web_interface/static/v3/js/widgets/day-selector.js

@@ -91,7 +91,7 @@
             const xOptions = config['x-options'] || config['x_options'] || {};
             const requestedFormat = xOptions.format || 'long';
             // Validate format exists in DAY_LABELS, default to 'long' if not
-            const format = DAY_LABELS.hasOwnProperty(requestedFormat) ? requestedFormat : 'long';
+            const format = Object.prototype.hasOwnProperty.call(DAY_LABELS, requestedFormat) ? requestedFormat : 'long';
             const layout = xOptions.layout || 'horizontal';
             const showSelectAll = xOptions.selectAll !== false;
 

web_interface/static/v3/plugins_manager.js

@@ -1227,6 +1227,8 @@ function initializePlugins() {
     // searchPluginStore renders Installed/Reinstall badges against it.
     loadInstalledPlugins().then(() => {
         searchPluginStore(!isReswapWarm);
+    }).catch(err => {
+        console.error('[PluginStore] loadInstalledPlugins failed:', err);
     });
 
     // Setup search functionality (with guard against duplicate listeners)