From d5775526fd49ef6db52486c0a2d33cace01bf434 Mon Sep 17 00:00:00 2001 From: infy1995 Date: Mon, 9 Nov 2026 22:58:39 +0100 Subject: [PATCH] fix: redact private data in device diagnostics Redact JSON documents stored as strings, such as the A01 product info data point, which exposed the serial number, Wi-Fi name, IP and MAC. Also redact ssid and iotName. Decode the A01 product info and voice pack data points, which the device reports as JSON strings and always decoded to None, and add the decoded A01 state to device diagnostics as traits. --- roborock/devices/device.py | 4 ++ roborock/devices/traits/a01/__init__.py | 28 +++++++++-- roborock/diagnostics.py | 29 +++++++++-- tests/devices/test_a01_traits.py | 39 +++++++++++++- tests/devices/traits/a01/test_init.py | 67 +++++++++++++++++++++++++ tests/test_diagnostics.py | 63 +++++++++++++++++++++++ 6 files changed, 222 insertions(+), 8 deletions(-) diff --git a/roborock/devices/device.py b/roborock/devices/device.py index 44c275f6c..d57b451b0 100644 --- a/roborock/devices/device.py +++ b/roborock/devices/device.py @@ -255,6 +255,10 @@ def diagnostic_data(self) -> dict[str, Any]: extra: dict[str, Any] = {} if self.v1_properties: extra["traits"] = self.v1_properties.as_dict() + elif self.dyad: + extra["traits"] = self.dyad.as_dict() + elif self.zeo: + extra["traits"] = self.zeo.as_dict() elif self.b01_q10_properties: extra["traits"] = self.b01_q10_properties.as_dict() return redact_device_data( diff --git a/roborock/devices/traits/a01/__init__.py b/roborock/devices/traits/a01/__init__.py index 7682bc7f8..152b76ece 100644 --- a/roborock/devices/traits/a01/__init__.py +++ b/roborock/devices/traits/a01/__init__.py @@ -31,7 +31,14 @@ from datetime import UTC, datetime, time from typing import Any, Generic, TypeVar -from roborock.data import DyadProductInfo, DyadScheduleTask, DyadSndState, HomeDataProduct, RoborockCategory +from roborock.data import ( + DyadProductInfo, + DyadScheduleTask, + DyadSndState, + HomeDataProduct, + RoborockBase, + RoborockCategory, +) from roborock.data.dyad.dyad_code_mappings import ( DyadBrushSpeed, DyadCleanAssistPower, @@ -144,8 +151,8 @@ def _decode_dyad_schedule(value: str) -> list[DyadScheduleTask]: RoborockDyadDataProtocol.SELF_CLEAN_CUR_TEMP: lambda val: DyadSelfCleanWaterTemp(val).name, RoborockDyadDataProtocol.DRY_CUR_MODE: lambda val: DyadDryingMode(val).name, RoborockDyadDataProtocol.SCHEDULE_TASK: _decode_dyad_schedule, - RoborockDyadDataProtocol.SND_STATE: lambda val: DyadSndState.from_dict(val), - RoborockDyadDataProtocol.PRODUCT_INFO: lambda val: DyadProductInfo.from_dict(val), + RoborockDyadDataProtocol.SND_STATE: lambda val: DyadSndState.from_dict(json.loads(val)), + RoborockDyadDataProtocol.PRODUCT_INFO: lambda val: DyadProductInfo.from_dict(json.loads(val)), } ZEO_PROTOCOL_ENTRIES: dict[RoborockZeoProtocol, Callable] = { @@ -191,6 +198,17 @@ def convert_zeo_value(protocol_value: RoborockZeoProtocol, value: Any) -> Any: return None +def _diagnostic_value(value: Any) -> Any: + """Convert a decoded value to plain data for diagnostics.""" + if isinstance(value, RoborockBase): + return value.as_dict() + if isinstance(value, list): + return [_diagnostic_value(item) for item in value] + if isinstance(value, time): + return value.isoformat() + return value + + _DYAD_PROTOCOL_VALUES = frozenset(protocol.value for protocol in RoborockDyadDataProtocol) _ZEO_PROTOCOL_VALUES = frozenset(protocol.value for protocol in RoborockZeoProtocol) @@ -225,6 +243,10 @@ def values(self) -> dict[_P, Any]: """ return dict(self._values) + def as_dict(self) -> dict[str, Any]: + """Return the latest known state as plain data for diagnostics.""" + return {protocol.name: _diagnostic_value(value) for protocol, value in self._values.items()} + @property def last_message_time(self) -> datetime | None: """Time the last message was received from the device. diff --git a/roborock/diagnostics.py b/roborock/diagnostics.py index 26d1e89c7..d81c0f1c0 100644 --- a/roborock/diagnostics.py +++ b/roborock/diagnostics.py @@ -9,6 +9,7 @@ DeviceManager. """ +import json import time from collections import Counter from collections.abc import Generator, Mapping @@ -106,7 +107,9 @@ def reset(self) -> None: "ipAddress", "ipAdress", "wifiName", + "ssid", "rawName", + "iotName", "lat", "long", } @@ -123,8 +126,28 @@ def reset(self) -> None: REDACTED = "**REDACTED**" +def _redact_json_string(value: str, path: str) -> str: + """Redact sensitive data in a JSON document stored as a string. + + Some devices report structured data this way, such as the A01 product info + data point. A string that looks like JSON but cannot be parsed is fully + redacted, since its contents cannot be checked. + """ + if not value.lstrip().startswith(("{", "[")): + return value + try: + decoded = json.loads(value) + except json.JSONDecodeError: + return REDACTED + redacted = redact_device_data(decoded, path) + return value if redacted == decoded else json.dumps(redacted) + + def redact_device_data(data: T, path: str = "") -> T | dict[str, Any]: - """Redact sensitive data in a dict.""" + """Redact sensitive data in a dict, including JSON documents stored as strings.""" + if isinstance(data, str): + return cast(T, _redact_json_string(data, path)) + if not isinstance(data, (Mapping, list)): return data @@ -141,10 +164,8 @@ def redact_device_data(data: T, path: str = "") -> T | dict[str, Any]: redacted[key] = REDACTED elif key == DEVICE_UID and isinstance(value, str): redacted[key] = redact_device_uid(value) - elif isinstance(value, dict): + else: redacted[key] = redact_device_data(value, curr_path) - elif isinstance(value, list): - redacted[key] = [redact_device_data(item, curr_path) for item in value] return redacted diff --git a/tests/devices/test_a01_traits.py b/tests/devices/test_a01_traits.py index 83a5b5cb3..b7bb0ca1b 100644 --- a/tests/devices/test_a01_traits.py +++ b/tests/devices/test_a01_traits.py @@ -1,9 +1,14 @@ +import json from unittest.mock import AsyncMock, Mock, patch import pytest -from roborock.devices.traits.a01 import DyadApi, ZeoApi +from roborock.data import HomeDataDevice +from roborock.devices.device import RoborockDevice +from roborock.devices.traits.a01 import DyadApi, ZeoApi, create from roborock.roborock_message import RoborockDyadDataProtocol, RoborockZeoProtocol +from roborock.testing.a01_simulator import DEFAULT_DYAD_PRODUCT +from tests import mock_data @pytest.fixture @@ -51,3 +56,35 @@ async def test_zeo_query_values(mock_channel): assert RoborockZeoProtocol.STATE in result assert result[RoborockZeoProtocol.STATE] == "spinning" assert result[RoborockZeoProtocol.COUNTDOWN] == 120 + + +def test_dyad_diagnostic_data_includes_redacted_traits(mock_channel): + """A01 diagnostics report the decoded state without identifiers.""" + device_status = { + "201": 3, + "10005": json.dumps( + { + "sn": "TESTSERIAL", + "ssid": "test-network", + "timezone": "Europe/Warsaw", + "posix_timezone": "CET-1CEST,M3.5.0/2,M10.5.0/3", + "ip": "192.0.2.1", + "mac": "00:00:5e:00:53:01", + "oba": {"bom": "A.01"}, + } + ), + } + device = RoborockDevice( + device_info=HomeDataDevice.from_dict({**mock_data.Q10_DEVICE_DATA, "deviceStatus": device_status}), + product=DEFAULT_DYAD_PRODUCT, + channel=AsyncMock(), + trait=create(DEFAULT_DYAD_PRODUCT, mock_channel, device_status=device_status), + ) + + diagnostics = device.diagnostic_data() + + dumped = json.dumps(diagnostics) + for private_value in ("TESTSERIAL", "test-network", "192.0.2.1", "00:00:5e:00:53:01"): + assert private_value not in dumped + assert diagnostics["traits"]["status"] == "charging" + assert diagnostics["traits"]["product_info"]["timezone"] == "Europe/Warsaw" diff --git a/tests/devices/traits/a01/test_init.py b/tests/devices/traits/a01/test_init.py index aea824e6f..ea9b53085 100644 --- a/tests/devices/traits/a01/test_init.py +++ b/tests/devices/traits/a01/test_init.py @@ -324,6 +324,73 @@ async def test_create_seeds_values_from_device_status(fake_channel: FakeChannel) } +PRODUCT_INFO_JSON = json.dumps( + { + "sn": "TESTSERIAL", + "ssid": "test-network", + "timezone": "Europe/Warsaw", + "posix_timezone": "CET-1CEST,M3.5.0/2,M10.5.0/3", + "ip": "192.0.2.1", + "mac": "00:00:5e:00:53:01", + "rssi": -53, + "oba": {"bom": "A.01"}, + } +) + + +async def test_dyad_decodes_json_string_data_points(fake_channel: FakeChannel): + """The device reports product info and voice pack state as JSON strings.""" + api = DyadApi( + fake_channel, # type: ignore[arg-type] + initial_status={ + 10004: '{"sid_in_use":21,"sid_version":3,"location":"de","bom":"A.01","language":"en"}', + 10005: PRODUCT_INFO_JSON, + }, + ) + + assert api.values[RoborockDyadDataProtocol.SND_STATE].sid_in_use == 21 + product_info = api.values[RoborockDyadDataProtocol.PRODUCT_INFO] + assert product_info.sn == "TESTSERIAL" + assert product_info.posix_timezone == "CET-1CEST,M3.5.0/2,M10.5.0/3" + assert product_info.oba == {"bom": "A.01"} + + +@pytest.mark.parametrize("value", ["", "not json", None]) +async def test_dyad_invalid_json_string_data_point(fake_channel: FakeChannel, value: str | None): + """A malformed JSON data point leaves the value unset.""" + api = DyadApi(fake_channel, initial_status={10005: value}) # type: ignore[arg-type] + + assert api.values == {} + + +async def test_dyad_as_dict_is_plain_data(fake_channel: FakeChannel): + """Decoded values are converted to JSON serializable data for diagnostics.""" + api = DyadApi( + fake_channel, # type: ignore[arg-type] + initial_status={ + 201: 3, + 227: 1320, + 10002: '[{"id":1,"on":false,"cron":"15 14 31 10 *","dpAction":{"200":1}}]', + 10005: PRODUCT_INFO_JSON, + }, + ) + + data = api.as_dict() + + json.dumps(data) + assert data["status"] == "charging" + assert data["silent_mode_start_time"] == "22:00:00" + assert data["schedule_task"][0]["cron"] == "15 14 31 10 *" + assert data["product_info"]["timezone"] == "Europe/Warsaw" + + +async def test_zeo_as_dict(fake_channel: FakeChannel): + """Zeo state is keyed by data point name for diagnostics.""" + api = ZeoApi(fake_channel, initial_status={RoborockZeoProtocol.COUNTDOWN.value: 5}) # type: ignore[arg-type] + + assert api.as_dict() == {RoborockZeoProtocol.COUNTDOWN.name: 5} + + async def test_dyad_close_stops_tracking(dyad_api: DyadApi, fake_channel: FakeChannel): """After close(), pushes no longer update `values`.""" await dyad_api.start() diff --git a/tests/test_diagnostics.py b/tests/test_diagnostics.py index 5de9a6950..f3c7e49e7 100644 --- a/tests/test_diagnostics.py +++ b/tests/test_diagnostics.py @@ -1,5 +1,8 @@ """Tests for diagnostics module.""" +import json +from typing import Any + import pytest from roborock.diagnostics import REDACTED, Diagnostics, redact_device_data, redact_device_uid, redact_topic_name @@ -106,3 +109,63 @@ def test_redact_q10_raw_room_name() -> None: assert redact_device_data( {"map": {"rooms": [{"rawName": "Owner’s bedroom"}], "currentRoom": {"rawName": "Salle d’eau"}}} ) == {"map": {"rooms": [{"rawName": REDACTED}], "currentRoom": {"rawName": REDACTED}}} + + +def test_redact_iot_room_name() -> None: + """Room names reported in the map list are as private as raw room names.""" + assert redact_device_data({"rooms": [{"iotName": "Owner’s bedroom", "tag": 1}]}) == { + "rooms": [{"iotName": REDACTED, "tag": 1}] + } + + +def test_redact_json_string_in_device_status() -> None: + """Identifiers inside a data point that holds a JSON string are redacted.""" + product_info = json.dumps( + { + "sn": "TESTSERIAL", + "ssid": "test-network", + "ip": "192.0.2.1", + "mac": "00:00:5e:00:53:01", + "rssi": -53, + "oba": {"bom": "A.01"}, + } + ) + + redacted = redact_device_data({"device": {"deviceStatus": {"10005": product_info, "209": 100}}}) + + status: dict[str, Any] = redacted["device"]["deviceStatus"] + assert status["209"] == 100 + assert json.loads(status["10005"]) == { + "sn": REDACTED, + "ssid": REDACTED, + "ip": REDACTED, + "mac": REDACTED, + "rssi": -53, + "oba": {"bom": "A.01"}, + } + + +def test_redact_json_string_in_list() -> None: + """JSON strings inside lists are redacted too.""" + redacted = redact_device_data({"items": ['{"ip": "192.0.2.1"}', "plain"]}) + + assert redacted["items"] == [json.dumps({"ip": REDACTED}), "plain"] + + +@pytest.mark.parametrize( + "value", + [ + "plain text", + "", + '{"RRPhotoPrivacyVersion": "1"}', # Not sensitive, so the formatting is kept + "[1, 2, 3]", + ], +) +def test_redact_keeps_strings_without_sensitive_data(value: str) -> None: + """Strings with nothing to redact are returned unchanged.""" + assert redact_device_data({"value": value}) == {"value": value} + + +def test_redact_unparsable_json_string() -> None: + """A string that looks like JSON but cannot be checked is fully redacted.""" + assert redact_device_data({"value": '{"sn": "TESTSERIAL"'}) == {"value": REDACTED}