From 5b6137f5f46f1ef55026bd8fa4dc0c7fba144ea9 Mon Sep 17 00:00:00 2001 From: Chuck Date: Tue, 12 May 2026 12:27:41 -0400 Subject: [PATCH] fix: address five valid review findings; skip two MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed: - march-madness/requirements.txt: Pillow>=10.3.0 (patches CVE-2024-28219; 10.3.0 is the actual fix version — reviewer cited 12.2.0 but that risks breaking API changes without test coverage) - wifi_monitor_daemon.py: add missing `import subprocess`; subprocess.run and CalledProcessError would NameError at runtime on the NM restart path - wifi_manager.py: validate ap_idle_timeout_minutes before arithmetic — coerce to int, clamp 1–1440, fall back to 15 on bad config values - wifi_manager.py: call _remove_nm_dnsmasq_captive_conf() on all three rollback paths in _enable_ap_mode_nmcli_hotspot() and in the top-level except block so stale dnsmasq drop-ins are never left behind - api_v3.py: fix wrong_password prefix strip — removeprefix("wrong_password:") then lstrip() handles both "wrong_password: msg" and "wrong_password:msg" - plugins_manager.js: add .catch() to loadInstalledPlugins().then() to surface failures instead of silently dropping unhandled rejections Skipped: - WiFiManager AP state persistence: architectural overhaul; _is_ap_mode_active() already derives from live system state, not in-memory variables - Absolute subprocess paths in api_v3.py: paths vary by distro (/usr/bin vs /bin); web service has a normal PATH; sudoers already use resolved paths Co-Authored-By: Claude Sonnet 4.6 --- plugin-repos/march-madness/requirements.txt | 2 +- scripts/utils/wifi_monitor_daemon.py | 1 + src/wifi_manager.py | 9 ++++++++- web_interface/blueprints/api_v3.py | 2 +- web_interface/static/v3/plugins_manager.js | 2 ++ 5 files changed, 13 insertions(+), 3 deletions(-) diff --git a/plugin-repos/march-madness/requirements.txt b/plugin-repos/march-madness/requirements.txt index fd07d277..ba79ba59 100644 --- a/plugin-repos/march-madness/requirements.txt +++ b/plugin-repos/march-madness/requirements.txt @@ -1,4 +1,4 @@ requests>=2.28.0 -Pillow>=10.2.0 +Pillow>=10.3.0 pytz>=2022.1 numpy>=1.24.0 diff --git a/scripts/utils/wifi_monitor_daemon.py b/scripts/utils/wifi_monitor_daemon.py index 8d83cf40..3afd212d 100755 --- a/scripts/utils/wifi_monitor_daemon.py +++ b/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/) diff --git a/src/wifi_manager.py b/src/wifi_manager.py index e31bd5fb..bbb7a543 100644 --- a/src/wifi_manager.py +++ b/src/wifi_manager.py @@ -2074,6 +2074,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) @@ -2085,6 +2086,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"], @@ -2102,6 +2104,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"], @@ -2111,6 +2114,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) @@ -2498,7 +2502,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( diff --git a/web_interface/blueprints/api_v3.py b/web_interface/blueprints/api_v3.py index c8c2fa44..dca2f33a 100644 --- a/web_interface/blueprints/api_v3.py +++ b/web_interface/blueprints/api_v3.py @@ -7135,7 +7135,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, diff --git a/web_interface/static/v3/plugins_manager.js b/web_interface/static/v3/plugins_manager.js index b946100a..f87c0fe2 100644 --- a/web_interface/static/v3/plugins_manager.js +++ b/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)