From 67cf907f74361e5f00f55762f5a5d5350d5589c5 Mon Sep 17 00:00:00 2001 From: Cobra Date: Thu, 9 Apr 2026 13:28:34 -0400 Subject: [PATCH] Fix on_arrival() to always cancel departure timer on device re-arrival MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Bug: device departs → timer starts, but last_departed_time not set yet If device re-arrives before timer fires, timer never cancelled Result: false DEPARTED alert sent after re-arrival debounce expires - Fix: Cancel any pending departure timer immediately on re-arrival, before checking last_departed_time - Test: Added regression test that verifies timer is cancelled when device re-arrives before 15-min debounce expires, preventing false alerts - All 10 tests passing --- net_alerter/net_alerter.py | 10 +++--- net_alerter/test_net_alerter.py | 64 +++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/net_alerter/net_alerter.py b/net_alerter/net_alerter.py index 6723265..c328fab 100644 --- a/net_alerter/net_alerter.py +++ b/net_alerter/net_alerter.py @@ -423,14 +423,16 @@ def on_arrival(mac: str, ip: str, hostname: str = "") -> None: # Check re-arrival suppression BEFORE acquiring lock (avoid nested locks) with departure_lock: + # ALWAYS cancel any pending departure timer when device re-arrives + if mac in departure_timers: + departure_timers[mac].cancel() + del departure_timers[mac] + 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)") - # Clean up departure timer if still pending - if mac in departure_timers: - departure_timers[mac].cancel() - del departure_timers[mac] # Re-add to known_devices to track presence with known_lock: if mac not in known_devices: diff --git a/net_alerter/test_net_alerter.py b/net_alerter/test_net_alerter.py index 4bdc224..8f6df10 100644 --- a/net_alerter/test_net_alerter.py +++ b/net_alerter/test_net_alerter.py @@ -221,6 +221,67 @@ def test_dhcp_renewal_dedup_30min(): net_alerter.send_alert = original_send_alert +def test_re_arrival_cancels_departure_timer(): + """ + Regression test: device departs → timer starts → device re-arrives BEFORE timer fires. + Timer should be cancelled and NO departure alert should be sent. + This tests the exact bug scenario: on_arrival() must cancel any pending timer. + """ + net_alerter.known_devices.clear() + net_alerter.departure_timers.clear() + net_alerter.last_departed_time.clear() + + alert_calls = [] + original_send_alert = net_alerter.send_alert + net_alerter.send_alert = lambda msg: alert_calls.append(msg) + + try: + mac = "aa:bb:cc:dd:ee:ff" + ip = "10.0.0.0" + + # Device exists in known_devices + now = time.time() + net_alerter.known_devices[mac] = { + "ip": ip, + "hostname": "testhost", + "vendor": "Test", + "first_seen": now, + "last_seen": now + } + + # Trigger departure (starts 15-min debounce timer) + # At this point: timer created but last_departed_time NOT set yet + net_alerter.on_departure(mac) + + # Verify timer was created + assert mac in net_alerter.departure_timers, "Departure timer should be created" + timer_ref = net_alerter.departure_timers[mac] + + # Device re-arrives BEFORE timer fires (and BEFORE last_departed_time is set) + # This is the critical bug scenario: on_arrival must cancel the timer + net_alerter.on_arrival(mac, ip, "testhost") + + # Timer should be cancelled (either not in dict or marked as cancelled) + assert mac not in net_alerter.departure_timers or not timer_ref.is_alive(), \ + "Departure timer should be cancelled after re-arrival" + + # NO departure alert should be sent (timer was cancelled before firing) + departure_alerts = [a for a in alert_calls if "DEPARTED" in a] + assert len(departure_alerts) == 0, \ + f"No departure alert should be sent when device re-arrives before timer expires, got: {departure_alerts}" + + # Re-arrival may or may not trigger ARRIVED alert depending on last_departed_time + # Since last_departed_time was not set (timer didn't fire yet), re-arrival will + # generate a new ARRIVED alert (not suppressed). This is correct behavior. + + finally: + # Clean up + for timer in net_alerter.departure_timers.values(): + timer.cancel() + net_alerter.departure_timers.clear() + net_alerter.send_alert = original_send_alert + + if __name__ == "__main__": test_hostname_dedup_when_hostname_equals_ip() print("✓ test_hostname_dedup_when_hostname_equals_ip") @@ -249,4 +310,7 @@ if __name__ == "__main__": test_dhcp_renewal_dedup_30min() print("✓ test_dhcp_renewal_dedup_30min") + test_re_arrival_cancels_departure_timer() + print("✓ test_re_arrival_cancels_departure_timer") + print("\nAll tests passed!")