fix(wifi): restore safe AP-enable trigger; decouple internet check from AP logic

The previous commit introduced _check_internet_connectivity() into
check_and_manage_ap_mode(), which shared the same _disconnected_checks counter
that triggers AP enable. This created a false-positive risk: 90 seconds of
packet loss on working WiFi would enable AP mode and kick off the connection.

Fix: restore nmcli association state as the sole AP-enable trigger (original,
safe behaviour). The internet connectivity check is now used only in the daemon
watchdog for the NM-restart escalation — matching how adsb-feeder-image actually
structures the two concerns (initial setup detection vs. ongoing monitoring).

Also clarify daemon comment: the connectivity check runs once per cycle in the
watchdog block, not inside check_and_manage_ap_mode, so there is no double-call.

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

README.md

@@ -1,5 +1,4 @@
 # LEDMatrix
-[![Codacy Badge](https://app.codacy.com/project/badge/Grade/77fc9b446a5948e5b0aed7a7aaeb1bab)](https://app.codacy.com/gh/ChuckBuilds/LEDMatrix/dashboard?utm_source=gh&utm_medium=referral&utm_content=&utm_campaign=Badge_grade)
 ## Welcome to LEDMatrix! 
 Welcome to the LEDMatrix Project! This open-source project enables you to run an information-rich display on a Raspberry Pi connected to an LED RGB Matrix panel. Whether you want to see your calendar, weather forecasts, sports scores, stock prices, or any other information at a glance, LEDMatrix brings it all together.
 

first_time_install.sh

@@ -1086,7 +1086,6 @@ 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
@@ -1102,23 +1101,10 @@ $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
-$ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/scripts/fix_perms/safe_plugin_rm.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>=12.2.0
+Pillow>=9.1.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.service"
+    echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix-web"
-    echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix-web.service"
+    echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix-web"
-    echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix-web.service"
+    echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix-web"
 
     # Optional: journalctl (non-critical — skip if not found)
     if [ -n "$JOURNALCTL_PATH" ]; then

scripts/utils/wifi_monitor_daemon.py

@@ -10,7 +10,6 @@ import sys
 import time
 import logging
 import signal
-import subprocess
 from pathlib import Path
 
 # Add project root to path (parent of scripts/utils/)
@@ -133,7 +132,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 "
@@ -141,23 +140,9 @@ class WiFiMonitorDaemon:
                         )
                         if self._consecutive_internet_failures >= self._nm_restart_threshold:
                             logger.warning("Restarting NetworkManager to recover internet connectivity")
-                            try:
+                            import subprocess as _sp
-                                subprocess.run(
+                            _sp.run(["sudo", "systemctl", "restart", "NetworkManager"],
-                                    ["/usr/bin/systemctl", "restart", "NetworkManager"],
+                                    capture_output=True, timeout=20)
-                                    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 (subprocess.SubprocessError, OSError) 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

src/wifi_manager.py

@@ -144,8 +144,6 @@ 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}, "
@@ -693,8 +691,9 @@ 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:
@@ -708,133 +707,113 @@ class WiFiManager:
 
     def _setup_iptables_redirect(self) -> bool:
         """
-        Add port 80 → 5000 redirect rules for the captive portal.
+        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.
 
-        Tries iptables first, falls back to nftables (used by Debian Trixie).
+        Uses _find_command_path() so binaries in /sbin or /usr/sbin are resolved even
-        When neither tool is available, logs a warning and returns True — the AP
+        when those directories are absent from PATH in service environments.
-        still works and DNS spoofing still triggers the OS popup; users just land
-        on port 5000 directly rather than being redirected from port 80.
 
-        Only returns False when a tool was found but the rule addition itself failed.
+        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.
         """
         try:
             iptables = self._find_command_path("iptables")
-            nft = self._find_command_path("nft")
+            if not iptables:
-
+                logger.debug("iptables unavailable; captive portal requires direct port-5000 access")
-            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."
-                )
-                self._redirect_backend = None
-                return True  # AP works; redirect is best-effort
-
-            if iptables:
-                return self._setup_iptables_redirect_iptables(iptables)
-            else:
-                return self._setup_iptables_redirect_nftables(nft)
-
-        except Exception as 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 redirect exception also failed: {cleanup_e}")
                 return False
 
-    def _setup_iptables_redirect_iptables(self, iptables: str) -> bool:
+            # Read ip_forward from /proc — reliable with no subprocess or PATH dependency.
-        """Set up port 80→5000 redirect using iptables."""
-        # Save ip_forward state before enabling
             try:
                 current_fwd = Path("/proc/sys/net/ipv4/ip_forward").read_text().strip()
             except OSError:
-            current_fwd = None
+                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
+                    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"
-            r = subprocess.run(["sudo", sysctl_bin, "-w", "net.ipv4.ip_forward=1"],
+                sysctl_r = subprocess.run(
-                               capture_output=True, text=True, timeout=5)
+                    ["sudo", sysctl_bin, "-w", "net.ipv4.ip_forward=1"],
-            if r.returncode != 0:
+                    capture_output=True, text=True, timeout=5
-                logger.error(f"Failed to enable ip_forward: {r.stderr.strip()}")
+                )
+                if sysctl_r.returncode != 0:
+                    logger.error(f"Failed to enable ip_forward: {sysctl_r.stderr.strip()}")
                     self._teardown_iptables_redirect()
                     return False
 
+            # 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:
-            r = subprocess.run(
+                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 r.returncode != 0:
+                if add_r.returncode != 0:
-                logger.error(f"Failed to add PREROUTING rule: {r.stderr.strip()}")
+                    logger.error(f"Failed to add PREROUTING rule: {add_r.stderr.strip()}")
                     self._teardown_iptables_redirect()
                     return False
 
+            # 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"],
+                 "-i", self._wifi_interface, "-p", "tcp", "--dport", "5000",
+                 "-j", "ACCEPT"],
                 capture_output=True, timeout=5
             ).returncode != 0:
-            r = subprocess.run(
+                add_r = subprocess.run(
                     ["sudo", iptables, "-A", "INPUT",
-                 "-i", self._wifi_interface, "-p", "tcp", "--dport", "5000", "-j", "ACCEPT"],
+                     "-i", self._wifi_interface, "-p", "tcp", "--dport", "5000",
+                     "-j", "ACCEPT"],
                     capture_output=True, text=True, timeout=5
                 )
-            if r.returncode != 0:
+                if add_r.returncode != 0:
-                logger.error(f"Failed to add INPUT rule: {r.stderr.strip()}")
+                    logger.error(f"Failed to add INPUT rule: {add_r.stderr.strip()}")
                     self._teardown_iptables_redirect()
                     return False
 
-        self._redirect_backend = "iptables"
+            logger.info("iptables: port 80→5000 redirect and INPUT accept-5000 rules added")
-        logger.info("iptables: port 80→5000 redirect rules added")
             return True
-
+        except Exception as e:
-    def _setup_iptables_redirect_nftables(self, nft: str) -> bool:
+            logger.warning(f"Could not set up iptables redirect: {e}")
-        """Set up port 80→5000 redirect using nftables (Debian Trixie / modern systems)."""
+            try:
-        # 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()
+            except Exception as cleanup_e:
+                logger.warning(f"Cleanup after iptables redirect exception also failed: {cleanup_e}")
             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."""
+        """Remove the port 80→5000 iptables rules and restore the saved ip_forward state.
-        try:
-            backend = self._redirect_backend
-            self._redirect_backend = None
 
-            if backend == "iptables":
+        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.
+        """
+        try:
             iptables = self._find_command_path("iptables")
-                if iptables:
+            if not iptables:
+                return
+
             subprocess.run(
                 ["sudo", iptables, "-t", "nat", "-D", "PREROUTING",
                  "-i", self._wifi_interface, "-p", "tcp", "--dport", "80",
@@ -847,7 +826,9 @@ class WiFiManager:
                  "-j", "ACCEPT"],
                 capture_output=True, timeout=5
             )
-                # Restore ip_forward only when we saved it
+
+            # 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()
@@ -856,27 +837,13 @@ class WiFiManager:
                     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}")
+                    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.debug("ip_forward not modified by setup; leaving unchanged")
+                logger.info("iptables redirect rules removed; ip_forward left unchanged (not modified by setup)")
-
-            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")
-
-            else:
-                # 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 port redirect: {e}")
+            logger.warning(f"Could not tear down iptables redirect: {e}")
 
     def _write_nm_dnsmasq_captive_conf(self, ap_ip: str = "192.168.4.1") -> None:
         """
@@ -933,22 +900,18 @@ class WiFiManager:
             if r.returncode == 0:
                 logger.debug("Internet connectivity confirmed via ping 8.8.8.8")
                 return True
-        except (subprocess.SubprocessError, OSError):
+        except Exception:
             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 OSError:
+        except Exception:
             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.
@@ -2044,10 +2007,10 @@ class WiFiManager:
                 # No 802-11-wireless-security section → open network
             ]
 
-            # PMF (Protected Management Frames) is only meaningful for WPA2/WPA3.
+            # On Trixie disable PMF which can prevent older clients from connecting
-            # An open AP has no security section, so adding 802-11-wireless-security.pmf
+            if self._is_trixie:
-            # would cause NM to require key-mgmt too, breaking the connection add on
+                cmd += ["802-11-wireless-security.pmf", "disable"]
-            # Trixie NM 1.52+. Leave PMF untouched — open APs have no frame protection.
+                logger.info("Trixie detected: disabling PMF for better client compatibility")
 
             result = subprocess.run(cmd, capture_output=True, text=True, timeout=30)
 
@@ -2071,7 +2034,6 @@ 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)
@@ -2083,7 +2045,6 @@ 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"],
@@ -2101,7 +2062,6 @@ 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"],
@@ -2111,7 +2071,6 @@ 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)
     
@@ -2499,10 +2458,7 @@ 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:
-                try:
+                idle_timeout_min = self.config.get("ap_idle_timeout_minutes", 15)
-                    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,15 +129,7 @@ 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"
-    # Verify the value for 802-11-wireless.mode is exactly "ap" — check the element
+    assert "ap" in add_calls[0], "Wireless mode value must be 'ap'"
-    # 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")
 
 
 # ---------------------------------------------------------------------------
@@ -201,8 +193,6 @@ 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

@@ -2,7 +2,6 @@ from flask import Blueprint, request, jsonify, Response, send_from_directory
 import json
 import os
 import re
-import shutil
 import socket
 import sys
 import subprocess
@@ -17,11 +16,6 @@ from typing import Optional, Tuple, Dict, Any, Type
 
 logger = logging.getLogger(__name__)
 
-SUDO_BIN = shutil.which("sudo") or "/usr/bin/sudo"
-SYSTEMCTL_BIN = shutil.which("systemctl") or "/usr/bin/systemctl"
-REBOOT_BIN = shutil.which("reboot") or "/usr/sbin/reboot"
-POWEROFF_BIN = shutil.which("poweroff") or "/usr/sbin/poweroff"
-
 # Import new infrastructure
 from src.web_interface.api_helpers import success_response, error_response, validate_request_json
 from src.web_interface.errors import ErrorCode
@@ -224,7 +218,7 @@ def _ensure_display_service_running():
     if status.get('active'):
         status['started'] = False
         return status
-    result = _run_systemctl_command([SUDO_BIN, SYSTEMCTL_BIN, 'start', 'ledmatrix.service'])
+    result = _run_systemctl_command(['sudo', 'systemctl', 'start', 'ledmatrix'])
     service_status = _get_display_service_status()
     result['started'] = result.get('returncode') == 0
     result['active'] = service_status.get('active')
@@ -233,7 +227,7 @@ def _ensure_display_service_running():
 
 def _stop_display_service():
     """Stop the ledmatrix display service."""
-    result = _run_systemctl_command([SUDO_BIN, SYSTEMCTL_BIN, 'stop', 'ledmatrix.service'])
+    result = _run_systemctl_command(['sudo', 'systemctl', 'stop', 'ledmatrix'])
     status = _get_display_service_status()
     result['active'] = status.get('active')
     result['status'] = status
@@ -1722,34 +1716,33 @@ 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_BIN, SYSTEMCTL_BIN, 'start', 'ledmatrix.service'],
+                result = subprocess.run(['sudo', 'systemctl', 'start', 'ledmatrix'],
-                                     capture_output=True, text=True, timeout=15)
+                                     capture_output=True, text=True)
                 return jsonify({
                     'status': 'success' if result.returncode == 0 else 'error',
-                    'message': f'Started display in {mode} mode' if result.returncode == 0
+                    'message': f'Started display in {mode} mode',
-                               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_BIN, SYSTEMCTL_BIN, 'start', 'ledmatrix.service'],
+                result = subprocess.run(['sudo', 'systemctl', 'start', 'ledmatrix'],
-                                     capture_output=True, text=True, timeout=15)
+                                     capture_output=True, text=True)
         elif action == 'stop_display':
-            result = subprocess.run([SUDO_BIN, SYSTEMCTL_BIN, 'stop', 'ledmatrix.service'],
+            result = subprocess.run(['sudo', 'systemctl', 'stop', 'ledmatrix'],
-                                 capture_output=True, text=True, timeout=15)
+                                 capture_output=True, text=True)
         elif action == 'enable_autostart':
-            result = subprocess.run([SUDO_BIN, SYSTEMCTL_BIN, 'enable', 'ledmatrix.service'],
+            result = subprocess.run(['sudo', 'systemctl', 'enable', 'ledmatrix'],
-                                 capture_output=True, text=True, timeout=15)
+                                 capture_output=True, text=True)
         elif action == 'disable_autostart':
-            result = subprocess.run([SUDO_BIN, SYSTEMCTL_BIN, 'disable', 'ledmatrix.service'],
+            result = subprocess.run(['sudo', 'systemctl', 'disable', 'ledmatrix'],
-                                 capture_output=True, text=True, timeout=15)
+                                 capture_output=True, text=True)
         elif action == 'reboot_system':
-            result = subprocess.run([SUDO_BIN, REBOOT_BIN],
+            result = subprocess.run(['sudo', 'reboot'],
-                                 capture_output=True, text=True, timeout=10)
+                                 capture_output=True, text=True)
         elif action == 'shutdown_system':
-            result = subprocess.run([SUDO_BIN, POWEROFF_BIN],
+            result = subprocess.run(['sudo', 'poweroff'],
-                                 capture_output=True, text=True, timeout=10)
+                                 capture_output=True, text=True)
         elif action == 'git_pull':
             # Use PROJECT_ROOT instead of hardcoded path
             project_dir = str(PROJECT_ROOT)
@@ -1830,11 +1823,12 @@ def execute_system_action():
                 'stderr': result.stderr
             })
         elif action == 'restart_display_service':
-            result = subprocess.run([SUDO_BIN, SYSTEMCTL_BIN, 'restart', 'ledmatrix.service'],
+            result = subprocess.run(['sudo', 'systemctl', 'restart', 'ledmatrix'],
-                                 capture_output=True, text=True, timeout=15)
+                                 capture_output=True, text=True)
         elif action == 'restart_web_service':
-            result = subprocess.run([SUDO_BIN, SYSTEMCTL_BIN, 'restart', 'ledmatrix-web.service'],
+            # Try to restart the web service (assuming it's ledmatrix-web.service)
-                                 capture_output=True, text=True, timeout=15)
+            result = subprocess.run(['sudo', 'systemctl', 'restart', 'ledmatrix-web'],
+                                 capture_output=True, text=True)
         else:
             return jsonify({'status': 'error', 'message': f'Unknown action: {action}'}), 400
 
@@ -1846,13 +1840,6 @@ def execute_system_action():
             'stderr': result.stderr
         })
 
-    except subprocess.TimeoutExpired:
-        if action == 'start_display' and mode:
-            msg = f'Failed to start display in {mode} mode: timed out'
-        else:
-            msg = f'Action {action} timed out'
-        logger.warning("[System] execute_system_action timed out: action=%s", action)
-        return jsonify({'status': 'error', 'message': msg, 'returncode': -1, 'stdout': '', 'stderr': 'timeout'}), 500
     except Exception as e:
         logger.exception("[System] execute_system_action failed")
         return jsonify({'status': 'error', 'message': 'Failed to execute system action'}), 500
@@ -7149,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:").lstrip() or "Failed to connect to network"
+            clean_message = (message or "").removeprefix("wrong_password: ") or "Failed to connect to network"
             return jsonify({
                 'status': 'error',
                 'message': clean_message,

web_interface/static/v3/plugins_manager.js

@@ -1225,9 +1225,7 @@ function initializePlugins() {
     window.pluginManager._reswap = false;
     // Await the installed-plugins fetch so window.installedPlugins is populated before
     // searchPluginStore renders Installed/Reinstall badges against it.
-    loadInstalledPlugins().catch(err => {
+    loadInstalledPlugins().then(() => {
-        console.error('[PluginStore] loadInstalledPlugins failed:', err);
-    }).finally(() => {
         searchPluginStore(!isReswapWarm);
     });
 
@@ -4853,9 +4851,9 @@ window.executePluginAction = function(actionId, actionIndex, pluginIdParam = nul
                     showNotification(data.message || 'Action completed successfully!', 'success');
                 }
             } else {
-                statusDiv.innerHTML = `<div class="text-red-600"><i class="fas fa-exclamation-circle mr-2"></i>${escapeHtml(data.message || 'Error')}</div>`;
+                statusDiv.innerHTML = `<div class="text-red-600"><i class="fas fa-exclamation-circle mr-2"></i>${data.message}</div>`;
                 if (data.output) {
-                    statusDiv.innerHTML += `<pre class="mt-2 text-xs bg-red-50 p-2 rounded overflow-auto max-h-32">${escapeHtml(data.output)}</pre>`;
+                    statusDiv.innerHTML += `<pre class="mt-2 text-xs bg-red-50 p-2 rounded overflow-auto max-h-32">${data.output}</pre>`;
                 }
                 btn.innerHTML = originalText;
                 btn.disabled = false;
@@ -4899,8 +4897,8 @@ window.executePluginAction = function(actionId, actionIndex, pluginIdParam = nul
                         </div>
                         <div class="mb-3">
                             <p class="text-sm text-blue-700 mb-2">1. Click the link below to authorize:</p>
-                            <a href="${data.auth_url && data.auth_url.startsWith('http') ? escapeHtml(data.auth_url) : '#'}" target="_blank" class="text-blue-600 hover:text-blue-800 underline break-all">
+                            <a href="${data.auth_url}" target="_blank" class="text-blue-600 hover:text-blue-800 underline break-all">
-                                ${escapeHtml(data.auth_url || '')}
+                                ${data.auth_url}
                             </a>
                         </div>
                         <div class="mb-2">
@@ -4922,7 +4920,7 @@ window.executePluginAction = function(actionId, actionIndex, pluginIdParam = nul
                         <div class="text-green-900 font-medium mb-2">
                             <i class="fas fa-check-circle mr-2"></i>${data.message || 'Action completed successfully'}
                         </div>
-                        ${data.output ? `<pre class="mt-2 text-xs bg-green-50 p-2 rounded overflow-auto max-h-32">${escapeHtml(data.output)}</pre>` : ''}
+                        ${data.output ? `<pre class="mt-2 text-xs bg-green-50 p-2 rounded overflow-auto max-h-32">${data.output}</pre>` : ''}
                     </div>
                 `;
                 btn.innerHTML = originalText;
@@ -4935,9 +4933,9 @@ window.executePluginAction = function(actionId, actionIndex, pluginIdParam = nul
             statusDiv.innerHTML = `
                 <div class="bg-red-50 border border-red-200 rounded p-3">
                     <div class="text-red-900 font-medium mb-2">
-                        <i class="fas fa-exclamation-circle mr-2"></i>${escapeHtml(data.message || 'Action failed')}
+                        <i class="fas fa-exclamation-circle mr-2"></i>${data.message || 'Action failed'}
                     </div>
-                    ${data.output ? `<pre class="mt-2 text-xs bg-red-50 p-2 rounded overflow-auto max-h-32">${escapeHtml(data.output)}</pre>` : ''}
+                    ${data.output ? `<pre class="mt-2 text-xs bg-red-50 p-2 rounded overflow-auto max-h-32">${data.output}</pre>` : ''}
                 </div>
             `;
             btn.innerHTML = originalText;
@@ -8067,3 +8065,4 @@ setTimeout(function() {
         }
     });
 })();
+