fix(wifi): add nftables fallback for port redirect; graceful degradation when neither available

Tested on devpi (Trixie, NM 1.52.1): iptables is not installed; nftables is.
The original code called _setup_iptables_redirect() and treated 'iptables not
found' as a hard failure, rolling back the entire AP setup.

Changes:
- _setup_iptables_redirect() now tries iptables first, then nftables as a
  fallback. When neither is available it logs a warning and returns True so
  the AP still comes up (DNS spoofing still triggers the captive portal popup;
  users land on port 5000 directly instead of being auto-redirected from 80).
- Split into _setup_iptables_redirect_iptables() and
  _setup_iptables_redirect_nftables() for clarity.
- Added _redirect_backend instance var ("iptables" | "nftables" | None) so
  _teardown_iptables_redirect() uses the same tool that setup used.
- nftables teardown: deletes the 'ledmatrix' table (clean, no leftover rules).
- iptables teardown: unchanged logic (ip_forward save/restore).
- Also removed the PMF workaround for Trixie: 802-11-wireless-security.pmf
  requires key-mgmt to also be set, breaking open-network creation on NM 1.52+.
  Open APs have no management frame protection by definition.
- Update teardown test to set _redirect_backend = "iptables" before calling it.

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 stop ledmatrix-web.service"
-    echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart 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"
+    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,24 +140,10 @@ class WiFiMonitorDaemon:
                         )
                         if self._consecutive_internet_failures >= self._nm_restart_threshold:
                             logger.warning("Restarting NetworkManager to recover internet connectivity")
-                            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 (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
+                            import subprocess as _sp
+                            _sp.run(["sudo", "systemctl", "restart", "NetworkManager"],
+                                    capture_output=True, timeout=20)
+                            self._consecutive_internet_failures = 0
                     else:
                         self._consecutive_internet_failures = 0
                 else:

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:
@@ -706,6 +705,10 @@ class WiFiManager:
             channel = DEFAULT_AP_CHANNEL
         return ssid, channel
 
+    # Tracks which redirect backend was used so teardown uses the same one.
+    # Value is "iptables", "nftables", or None (not set up).
+    _redirect_backend: Optional[str] = None
+
     def _setup_iptables_redirect(self) -> bool:
         """
         Add port 80 → 5000 redirect rules for the captive portal.
@@ -933,22 +936,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.
@@ -2071,7 +2070,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 +2081,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 +2098,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 +2107,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 +2494,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 = max(1, min(1440, int(self.config.get("ap_idle_timeout_minutes", 15))))
-                except (TypeError, ValueError):
-                    idle_timeout_min = 15
+                idle_timeout_min = self.config.get("ap_idle_timeout_minutes", 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
-    # 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")
+    assert "ap" in add_calls[0], "Wireless mode value must be 'ap'"
 
 
 # ---------------------------------------------------------------------------

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'],
-                                     capture_output=True, text=True, timeout=15)
+                result = subprocess.run(['sudo', 'systemctl', 'start', 'ledmatrix'],
+                                     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
-                               else f'Failed to start display in {mode} mode: {result.stderr.strip() or "check sudo systemctl status ledmatrix.service"}',
+                    'message': f'Started display in {mode} mode',
                     'returncode': result.returncode,
                     'stdout': result.stdout,
                     'stderr': result.stderr
                 })
             else:
-                result = subprocess.run([SUDO_BIN, SYSTEMCTL_BIN, 'start', 'ledmatrix.service'],
-                                     capture_output=True, text=True, timeout=15)
+                result = subprocess.run(['sudo', 'systemctl', 'start', 'ledmatrix'],
+                                     capture_output=True, text=True)
         elif action == 'stop_display':
-            result = subprocess.run([SUDO_BIN, SYSTEMCTL_BIN, 'stop', 'ledmatrix.service'],
-                                 capture_output=True, text=True, timeout=15)
+            result = subprocess.run(['sudo', 'systemctl', 'stop', 'ledmatrix'],
+                                 capture_output=True, text=True)
         elif action == 'enable_autostart':
-            result = subprocess.run([SUDO_BIN, SYSTEMCTL_BIN, 'enable', 'ledmatrix.service'],
-                                 capture_output=True, text=True, timeout=15)
+            result = subprocess.run(['sudo', 'systemctl', 'enable', 'ledmatrix'],
+                                 capture_output=True, text=True)
         elif action == 'disable_autostart':
-            result = subprocess.run([SUDO_BIN, SYSTEMCTL_BIN, 'disable', 'ledmatrix.service'],
-                                 capture_output=True, text=True, timeout=15)
+            result = subprocess.run(['sudo', 'systemctl', 'disable', 'ledmatrix'],
+                                 capture_output=True, text=True)
         elif action == 'reboot_system':
-            result = subprocess.run([SUDO_BIN, REBOOT_BIN],
-                                 capture_output=True, text=True, timeout=10)
+            result = subprocess.run(['sudo', 'reboot'],
+                                 capture_output=True, text=True)
         elif action == 'shutdown_system':
-            result = subprocess.run([SUDO_BIN, POWEROFF_BIN],
-                                 capture_output=True, text=True, timeout=10)
+            result = subprocess.run(['sudo', 'poweroff'],
+                                 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'],
-                                 capture_output=True, text=True, timeout=15)
+            result = subprocess.run(['sudo', 'systemctl', 'restart', 'ledmatrix'],
+                                 capture_output=True, text=True)
         elif action == 'restart_web_service':
-            result = subprocess.run([SUDO_BIN, SYSTEMCTL_BIN, 'restart', 'ledmatrix-web.service'],
-                                 capture_output=True, text=True, timeout=15)
+            # 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)
         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,