- 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>
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 <noreply@anthropic.com>
The counter reset after NM restart already fully prevents the SSH-lockout
cascade: _disconnected_checks can never accumulate across NM restarts
because it is reset to 0 before the next daemon iteration runs.
The 3→6 increase provided no additional fix for the described problem and
caused a UX regression: fresh Pi devices with no WiFi configured would
wait 3 minutes instead of 90 seconds for the LEDMatrix-Setup hotspot to
appear.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two user-reported issues after fresh install:
1. All service buttons (Start/Stop/Restart Display, Restart Web Service)
failed silently — only Reboot worked.
Root cause: sudoers rules use `ledmatrix.service` (with suffix) but
api_v3.py called `sudo systemctl start ledmatrix` (no suffix). sudo
does exact string matching, so every service action was rejected with
returncode=1. Also missing from sudoers: ledmatrix-web, journalctl,
and is-active entries.
Fix:
- Add `.service` suffix to all 8 sudo systemctl call sites in
api_v3.py (_ensure_display_service_running, _stop_display_service,
and all execute_system_action branches).
- Add timeout=15 to all subprocess.run calls in execute_system_action
(previously could hang indefinitely).
- Add missing sudoers rules to first_time_install.sh and
configure_web_sudo.sh: ledmatrix-web.service start/stop/restart,
is-active for both name forms, and journalctl -u/-t ledmatrix rules.
2. SSH and web UI became inaccessible after ~1 hour even though the
display kept running.
Root cause: wifi_monitor_daemon restarts NetworkManager after 5
consecutive internet failures (~2.5 min). Each NM restart drops WiFi
briefly. During that window check_and_manage_ap_mode() increments
_disconnected_checks but the daemon never reset it after the restart.
After 3 such NM-restart cycles, _disconnected_checks reached 3 and
AP mode activated — changing the Pi from WiFi client to hotspot
(192.168.4.1) and killing SSH on the old IP.
Fix:
- Reset wifi_manager._disconnected_checks = 0 in the daemon
immediately after a successful NM restart so the brief drop it
causes doesn't count toward AP-mode activation.
- Increase _disconnected_checks_required from 3 to 6 (90s → 3min)
as an additional buffer against transient network flaps.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pages_v3.py: plugin_id is taken directly from the URL path and was
interpolated into a returned HTML fragment without escaping. A crafted
URL like /partials/plugin-config/<script>alert(1)</script> would inject
arbitrary HTML into any page that loads this HTMX partial.
Fix: wrap with html.escape() from the stdlib.
march-madness/requirements.txt: Pillow>=9.1.0 is vulnerable to
CVE-2023-50447 (arbitrary code execution via the environment parameter).
Bump minimum to >=10.2.0 which contains the fix.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Direct .hasOwnProperty() calls on objects can be shadowed if the object
itself has a property named hasOwnProperty. Using Object.prototype.
hasOwnProperty.call(obj, key) is the safe, ESLint-compliant form.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
wifi_manager.py:
- Add public check_internet_connectivity() wrapping the private method so the
daemon does not reach into the private API
wifi_monitor_daemon.py:
- Call wifi_manager.check_internet_connectivity() instead of the private
_check_internet_connectivity()
- Use /usr/bin/systemctl (absolute path) instead of bare "systemctl"
- Wrap NM restart in try/except with check=True; only reset
_consecutive_internet_failures on success — on CalledProcessError or other
exception, log the error and leave the counter unchanged so the next cycle
retries
test/test_wifi_manager_ap.py:
- Replace loose `assert "ap" in add_calls[0]` (list-membership check that
could be satisfied by any element equal to "ap") with an explicit key/value
check: locate "802-11-wireless.mode" in the command list and assert the next
element is exactly "ap"
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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>
802-11-wireless-security.pmf is only valid within a security section that also
includes key-mgmt. Adding it to an open-network profile causes NM 1.52+ to
reject the connection add with 'key-mgmt: property is missing'. PMF has no
meaning for open APs (it only applies to WPA2/WPA3), so the setting is simply
removed rather than worked around.
Found by testing on devpi (Trixie, NM 1.52.1).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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>
Inspired by the production-proven approach in dirkhh/adsb-feeder-image.
1. DNS spoofing for automatic captive-portal popup (Change 1 — Critical)
Write /etc/NetworkManager/dnsmasq-shared.d/ledmatrix-captive.conf with
address=/#/192.168.4.1 before nmcli connection up so NM's built-in
dnsmasq (ipv4.method=shared) resolves every hostname to the AP IP.
This triggers the OS captive-portal popup automatically on iOS / Android /
Windows / macOS — no manual navigation to 192.168.4.1:5000/setup required.
New helpers: _write_nm_dnsmasq_captive_conf / _remove_nm_dnsmasq_captive_conf.
New constants: NM_DNSMASQ_SHARED_DIR / NM_DNSMASQ_SHARED_CONF.
2. Real internet connectivity check (Change 2 — High)
Add _check_internet_connectivity() (ping 8.8.8.8 + HTTP fallback).
check_and_manage_ap_mode() now considers a device "disconnected" when nmcli
shows connected but no real internet reachability, matching adsb-feeder's
multi-method gateway/DNS/HTTP test approach.
3. AP idle timeout (Change 3 — Medium)
Track _ap_enabled_at timestamp in enable_ap_mode(). Add _has_ap_clients()
using 'iw dev <iface> station dump'. check_and_manage_ap_mode() auto-disables
AP after ap_idle_timeout_minutes (default 15) with no associated clients.
4. Wrong-password error feedback (Change 4 — Medium)
_connect_nmcli() detects "Secrets were required" / "authentication rejected"
in nmcli stderr and prefixes the message with "wrong_password: ".
The /api/v3/wifi/connect route propagates error_type="wrong_password" in the
JSON response. captive_setup.html shows "Incorrect password — try again"
(keeping the form active) instead of the generic failure message.
5. Escalating watchdog NM restart (Change 5 — Low)
wifi_monitor_daemon.py tracks _consecutive_internet_failures. After
_nm_restart_threshold (5) consecutive checks where nmcli shows connected but
internet is unreachable, restart NetworkManager as a recovery step.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Six pytest unit tests covering the five review scenarios. All subprocess and
filesystem side-effects are mocked so the tests run without root, hardware, or
a Pi OS environment.
1. test_nmcli_ap_profile_has_no_security_params — asserts the nmcli connection
add command has no key-mgmt / psk / WPA arguments and sets mode=ap.
2. test_iptables_nat_rules_added_on_ap_start — verifies _setup_iptables_redirect
emits a PREROUTING REDIRECT 80→5000 rule and an INPUT ACCEPT rule for port
5000 (not 80, which never hits INPUT after PREROUTING rewrites it).
3. test_iptables_rules_and_ip_forward_reverted_on_teardown — verifies the -D
PREROUTING/-D INPUT calls and that sysctl restores the saved ip_forward value
and removes the save file.
4. test_ip_forward_not_restored_when_save_file_absent — verifies teardown skips
sysctl when the save file was never written, preventing blind ip_forward=0 on
systems using ip_forward for VPNs or NM shared mode.
5. test_led_message_shows_ssid_no_password_and_url — asserts the LED message
includes the SSID, 'No password', and the 192.168.4.1:5000 setup URL.
6. test_existing_ap_profiles_deleted_before_new_profile_created — asserts all
known profile names are targeted for deletion before 'nmcli connection add'.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Both AP startup paths (hostapd and nmcli) now check the bool returned by
_setup_iptables_redirect() and treat False as a hard failure: the hostapd
path stops hostapd/dnsmasq and returns an error tuple; the nmcli path brings
down and deletes the LEDMatrix-Setup-AP profile and clears the LED message
- _enable_ap_mode_hostapd's LED message now calls _validate_ap_config() to get
the same sanitized SSID that _create_hostapd_config() uses, so the displayed
name always matches the AP actually broadcast by hostapd
- _setup_iptables_redirect's outer except block now calls
_teardown_iptables_redirect() before returning False so partial iptables/
ip_forward state is always cleaned up on unexpected exceptions; cleanup
exceptions are caught and logged separately
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add _find_command_path() helper that extends _check_command()'s sbin-aware lookup to
return the absolute binary path rather than a boolean. Use it in
_setup_iptables_redirect and _teardown_iptables_redirect so iptables and sysctl are
resolved via /sbin or /usr/sbin even when those directories are absent from PATH in
systemd service environments.
Also harden the ip_forward save/restore logic:
- Read ip_forward from /proc/sys/net/ipv4/ip_forward (no subprocess, no PATH
dependency) instead of spawning sysctl -n
- Skip the sysctl -w ip_forward=1 write when the value is already "1" to avoid
mutating state owned by another service (VPN, NM shared mode, bridge)
- Track save success via presence of the save file: if the /proc read or file write
fails, leave the file absent so teardown knows not to restore
- In _teardown_iptables_redirect, only restore ip_forward when the save file exists;
if absent, leave the current value untouched rather than forcing "0"
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
wifi_manager.py:
- _create_hostapd_config: use _validate_ap_config() for ssid/channel instead
of raw self.config values; strip newlines from SSID to prevent config-file
injection via the generated hostapd.conf
- _setup_iptables_redirect: check return codes of sysctl ip_forward enable and
both iptables -A calls; on any failure log the error output, call
_teardown_iptables_redirect() to restore state, and return False instead of
silently succeeding
- _enable_ap_mode_nmcli_hotspot: on AP verification failure roll back fully —
tear down iptables redirect, delete the LEDMatrix-Setup-AP connection profile,
clear the LED message — before returning False
plugins_manager.js:
- initializePlugins: chain searchPluginStore(!isReswapWarm) inside
loadInstalledPlugins().then() so window.installedPlugins is populated before
the store renders Installed/Reinstall badges (same pattern applied to
refreshPlugins() in the previous commit)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
refreshPlugins() called searchPluginStore(true) and showNotification() immediately
after refreshInstalledPlugins() without awaiting the returned Promise, so
window.installedPlugins could still be stale when the store rendered its
Installed/Reinstall badges. Chain .then() so both run only after the fetch
completes.
In initializePlugins(), the re-swap path always passed fetchCommitInfo=false to
searchPluginStore, skipping GitHub metadata even when the 5-minute cache TTL had
expired. Add storeCacheExpired() helper and compute isReswapWarm = _reswap &&
!storeCacheExpired() so fresh metadata is fetched whenever the cache is cold,
regardless of whether the render is a first load or a tab re-swap.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Validate ap_ssid/ap_channel from config before passing to subprocess
(printable ASCII ≤32 chars; channel 1-14) to prevent command injection
- Fix INPUT iptables rule: PREROUTING redirects port 80→5000 so the INPUT
chain sees dport=5000, not 80. Old INPUT rule on port 80 was a no-op.
- Refactor iptables setup/teardown into _setup_iptables_redirect() and
_teardown_iptables_redirect() helpers, eliminating duplicate logic in
the hostapd and nmcli paths
- Save/restore ip_forward state (via /tmp/ledmatrix_ip_forward_saved)
instead of forcing it to 0 on cleanup, which could break VPNs or
bridges already relying on forwarding
- nmcli path skips ip_forward management entirely: NM's ipv4.method=shared
already manages it for the duration of the connection
- Fix _get_ap_status_nmcli() verification: new 'connection add type wifi'
profiles have type '802-11-wireless', not 'hotspot', so verification was
always returning False. Now also matches by our known connection name.
- Remove SSID-based connection deletion: deleting any profile whose SSID
matched the AP SSID could destroy a user's saved home WiFi profile.
Now only deletes by our application-managed profile names.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
nmcli device wifi hotspot always attaches a WPA2 PSK on Bookworm/Trixie
and silently ignores post-creation security modifications, causing users
to be prompted for an unknown password. Switch to nmcli connection add
with 802-11-wireless.mode ap and no security section — NM cannot auto-add
a password to a profile that has no 802-11-wireless-security block.
Also:
- Remove dead DEFAULT_AP_PASSWORD / ap_password config field (stored but
never passed to hostapd or nmcli, causing user confusion)
- Add iptables port 80→5000 redirect to the nmcli AP path so captive portal
auto-popup works on phones without hostapd (previously only worked on
the hostapd path)
- Clean up iptables rules on disable for the nmcli path
- Improve LED message on AP enable: show SSID, "No password", and IP:port
on both paths so users know exactly how to connect
- Fix systemd template: replace hardcoded /home/ledpi/LEDMatrix/ with
__PROJECT_ROOT_DIR__ placeholder (install script already writes correct path)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
