From a78504cd2f4cfe3231e0d91907a9a29d9a59ad0c Mon Sep 17 00:00:00 2001 From: Christian Glombek Date: Thu, 30 Jul 2026 01:16:10 +0200 Subject: [PATCH 1/4] Add an OTBR action to migrate the whole Thread network Joining a border router to another Thread network meant replacing its active dataset with Thread down, which moves only the router and strands every device on the network it leaves. Thread has a graceful way to do this that the integration only used for channel changes: a pending operational dataset, disseminated by the router to every device, applied by all of them together when its delay expires. The new otbr.migrate_network action generalises that mechanism to a change of network. The target dataset - given as TLVs, or the preferred Thread network by default - must describe the network completely: the router merges a pending dataset over a base it chooses, so any field left out would come from that base and the mesh would migrate onto settings nobody picked. The dataset is re-stamped newer than the network being left - a not-newer pending dataset is silently ignored by the mesh - and otherwise taken verbatim. Rotating credentials on the same network is the same operation, so it works too; only a fully identical dataset is a no-op. A migration is refused outright while a pending dataset is already in flight, and the user is told so. Superseding one is never safe to do implicitly: the replacement races the delay timer on every device already holding the old dataset, so a late replacement can split the mesh, and it would silently undo whatever the in-flight dataset was doing, such as a channel change the user never heard about. The check runs before anything is recorded, and the library's own guard on the write - If-None-Match on border routers that honor it - backstops the race where a pending dataset appears between the check and the write; that refusal surfaces as the same error. A migration off a channel that another radio pins in multiprotocol setups is refused, the same way selecting a different network already is. The border router is picked with a config entry selector, needed only when several are set up. The Thread dataset store learns the re-stamped dataset, bound to the router like setup binds datasets, and when the network being left was the preferred one, the preferred pointer moves along - everything handing out Thread credentials starts there, and the action's own default would otherwise migrate a router back onto the abandoned network. Migrations are serialised per router: two racing calls would compute identical timestamps and the loser would be ignored by the mesh while its credentials sat in the store. The TLV construction and the pending-dataset write live in python-otbr-api 2.11.0 (set_pending_dataset_tlvs, Timestamp.from_values, DelayTimer.from_milliseconds); the integration validates, compares and stamps. Assisted-By: Claude Fable 5 --- homeassistant/components/otbr/__init__.py | 2 + homeassistant/components/otbr/config_flow.py | 11 + homeassistant/components/otbr/icons.json | 7 + homeassistant/components/otbr/manifest.json | 2 +- homeassistant/components/otbr/services.py | 332 +++++++++ homeassistant/components/otbr/services.yaml | 21 + .../components/otbr/silabs_multiprotocol.py | 19 +- homeassistant/components/otbr/strings.json | 52 ++ homeassistant/components/otbr/util.py | 25 + .../components/otbr/websocket_api.py | 174 ++--- homeassistant/components/thread/manifest.json | 2 +- requirements_all.txt | 2 +- tests/components/otbr/test_services.py | 639 ++++++++++++++++++ 13 files changed, 1195 insertions(+), 93 deletions(-) create mode 100644 homeassistant/components/otbr/icons.json create mode 100644 homeassistant/components/otbr/services.py create mode 100644 homeassistant/components/otbr/services.yaml create mode 100644 tests/components/otbr/test_services.py diff --git a/homeassistant/components/otbr/__init__.py b/homeassistant/components/otbr/__init__.py index 38c0bcc4aaee25..bab4f98442384a 100644 --- a/homeassistant/components/otbr/__init__.py +++ b/homeassistant/components/otbr/__init__.py @@ -18,6 +18,7 @@ from . import homeassistant_hardware, websocket_api from .const import DOMAIN +from .services import async_setup_services from .types import OTBRConfigEntry from .util import ( GetBorderAgentIdNotSupported, @@ -34,6 +35,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: """Set up the Open Thread Border Router component.""" websocket_api.async_setup(hass) + async_setup_services(hass) async_register_firmware_info_provider(hass, DOMAIN, homeassistant_hardware) diff --git a/homeassistant/components/otbr/config_flow.py b/homeassistant/components/otbr/config_flow.py index 9f6c6151874207..72a7b5211e96f5 100644 --- a/homeassistant/components/otbr/config_flow.py +++ b/homeassistant/components/otbr/config_flow.py @@ -29,6 +29,7 @@ from .const import DEFAULT_CHANNEL, DOMAIN from .util import ( + DATASET_LOCK, compose_default_network_name, generate_random_pan_id, get_allowed_channel, @@ -87,6 +88,16 @@ class OTBRConfigFlow(ConfigFlow, domain=DOMAIN): async def _set_dataset(self, api: python_otbr_api.OTBR, otbr_url: str) -> None: """Connect to the OTBR and create or apply a dataset if it doesn't have one.""" + # Held across the read and the write: the preferred dataset this adopts + # can be repointed by a network migration, and a router provisioned from + # the old pointer would come up on the network everything else just left. + async with DATASET_LOCK: + await self._async_set_dataset(api, otbr_url) + + async def _async_set_dataset( + self, api: python_otbr_api.OTBR, otbr_url: str + ) -> None: + """Create or apply a dataset. The caller holds DATASET_LOCK.""" if await api.get_active_dataset_tlvs() is None: allowed_channel = await get_allowed_channel(self.hass, otbr_url) diff --git a/homeassistant/components/otbr/icons.json b/homeassistant/components/otbr/icons.json new file mode 100644 index 00000000000000..195ed1c5ee2d04 --- /dev/null +++ b/homeassistant/components/otbr/icons.json @@ -0,0 +1,7 @@ +{ + "services": { + "migrate_network": { + "service": "mdi:swap-horizontal-variant" + } + } +} diff --git a/homeassistant/components/otbr/manifest.json b/homeassistant/components/otbr/manifest.json index 1f10ce2456d2b8..9678fd9e0c946d 100644 --- a/homeassistant/components/otbr/manifest.json +++ b/homeassistant/components/otbr/manifest.json @@ -8,5 +8,5 @@ "documentation": "https://www.home-assistant.io/integrations/otbr", "integration_type": "service", "iot_class": "local_polling", - "requirements": ["python-otbr-api==2.10.0"] + "requirements": ["python-otbr-api==3.0.0"] } diff --git a/homeassistant/components/otbr/services.py b/homeassistant/components/otbr/services.py new file mode 100644 index 00000000000000..9427c44689e1b7 --- /dev/null +++ b/homeassistant/components/otbr/services.py @@ -0,0 +1,332 @@ +"""Actions for the Open Thread Border Router integration.""" + +from typing import TYPE_CHECKING, Any + +from python_otbr_api import PENDING_DATASET_DELAY_TIMER, tlv_parser +from python_otbr_api.tlv_parser import MeshcopTLVType +import voluptuous as vol + +from homeassistant.components.thread import ( + async_add_dataset, + async_get_preferred_dataset, +) +from homeassistant.components.thread.dataset_store import async_get_store +from homeassistant.core import HomeAssistant, ServiceCall, SupportsResponse, callback +from homeassistant.exceptions import HomeAssistantError, ServiceValidationError +from homeassistant.helpers import config_validation as cv, service +from homeassistant.helpers.selector import ConfigEntrySelector + +from .const import DOMAIN +from .util import DATASET_LOCK, get_allowed_channel, update_issues + +if TYPE_CHECKING: + from .types import OTBRConfigEntry + +SERVICE_MIGRATE_NETWORK = "migrate_network" + +ATTR_CONFIG_ENTRY = "config_entry" +ATTR_DATASET = "dataset" +ATTR_DELAY = "delay" + +# The delay recommended for a channel change; a network change needs the +# same grace for sleepy devices to hear about it. +DEFAULT_DELAY_S = PENDING_DATASET_DELAY_TIMER // 1000 + +# The pending dataset is merged by the router over a base it chooses: an +# in-flight pending dataset, or a freshly generated random network. Any +# field left out here would come from that base, so a partial dataset +# would migrate the mesh onto settings nobody picked. Require the full +# set of network-defining fields instead. +_REQUIRED_DATASET_TLVS = ( + MeshcopTLVType.CHANNEL, + MeshcopTLVType.CHANNELMASK, + MeshcopTLVType.EXTPANID, + MeshcopTLVType.MESHLOCALPREFIX, + MeshcopTLVType.NETWORKKEY, + MeshcopTLVType.NETWORKNAME, + MeshcopTLVType.PANID, + MeshcopTLVType.PSKC, + MeshcopTLVType.SECURITYPOLICY, +) + +SERVICE_MIGRATE_NETWORK_SCHEMA = vol.Schema( + { + vol.Optional(ATTR_CONFIG_ENTRY): ConfigEntrySelector({"integration": DOMAIN}), + vol.Optional(ATTR_DATASET): cv.string, + vol.Optional(ATTR_DELAY, default=DEFAULT_DELAY_S): vol.All( + vol.Coerce(int), vol.Range(min=30, max=3600) + ), + } +) + + +def _timestamp_parts( + entries: dict[MeshcopTLVType | int, tlv_parser.MeshcopTLVItem], + tag: MeshcopTLVType, +) -> tuple[int, int]: + """Return the (seconds, ticks) of the timestamp under tag, (0, 0) when absent. + + Thread orders timestamps by the pair, and so does the dataset store, so + comparing seconds alone would miss a dataset that is newer by ticks. + """ + item = entries.get(tag) + if isinstance(item, tlv_parser.Timestamp): + return (item.seconds, item.ticks) + return (0, 0) + + +async def _target_dataset(call: ServiceCall) -> bytes: + """Return the dataset named by the call, or the preferred one.""" + # Presence, not truthiness: an empty dataset is a caller mistake (a + # template that resolved to nothing), not a request for the default. + if (dataset_hex := call.data.get(ATTR_DATASET)) is not None: + try: + dataset = bytes.fromhex(dataset_hex) + except ValueError as err: + raise ServiceValidationError( + translation_domain=DOMAIN, translation_key="invalid_dataset" + ) from err + if dataset: + return dataset + raise ServiceValidationError( + translation_domain=DOMAIN, translation_key="invalid_dataset" + ) + preferred = await async_get_preferred_dataset(call.hass) + if preferred is None: + raise ServiceValidationError( + translation_domain=DOMAIN, translation_key="no_preferred_dataset" + ) + return bytes.fromhex(preferred) + + +def _same_network_settings( + active: dict[MeshcopTLVType | int, tlv_parser.MeshcopTLVItem], + target: dict[MeshcopTLVType | int, tlv_parser.MeshcopTLVItem], +) -> bool: + """Return whether both datasets describe the same network settings. + + The timestamps say when a dataset was made, not what it configures, and + this one is re-stamped before it is sent, so they are left out of the + comparison. + """ + ignored = { + MeshcopTLVType.ACTIVETIMESTAMP, + MeshcopTLVType.PENDINGTIMESTAMP, + MeshcopTLVType.DELAYTIMER, + } + return {k: v.data for k, v in active.items() if k not in ignored} == { + k: v.data for k, v in target.items() if k not in ignored + } + + +async def _async_repoint_preferred_dataset( + hass: HomeAssistant, source_extended_pan_id: str, target_extended_pan_id: str +) -> None: + """Move the preferred dataset along with a migration away from it. + + Everything that hands out Thread credentials (the config flow, HomeKit + bridges, the Thread panel) starts from the preferred dataset. Leaving + it on the abandoned network would keep sharing credentials no network + runs any more - and this action's own no-dataset default would migrate + a router back onto them. + + When no preference exists yet the target becomes it. The store picks a + preference on its own when a router's first dataset arrives and it finds + that router alone on its network, after a discovery wait; a migration + started inside that wait would otherwise see the abandoned network + chosen once it ends, with the same consequences. + """ + store = await async_get_store(hass) + source_id = None + target_id = None + # Two independent matches: a credential rotation keeps the network, so + # source and target are the same entry and must both resolve to it. + for entry in store.datasets.values(): + if entry.extended_pan_id.lower() == source_extended_pan_id.lower(): + source_id = entry.id + if entry.extended_pan_id.lower() == target_extended_pan_id.lower(): + target_id = entry.id + # With no source entry -- a router re-provisioned by another controller + # runs a network the store never saw -- the promotion still applies: + # the membership test then only matches a missing preference. + if target_id and store.preferred_dataset in (source_id, None): + store.preferred_dataset = target_id + + +async def _async_migrate_network(call: ServiceCall) -> dict[str, Any]: + """Migrate a border router and every device on its network. + + The target dataset is re-stamped newer than the network being left, so + it wins dataset propagation, and handed to the router as a pending + dataset with a delay. The router spreads it; the network switches as + one when the delay expires. + """ + entry: OTBRConfigEntry = service.async_get_config_entry( + call.hass, DOMAIN, call.data.get(ATTR_CONFIG_ENTRY) + ) + data = entry.runtime_data + delay: int = call.data[ATTR_DELAY] + + async with DATASET_LOCK: + # Resolved under the lock: a queued no-dataset call must see the + # preferred dataset as repointed by the migration it waited for, + # or it would migrate the router straight back. + dataset = await _target_dataset(call) + + try: + target = tlv_parser.parse_tlv(dataset.hex()) + except tlv_parser.TLVError as err: + raise ServiceValidationError( + translation_domain=DOMAIN, translation_key="invalid_dataset" + ) from err + if missing := [tag.name for tag in _REQUIRED_DATASET_TLVS if tag not in target]: + raise ServiceValidationError( + translation_domain=DOMAIN, + translation_key="incomplete_dataset", + translation_placeholders={"missing": ", ".join(missing)}, + ) + + active_tlvs = await data.get_active_dataset_tlvs() + if active_tlvs is None: + # An unprovisioned router has no network to migrate; joining one + # is what the existing configuration flows are for. + raise ServiceValidationError( + translation_domain=DOMAIN, translation_key="no_active_network" + ) + try: + active = tlv_parser.parse_tlv(active_tlvs.hex()) + except tlv_parser.TLVError as err: + raise HomeAssistantError( + translation_domain=DOMAIN, translation_key="router_dataset_invalid" + ) from err + + # A pending dataset in flight means the mesh is mid-change: a + # migration or a channel change is propagating, and every device + # holding that dataset is counting down towards it. Superseding it + # would race those timers -- a late replacement splits the mesh -- + # and would silently undo the change the user may not even know is + # queued. Refuse and say so; the library's own guard on the write + # backstops the race where one appears after this read. + if await data.get_pending_dataset_tlvs() is not None: + raise HomeAssistantError( + translation_domain=DOMAIN, translation_key="pending_dataset_in_place" + ) + + # Only an identical dataset is a no-op. Comparing the extended PAN + # ID alone would silently ignore a dataset that keeps the network + # but replaces its credentials, which is how a network key is + # rotated. + if _same_network_settings(active, target): + return {"status": "already_on_network"} + + # A different radio (like Zigbee in multiprotocol setups) may pin + # the channel; refuse a migration that would move off it, as a + # channel change does. The completeness check above guarantees the + # target names its channel, so the guard cannot be skipped. + channel_item = target[MeshcopTLVType.CHANNEL] + if TYPE_CHECKING: + assert isinstance(channel_item, tlv_parser.Channel) + target_channel = channel_item.channel + allowed_channel = await get_allowed_channel(call.hass, entry.data["url"]) + if allowed_channel and target_channel != allowed_channel: + raise ServiceValidationError( + translation_domain=DOMAIN, + translation_key="channel_conflict", + translation_placeholders={ + "target_channel": str(target_channel), + "allowed_channel": str(allowed_channel), + }, + ) + + # The pending dataset must carry timestamps newer than the network + # being left: a not-newer pending dataset is silently ignored by + # the mesh while this action would still report success. + newest = max( + _timestamp_parts(active, MeshcopTLVType.ACTIVETIMESTAMP), + _timestamp_parts(target, MeshcopTLVType.ACTIVETIMESTAMP), + ) + # The dataset store silently keeps an existing entry for the same + # extended PAN ID unless the update is newer, so stamp above the + # stored dataset too - or the mesh would migrate while the store + # kept (and the preferred pointer shared) the old credentials. + store = await async_get_store(call.hass) + target_xpan = str(target[MeshcopTLVType.EXTPANID]).lower() + for entry_ in store.datasets.values(): + if entry_.extended_pan_id.lower() == target_xpan: + newest = max( + newest, + _timestamp_parts(entry_.dataset, MeshcopTLVType.ACTIVETIMESTAMP), + ) + # Always step the seconds, never the ticks: python_otbr_api's channel + # change stamps seconds + 1 and ignores ticks, so a network left at the + # last representable second would wrap that write to zero and have the + # mesh ignore every later channel change. + newest_seconds = newest[0] + if newest_seconds >= 2**48 - 1: + raise HomeAssistantError( + translation_domain=DOMAIN, translation_key="timestamp_exhausted" + ) + seconds = newest_seconds + 1 + + pending = dict(target) + pending[MeshcopTLVType.ACTIVETIMESTAMP] = tlv_parser.Timestamp.from_values( + MeshcopTLVType.ACTIVETIMESTAMP, seconds=seconds + ) + pending[MeshcopTLVType.PENDINGTIMESTAMP] = tlv_parser.Timestamp.from_values( + MeshcopTLVType.PENDINGTIMESTAMP, seconds=seconds + ) + pending[MeshcopTLVType.DELAYTIMER] = tlv_parser.DelayTimer.from_milliseconds( + delay * 1000 + ) + + # Fetched before the write: a failure here must abort the action + # before the mesh starts migrating, not after. + border_agent_id = (await data.get_border_agent_id()).hex() + extended_address = (await data.get_extended_address()).hex() + + await data.set_pending_dataset_tlvs( + bytes.fromhex(tlv_parser.encode_tlv(pending)) + ) + + # What the network will run after the delay is the re-stamped + # dataset; record it so Home Assistant's view of the network stays + # current, bound to this router the same way setup binds datasets. + del pending[MeshcopTLVType.PENDINGTIMESTAMP] + del pending[MeshcopTLVType.DELAYTIMER] + migrated_tlvs = bytes.fromhex(tlv_parser.encode_tlv(pending)) + await async_add_dataset( + call.hass, + DOMAIN, + migrated_tlvs.hex(), + preferred_border_agent_id=border_agent_id, + preferred_extended_address=extended_address, + ) + # The repair issues describe the credentials the network is adopting, + # the same way the create and set-network paths report them. + await update_issues(call.hass, data, migrated_tlvs) + if (source_xpan := active.get(MeshcopTLVType.EXTPANID)) is not None: + await _async_repoint_preferred_dataset( + call.hass, str(source_xpan), str(target[MeshcopTLVType.EXTPANID]) + ) + + name_item = pending[MeshcopTLVType.NETWORKNAME] + if TYPE_CHECKING: + assert isinstance(name_item, tlv_parser.NetworkName) + return { + "status": "migrating", + "delay": delay, + "network_name": name_item.name, + } + + +@callback +def async_setup_services(hass: HomeAssistant) -> None: + """Register the actions of the integration.""" + service.async_register_admin_service( + hass, + DOMAIN, + SERVICE_MIGRATE_NETWORK, + _async_migrate_network, + schema=SERVICE_MIGRATE_NETWORK_SCHEMA, + supports_response=SupportsResponse.OPTIONAL, + ) diff --git a/homeassistant/components/otbr/services.yaml b/homeassistant/components/otbr/services.yaml new file mode 100644 index 00000000000000..93b339babaecde --- /dev/null +++ b/homeassistant/components/otbr/services.yaml @@ -0,0 +1,21 @@ +migrate_network: + fields: + config_entry: + required: false + selector: + config_entry: + integration: otbr + dataset: + required: false + selector: + text: + type: password + delay: + required: false + default: 300 + selector: + number: + min: 30 + max: 3600 + unit_of_measurement: seconds + mode: box diff --git a/homeassistant/components/otbr/silabs_multiprotocol.py b/homeassistant/components/otbr/silabs_multiprotocol.py index dde80bf103b1d9..867f57ec450cae 100644 --- a/homeassistant/components/otbr/silabs_multiprotocol.py +++ b/homeassistant/components/otbr/silabs_multiprotocol.py @@ -17,7 +17,7 @@ from homeassistant.exceptions import HomeAssistantError from .const import DOMAIN -from .util import OTBRData +from .util import DATASET_LOCK, OTBRData if TYPE_CHECKING: from . import OTBRConfigEntry @@ -70,13 +70,16 @@ async def async_change_channel( Does nothing if not configured. """ - await data.set_channel(channel, delay) - - # Import the new dataset - dataset_tlvs = await data.get_pending_dataset_tlvs() - if dataset_tlvs is None: - # The activation timer may have expired already - dataset_tlvs = await data.get_active_dataset_tlvs() + # Held across the write and the read-back, so the dataset imported below + # is the one this call created. + async with DATASET_LOCK: + await data.set_channel(channel, delay) + + # Import the new dataset + dataset_tlvs = await data.get_pending_dataset_tlvs() + if dataset_tlvs is None: + # The activation timer may have expired already + dataset_tlvs = await data.get_active_dataset_tlvs() if dataset_tlvs is None: # Don't try to import a None dataset return diff --git a/homeassistant/components/otbr/strings.json b/homeassistant/components/otbr/strings.json index c03d42a6c33405..5d89d683e4f8ad 100644 --- a/homeassistant/components/otbr/strings.json +++ b/homeassistant/components/otbr/strings.json @@ -17,6 +17,38 @@ } } }, + "exceptions": { + "channel_conflict": { + "message": "The target network uses channel {target_channel}, but another radio pins the Thread channel to {allowed_channel}." + }, + "incomplete_dataset": { + "message": "The dataset must describe the target network completely; it is missing {missing}. A partial dataset would be filled in by the border router with settings nobody picked." + }, + "invalid_dataset": { + "message": "The dataset must be Thread operational dataset TLVs in hex." + }, + "no_active_network": { + "message": "The border router has no active network to migrate; join it to a network first." + }, + "no_preferred_dataset": { + "message": "No dataset was given and no preferred Thread network is configured." + }, + "pending_dataset_in_place": { + "message": "A pending dataset is already in place: a migration or channel change is propagating through the mesh. Wait for it to complete, then try again." + }, + "pinned_router_unreachable": { + "message": "The border router {router} pins the Thread channel for another radio, but could not be read to check whether it is on this network. Try again once it is reachable." + }, + "preferred_dataset_changed": { + "message": "The preferred Thread network changed while the border router was being read, so nothing was migrated. Try again." + }, + "router_dataset_invalid": { + "message": "The border router returned a dataset that could not be parsed." + }, + "timestamp_exhausted": { + "message": "The network's dataset timestamp cannot be increased any further." + } + }, "issues": { "get_get_border_agent_id_unsupported": { "description": "Your OTBR does not support Border Agent ID.\n\nTo fix this issue, update the OTBR to the latest version and restart Home Assistant.\nIf you are using an OTBR integrated in Home Assistant, update either the OpenThread Border Router app or the Silicon Labs Multiprotocol app. Otherwise update your self-managed OTBR.", @@ -30,5 +62,25 @@ "description": "When OTBR and ZHA share the radio, they must use the same network channel.\n\nIf OTBR and ZHA attempt to connect to networks on different channels, neither Thread/Matter nor Zigbee will work.\n\nOTBR is configured with a Thread network on channel {otbr_channel}, ZHA is configured with a Zigbee network on channel {zha_channel}.", "title": "OTBR and ZHA share the same radio but use different channels" } + }, + "services": { + "migrate_network": { + "description": "Moves the border router and every device on its Thread network onto another network, using a pending dataset that all devices apply together after a delay. Thread never goes down, and devices that hear about the change do not need to be re-joined; a device powered off for the whole delay misses it and may need to be re-joined. Warning: this migrates the entire network, including devices commissioned by other systems.", + "fields": { + "config_entry": { + "description": "The border router to migrate. Needed only when more than one is set up.", + "name": "Border router" + }, + "dataset": { + "description": "Thread operational dataset of the target network as TLVs in hex. Defaults to the preferred network from the Thread settings.", + "name": "Dataset" + }, + "delay": { + "description": "How long devices wait before switching, giving sleepy devices time to hear about the change.", + "name": "Delay" + } + }, + "name": "Migrate Thread network" + } } } diff --git a/homeassistant/components/otbr/util.py b/homeassistant/components/otbr/util.py index bdd66a9d3625c2..e45e8749132c48 100644 --- a/homeassistant/components/otbr/util.py +++ b/homeassistant/components/otbr/util.py @@ -1,5 +1,6 @@ """Utility functions for the Open Thread Border Router integration.""" +import asyncio from collections.abc import Callable, Coroutine import dataclasses from functools import wraps @@ -31,6 +32,14 @@ _LOGGER = logging.getLogger(__name__) +# Serializes dataset mutations across all config entries. Acquired by callers +# rather than by the OTBRData methods, so a sequence that reads the router's +# state and writes it back stays atomic: concurrent writers would otherwise +# work from state the other has already replaced, and the mesh silently +# ignores whichever pending dataset is not the newest while its writer still +# reports success. +DATASET_LOCK = asyncio.Lock() + INSECURE_NETWORK_KEYS = ( # Thread web UI default bytes.fromhex("00112233445566778899AABBCCDDEEFF"), @@ -143,6 +152,22 @@ async def set_active_dataset_tlvs(self, dataset: bytes) -> None: """Set current active operational dataset in TLVS format.""" await self.api.set_active_dataset_tlvs(dataset) + @_handle_otbr_error + async def set_pending_dataset_tlvs(self, dataset: bytes) -> None: + """Set the pending operational dataset in TLVS format. + + Refused while a pending dataset is in place. The refusal is surfaced + as its own error: nothing was written, and the caller's answer is to + wait out the in-flight change, not to retry. + """ + try: + await self.api.set_pending_dataset_tlvs(dataset) + except python_otbr_api.PendingDatasetConflictError as exc: + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key="pending_dataset_in_place", + ) from exc + @_handle_otbr_error async def set_channel( self, channel: int, delay: float = PENDING_DATASET_DELAY_TIMER / 1000 diff --git a/homeassistant/components/otbr/websocket_api.py b/homeassistant/components/otbr/websocket_api.py index 2bcd0da8f16c50..ea90b1e12826c4 100644 --- a/homeassistant/components/otbr/websocket_api.py +++ b/homeassistant/components/otbr/websocket_api.py @@ -19,6 +19,7 @@ from .const import DEFAULT_CHANNEL, DOMAIN from .util import ( + DATASET_LOCK, OTBRData, compose_default_network_name, generate_random_pan_id, @@ -149,54 +150,57 @@ async def websocket_create_network( data: OTBRData, ) -> None: """Create a new Thread network.""" - channel = await get_allowed_channel(hass, data.url) or DEFAULT_CHANNEL + # Held from the first read: the channel this picks and the dataset it + # creates must not be decided from state another writer is replacing. + async with DATASET_LOCK: + channel = await get_allowed_channel(hass, data.url) or DEFAULT_CHANNEL - try: - await data.set_enabled(False) - except HomeAssistantError as exc: - connection.send_error(msg["id"], "set_enabled_failed", str(exc)) - return + try: + await data.set_enabled(False) + except HomeAssistantError as exc: + connection.send_error(msg["id"], "set_enabled_failed", str(exc)) + return - try: - await data.factory_reset(hass) - except HomeAssistantError as exc: - connection.send_error(msg["id"], "factory_reset_failed", str(exc)) - return + try: + await data.factory_reset(hass) + except HomeAssistantError as exc: + connection.send_error(msg["id"], "factory_reset_failed", str(exc)) + return - pan_id = generate_random_pan_id() - try: - await data.create_active_dataset( - python_otbr_api.ActiveDataSet( - channel=channel, - network_name=compose_default_network_name(pan_id), - pan_id=pan_id, + pan_id = generate_random_pan_id() + try: + await data.create_active_dataset( + python_otbr_api.ActiveDataSet( + channel=channel, + network_name=compose_default_network_name(pan_id), + pan_id=pan_id, + ) ) - ) - except HomeAssistantError as exc: - connection.send_error(msg["id"], "create_active_dataset_failed", str(exc)) - return + except HomeAssistantError as exc: + connection.send_error(msg["id"], "create_active_dataset_failed", str(exc)) + return - try: - await data.set_enabled(True) - except HomeAssistantError as exc: - connection.send_error(msg["id"], "set_enabled_failed", str(exc)) - return + try: + await data.set_enabled(True) + except HomeAssistantError as exc: + connection.send_error(msg["id"], "set_enabled_failed", str(exc)) + return - try: - dataset_tlvs = await data.get_active_dataset_tlvs() - except HomeAssistantError as exc: - connection.send_error(msg["id"], "get_active_dataset_tlvs_failed", str(exc)) - return - if not dataset_tlvs: - connection.send_error(msg["id"], "get_active_dataset_tlvs_empty", "") - return + try: + dataset_tlvs = await data.get_active_dataset_tlvs() + except HomeAssistantError as exc: + connection.send_error(msg["id"], "get_active_dataset_tlvs_failed", str(exc)) + return + if not dataset_tlvs: + connection.send_error(msg["id"], "get_active_dataset_tlvs_empty", "") + return - await async_add_dataset(hass, DOMAIN, dataset_tlvs.hex()) + await async_add_dataset(hass, DOMAIN, dataset_tlvs.hex()) - # Update repair issues - await update_issues(hass, data, dataset_tlvs) + # Update repair issues + await update_issues(hass, data, dataset_tlvs) - connection.send_result(msg["id"]) + connection.send_result(msg["id"]) @websocket_api.websocket_command( @@ -216,48 +220,52 @@ async def websocket_set_network( data: OTBRData, ) -> None: """Set the Thread network to be used by the OTBR.""" - dataset_tlv = await async_get_dataset(hass, msg["dataset_id"]) - - if not dataset_tlv: - connection.send_error(msg["id"], "unknown_dataset", "Unknown dataset") - return - dataset = tlv_parser.parse_tlv(dataset_tlv) - if channel := dataset.get(MeshcopTLVType.CHANNEL): - thread_dataset_channel = cast(tlv_parser.Channel, channel).channel - - allowed_channel = await get_allowed_channel(hass, data.url) - - if allowed_channel and thread_dataset_channel != allowed_channel: - connection.send_error( - msg["id"], - "channel_conflict", - f"Can't connect to network on channel {thread_dataset_channel}, ZHA is " - f"using channel {allowed_channel}", - ) - return + # Held from the first read: the dataset read here is what gets written + # below, so a concurrent writer must not replace it in between -- a key + # rotation landing in that window would push the superseded credentials. + async with DATASET_LOCK: + dataset_tlv = await async_get_dataset(hass, msg["dataset_id"]) + + if not dataset_tlv: + connection.send_error(msg["id"], "unknown_dataset", "Unknown dataset") + return + dataset = tlv_parser.parse_tlv(dataset_tlv) + if channel := dataset.get(MeshcopTLVType.CHANNEL): + thread_dataset_channel = cast(tlv_parser.Channel, channel).channel + + allowed_channel = await get_allowed_channel(hass, data.url) + + if allowed_channel and thread_dataset_channel != allowed_channel: + connection.send_error( + msg["id"], + "channel_conflict", + f"Can't connect to network on channel {thread_dataset_channel}, ZHA is " + f"using channel {allowed_channel}", + ) + return - try: - await data.set_enabled(False) - except HomeAssistantError as exc: - connection.send_error(msg["id"], "set_enabled_failed", str(exc)) - return + try: + await data.set_enabled(False) + except HomeAssistantError as exc: + connection.send_error(msg["id"], "set_enabled_failed", str(exc)) + return - try: - await data.set_active_dataset_tlvs(bytes.fromhex(dataset_tlv)) - except HomeAssistantError as exc: - connection.send_error(msg["id"], "set_active_dataset_tlvs_failed", str(exc)) - return + try: + await data.set_active_dataset_tlvs(bytes.fromhex(dataset_tlv)) + except HomeAssistantError as exc: + connection.send_error(msg["id"], "set_active_dataset_tlvs_failed", str(exc)) + return - try: - await data.set_enabled(True) - except HomeAssistantError as exc: - connection.send_error(msg["id"], "set_enabled_failed", str(exc)) - return + try: + await data.set_enabled(True) + except HomeAssistantError as exc: + connection.send_error(msg["id"], "set_enabled_failed", str(exc)) + return - # Update repair issues - await update_issues(hass, data, bytes.fromhex(dataset_tlv)) + # Update repair issues + await update_issues(hass, data, bytes.fromhex(dataset_tlv)) - connection.send_result(msg["id"]) + connection.send_result(msg["id"]) @websocket_api.websocket_command( @@ -288,10 +296,12 @@ async def websocket_set_channel( channel: int = msg["channel"] delay: float = PENDING_DATASET_DELAY_TIMER / 1000 - try: - await data.set_channel(channel) - except HomeAssistantError as exc: - connection.send_error(msg["id"], "set_channel_failed", str(exc)) - return + # Serialized against other dataset writers; a channel change is a pending-dataset write. + async with DATASET_LOCK: + try: + await data.set_channel(channel) + except HomeAssistantError as exc: + connection.send_error(msg["id"], "set_channel_failed", str(exc)) + return - connection.send_result(msg["id"], {"delay": delay}) + connection.send_result(msg["id"], {"delay": delay}) diff --git a/homeassistant/components/thread/manifest.json b/homeassistant/components/thread/manifest.json index 4daa49dc360a76..fdce5ebdbe4867 100644 --- a/homeassistant/components/thread/manifest.json +++ b/homeassistant/components/thread/manifest.json @@ -7,7 +7,7 @@ "documentation": "https://www.home-assistant.io/integrations/thread", "integration_type": "service", "iot_class": "local_polling", - "requirements": ["python-otbr-api==2.10.0", "pyroute2==0.9.6"], + "requirements": ["python-otbr-api==3.0.0", "pyroute2==0.9.6"], "single_config_entry": true, "zeroconf": ["_meshcop._udp.local."] } diff --git a/requirements_all.txt b/requirements_all.txt index 55119bdec4cb45..2cbb3abc642a03 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -2778,7 +2778,7 @@ python-opensky==1.0.1 # homeassistant.components.otbr # homeassistant.components.thread -python-otbr-api==2.10.0 +python-otbr-api==3.0.0 # homeassistant.components.overseerr python-overseerr==0.9.0 diff --git a/tests/components/otbr/test_services.py b/tests/components/otbr/test_services.py new file mode 100644 index 00000000000000..010772e30c297e --- /dev/null +++ b/tests/components/otbr/test_services.py @@ -0,0 +1,639 @@ +"""Test the Open Thread Border Router actions.""" + +import asyncio +from http import HTTPStatus +import re +from unittest.mock import AsyncMock, Mock, patch + +import pytest +from python_otbr_api import tlv_parser +from python_otbr_api.tlv_parser import DelayTimer, MeshcopTLVType, Timestamp + +from homeassistant.components.otbr import ( + silabs_multiprotocol as otbr_silabs_multiprotocol, +) +from homeassistant.components.otbr.util import DATASET_LOCK, INSECURE_NETWORK_KEYS +from homeassistant.components.thread import async_add_dataset +from homeassistant.components.thread.dataset_store import async_get_store +from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError, ServiceValidationError +from homeassistant.helpers import issue_registry as ir + +from . import BASE_URL, DATASET_CH16 + +from tests.test_util.aiohttp import AiohttpClientMocker + +# A different network from the one under test: ts 1003, channel 15. Carries +# every network-defining TLV plus a wakeup-channel TLV (0x4a) to prove +# unrelated fields survive the migration untouched. +TARGET = ( + "0e080000000003eb0000000300000f4a0300001035060004001fffe002081111111122222222" + "0708fd111111222222220510aaaaaaaaaaaaaaaabbbbbbbbbbbbbbbb030f4f70656e546872" + "6561642048412032010212340410ccccccccccccccccdddddddddddddddd0c0402a0f7f8" +) + + +pytestmark = pytest.mark.usefixtures("multiprotocol_addon_manager_mock") + + +async def call_migrate(hass: HomeAssistant, **data) -> dict: + """Invoke the migrate_network action.""" + return await hass.services.async_call( + "otbr", + "migrate_network", + data, + blocking=True, + return_response=True, + ) + + +def expected_pending(target_hex: str, seconds: int, delay_ms: int) -> dict: + """Return the dataset the router must receive for a migration.""" + expected = tlv_parser.parse_tlv(target_hex) + expected[MeshcopTLVType.ACTIVETIMESTAMP] = Timestamp.from_values( + MeshcopTLVType.ACTIVETIMESTAMP, seconds=seconds + ) + expected[MeshcopTLVType.PENDINGTIMESTAMP] = Timestamp.from_values( + MeshcopTLVType.PENDINGTIMESTAMP, seconds=seconds + ) + expected[MeshcopTLVType.DELAYTIMER] = DelayTimer.from_milliseconds(delay_ms) + return expected + + +def mock_pending_endpoint( + aioclient_mock: AiohttpClientMocker, + in_flight: str | None = None, + put_status: HTTPStatus = HTTPStatus.CREATED, +) -> None: + """Mock the router's pending-dataset endpoint.""" + aioclient_mock.clear_requests() + aioclient_mock.get(re.compile(r".*/api/actions$"), status=HTTPStatus.OK) + if in_flight is None: + aioclient_mock.get( + f"{BASE_URL}/node/dataset/pending", status=HTTPStatus.NO_CONTENT + ) + else: + aioclient_mock.get(f"{BASE_URL}/node/dataset/pending", text=in_flight) + aioclient_mock.put(f"{BASE_URL}/node/dataset/pending", status=put_status) + + +def pending_calls(aioclient_mock: AiohttpClientMocker) -> list: + """Return the PUT calls the router received.""" + return [call for call in aioclient_mock.mock_calls if call[0] == "PUT"] + + +async def test_network_is_migrated( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """The pending dataset is the target network, only re-stamped.""" + mock_pending_endpoint(aioclient_mock) + + response = await call_migrate(hass, dataset=TARGET) + + assert response == { + "status": "migrating", + "delay": 300, + "network_name": "OpenThread HA 2", + } + + # Newer than both the network being left (ts 1) and the target (ts 1003), + # and taken verbatim otherwise - including the wakeup-channel TLV. + puts = pending_calls(aioclient_mock) + assert len(puts) == 1 + assert tlv_parser.parse_tlv(puts[0][2]) == expected_pending(TARGET, 1004, 300000) + + # The store learns what the network will run - the re-stamped dataset + # without the pending machinery - bound to this router. + store = await async_get_store(hass) + entries = [ + entry + for entry in store.datasets.values() + if entry.extended_pan_id.lower() == "1111111122222222" + ] + assert len(entries) == 1 + stored = tlv_parser.parse_tlv(entries[0].tlv) + expected = expected_pending(TARGET, 1004, 300000) + del expected[MeshcopTLVType.PENDINGTIMESTAMP] + del expected[MeshcopTLVType.DELAYTIMER] + assert stored == expected + + +async def test_migration_repoints_preferred_dataset( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """Migrating away from the preferred network moves the preferred pointer. + + Everything handing out Thread credentials starts from the preferred + dataset; left behind it would keep sharing a network nobody runs. + """ + mock_pending_endpoint(aioclient_mock) + await async_add_dataset(hass, "test", DATASET_CH16.hex()) + store = await async_get_store(hass) + source_id = next(iter(store.datasets.values())).id + store.preferred_dataset = source_id + + await call_migrate(hass, dataset=TARGET) + + preferred = store.datasets[store.preferred_dataset] + assert preferred.extended_pan_id.lower() == "1111111122222222" + + +async def test_migration_sets_preferred_dataset_when_none_is_chosen( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """Without a preferred network yet, the target becomes it. + + The store picks a preference on its own once a router's first dataset + has been through discovery; a migration started inside that wait must + not see the abandoned network chosen when the wait ends. + """ + mock_pending_endpoint(aioclient_mock) + store = await async_get_store(hass) + assert store.preferred_dataset is None + + await call_migrate(hass, dataset=TARGET) + + preferred = store.datasets[store.preferred_dataset] + assert preferred.extended_pan_id.lower() == "1111111122222222" + + +async def test_migration_sets_preferred_dataset_for_unknown_source( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, + get_active_dataset_tlvs: AsyncMock, +) -> None: + """The promotion also covers a router whose network the store never saw. + + A router re-provisioned by another controller runs a network the store + has no entry for; with no preference chosen yet, the migration target + still becomes it. + """ + mock_pending_endpoint(aioclient_mock) + foreign = dict(tlv_parser.parse_tlv(DATASET_CH16.hex())) + foreign[MeshcopTLVType.EXTPANID] = tlv_parser.MeshcopTLVItem( + MeshcopTLVType.EXTPANID, bytes.fromhex("5555666677778888") + ) + get_active_dataset_tlvs.return_value = bytes.fromhex(tlv_parser.encode_tlv(foreign)) + store = await async_get_store(hass) + assert store.preferred_dataset is None + + await call_migrate(hass, dataset=TARGET) + + preferred = store.datasets[store.preferred_dataset] + assert preferred.extended_pan_id.lower() == "1111111122222222" + + +async def test_migration_leaves_an_unrelated_preference_alone( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """A preference for a third network is not moved by the migration.""" + mock_pending_endpoint(aioclient_mock) + unrelated = dict(tlv_parser.parse_tlv(TARGET)) + unrelated[MeshcopTLVType.EXTPANID] = tlv_parser.MeshcopTLVItem( + MeshcopTLVType.EXTPANID, bytes.fromhex("3333333344444444") + ) + await async_add_dataset(hass, "test", tlv_parser.encode_tlv(unrelated)) + store = await async_get_store(hass) + unrelated_id = next( + entry.id + for entry in store.datasets.values() + if entry.extended_pan_id.lower() == "3333333344444444" + ) + store.preferred_dataset = unrelated_id + + await call_migrate(hass, dataset=TARGET) + + assert store.preferred_dataset == unrelated_id + + +async def test_credentials_are_rotated_on_the_same_network( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """A dataset that keeps the network but replaces its key is a migration.""" + mock_pending_endpoint(aioclient_mock) + + rotated = tlv_parser.parse_tlv(DATASET_CH16.hex()) + rotated[MeshcopTLVType.NETWORKKEY] = tlv_parser.MeshcopTLVItem( + MeshcopTLVType.NETWORKKEY, bytes.fromhex("11111111222222223333333344444444") + ) + rotated_hex = tlv_parser.encode_tlv(rotated) + + response = await call_migrate(hass, dataset=rotated_hex) + + assert response["status"] == "migrating" + puts = pending_calls(aioclient_mock) + assert len(puts) == 1 + # Active and target timestamps are both 1 here. + assert tlv_parser.parse_tlv(puts[0][2]) == expected_pending(rotated_hex, 2, 300000) + + +async def test_rotation_sets_preferred_dataset_when_none_is_chosen( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """A rotation promotes the network when no preference exists yet. + + Source and target are the same network then, so resolving the target + entry must not depend on it differing from the source. + """ + mock_pending_endpoint(aioclient_mock) + + rotated = tlv_parser.parse_tlv(DATASET_CH16.hex()) + rotated[MeshcopTLVType.NETWORKKEY] = tlv_parser.MeshcopTLVItem( + MeshcopTLVType.NETWORKKEY, bytes.fromhex("11111111222222223333333344444444") + ) + store = await async_get_store(hass) + assert store.preferred_dataset is None + + await call_migrate(hass, dataset=tlv_parser.encode_tlv(rotated)) + + preferred = store.datasets[store.preferred_dataset] + assert preferred.extended_pan_id.lower() == ( + rotated[MeshcopTLVType.EXTPANID].data.hex().lower() + ) + + +async def test_migration_refuses_while_a_pending_dataset_is_in_flight( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """A pending dataset in flight refuses the migration outright. + + Superseding it would race the delay timer on every device already + holding it, so a late replacement can split the mesh, and it would + silently undo whatever that dataset was doing. Nothing is written and + nothing is recorded; the user is told to wait it out. + """ + mock_pending_endpoint(aioclient_mock, in_flight=TARGET) + + with pytest.raises(HomeAssistantError) as exc_info: + await call_migrate(hass, dataset=TARGET) + + assert exc_info.value.translation_key == "pending_dataset_in_place" + assert not pending_calls(aioclient_mock) + + +async def test_pending_dataset_appearing_mid_flight_is_surfaced( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """A pending dataset that appears between the read and the write refuses. + + The early check cannot see it, so the refusal comes back from the router + (the If-None-Match precondition) through the library, and must surface + as the same error rather than a generic failure. + """ + mock_pending_endpoint(aioclient_mock, put_status=HTTPStatus.PRECONDITION_FAILED) + + with pytest.raises(HomeAssistantError) as exc_info: + await call_migrate(hass, dataset=TARGET) + + assert exc_info.value.translation_key == "pending_dataset_in_place" + + +async def test_delay_is_applied( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """A non-default delay reaches the router and the response.""" + mock_pending_endpoint(aioclient_mock) + + response = await call_migrate(hass, dataset=TARGET, delay=60) + + assert response["delay"] == 60 + puts = pending_calls(aioclient_mock) + assert tlv_parser.parse_tlv(puts[0][2]) == expected_pending(TARGET, 1004, 60000) + + +async def test_already_on_network( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """Migrating to the network the router already runs does nothing.""" + mock_pending_endpoint(aioclient_mock) + + response = await call_migrate(hass, dataset=DATASET_CH16.hex()) + + assert response == {"status": "already_on_network"} + assert not pending_calls(aioclient_mock) + + +async def test_router_refusal_is_reported( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """A router refusing the pending dataset surfaces as an error.""" + aioclient_mock.clear_requests() + aioclient_mock.get(re.compile(r".*/api/actions$"), status=HTTPStatus.OK) + aioclient_mock.get(f"{BASE_URL}/node/dataset/pending", status=HTTPStatus.NO_CONTENT) + aioclient_mock.put( + f"{BASE_URL}/node/dataset/pending", status=HTTPStatus.BAD_REQUEST + ) + + # Setup recorded the router's own dataset; a failed migration must + # not add or change anything. + store = await async_get_store(hass) + before = dict(store.datasets) + + with pytest.raises(HomeAssistantError): + await call_migrate(hass, dataset=TARGET) + + assert store.datasets == before + + +@pytest.mark.parametrize( + "bad_dataset", + [ + "zz", + # An empty dataset is a mistake, not a request for the default. + "", + # Truncated TLV: NETWORKNAME announcing 14 bytes, carrying 13. + "030e4f70656e54687265616444656d", + ], +) +async def test_invalid_dataset_is_rejected( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, + bad_dataset: str, +) -> None: + """A dataset that does not parse must not be sent anywhere.""" + aioclient_mock.clear_requests() + + with pytest.raises(ServiceValidationError) as exc_info: + await call_migrate(hass, dataset=bad_dataset) + assert exc_info.value.translation_key == "invalid_dataset" + assert not aioclient_mock.mock_calls + + +async def test_incomplete_dataset_is_rejected( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """A partial dataset would be completed by the router with random settings.""" + aioclient_mock.clear_requests() + + # Extended PAN id and network key only. + with pytest.raises(ServiceValidationError) as exc_info: + await call_migrate( + hass, + dataset="020811111111222222220510aaaaaaaaaaaaaaaabbbbbbbbbbbbbbbb", + ) + assert exc_info.value.translation_key == "incomplete_dataset" + assert "NETWORKNAME" in exc_info.value.translation_placeholders["missing"] + assert not aioclient_mock.mock_calls + + +async def test_no_active_network( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + get_active_dataset_tlvs: AsyncMock, + aioclient_mock: AiohttpClientMocker, +) -> None: + """An unprovisioned router has no network to migrate.""" + aioclient_mock.clear_requests() + get_active_dataset_tlvs.return_value = None + + with pytest.raises(ServiceValidationError) as exc_info: + await call_migrate(hass, dataset=TARGET) + assert exc_info.value.translation_key == "no_active_network" + + +async def test_no_preferred_dataset( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """Without a dataset and without a preferred network there is no target.""" + aioclient_mock.clear_requests() + + # Setup stored the router's dataset, but nothing marked one preferred. + store = await async_get_store(hass) + assert store.preferred_dataset is None + + with pytest.raises(ServiceValidationError) as exc_info: + await call_migrate(hass) + assert exc_info.value.translation_key == "no_preferred_dataset" + + +async def test_unknown_config_entry( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """Naming a config entry that does not exist is a validation error.""" + aioclient_mock.clear_requests() + + with pytest.raises(ServiceValidationError): + await call_migrate(hass, dataset=TARGET, config_entry="not-an-entry-id") + + +async def test_config_entry_is_honoured( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """The named config entry is the router that is migrated.""" + mock_pending_endpoint(aioclient_mock) + + response = await call_migrate( + hass, dataset=TARGET, config_entry=otbr_config_entry_multipan + ) + + assert response["status"] == "migrating" + assert len(pending_calls(aioclient_mock)) == 1 + + +async def test_pinned_channel_conflict( + hass: HomeAssistant, + multiprotocol_addon_manager_mock: Mock, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """A migration off a channel another radio pins is refused.""" + aioclient_mock.clear_requests() + aioclient_mock.get(re.compile(r".*/api/actions$"), status=HTTPStatus.OK) + aioclient_mock.get(f"{BASE_URL}/node/dataset/pending", status=HTTPStatus.NO_CONTENT) + multiprotocol_addon_manager_mock.async_get_channel.return_value = 25 + + with pytest.raises(ServiceValidationError) as exc_info: + await call_migrate(hass, dataset=TARGET) + assert exc_info.value.translation_key == "channel_conflict" + assert not pending_calls(aioclient_mock) + + +async def test_default_target_is_preferred_dataset( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """Without a dataset, the preferred Thread network is the target.""" + mock_pending_endpoint(aioclient_mock) + await async_add_dataset(hass, "test", TARGET) + store = await async_get_store(hass) + preferred_id = next( + entry.id + for entry in store.datasets.values() + if entry.extended_pan_id.lower() == "1111111122222222" + ) + store.preferred_dataset = preferred_id + + response = await call_migrate(hass) + + assert response["status"] == "migrating" + puts = pending_calls(aioclient_mock) + assert len(puts) == 1 + assert tlv_parser.parse_tlv(puts[0][2]) == expected_pending(TARGET, 1004, 300000) + + +async def test_targeting_the_current_network_also_refuses_while_pending( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """With a move away queued, re-targeting the current network refuses too. + + It must not be swallowed as "already on network": the mesh is about to + leave it. But refusing is all this action can offer, since a counter + dataset would race the delay timers the same as any other replacement. + """ + mock_pending_endpoint(aioclient_mock, in_flight=TARGET) + + with pytest.raises(HomeAssistantError) as exc_info: + await call_migrate(hass, dataset=DATASET_CH16.hex()) + + assert exc_info.value.translation_key == "pending_dataset_in_place" + assert not pending_calls(aioclient_mock) + + +async def test_concurrent_migrations_are_serialized( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """Overlapping migrations never produce equal timestamps. + + The lock serializes them and the store-aware stamping makes the second + write strictly newer, so neither is silently ignored by the mesh. + """ + mock_pending_endpoint(aioclient_mock) + + r1, r2 = await asyncio.gather( + call_migrate(hass, dataset=TARGET), call_migrate(hass, dataset=TARGET) + ) + + assert r1["status"] == "migrating" + assert r2["status"] == "migrating" + puts = pending_calls(aioclient_mock) + assert len(puts) == 2 + stamps = { + tlv_parser.parse_tlv(p[2])[MeshcopTLVType.ACTIVETIMESTAMP].seconds for p in puts + } + assert stamps == {1004, 1005} + + +async def test_channel_change_waits_for_dataset_lock( + hass: HomeAssistant, + otbr_config_entry_multipan: str, +) -> None: + """A channel change cannot write while a migration holds the lock.""" + with ( + patch("python_otbr_api.OTBR.set_channel") as set_channel, + patch( + "python_otbr_api.OTBR.get_pending_dataset_tlvs", + return_value=DATASET_CH16, + ), + ): + async with DATASET_LOCK: + task = hass.async_create_task( + otbr_silabs_multiprotocol.async_change_channel(hass, 15, delay=300) + ) + for _ in range(5): + await asyncio.sleep(0) + # Blocked on the shared lock before touching the router. + assert not task.done() + set_channel.assert_not_awaited() + + await task + set_channel.assert_awaited_once() + + +async def test_exhausted_seconds_are_refused( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """A timestamp that cannot be stepped by a second is an error, not a wrap.""" + mock_pending_endpoint(aioclient_mock) + maxed = dict(tlv_parser.parse_tlv(TARGET)) + maxed[MeshcopTLVType.ACTIVETIMESTAMP] = Timestamp.from_values( + MeshcopTLVType.ACTIVETIMESTAMP, seconds=2**48 - 1 + ) + + with pytest.raises(HomeAssistantError) as exc_info: + await call_migrate(hass, dataset=tlv_parser.encode_tlv(maxed)) + assert exc_info.value.translation_key == "timestamp_exhausted" + assert not pending_calls(aioclient_mock) + + +async def test_ticks_count_in_the_comparison( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """A dataset newer only by ticks is still out-stamped.""" + mock_pending_endpoint(aioclient_mock) + stored = dict(tlv_parser.parse_tlv(TARGET)) + stored[MeshcopTLVType.ACTIVETIMESTAMP] = Timestamp.from_values( + MeshcopTLVType.ACTIVETIMESTAMP, seconds=1003, ticks=42 + ) + await async_add_dataset(hass, "test", tlv_parser.encode_tlv(stored)) + + await call_migrate(hass, dataset=TARGET) + + stamp = tlv_parser.parse_tlv(pending_calls(aioclient_mock)[0][2])[ + MeshcopTLVType.ACTIVETIMESTAMP + ] + assert (stamp.seconds, stamp.ticks) == (1004, 0) + + +async def test_migration_refreshes_repair_issues( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, + issue_registry: ir.IssueRegistry, +) -> None: + """Migrating onto insecure credentials raises the repair issue for them.""" + mock_pending_endpoint(aioclient_mock) + insecure = dict(tlv_parser.parse_tlv(TARGET)) + insecure[MeshcopTLVType.NETWORKKEY] = tlv_parser.MeshcopTLVItem( + MeshcopTLVType.NETWORKKEY, INSECURE_NETWORK_KEYS[0] + ) + + assert not issue_registry.async_get_issue( + domain="otbr", issue_id=f"insecure_thread_network_{otbr_config_entry_multipan}" + ) + + await call_migrate(hass, dataset=tlv_parser.encode_tlv(insecure)) + + assert issue_registry.async_get_issue( + domain="otbr", issue_id=f"insecure_thread_network_{otbr_config_entry_multipan}" + ) From 55b5cbbdf40fd26ed4322c8a11eaec3d0f856e7a Mon Sep 17 00:00:00 2001 From: Christian Glombek Date: Thu, 6 Aug 2026 04:55:59 +0200 Subject: [PATCH 2/4] Report when a migrated Thread network could not be recorded otbr.migrate_network hands the border router a pending dataset and then records what the network will run. That record can be dropped: the Thread dataset store keeps the newest dataset per network, so another writer storing newer credentials for the same network while the router is being written to wins, and the action would still report a plain success. The store now says which happened, so report the case where Home Assistant's copy of the credentials is not the dataset the mesh is switching to. The mesh cannot be called back, so the repair issues and the preferred dataset are still brought up to date first -- they describe which network is being adopted rather than with which credentials, and the preferred pointer is what this action's own default target reads. Assisted-By: Claude Fable 5 --- homeassistant/components/otbr/config_flow.py | 6 +- homeassistant/components/otbr/services.py | 21 ++++- .../components/otbr/silabs_multiprotocol.py | 4 +- homeassistant/components/otbr/strings.json | 3 + homeassistant/components/otbr/util.py | 28 +++++-- .../components/otbr/websocket_api.py | 8 +- tests/components/otbr/test_services.py | 76 ++++++++++++++++++- 7 files changed, 124 insertions(+), 22 deletions(-) diff --git a/homeassistant/components/otbr/config_flow.py b/homeassistant/components/otbr/config_flow.py index 72a7b5211e96f5..39de74a88cab89 100644 --- a/homeassistant/components/otbr/config_flow.py +++ b/homeassistant/components/otbr/config_flow.py @@ -29,7 +29,7 @@ from .const import DEFAULT_CHANNEL, DOMAIN from .util import ( - DATASET_LOCK, + async_get_dataset_lock, compose_default_network_name, generate_random_pan_id, get_allowed_channel, @@ -91,13 +91,13 @@ async def _set_dataset(self, api: python_otbr_api.OTBR, otbr_url: str) -> None: # Held across the read and the write: the preferred dataset this adopts # can be repointed by a network migration, and a router provisioned from # the old pointer would come up on the network everything else just left. - async with DATASET_LOCK: + async with async_get_dataset_lock(self.hass): await self._async_set_dataset(api, otbr_url) async def _async_set_dataset( self, api: python_otbr_api.OTBR, otbr_url: str ) -> None: - """Create or apply a dataset. The caller holds DATASET_LOCK.""" + """Create or apply a dataset. The caller holds the dataset lock.""" if await api.get_active_dataset_tlvs() is None: allowed_channel = await get_allowed_channel(self.hass, otbr_url) diff --git a/homeassistant/components/otbr/services.py b/homeassistant/components/otbr/services.py index 9427c44689e1b7..4bdcf9b7fe5e48 100644 --- a/homeassistant/components/otbr/services.py +++ b/homeassistant/components/otbr/services.py @@ -7,6 +7,7 @@ import voluptuous as vol from homeassistant.components.thread import ( + DatasetAddResult, async_add_dataset, async_get_preferred_dataset, ) @@ -17,7 +18,7 @@ from homeassistant.helpers.selector import ConfigEntrySelector from .const import DOMAIN -from .util import DATASET_LOCK, get_allowed_channel, update_issues +from .util import async_get_dataset_lock, get_allowed_channel, update_issues if TYPE_CHECKING: from .types import OTBRConfigEntry @@ -167,7 +168,7 @@ async def _async_migrate_network(call: ServiceCall) -> dict[str, Any]: data = entry.runtime_data delay: int = call.data[ATTR_DELAY] - async with DATASET_LOCK: + async with async_get_dataset_lock(call.hass): # Resolved under the lock: a queued no-dataset call must see the # preferred dataset as repointed by the migration it waited for, # or it would migrate the router straight back. @@ -294,7 +295,7 @@ async def _async_migrate_network(call: ServiceCall) -> dict[str, Any]: del pending[MeshcopTLVType.PENDINGTIMESTAMP] del pending[MeshcopTLVType.DELAYTIMER] migrated_tlvs = bytes.fromhex(tlv_parser.encode_tlv(pending)) - await async_add_dataset( + result = await async_add_dataset( call.hass, DOMAIN, migrated_tlvs.hex(), @@ -308,6 +309,20 @@ async def _async_migrate_network(call: ServiceCall) -> dict[str, Any]: await _async_repoint_preferred_dataset( call.hass, str(source_xpan), str(target[MeshcopTLVType.EXTPANID]) ) + if result is DatasetAddResult.DISCARDED: + # Newer credentials for this network were stored while the router + # was being written to. The mesh is migrating to the dataset above + # and cannot be called back, so say so rather than report a success + # Home Assistant cannot back up. + # + # Reported after the two calls above on purpose: they describe + # which network is being adopted rather than with which + # credentials, so they are right either way and must still run -- + # in particular the preferred pointer, which this action's own + # default target reads. + raise HomeAssistantError( + translation_domain=DOMAIN, translation_key="dataset_discarded" + ) name_item = pending[MeshcopTLVType.NETWORKNAME] if TYPE_CHECKING: diff --git a/homeassistant/components/otbr/silabs_multiprotocol.py b/homeassistant/components/otbr/silabs_multiprotocol.py index 867f57ec450cae..a0c602e338ce44 100644 --- a/homeassistant/components/otbr/silabs_multiprotocol.py +++ b/homeassistant/components/otbr/silabs_multiprotocol.py @@ -17,7 +17,7 @@ from homeassistant.exceptions import HomeAssistantError from .const import DOMAIN -from .util import DATASET_LOCK, OTBRData +from .util import OTBRData, async_get_dataset_lock if TYPE_CHECKING: from . import OTBRConfigEntry @@ -72,7 +72,7 @@ async def async_change_channel( """ # Held across the write and the read-back, so the dataset imported below # is the one this call created. - async with DATASET_LOCK: + async with async_get_dataset_lock(hass): await data.set_channel(channel, delay) # Import the new dataset diff --git a/homeassistant/components/otbr/strings.json b/homeassistant/components/otbr/strings.json index 5d89d683e4f8ad..aef5180bdb8e6e 100644 --- a/homeassistant/components/otbr/strings.json +++ b/homeassistant/components/otbr/strings.json @@ -21,6 +21,9 @@ "channel_conflict": { "message": "The target network uses channel {target_channel}, but another radio pins the Thread channel to {allowed_channel}." }, + "dataset_discarded": { + "message": "The network is migrating, but Home Assistant could not record its credentials: credentials for the same network, at least as new as these, were stored while the border router was being updated. Check the Thread panel; the stored network may not match the one the mesh is switching to." + }, "incomplete_dataset": { "message": "The dataset must describe the target network completely; it is missing {missing}. A partial dataset would be filled in by the border router with settings nobody picked." }, diff --git a/homeassistant/components/otbr/util.py b/homeassistant/components/otbr/util.py index e45e8749132c48..856a79c2580250 100644 --- a/homeassistant/components/otbr/util.py +++ b/homeassistant/components/otbr/util.py @@ -20,9 +20,10 @@ is_multiprotocol_url, ) from homeassistant.config_entries import SOURCE_USER -from homeassistant.core import HomeAssistant +from homeassistant.core import HomeAssistant, callback from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import issue_registry as ir +from homeassistant.util.hass_dict import HassKey from .const import DOMAIN @@ -32,13 +33,24 @@ _LOGGER = logging.getLogger(__name__) -# Serializes dataset mutations across all config entries. Acquired by callers -# rather than by the OTBRData methods, so a sequence that reads the router's -# state and writes it back stays atomic: concurrent writers would otherwise -# work from state the other has already replaced, and the mesh silently -# ignores whichever pending dataset is not the newest while its writer still -# reports success. -DATASET_LOCK = asyncio.Lock() +DATASET_LOCK_KEY: HassKey[asyncio.Lock] = HassKey("otbr_dataset_lock") + + +@callback +def async_get_dataset_lock(hass: HomeAssistant) -> asyncio.Lock: + """Return the lock serializing dataset mutations. + + It covers every config entry, and is acquired by callers rather than by + the OTBRData methods, so a sequence that reads the router's state and + writes it back stays atomic: concurrent writers would otherwise work from + state the other has already replaced, and the mesh silently ignores + whichever pending dataset is not the newest while its writer still + reports success. + """ + if (lock := hass.data.get(DATASET_LOCK_KEY)) is None: + lock = hass.data[DATASET_LOCK_KEY] = asyncio.Lock() + return lock + INSECURE_NETWORK_KEYS = ( # Thread web UI default diff --git a/homeassistant/components/otbr/websocket_api.py b/homeassistant/components/otbr/websocket_api.py index ea90b1e12826c4..51d0581ea32d01 100644 --- a/homeassistant/components/otbr/websocket_api.py +++ b/homeassistant/components/otbr/websocket_api.py @@ -19,8 +19,8 @@ from .const import DEFAULT_CHANNEL, DOMAIN from .util import ( - DATASET_LOCK, OTBRData, + async_get_dataset_lock, compose_default_network_name, generate_random_pan_id, get_allowed_channel, @@ -152,7 +152,7 @@ async def websocket_create_network( """Create a new Thread network.""" # Held from the first read: the channel this picks and the dataset it # creates must not be decided from state another writer is replacing. - async with DATASET_LOCK: + async with async_get_dataset_lock(hass): channel = await get_allowed_channel(hass, data.url) or DEFAULT_CHANNEL try: @@ -223,7 +223,7 @@ async def websocket_set_network( # Held from the first read: the dataset read here is what gets written # below, so a concurrent writer must not replace it in between -- a key # rotation landing in that window would push the superseded credentials. - async with DATASET_LOCK: + async with async_get_dataset_lock(hass): dataset_tlv = await async_get_dataset(hass, msg["dataset_id"]) if not dataset_tlv: @@ -297,7 +297,7 @@ async def websocket_set_channel( delay: float = PENDING_DATASET_DELAY_TIMER / 1000 # Serialized against other dataset writers; a channel change is a pending-dataset write. - async with DATASET_LOCK: + async with async_get_dataset_lock(hass): try: await data.set_channel(channel) except HomeAssistantError as exc: diff --git a/tests/components/otbr/test_services.py b/tests/components/otbr/test_services.py index 010772e30c297e..9c081250e1530f 100644 --- a/tests/components/otbr/test_services.py +++ b/tests/components/otbr/test_services.py @@ -12,7 +12,10 @@ from homeassistant.components.otbr import ( silabs_multiprotocol as otbr_silabs_multiprotocol, ) -from homeassistant.components.otbr.util import DATASET_LOCK, INSECURE_NETWORK_KEYS +from homeassistant.components.otbr.util import ( + INSECURE_NETWORK_KEYS, + async_get_dataset_lock, +) from homeassistant.components.thread import async_add_dataset from homeassistant.components.thread.dataset_store import async_get_store from homeassistant.core import HomeAssistant @@ -562,7 +565,7 @@ async def test_channel_change_waits_for_dataset_lock( return_value=DATASET_CH16, ), ): - async with DATASET_LOCK: + async with async_get_dataset_lock(hass): task = hass.async_create_task( otbr_silabs_multiprotocol.async_change_channel(hass, 15, delay=300) ) @@ -637,3 +640,72 @@ async def test_migration_refreshes_repair_issues( assert issue_registry.async_get_issue( domain="otbr", issue_id=f"insecure_thread_network_{otbr_config_entry_multipan}" ) + + +async def test_migration_reports_a_discarded_store_write( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, + issue_registry: ir.IssueRegistry, +) -> None: + """Test a store write lost to a concurrent writer is reported. + + The router has already been told to migrate and cannot be called back, so + the repair issues and the preferred dataset must still be brought up to + date before the failure is raised. + """ + mock_pending_endpoint(aioclient_mock) + await async_add_dataset(hass, "test", DATASET_CH16.hex()) + store = await async_get_store(hass) + store.preferred_dataset = next(iter(store.datasets.values())).id + + # Migrate onto insecure credentials, so the repair issue below can only + # exist if update_issues ran before the failure was raised. + insecure = dict(tlv_parser.parse_tlv(TARGET)) + insecure[MeshcopTLVType.NETWORKKEY] = tlv_parser.MeshcopTLVItem( + MeshcopTLVType.NETWORKKEY, INSECURE_NETWORK_KEYS[0] + ) + + async def store_newer_dataset( + dataset: bytes, *, allow_replace: bool = False + ) -> None: + """Store newer credentials for the target network mid-migration.""" + interloper = dict(tlv_parser.parse_tlv(TARGET)) + interloper[MeshcopTLVType.ACTIVETIMESTAMP] = Timestamp.from_values( + MeshcopTLVType.ACTIVETIMESTAMP, seconds=2000 + ) + await async_add_dataset(hass, "other", tlv_parser.encode_tlv(interloper)) + + with ( + patch( + "homeassistant.components.otbr.util.OTBRData.set_pending_dataset_tlvs", + side_effect=store_newer_dataset, + ), + pytest.raises(HomeAssistantError) as exc_info, + ): + await call_migrate(hass, dataset=tlv_parser.encode_tlv(insecure)) + + assert exc_info.value.translation_key == "dataset_discarded" + + # The store kept the newer credentials ... + stored = next( + entry + for entry in store.datasets.values() + if entry.extended_pan_id.lower() == "1111111122222222" + ) + assert _timestamp_parts_seconds(stored.tlv) == 2000 + # ... and the preferred pointer and repair issues still followed the + # network the mesh is switching to. + assert store.datasets[store.preferred_dataset].extended_pan_id.lower() == ( + "1111111122222222" + ) + assert issue_registry.async_get_issue( + domain="otbr", issue_id=f"insecure_thread_network_{otbr_config_entry_multipan}" + ) + + +def _timestamp_parts_seconds(tlv: str) -> int: + """Return the active timestamp seconds of a dataset.""" + stamp = tlv_parser.parse_tlv(tlv)[MeshcopTLVType.ACTIVETIMESTAMP] + assert isinstance(stamp, Timestamp) + return stamp.seconds From f76cfd3116a87dc7ff195c6f0f5ecbcbf407ce11 Mon Sep 17 00:00:00 2001 From: Christian Glombek Date: Sat, 22 Aug 2026 20:29:36 +0200 Subject: [PATCH 3/4] Account for the other routers on a mesh when migrating it A migration is handed to one border router, but the pending dataset it sends reaches every router on the mesh, and Home Assistant may be managing several of them. Three things then go wrong for the others: A router sharing its radio with Zigbee is pinned to a channel, which the migration only respected when started through that router. Ask the other routers on the same network whether they are pinned before choosing. A pinned router that cannot be read refuses the migration rather than being skipped: its REST API being down says nothing about its radio, which may still be on the mesh and would follow the pending dataset off the shared channel. A second migration of the same mesh -- another border router on it, moving to a different network -- can still read the old active dataset and no pending one, and would pick the same timestamp as the first. Stamp above what this integration has already issued for that mesh, keyed by extended PAN ID so one busy network does not exhaust the rest. That record is written to disk before the router is, since a restart inside the delay window would otherwise forget what is still in flight. The target taken from the preferred dataset can be replaced while the router is being read. Sending that snapshot would put credentials on the mesh that Home Assistant has already superseded and stamped newer, so the newer ones would lose. Nothing has been written yet at that point, so refuse and let the caller try again. Assisted-By: Claude Fable 5 --- homeassistant/components/otbr/services.py | 107 +++++++- homeassistant/components/otbr/util.py | 59 +++++ tests/components/otbr/test_services.py | 291 +++++++++++++++++++++- 3 files changed, 453 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/otbr/services.py b/homeassistant/components/otbr/services.py index 4bdcf9b7fe5e48..f591b45ae426b7 100644 --- a/homeassistant/components/otbr/services.py +++ b/homeassistant/components/otbr/services.py @@ -1,5 +1,6 @@ """Actions for the Open Thread Border Router integration.""" +import logging from typing import TYPE_CHECKING, Any from python_otbr_api import PENDING_DATASET_DELAY_TIMER, tlv_parser @@ -12,17 +13,25 @@ async_get_preferred_dataset, ) from homeassistant.components.thread.dataset_store import async_get_store +from homeassistant.config_entries import ConfigEntryState from homeassistant.core import HomeAssistant, ServiceCall, SupportsResponse, callback from homeassistant.exceptions import HomeAssistantError, ServiceValidationError from homeassistant.helpers import config_validation as cv, service from homeassistant.helpers.selector import ConfigEntrySelector from .const import DOMAIN -from .util import async_get_dataset_lock, get_allowed_channel, update_issues +from .util import ( + async_get_dataset_lock, + async_get_issued_timestamps, + get_allowed_channel, + update_issues, +) if TYPE_CHECKING: from .types import OTBRConfigEntry +_LOGGER = logging.getLogger(__name__) + SERVICE_MIGRATE_NETWORK = "migrate_network" ATTR_CONFIG_ENTRY = "config_entry" @@ -154,6 +163,65 @@ async def _async_repoint_preferred_dataset( store.preferred_dataset = target_id +async def _pinned_channel_of_another_router( + hass: HomeAssistant, + entry: OTBRConfigEntry, + active: dict[MeshcopTLVType | int, tlv_parser.MeshcopTLVItem], +) -> int | None: + """Return a channel another router on the same network is pinned to. + + Only routers that are actually pinned are asked which network they are + on, so the common setup pays for no extra calls. A pinned router that + cannot be read is an error rather than skipped: its REST API being down + says nothing about its radio, which may still be on this mesh and would + follow the pending dataset off the channel it shares with Zigbee. + Configured entries count even when they are not loaded, for the same + reason: a failed setup or an unload does not stop the radio. + """ + source_xpan = active.get(MeshcopTLVType.EXTPANID) + if source_xpan is None: + return None + + other: OTBRConfigEntry + for other in hass.config_entries.async_entries(DOMAIN): + if other.entry_id == entry.entry_id: + continue + # An ignored discovery has no URL to judge; nothing to check. + if (url := other.data.get("url")) is None: + continue + pinned = await get_allowed_channel(hass, url) + if pinned is None: + continue + # Not loaded means it cannot be asked which network it is on; + # fail safe, exactly like a router whose read fails below. + if other.state is not ConfigEntryState.LOADED: + raise ServiceValidationError( + translation_domain=DOMAIN, + translation_key="pinned_router_unreachable", + translation_placeholders={"router": other.title}, + ) + try: + other_tlvs = await other.runtime_data.get_active_dataset_tlvs() + other_active = ( + tlv_parser.parse_tlv(other_tlvs.hex()) + if other_tlvs is not None + else None + ) + except (HomeAssistantError, tlv_parser.TLVError) as err: + raise ServiceValidationError( + translation_domain=DOMAIN, + translation_key="pinned_router_unreachable", + translation_placeholders={"router": other.title}, + ) from err + # No active dataset: the router is not on any mesh. + if other_active is None: + continue + if other_active.get(MeshcopTLVType.EXTPANID) == source_xpan: + return pinned + + return None + + async def _async_migrate_network(call: ServiceCall) -> dict[str, Any]: """Migrate a border router and every device on its network. @@ -229,6 +297,13 @@ async def _async_migrate_network(call: ServiceCall) -> dict[str, Any]: assert isinstance(channel_item, tlv_parser.Channel) target_channel = channel_item.channel allowed_channel = await get_allowed_channel(call.hass, entry.data["url"]) + if allowed_channel is None: + # The pending dataset reaches every router on the mesh, not only + # the one it is handed to, so a router that shares its radio has + # a say even when the migration is started through another. + allowed_channel = await _pinned_channel_of_another_router( + call.hass, entry, active + ) if allowed_channel and target_channel != allowed_channel: raise ServiceValidationError( translation_domain=DOMAIN, @@ -258,6 +333,17 @@ async def _async_migrate_network(call: ServiceCall) -> dict[str, Any]: newest, _timestamp_parts(entry_.dataset, MeshcopTLVType.ACTIVETIMESTAMP), ) + # A pending dataset takes its delay to propagate, so a second + # migration of the same mesh -- another border router on it, moving to + # a different network -- can still read the old active dataset and no + # pending one, and would otherwise pick the same timestamp. Stamp + # above what this integration has already handed out for this mesh. + issued = await async_get_issued_timestamps(call.hass) + source_xpan_item = active.get(MeshcopTLVType.EXTPANID) + source_xpan = str(source_xpan_item).lower() if source_xpan_item else None + if source_xpan is not None: + newest = max(newest, issued.get(source_xpan)) + # Always step the seconds, never the ticks: python_otbr_api's channel # change stamps seconds + 1 and ignores ticks, so a network left at the # last representable second would wrap that write to zero and have the @@ -285,6 +371,21 @@ async def _async_migrate_network(call: ServiceCall) -> dict[str, Any]: border_agent_id = (await data.get_border_agent_id()).hex() extended_address = (await data.get_extended_address()).hex() + if call.data.get(ATTR_DATASET) is None: + # The target came from the preferred dataset, which another writer + # can replace while the router is being read. Sending the snapshot + # now would put credentials on the mesh that Home Assistant has + # already superseded -- and stamped newer, so the newer ones would + # be lost. Nothing has been written yet, so this can still refuse. + preferred = await async_get_preferred_dataset(call.hass) + if preferred is not None and bytes.fromhex(preferred) != dataset: + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key="preferred_dataset_changed", + ) + + if source_xpan is not None: + await issued.async_set(source_xpan, (seconds, 0)) await data.set_pending_dataset_tlvs( bytes.fromhex(tlv_parser.encode_tlv(pending)) ) @@ -305,9 +406,9 @@ async def _async_migrate_network(call: ServiceCall) -> dict[str, Any]: # The repair issues describe the credentials the network is adopting, # the same way the create and set-network paths report them. await update_issues(call.hass, data, migrated_tlvs) - if (source_xpan := active.get(MeshcopTLVType.EXTPANID)) is not None: + if source_xpan is not None: await _async_repoint_preferred_dataset( - call.hass, str(source_xpan), str(target[MeshcopTLVType.EXTPANID]) + call.hass, source_xpan, str(target[MeshcopTLVType.EXTPANID]) ) if result is DatasetAddResult.DISCARDED: # Newer credentials for this network were stored while the router diff --git a/homeassistant/components/otbr/util.py b/homeassistant/components/otbr/util.py index 856a79c2580250..30130a0812547b 100644 --- a/homeassistant/components/otbr/util.py +++ b/homeassistant/components/otbr/util.py @@ -23,6 +23,7 @@ from homeassistant.core import HomeAssistant, callback from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import issue_registry as ir +from homeassistant.helpers.storage import Store from homeassistant.util.hass_dict import HassKey from .const import DOMAIN @@ -34,6 +35,64 @@ DATASET_LOCK_KEY: HassKey[asyncio.Lock] = HassKey("otbr_dataset_lock") +ISSUED_TIMESTAMPS_KEY: HassKey[IssuedTimestamps] = HassKey("otbr_issued_timestamps") +ISSUED_TIMESTAMPS_STORAGE_KEY = f"{DOMAIN}.issued_timestamps" +ISSUED_TIMESTAMPS_STORAGE_VERSION = 1 + + +class IssuedTimestamps: + """The newest timestamp this integration has issued, per source network. + + Keyed by extended PAN ID: a busy network must not raise the floor for the + others, which would eventually exhaust their timestamps too. + + Kept on disk, not only in memory: a pending dataset takes its delay to + reach every router on the mesh, and a restart inside that window must not + let the next migration hand out the stamp again. + """ + + def __init__(self, hass: HomeAssistant) -> None: + """Initialize the record.""" + self._store = Store[dict[str, list[int]]]( + hass, + ISSUED_TIMESTAMPS_STORAGE_VERSION, + ISSUED_TIMESTAMPS_STORAGE_KEY, + # This file exists to survive an ill-timed restart; the same + # restart must not be able to truncate an in-place rewrite. + atomic_writes=True, + ) + self._issued: dict[str, tuple[int, int]] = {} + + async def async_load(self) -> None: + """Load what was issued before the last restart.""" + if data := await self._store.async_load(): + self._issued = { + xpan: (seconds, ticks) for xpan, (seconds, ticks) in data.items() + } + + def get(self, extended_pan_id: str) -> tuple[int, int]: + """Return the newest timestamp issued for a network.""" + return self._issued.get(extended_pan_id, (0, 0)) + + async def async_set(self, extended_pan_id: str, timestamp: tuple[int, int]) -> None: + """Record a timestamp about to be issued for a network. + + Written through before the caller hands the dataset to the router: + delaying the save would reopen the window this record exists to close. + """ + self._issued[extended_pan_id] = timestamp + await self._store.async_save( + {xpan: list(stamp) for xpan, stamp in self._issued.items()} + ) + + +async def async_get_issued_timestamps(hass: HomeAssistant) -> IssuedTimestamps: + """Return the record of issued timestamps, loading it on first use.""" + if (issued := hass.data.get(ISSUED_TIMESTAMPS_KEY)) is None: + issued = IssuedTimestamps(hass) + await issued.async_load() + hass.data[ISSUED_TIMESTAMPS_KEY] = issued + return issued @callback diff --git a/tests/components/otbr/test_services.py b/tests/components/otbr/test_services.py index 9c081250e1530f..072a4d4bbae7c8 100644 --- a/tests/components/otbr/test_services.py +++ b/tests/components/otbr/test_services.py @@ -3,6 +3,7 @@ import asyncio from http import HTTPStatus import re +from typing import Any from unittest.mock import AsyncMock, Mock, patch import pytest @@ -14,6 +15,8 @@ ) from homeassistant.components.otbr.util import ( INSECURE_NETWORK_KEYS, + ISSUED_TIMESTAMPS_KEY, + ISSUED_TIMESTAMPS_STORAGE_KEY, async_get_dataset_lock, ) from homeassistant.components.thread import async_add_dataset @@ -678,7 +681,7 @@ async def store_newer_dataset( with ( patch( - "homeassistant.components.otbr.util.OTBRData.set_pending_dataset_tlvs", + "python_otbr_api.OTBR.set_pending_dataset_tlvs", side_effect=store_newer_dataset, ), pytest.raises(HomeAssistantError) as exc_info, @@ -709,3 +712,289 @@ def _timestamp_parts_seconds(tlv: str) -> int: stamp = tlv_parser.parse_tlv(tlv)[MeshcopTLVType.ACTIVETIMESTAMP] assert isinstance(stamp, Timestamp) return stamp.seconds + + +async def test_migrations_of_one_mesh_do_not_share_a_timestamp( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """Test two migrations of the same mesh get distinct timestamps. + + A pending dataset takes its delay to propagate, so a second border router + on the same mesh still reports the old active dataset and no pending one. + Targeting a different network, nothing in the router's or the store's + state would separate the two stamps. + """ + mock_pending_endpoint(aioclient_mock) + other_target = dict(tlv_parser.parse_tlv(TARGET)) + other_target[MeshcopTLVType.EXTPANID] = tlv_parser.MeshcopTLVItem( + MeshcopTLVType.EXTPANID, bytes.fromhex("3333333344444444") + ) + other_target[MeshcopTLVType.ACTIVETIMESTAMP] = Timestamp.from_values( + MeshcopTLVType.ACTIVETIMESTAMP, seconds=1003 + ) + + await call_migrate(hass, dataset=TARGET) + await call_migrate(hass, dataset=tlv_parser.encode_tlv(other_target)) + + stamps = [ + tlv_parser.parse_tlv(put[2])[MeshcopTLVType.ACTIVETIMESTAMP].seconds + for put in pending_calls(aioclient_mock) + ] + assert len(stamps) == 2 + assert stamps[0] != stamps[1] + assert stamps == sorted(stamps) + + +async def test_issued_timestamps_survive_a_restart( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, + hass_storage: dict[str, Any], +) -> None: + """Test the per-mesh timestamp floor is not lost with a restart. + + The pending dataset takes its delay to reach the other routers, and a + restart in that window leaves them still reporting the old active dataset. + Only what was written to disk separates the next migration's stamp from + the one already in flight. + """ + mock_pending_endpoint(aioclient_mock) + other_target = dict(tlv_parser.parse_tlv(TARGET)) + other_target[MeshcopTLVType.EXTPANID] = tlv_parser.MeshcopTLVItem( + MeshcopTLVType.EXTPANID, bytes.fromhex("3333333344444444") + ) + other_target[MeshcopTLVType.ACTIVETIMESTAMP] = Timestamp.from_values( + MeshcopTLVType.ACTIVETIMESTAMP, seconds=1003 + ) + + await call_migrate(hass, dataset=TARGET) + # Written through before the router was, not on the lazy save timer. + (source_xpan,) = hass_storage[ISSUED_TIMESTAMPS_STORAGE_KEY]["data"] + + # A restart drops everything held in memory; the file stays. + del hass.data[ISSUED_TIMESTAMPS_KEY] + await call_migrate(hass, dataset=tlv_parser.encode_tlv(other_target)) + + stamps = [ + tlv_parser.parse_tlv(put[2])[MeshcopTLVType.ACTIVETIMESTAMP].seconds + for put in pending_calls(aioclient_mock) + ] + assert len(stamps) == 2 + assert stamps[0] < stamps[1] + assert hass_storage[ISSUED_TIMESTAMPS_STORAGE_KEY]["data"] == { + source_xpan: [stamps[1], 0] + } + + +async def test_preferred_dataset_replaced_while_reading_the_router( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, + get_active_dataset_tlvs: AsyncMock, +) -> None: + """Test a superseded preferred dataset is refused before anything is sent. + + The default target is a snapshot of the preferred dataset. Sending it + after another writer replaced it would put credentials on the mesh that + Home Assistant has already superseded -- stamped newer, so the newer ones + would be lost. + """ + mock_pending_endpoint(aioclient_mock) + await async_add_dataset(hass, "test", TARGET) + store = await async_get_store(hass) + # Setup already imported the router's own network, so pick the target. + store.preferred_dataset = next( + entry.id + for entry in store.datasets.values() + if entry.extended_pan_id.lower() == "1111111122222222" + ) + + async def replace_preferred_dataset() -> bytes: + """Rotate the preferred network's key while the router is read.""" + rotated = dict(tlv_parser.parse_tlv(TARGET)) + rotated[MeshcopTLVType.NETWORKKEY] = tlv_parser.MeshcopTLVItem( + MeshcopTLVType.NETWORKKEY, bytes.fromhex("99999999888888887777777766666666") + ) + rotated[MeshcopTLVType.ACTIVETIMESTAMP] = Timestamp.from_values( + MeshcopTLVType.ACTIVETIMESTAMP, seconds=1010 + ) + await async_add_dataset(hass, "panel", tlv_parser.encode_tlv(rotated)) + return DATASET_CH16 + + get_active_dataset_tlvs.side_effect = replace_preferred_dataset + + with pytest.raises(HomeAssistantError) as exc_info: + await call_migrate(hass) + + assert exc_info.value.translation_key == "preferred_dataset_changed" + # Nothing reached the router, so the rotated credentials still stand. + assert not pending_calls(aioclient_mock) + stored = next( + entry + for entry in store.datasets.values() + if entry.extended_pan_id.lower() == "1111111122222222" + ) + assert tlv_parser.parse_tlv(stored.tlv)[MeshcopTLVType.NETWORKKEY].data.hex() == ( + "99999999888888887777777766666666" + ) + + +async def test_timestamp_watermark_is_per_mesh( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, + get_active_dataset_tlvs: AsyncMock, +) -> None: + """Test one mesh's timestamps do not raise the floor for another. + + A shared watermark would let a network with a high timestamp push every + other network's stamps up, and eventually exhaust them. + """ + mock_pending_endpoint(aioclient_mock) + high = dict(tlv_parser.parse_tlv(DATASET_CH16.hex())) + high[MeshcopTLVType.ACTIVETIMESTAMP] = Timestamp.from_values( + MeshcopTLVType.ACTIVETIMESTAMP, seconds=900_000 + ) + get_active_dataset_tlvs.return_value = bytes.fromhex(tlv_parser.encode_tlv(high)) + + # A migration of the high-timestamp mesh ... + await call_migrate(hass, dataset=TARGET) + + # ... must not push a migration of an unrelated mesh up with it. Both the + # source and the target differ, so nothing but a shared watermark could. + other_source = dict(tlv_parser.parse_tlv(DATASET_CH16.hex())) + other_source[MeshcopTLVType.EXTPANID] = tlv_parser.MeshcopTLVItem( + MeshcopTLVType.EXTPANID, bytes.fromhex("5555555566666666") + ) + get_active_dataset_tlvs.return_value = bytes.fromhex( + tlv_parser.encode_tlv(other_source) + ) + other_target = dict(tlv_parser.parse_tlv(TARGET)) + other_target[MeshcopTLVType.EXTPANID] = tlv_parser.MeshcopTLVItem( + MeshcopTLVType.EXTPANID, bytes.fromhex("7777777788888888") + ) + + await call_migrate(hass, dataset=tlv_parser.encode_tlv(other_target)) + + stamps = [ + tlv_parser.parse_tlv(put[2])[MeshcopTLVType.ACTIVETIMESTAMP].seconds + for put in pending_calls(aioclient_mock) + ] + assert stamps[0] == 900_001 + # The second mesh's own timestamps are small; it keeps its own floor. + assert stamps[1] == 1004 + + +async def test_channel_pinned_by_another_router_on_the_mesh( + hass: HomeAssistant, + multiprotocol_addon_manager_mock: Mock, + otbr_config_entry_thread: None, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """Test a pinned router on the mesh is respected through another router. + + The pending dataset reaches every router on the network, so migrating + through a router that shares no radio would still move one that does. + """ + mock_pending_endpoint(aioclient_mock) + # The router the migration is handed to speaks over its serial path. + aioclient_mock.get( + "/dev/ttyAMA1/node/dataset/pending", status=HTTPStatus.NO_CONTENT + ) + aioclient_mock.put("/dev/ttyAMA1/node/dataset/pending", status=HTTPStatus.CREATED) + multiprotocol_addon_manager_mock.async_get_channel.return_value = 25 + + # Target the router that is not sharing its radio; the multiprotocol one + # is on the same network and pinned to another channel. + thread_entry = next( + entry + for entry in hass.config_entries.async_loaded_entries("otbr") + if entry.entry_id != otbr_config_entry_multipan + ) + + with pytest.raises(ServiceValidationError) as exc_info: + await call_migrate(hass, dataset=TARGET, config_entry=thread_entry.entry_id) + + assert exc_info.value.translation_key == "channel_conflict" + assert not pending_calls(aioclient_mock) + + +async def test_unreadable_pinned_router_refuses_the_migration( + hass: HomeAssistant, + multiprotocol_addon_manager_mock: Mock, + otbr_config_entry_thread: None, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """Test a pinned router that cannot be read is an error, not skipped. + + Its REST API being down says nothing about its radio, which may still be + on this mesh and would follow the pending dataset off the shared channel. + """ + mock_pending_endpoint(aioclient_mock) + aioclient_mock.get( + "/dev/ttyAMA1/node/dataset/pending", status=HTTPStatus.NO_CONTENT + ) + aioclient_mock.put("/dev/ttyAMA1/node/dataset/pending", status=HTTPStatus.CREATED) + multiprotocol_addon_manager_mock.async_get_channel.return_value = 25 + + thread_entry = next( + entry + for entry in hass.config_entries.async_loaded_entries("otbr") + if entry.entry_id != otbr_config_entry_multipan + ) + pinned_entry = hass.config_entries.async_get_entry(otbr_config_entry_multipan) + assert pinned_entry is not None + + with ( + patch.object( + pinned_entry.runtime_data, + "get_active_dataset_tlvs", + side_effect=HomeAssistantError("unreachable"), + ), + pytest.raises(ServiceValidationError) as exc_info, + ): + await call_migrate(hass, dataset=TARGET, config_entry=thread_entry.entry_id) + + assert exc_info.value.translation_key == "pinned_router_unreachable" + assert exc_info.value.translation_placeholders == {"router": pinned_entry.title} + assert not pending_calls(aioclient_mock) + + +async def test_unloaded_pinned_router_refuses_the_migration( + hass: HomeAssistant, + multiprotocol_addon_manager_mock: Mock, + otbr_config_entry_thread: None, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, +) -> None: + """Test a pinned router whose entry is not loaded is an error, not skipped. + + A failed setup or an unload does not stop the radio, which may still be + on this mesh and would follow the pending dataset off the shared channel. + """ + mock_pending_endpoint(aioclient_mock) + aioclient_mock.get( + "/dev/ttyAMA1/node/dataset/pending", status=HTTPStatus.NO_CONTENT + ) + aioclient_mock.put("/dev/ttyAMA1/node/dataset/pending", status=HTTPStatus.CREATED) + multiprotocol_addon_manager_mock.async_get_channel.return_value = 25 + + thread_entry = next( + entry + for entry in hass.config_entries.async_loaded_entries("otbr") + if entry.entry_id != otbr_config_entry_multipan + ) + assert await hass.config_entries.async_unload(otbr_config_entry_multipan) + pinned_entry = hass.config_entries.async_get_entry(otbr_config_entry_multipan) + assert pinned_entry is not None + + with pytest.raises(ServiceValidationError) as exc_info: + await call_migrate(hass, dataset=TARGET, config_entry=thread_entry.entry_id) + + assert exc_info.value.translation_key == "pinned_router_unreachable" + assert exc_info.value.translation_placeholders == {"router": pinned_entry.title} + assert not pending_calls(aioclient_mock) From 8037a03b47bca047a02e73d0a25d9b907542a6cc Mon Sep 17 00:00:00 2001 From: Christian Glombek Date: Thu, 3 Sep 2026 21:18:06 +0200 Subject: [PATCH 4/4] Persist a Thread migration before reporting success The migration writes the migrated dataset and the repointed preferred dataset into the Thread dataset store, which saves on a delay. A normal restart flushes that; a crash inside the delay does not. The mesh is migrating either way once the router accepted the write, so the store must not be left behind it: the dataset entry would be re-imported from the router on the next setup, but the preferred pointer would stay on the abandoned network until someone noticed, and everything that hands out Thread credentials, this action's own default target included, reads that pointer. Give the dataset store a public async_save() that writes now, ahead of any scheduled save, and have the migration call it once its store writes are done. The issued-timestamp watermark was already written through, for the same reason. Assisted-By: Claude Fable 5.1 Signed-off-by: Christian Glombek --- homeassistant/components/otbr/services.py | 6 ++++ .../components/thread/dataset_store.py | 10 +++++++ tests/components/otbr/test_services.py | 29 ++++++++++++++++++- tests/components/thread/test_dataset_store.py | 16 ++++++++++ 4 files changed, 60 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/otbr/services.py b/homeassistant/components/otbr/services.py index f591b45ae426b7..1bd3905f26cfae 100644 --- a/homeassistant/components/otbr/services.py +++ b/homeassistant/components/otbr/services.py @@ -410,6 +410,12 @@ async def _async_migrate_network(call: ServiceCall) -> dict[str, Any]: await _async_repoint_preferred_dataset( call.hass, source_xpan, str(target[MeshcopTLVType.EXTPANID]) ) + # The store saves on a delay, and a normal restart flushes it; a crash + # inside that delay would not. The mesh is migrating either way, so + # write now: the dataset entry would be re-imported from the router + # on the next setup, but the preferred pointer would stay on the + # abandoned network until someone noticed. + await store.async_save() if result is DatasetAddResult.DISCARDED: # Newer credentials for this network were stored while the router # was being written to. The mesh is migrating to the dataset above diff --git a/homeassistant/components/thread/dataset_store.py b/homeassistant/components/thread/dataset_store.py index 6fcfbf77e4d09b..46181339c48032 100644 --- a/homeassistant/components/thread/dataset_store.py +++ b/homeassistant/components/thread/dataset_store.py @@ -564,6 +564,16 @@ def async_schedule_save(self) -> None: """Schedule saving the dataset store.""" self._store.async_delay_save(self._data_to_save, SAVE_DELAY) + async def async_save(self) -> None: + """Write the dataset store now, ahead of any scheduled save. + + For a caller whose write has an external side effect that cannot be + called back, such as a mesh that is already migrating to the stored + credentials, so that a crash inside the save delay cannot leave the + store behind the network. + """ + await self._store.async_save(self._data_to_save()) + @callback def _data_to_save(self) -> dict[str, list[dict[str, str | None]]]: """Return data of datasets to store in a file.""" diff --git a/tests/components/otbr/test_services.py b/tests/components/otbr/test_services.py index 072a4d4bbae7c8..7cc0d0e8b25669 100644 --- a/tests/components/otbr/test_services.py +++ b/tests/components/otbr/test_services.py @@ -20,7 +20,7 @@ async_get_dataset_lock, ) from homeassistant.components.thread import async_add_dataset -from homeassistant.components.thread.dataset_store import async_get_store +from homeassistant.components.thread.dataset_store import STORAGE_KEY, async_get_store from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError, ServiceValidationError from homeassistant.helpers import issue_registry as ir @@ -621,6 +621,33 @@ async def test_ticks_count_in_the_comparison( assert (stamp.seconds, stamp.ticks) == (1004, 0) +async def test_migration_is_persisted_before_success_is_reported( + hass: HomeAssistant, + otbr_config_entry_multipan: str, + aioclient_mock: AiohttpClientMocker, + hass_storage: dict[str, Any], +) -> None: + """The store is written before the action returns, not on the save delay. + + The mesh is migrating either way once the router accepted the write; a + crash inside the store's save delay must not leave Home Assistant with + the abandoned network as its preferred one. + """ + mock_pending_endpoint(aioclient_mock) + await async_add_dataset(hass, "test", DATASET_CH16.hex()) + store = await async_get_store(hass) + store.preferred_dataset = next(iter(store.datasets.values())).id + + await call_migrate(hass, dataset=TARGET) + + saved = hass_storage[STORAGE_KEY]["data"] + by_id = {entry["id"]: entry for entry in saved["datasets"]} + preferred = by_id[saved["preferred_dataset"]] + assert tlv_parser.parse_tlv(preferred["tlv"])[MeshcopTLVType.EXTPANID].data == ( + bytes.fromhex("1111111122222222") + ) + + async def test_migration_refreshes_repair_issues( hass: HomeAssistant, otbr_config_entry_multipan: str, diff --git a/tests/components/thread/test_dataset_store.py b/tests/components/thread/test_dataset_store.py index 5284a9fc1b80d4..77ef2701b87c7a 100644 --- a/tests/components/thread/test_dataset_store.py +++ b/tests/components/thread/test_dataset_store.py @@ -81,6 +81,22 @@ ) +async def test_save_writes_immediately( + hass: HomeAssistant, hass_storage: dict[str, Any] +) -> None: + """Test async_save writes the store ahead of the scheduled save.""" + await dataset_store.async_add_dataset(hass, "source", DATASET_1) + store = await dataset_store.async_get_store(hass) + store.preferred_dataset = next(iter(store.datasets.values())).id + assert dataset_store.STORAGE_KEY not in hass_storage + + await store.async_save() + + saved = hass_storage[dataset_store.STORAGE_KEY]["data"] + assert saved["datasets"][0]["tlv"] == DATASET_1 + assert saved["preferred_dataset"] == store.preferred_dataset + + async def test_add_invalid_dataset(hass: HomeAssistant) -> None: """Test adding an invalid dataset.""" with pytest.raises(TLVError, match="expected 173 bytes for tag 222, got 2"):