fix: Improve debug logs redaction

Put all redaction logic in one place. Redact identifiers from topic names and duids.

Author: allenporter

roborock/devices/device.py

@@ -8,11 +8,12 @@
 import datetime
 import logging
 from abc import ABC
-from collections.abc import Callable, Mapping
-from typing import Any, TypeVar, cast
+from collections.abc import Callable
+from typing import Any
 
 from roborock.callbacks import CallbackList
 from roborock.data import HomeDataDevice, HomeDataProduct
+from roborock.diagnostics import redact_device_data
 from roborock.exceptions import RoborockException
 from roborock.roborock_message import RoborockMessage
 from roborock.util import RoborockLoggerAdapter
@@ -65,7 +66,7 @@ def __init__(
         """
         TraitsMixin.__init__(self, trait)
         self._duid = device_info.duid
-        self._logger = RoborockLoggerAdapter(self._duid, _LOGGER)
+        self._logger = RoborockLoggerAdapter(duid=self._duid, logger=_LOGGER)
         self._name = device_info.name
         self._device_info = device_info
         self._product = product
@@ -223,52 +224,9 @@ def diagnostic_data(self) -> dict[str, Any]:
         """Return diagnostics information about the device."""
         extra: dict[str, Any] = {}
         if self.v1_properties:
-            extra["traits"] = _redact_data(self.v1_properties.as_dict())
+            extra["traits"] = redact_device_data(self.v1_properties.as_dict())
         return {
-            "device": _redact_data(self.device_info.as_dict()),
-            "product": _redact_data(self.product.as_dict()),
+            "device": redact_device_data(self.device_info.as_dict()),
+            "product": redact_device_data(self.product.as_dict()),
             **extra,
         }
-
-
-T = TypeVar("T")
-
-REDACT_KEYS = {
-    # Potential identifiers
-    "duid",
-    "localKey",
-    "mac",
-    "bssid",
-    "sn",
-    "ip",
-    "u",
-    "s",
-    "h",
-    "k",
-    # Large binary blobs are entirely omitted
-    "imageContent",
-    "mapData",
-    "rawApiResponse",
-}
-REDACTED = "**REDACTED**"
-
-
-def _redact_data(data: T) -> T | dict[str, Any]:
-    """Redact sensitive data in a dict."""
-    if not isinstance(data, (Mapping, list)):
-        return data
-
-    if isinstance(data, list):
-        return cast(T, [_redact_data(item) for item in data])
-
-    redacted = {**data}
-
-    for key, value in redacted.items():
-        if key in REDACT_KEYS:
-            redacted[key] = REDACTED
-        elif isinstance(value, dict):
-            redacted[key] = _redact_data(value)
-        elif isinstance(value, list):
-            redacted[key] = [_redact_data(item) for item in value]
-
-    return redacted

roborock/devices/local_channel.py

@@ -54,7 +54,7 @@ class LocalChannel(Channel):
 
     def __init__(self, host: str, local_key: str, device_uid: str) -> None:
         self._host = host
-        self._logger = RoborockLoggerAdapter(device_uid, _LOGGER)
+        self._logger = RoborockLoggerAdapter(duid=device_uid, logger=_LOGGER)
         self._transport: asyncio.Transport | None = None
         self._protocol: _LocalProtocol | None = None
         self._subscribers: CallbackList[RoborockMessage] = CallbackList(self._logger)

roborock/devices/mqtt_channel.py

@@ -27,7 +27,7 @@ class MqttChannel(Channel):
     def __init__(self, mqtt_session: MqttSession, duid: str, local_key: str, rriot: RRiot, mqtt_params: MqttParams):
         self._mqtt_session = mqtt_session
         self._duid = duid
-        self._logger = RoborockLoggerAdapter(duid, _LOGGER)
+        self._logger = RoborockLoggerAdapter(duid=duid, logger=_LOGGER)
         self._local_key = local_key
         self._rriot = rriot
         self._mqtt_params = mqtt_params

roborock/devices/v1_channel.py

@@ -178,7 +178,7 @@ def __init__(
     ) -> None:
         """Initialize the V1Channel."""
         self._device_uid = device_uid
-        self._logger = RoborockLoggerAdapter(device_uid, _LOGGER)
+        self._logger = RoborockLoggerAdapter(duid=device_uid, logger=_LOGGER)
         self._security_data = security_data
         self._mqtt_channel = mqtt_channel
         self._local_session = local_session

roborock/diagnostics.py

@@ -15,7 +15,7 @@
 from collections import Counter
 from collections.abc import Generator, Mapping
 from contextlib import contextmanager
-from typing import Any
+from typing import Any, TypeVar, cast
 
 
 class Diagnostics:
@@ -82,3 +82,70 @@ def reset(self) -> None:
         self._counter = Counter()
         for d in self._subkeys.values():
             d.reset()
+
+
+T = TypeVar("T")
+
+REDACT_KEYS = {
+    # Potential identifiers
+    "localKey",
+    "mac",
+    "bssid",
+    "sn",
+    "ip",
+    "u",
+    "s",
+    "h",
+    "k",
+    # Large binary blobs are entirely omitted
+    "imageContent",
+    "mapData",
+    "rawApiResponse",
+}
+DEVICE_UID = "duid"
+REDACTED = "**REDACTED**"
+
+
+def redact_device_data(data: T) -> T | dict[str, Any]:
+    """Redact sensitive data in a dict."""
+    if not isinstance(data, (Mapping, list)):
+        return data
+
+    if isinstance(data, list):
+        return cast(T, [redact_device_data(item) for item in data])
+
+    redacted = {**data}
+
+    for key, value in redacted.items():
+        if key in REDACT_KEYS:
+            redacted[key] = REDACTED
+        elif key == DEVICE_UID and isinstance(value, str):
+            redacted[key] = redact_device_uid(value)
+        elif isinstance(value, dict):
+            redacted[key] = redact_device_data(value)
+        elif isinstance(value, list):
+            redacted[key] = [redact_device_data(item) for item in value]
+
+    return redacted
+
+
+def redact_topic_name(topic: str) -> str:
+    """Redact potentially identifying information from a topic name."""
+    parts = topic.split("/")
+    if len(parts) < 5:
+        return topic
+
+    redacted_parts = parts[:4]
+    for part in parts[4:]:
+        if len(part) <= 5:
+            redacted_parts.append("*****")
+        else:
+            redacted_parts.append("*****" + part[-5:])
+    return "/".join(redacted_parts)
+
+
+def redact_device_uid(duid: str) -> str:
+    """Redact a device UID to hide identifying information."""
+    if len(duid) <= 5:
+        return "*****"
+    return "******" + duid[-5:]

roborock/util.py

@@ -10,6 +10,7 @@
 from typing import Any, TypeVar
 
 from roborock import RoborockException
+from roborock.diagnostics import redact_device_uid
 
 T = TypeVar("T")
 DEFAULT_TIME_ZONE: datetime.tzinfo | None = datetime.datetime.now().astimezone().tzinfo
@@ -81,9 +82,19 @@ async def reset(self):
 
 
 class RoborockLoggerAdapter(logging.LoggerAdapter):
-    def __init__(self, prefix: str, logger: logging.Logger) -> None:
-        super().__init__(logger, {})
-        self.prefix = prefix
+    def __init__(
+        self,
+        duid: str | None = None,
+        name: str | None = None,
+        logger: logging.Logger | None = None,
+    ) -> None:
+        super().__init__(logger or logging.getLogger(__name__), {})
+        if name is not None:
+            self.prefix = name
+        elif duid is not None:
+            self.prefix = redact_device_uid(duid)
+        else:
+            raise ValueError("Either duid or name must be provided")
 
     def process(self, msg: str, kwargs: MutableMapping[str, Any]) -> tuple[str, MutableMapping[str, Any]]:
         return f"[{self.prefix}] {msg}", kwargs

roborock/version_1_apis/roborock_local_client_v1.py

@@ -58,7 +58,7 @@ def __init__(
         self._ack_nonce: int | None = None
         self._set_encoder_decoder()
         self.queue_timeout = queue_timeout
-        self._logger = RoborockLoggerAdapter(device_data.device.name, _LOGGER)
+        self._logger = RoborockLoggerAdapter(name=device_data.device.name, logger=_LOGGER)
 
     @property
     def local_protocol_version(self) -> LocalProtocolVersion:

roborock/version_1_apis/roborock_mqtt_client_v1.py

@@ -32,7 +32,7 @@ def __init__(self, user_data: UserData, device_info: DeviceData, queue_timeout:
         security_data = create_security_data(rriot)
         RoborockClientV1.__init__(self, device_info, security_data=security_data)
         self.queue_timeout = queue_timeout
-        self._logger = RoborockLoggerAdapter(device_info.device.name, _LOGGER)
+        self._logger = RoborockLoggerAdapter(name=device_info.device.name, logger=_LOGGER)
         self._security_data = security_data
 
     async def _send_command(

roborock/version_a01_apis/roborock_mqtt_client_a01.py

@@ -37,7 +37,7 @@ def __init__(
         RoborockMqttClient.__init__(self, user_data, device_info)
         RoborockClientA01.__init__(self, device_info, category)
         self.queue_timeout = queue_timeout
-        self._logger = RoborockLoggerAdapter(device_info.device.name, _LOGGER)
+        self._logger = RoborockLoggerAdapter(name=device_info.device.name, logger=_LOGGER)
 
     async def _send_message(self, roborock_message: RoborockMessage):
         await self._validate_connection()

tests/devices/__snapshots__/test_v1_device.ambr

@@ -18,7 +18,7 @@
         '128': 0,
         '133': 1,
       }),
-      'duid': '**REDACTED**',
+      'duid': '******bc123',
       'extra': '{"RRPhotoPrivacyVersion": "1"}',
       'featureSet': '2234201184108543',
       'fv': '02.56.02',
@@ -444,7 +444,7 @@
         '128': 0,
         '133': 1,
       }),
-      'duid': '**REDACTED**',
+      'duid': '******bc123',
       'extra': '{"RRPhotoPrivacyVersion": "1"}',
       'featureSet': '2234201184108543',
       'fv': '02.56.02',
@@ -850,7 +850,7 @@
         '128': 0,
         '133': 1,
       }),
-      'duid': '**REDACTED**',
+      'duid': '******bc123',
       'extra': '{"RRPhotoPrivacyVersion": "1"}',
       'featureSet': '2234201184108543',
       'fv': '02.56.02',
@@ -1227,7 +1227,7 @@
         '128': 0,
         '133': 1,
       }),
-      'duid': '**REDACTED**',
+      'duid': '******bc123',
       'extra': '{"RRPhotoPrivacyVersion": "1"}',
       'featureSet': '2234201184108543',
       'fv': '02.56.02',