feat(diagnostics): capture /oic/p and /oic/d identity
Device-type detection currently parses board part numbers out of /information/vs/0's modelNum. OCF has a standard field for exactly this question -- /oic/d's `rt` -- and read_identity() already fetches the resource, but kept only `n` and threw the rest away. No captured dump has ever included it either: /device/0 batch responses don't carry /oic/d, and diagnostics didn't report it, so there's no evidence on whether real hardware populates it usefully. Keep `rt` as DeviceIdentity.device_types, keep both raw payloads whole (we don't yet know which of their fields identify a type), and surface them in diagnostics so incoming issue reports answer the question. Nothing routes on it yet. /oic/d and /oic/p identify the unit with bare two-letter keys -- 'di' and 'pi' -- as sensitive as the serial number redact.py already covers but far too short to match on: 'di' alone is a substring of 'condition', 'display' and 'dispenser'. Add a whole-key match alongside the substring rules.
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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},
|
||||
)
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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'
|
||||
|
||||
@@ -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',
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user