diff --git a/custom_components/localthings/diagnostics.py b/custom_components/localthings/diagnostics.py index b42f821..b22896b 100644 --- a/custom_components/localthings/diagnostics.py +++ b/custom_components/localthings/diagnostics.py @@ -31,9 +31,19 @@ async def async_get_config_entry_diagnostics( # detector when called inline here. Offload it to the executor. stl_version = await hass.async_add_executor_job(pkg_version, "smartthings-local") + # /oic/p and /oic/d sit outside the /device/0 batch captured below, so + # they'd otherwise never reach an issue report. /oic/d's `rt` is OCF's + # standard device-type declaration -- see registry/identity.py. + identity = coordinator._identity return { "device_type": coordinator.device_type_name or "unknown", "one_ui_version": coordinator.one_ui_version, + "identity": { + "manufacturer": identity.manufacturer, + "model": identity.model, + "device_types": list(identity.device_types), + "resources": redact_resources(identity.raw), + } if identity is not None else None, "unbound_hrefs": sorted(coordinator._unbound_hrefs), "resources": redact_resources(coordinator.last_resources), "integration_version": integration.version, diff --git a/custom_components/localthings/registry/identity.py b/custom_components/localthings/registry/identity.py index 7c16fe9..c5445d1 100644 --- a/custom_components/localthings/registry/identity.py +++ b/custom_components/localthings/registry/identity.py @@ -1,7 +1,7 @@ """Read device identity from standard OCF resources (/oic/p, /oic/d).""" from __future__ import annotations -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import Optional import cbor2 @@ -13,6 +13,8 @@ class DeviceIdentity: model: str name: str serial: Optional[str] + device_types: tuple[str, ...] = () + raw: dict[str, dict] = field(default_factory=dict) def _get(sess, path) -> dict: @@ -26,6 +28,27 @@ def _get(sess, path) -> dict: return {} +def _device_types(d: dict) -> tuple[str, ...]: + """/oic/d's `rt` -- the device's own OCF device-type declaration. + + In OCF this is the one standardized "what am I" field: alongside the + generic 'oic.wk.d' it carries a concrete type such as 'oic.d.airconditioner' + or a Samsung 'x.com.samsung.da.*' equivalent. Nothing routes on it yet -- + device-type detection currently parses board part numbers out of + /information/vs/0's modelNum instead (see registry/by_type/__init__.py) -- + because no captured dump has ever included it: /device/0 batch responses + don't carry /oic/d, and diagnostics didn't report it. It's surfaced in + diagnostics so incoming issue reports can tell us whether real hardware + populates it usefully enough to route on. + """ + rt = d.get('rt') + if isinstance(rt, str): + rt = [rt] + if not isinstance(rt, (list, tuple)): + return () + return tuple(t for t in rt if isinstance(t, str)) + + def read_identity(sess, serial: Optional[str]) -> DeviceIdentity: p = _get(sess, ['oic', 'p']) d = _get(sess, ['oic', 'd']) @@ -34,4 +57,9 @@ def read_identity(sess, serial: Optional[str]) -> DeviceIdentity: model=p.get('mnmo') or '', name=d.get('n') or '', serial=serial, + device_types=_device_types(d), + # Kept whole rather than field-by-field: these resources are outside + # the /device/0 dump diagnostics already captures, and we don't yet + # know which of their fields will turn out to identify a device type. + raw={'/oic/p': p, '/oic/d': d}, ) diff --git a/custom_components/localthings/registry/redact.py b/custom_components/localthings/registry/redact.py index 9ff6f60..2d4d0f0 100644 --- a/custom_components/localthings/registry/redact.py +++ b/custom_components/localthings/registry/redact.py @@ -19,9 +19,18 @@ _SENSITIVE_SUBSTRINGS = ( 'userid', 'deviceid', 'uuid', 'duid', 'password', 'secret', ) +# Matched whole, not as substrings. OCF's /oic/d and /oic/p identify the unit +# with bare two-letter keys -- 'di' (device UUID) and 'pi' (platform UUID) -- +# which are as identifying as the serial number above but far too short to +# match on: 'di' alone is a substring of 'condition', 'display', 'dispenser' +# and plenty of other perfectly ordinary appliance fields. +_SENSITIVE_EXACT = frozenset({'di', 'pi'}) + def _is_sensitive_key(key: str) -> bool: lowered = key.lower() + if lowered in _SENSITIVE_EXACT: + return True return any(s in lowered for s in _SENSITIVE_SUBSTRINGS) diff --git a/tests/localthings/test_diagnostics.py b/tests/localthings/test_diagnostics.py index 13fde6b..6385e05 100644 --- a/tests/localthings/test_diagnostics.py +++ b/tests/localthings/test_diagnostics.py @@ -38,6 +38,55 @@ async def test_diagnostics_shape_and_redaction( assert resources['/status/lock/vs/0']['x.com.samsung.da.ado.devicecontrol'] == 'On' +async def test_diagnostics_include_ocf_identity( + hass: HomeAssistant, mock_entry, mock_coordinator_session +) -> None: + """/oic/p and /oic/d are outside the /device/0 batch, so diagnostics is + the only place an issue report can carry them -- and `rt` there is OCF's + own device-type declaration.""" + from custom_components.localthings.registry.identity import DeviceIdentity + + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + + coordinator = hass.data[DOMAIN][mock_entry.entry_id] + coordinator._identity = DeviceIdentity( + manufacturer='Samsung Electronics', + model='RF9000B', + name='Family Hub', + serial=None, + device_types=('oic.wk.d', 'oic.d.refrigerator'), + raw={ + '/oic/p': {'mnmn': 'Samsung Electronics', 'pi': '12-34-56'}, + '/oic/d': {'n': 'Family Hub', 'di': 'ab-cd-ef'}, + }, + ) + + diag = await async_get_config_entry_diagnostics(hass, mock_entry) + + identity = diag['identity'] + assert identity['model'] == 'RF9000B' + assert identity['device_types'] == ['oic.wk.d', 'oic.d.refrigerator'] + # The raw payloads ride along redacted -- we don't yet know which of + # their fields identify a device type, so none are dropped up front. + assert identity['resources']['/oic/p']['mnmn'] == 'Samsung Electronics' + assert identity['resources']['/oic/d']['di'] == REDACTED + assert identity['resources']['/oic/p']['pi'] == REDACTED + + +async def test_diagnostics_identity_none_when_unavailable( + hass: HomeAssistant, mock_entry, mock_coordinator_session +) -> None: + """read_identity is best-effort: a device that answers neither resource + (or a session that never connected) must not break the download.""" + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + + diag = await async_get_config_entry_diagnostics(hass, mock_entry) + + assert diag['identity'] is None + + async def test_diagnostics_include_observe_mode_fields( hass: HomeAssistant, mock_entry, mock_coordinator_session ) -> None: diff --git a/tests/test_identity.py b/tests/test_identity.py index 4d0a138..d9fa4dd 100644 --- a/tests/test_identity.py +++ b/tests/test_identity.py @@ -31,3 +31,41 @@ def test_read_identity_tolerates_missing_resources(): assert ident.manufacturer == 'Samsung' assert ident.model == '' assert ident.serial is None + assert ident.device_types == () + assert ident.raw == {'/oic/p': {}, '/oic/d': {}} + + +def test_read_identity_captures_oic_d_device_types(): + """/oic/d's `rt` is OCF's own device-type declaration -- captured so + diagnostics can show whether real hardware populates it usefully.""" + sess = FakeSession({ + ('oic', 'd'): { + 'n': 'Living Room AC', + 'rt': ['oic.wk.d', 'oic.d.airconditioner'], + }, + }) + ident = read_identity(sess, serial=None) + assert ident.device_types == ('oic.wk.d', 'oic.d.airconditioner') + + +def test_read_identity_normalizes_scalar_and_malformed_rt(): + """Firmware that reports a bare string, or a non-list, must not explode.""" + assert read_identity( + FakeSession({('oic', 'd'): {'rt': 'oic.d.refrigerator'}}), None + ).device_types == ('oic.d.refrigerator',) + assert read_identity( + FakeSession({('oic', 'd'): {'rt': 42}}), None + ).device_types == () + assert read_identity( + FakeSession({('oic', 'd'): {'rt': ['oic.wk.d', 7, None]}}), None + ).device_types == ('oic.wk.d',) + + +def test_read_identity_keeps_raw_payloads_for_diagnostics(): + sess = FakeSession({ + ('oic', 'p'): {'mnmn': 'Samsung Electronics', 'mnmo': 'RF9000B'}, + ('oic', 'd'): {'n': 'Family Hub', 'di': 'abc-123'}, + }) + ident = read_identity(sess, serial=None) + assert ident.raw['/oic/p']['mnmo'] == 'RF9000B' + assert ident.raw['/oic/d']['di'] == 'abc-123' diff --git a/tests/test_redact.py b/tests/test_redact.py index 21287a0..b80542b 100644 --- a/tests/test_redact.py +++ b/tests/test_redact.py @@ -72,3 +72,34 @@ def test_redact_resources_does_not_mutate_input(): redact_resources(resources) assert resources['/information/vs/0']['x.com.samsung.da.serialNum'] == original_serial + + +def test_redacts_bare_ocf_identity_keys(): + """/oic/d and /oic/p identify the unit with two-letter keys ('di', 'pi') + that the substring rules can't see.""" + redacted = redact_resources({ + '/oic/d': {'di': 'ab-cd-ef', 'n': 'Family Hub'}, + '/oic/p': {'pi': '12-34-56', 'mnmo': 'RF9000B'}, + }) + + assert redacted['/oic/d']['di'] == REDACTED + assert redacted['/oic/p']['pi'] == REDACTED + # Non-identifying neighbours in the same payloads survive. + assert redacted['/oic/d']['n'] == 'Family Hub' + assert redacted['/oic/p']['mnmo'] == 'RF9000B' + + +def test_bare_key_redaction_does_not_leak_into_substring_matching(): + """'di'/'pi' are whole-key matches only -- plenty of ordinary appliance + fields contain those two letters and must survive untouched.""" + redacted = redact_resources({ + '/x': { + 'condition': 'Normal', 'display': 'On', 'dispenser': 'Cubed', + 'humidity': '45', 'spinSpeed': '1200', + }, + }) + + assert redacted['/x'] == { + 'condition': 'Normal', 'display': 'On', 'dispenser': 'Cubed', + 'humidity': '45', 'spinSpeed': '1200', + }