diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8a93ebf..e13ff89 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -17,6 +17,16 @@ Fixed cloud reported no code, which is not the same claim as ``NO_ERROR``. Callers that treated the field as always-present should handle ``None``. +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