diff --git a/net_alerter/net_alerter.py b/net_alerter/net_alerter.py index a6d54c6..f879d20 100644 --- a/net_alerter/net_alerter.py +++ b/net_alerter/net_alerter.py @@ -13,6 +13,7 @@ Three concurrent threads: import json import logging import os +import re import socket import struct import sys @@ -365,9 +366,15 @@ def setup_logging(): return logger +# MAC address validation regex: exactly 12 hex digits (after normalization) +_MAC_RE = re.compile(r'^[0-9A-F]{12}$') + def _normalize_mac(mac: str) -> str: - """Normalize MAC address: remove colons/hyphens, uppercase.""" - return mac.replace(":", "").replace("-", "").upper() + """Normalize MAC address: remove colons/hyphens, uppercase. Returns empty string if invalid.""" + normalized = mac.replace(":", "").replace("-", "").upper() + if not _MAC_RE.match(normalized): + return "" + return normalized def load_personal_macs_from_env() -> None: @@ -380,6 +387,9 @@ def load_personal_macs_from_env() -> None: with personal_lock: for mac in macs: normalized = _normalize_mac(mac) + if not normalized: # Skip invalid MACs + logging.warning(f"Skipping invalid MAC from NET_ALERTER_PERSONAL_MACS: {mac}") + continue personal_devices.add(normalized) logging.info(f"Added personal device MAC: {normalized}") @@ -425,6 +435,9 @@ def is_personal_device(mac: str) -> bool: """ # Normalize MAC mac_normalized = _normalize_mac(mac) + if not mac_normalized: # Skip invalid MACs + return False + oui = mac_normalized[:6] # Check personal_devices set first (manually configured) @@ -629,46 +642,52 @@ def on_arrival(mac: str, ip: str, hostname: str = "") -> None: now = time.time() # Lock order: known_lock, then departure_lock, then occupancy_lock + # Extract timers to cancel OUTSIDE all locks to avoid deadlock + timers_to_cancel = [] + + with departure_lock: + if mac in departure_timers: + timers_to_cancel.append(departure_timers.pop(mac)) + + # Cancel all timers BEFORE acquiring any other locks + for timer in timers_to_cancel: + timer.cancel() + with known_lock: with departure_lock: # Check if device marked departing (rapid flap recovery) if mac in known_devices and known_devices[mac].get('departing'): # Device returned during departure window — cancel timer, clear flag, no alert - if mac in departure_timers: - departure_timers[mac].cancel() - del departure_timers[mac] known_devices[mac]['departing'] = False with occupancy_lock: _update_occupancy_state_unlocked() last_departed_time.pop(mac, None) logging.debug(f"Device {mac} re-arrived during departure window — suppressed alert") return - - # ALWAYS cancel any pending departure timer when device re-arrives - if mac in departure_timers: - departure_timers[mac].cancel() - del departure_timers[mac] + + # Log if we cancelled a timer + if timers_to_cancel: logging.debug(f"Cancelled departure timer for {mac} — device re-arrived before debounce expired") - if mac in last_departed_time: - time_since_departure = now - last_departed_time[mac] - if time_since_departure < 300: # 5 min - logging.debug(f"Suppressing re-arrival alert for {mac} (departed {int(time_since_departure)}s ago)") - # Re-add to known_devices to track presence - if mac not in known_devices: - vendor = lookup_oui(mac) - known_devices[mac] = { - 'ip': ip, - 'hostname': hostname, - 'vendor': vendor, - 'first_seen': now, - 'last_seen': now - } - else: - known_devices[mac]['last_seen'] = now - with occupancy_lock: - _update_occupancy_state_unlocked() - return + if mac in last_departed_time: + time_since_departure = now - last_departed_time[mac] + if time_since_departure < 300: # 5 min + logging.debug(f"Suppressing re-arrival alert for {mac} (departed {int(time_since_departure)}s ago)") + # Re-add to known_devices to track presence + if mac not in known_devices: + vendor = lookup_oui(mac) + known_devices[mac] = { + 'ip': ip, + 'hostname': hostname, + 'vendor': vendor, + 'first_seen': now, + 'last_seen': now + } + else: + known_devices[mac]['last_seen'] = now + with occupancy_lock: + _update_occupancy_state_unlocked() + return # Still inside known_lock, but not inside departure_lock now if mac in known_devices: @@ -732,11 +751,18 @@ def on_departure(mac: str) -> None: known_devices[mac]['departing'] = True # Start 15-minute debounce timer + # Extract timer to cancel outside of lock to avoid deadlock + timer_to_cancel = None with departure_lock: # Cancel any existing timer for this MAC if mac in departure_timers: - departure_timers[mac].cancel() - + timer_to_cancel = departure_timers.pop(mac) + + # Cancel outside lock + if timer_to_cancel: + timer_to_cancel.cancel() + + with departure_lock: def send_departure_alert(): """Callback to send alert after debounce expires.""" # Lock order: known_lock, then departure_lock, then occupancy_lock