From 89e8b2afdb2e2c21103015c942f9bf96ca498e9f Mon Sep 17 00:00:00 2001
From: Emmanuel Levijarvi <emansl@gmail.com>
Date: Sat, 29 Aug 2026 07:59:17 -0700
Subject: [PATCH] Redact the MAC in the reservations unsubscribe warning

fetch_reservations logged device_info.mac_address in the clear when its
cleanup unsubscribe raised. The identical warning in
update_reservations_confirmed, seventy lines below, already passed the
value through redact_mac - this call site was simply missed.

CodeQL has flagged it as py/clear-text-logging-sensitive-data (high)
since 2026-07-23; it shipped in 9.3.1.

The local `from .mqtt.utils import redact_mac` matches the sibling and
the three other call sites in the package, which import it inside the
function rather than at module scope.

A test drives the cleanup path with a failing unsubscribe and asserts
the MAC does not appear in the emitted warning; it fails against the
unredacted version.

Claude-Session: https://claude.ai/code/session_01XVj9BYvuLj7Th3iFUVoeCn
---
 CHANGELOG.rst              | 10 ++++++++++
 src/nwp500/reservations.py |  4 +++-
 tests/test_reservations.py | 21 +++++++++++++++++++++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 3377119..1d05f1a 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -5,6 +5,16 @@ Changelog
 Unreleased
 ==========
 
+Security
+--------
+- **MAC address no longer logged in the clear when a reservations
+  unsubscribe fails.** ``fetch_reservations`` logged
+  ``device_info.mac_address`` unredacted in its cleanup warning, while the
+  identical warning in ``update_reservations_confirmed`` a few lines below
+  passed it through ``redact_mac``. CodeQL flagged it as
+  ``py/clear-text-logging-sensitive-data`` (high). Both call sites now
+  redact.
+
 Version 9.3.1 (2026-08-29)
 ==========================
 
diff --git a/src/nwp500/reservations.py b/src/nwp500/reservations.py
index fc4602e..fe890c1 100644
--- a/src/nwp500/reservations.py
+++ b/src/nwp500/reservations.py
@@ -71,10 +71,12 @@ def on_schedule(schedule: ReservationSchedule) -> None:
         try:
             await mqtt.unsubscribe_reservation_response(device, on_schedule)
         except Exception:
+            from .mqtt.utils import redact_mac
+
             _logger.warning(
                 "Failed to unsubscribe reservations response handler for "
                 "device %s",
-                device.device_info.mac_address,
+                redact_mac(device.device_info.mac_address),
                 exc_info=True,
             )
 
diff --git a/tests/test_reservations.py b/tests/test_reservations.py
index 6cc267b..f431e78 100644
--- a/tests/test_reservations.py
+++ b/tests/test_reservations.py
@@ -1,5 +1,6 @@
 """Tests for the nwp500.reservations public helpers."""
 
+import logging
 from typing import Any
 from unittest.mock import ANY, AsyncMock, MagicMock, patch
 
@@ -119,6 +120,26 @@ async def test_fetch_reservations_timeout(
     )
 
 
+@pytest.mark.asyncio
+async def test_fetch_reservations_redacts_mac_when_unsubscribe_fails(
+    mock_mqtt: MagicMock, mock_device: MagicMock, caplog: Any
+) -> None:
+    """The unsubscribe warning must not log the MAC address in the clear."""
+    mac = "0123456789ab"
+    mock_device.device_info.mac_address = mac
+    mock_mqtt.request_reservations = AsyncMock()  # never fires callback
+    mock_mqtt.unsubscribe_reservation_response = AsyncMock(
+        side_effect=RuntimeError("boom")
+    )
+
+    with caplog.at_level(logging.WARNING, logger="nwp500.reservations"):
+        await fetch_reservations(mock_mqtt, mock_device, timeout=0.01)
+
+    messages = [r.getMessage() for r in caplog.records]
+    assert any("Failed to unsubscribe" in m for m in messages)
+    assert not any(mac in m for m in messages)
+
+
 @pytest.mark.asyncio
 async def test_fetch_reservations_ignores_multiple_responses(
     mock_mqtt: MagicMock, mock_device: MagicMock