From f5dc5aa9ec49f6749d0988c4d9a24868ab0c0597 Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Sat, 8 Aug 2026 18:05:57 +0200 Subject: [PATCH] Don't allow devices setting the via device link to itself (#178194) Co-authored-by: Artur Pragacz <49985303+arturpragacz@users.noreply.github.com> --- homeassistant/helpers/device_registry.py | 42 +++- tests/components/hydrawise/test_init.py | 32 --- tests/helpers/test_device_registry.py | 247 +++++++++++++++++++++++ 3 files changed, 287 insertions(+), 34 deletions(-) diff --git a/homeassistant/helpers/device_registry.py b/homeassistant/helpers/device_registry.py index 65b01ff67163..b571bc1375c2 100644 --- a/homeassistant/helpers/device_registry.py +++ b/homeassistant/helpers/device_registry.py @@ -64,7 +64,7 @@ EVENT_DEVICE_REGISTRY_UPDATED: EventType[EventDeviceRegistryUpdatedData] = Event ) STORAGE_KEY = "core.device_registry" STORAGE_VERSION_MAJOR = 3 -STORAGE_VERSION_MINOR = 2 +STORAGE_VERSION_MINOR = 3 CLEANUP_DELAY = 10 @@ -1005,6 +1005,13 @@ class DeviceRegistryStore(storage.Store[dict[str, list[dict[str, Any]]]]): else: device["via_device_id"] = None + if old_major_version < 3 or (old_major_version == 3 and old_minor_version < 3): + # Version 3.3, introduced in 2026.8, clears via_device_id self-references, + # which are no longer allowed. + for device in old_data["devices"]: + if device["via_device_id"] == device["id"]: + device["via_device_id"] = None + if old_major_version > 3: raise NotImplementedError return old_data @@ -1989,7 +1996,19 @@ class DeviceRegistry(BaseRegistry[dict[str, list[dict[str, Any]]]]): core_behavior=ReportBehavior.LOG, breaks_in_ha_version="2025.12.0", ) - via_device_id = via.id if via else UNDEFINED + via_device_id = UNDEFINED + elif via.id == device.id: + # A device can not be its own via device. Ignore the self-reference; + # this will raise in HA Core 2027.8. + report_usage( + "calls `device_registry.async_get_or_create` with a `via_device` " + "referencing the device itself; the via device is ignored", + core_behavior=ReportBehavior.LOG, + breaks_in_ha_version="2027.8.0", + ) + via_device_id = UNDEFINED + else: + via_device_id = via.id elif via_device is None: # An explicit `via_device=None` means "no via device" (a via_device_id # alongside it is rejected above). @@ -2163,6 +2182,9 @@ class DeviceRegistry(BaseRegistry[dict[str, list[dict[str, Any]]]]): f"Can't link device to unknown via device {via_device_id}" ) + if via_device_id == device_id: + raise HomeAssistantError("A device can not be its own via device") + # A device belongs to exactly one config entry and subentry: # - add_config_entry_id (with an optional add_config_subentry_id) records a # transient pending move to that config entry and subentry; on its own it does @@ -2228,6 +2250,18 @@ class DeviceRegistry(BaseRegistry[dict[str, list[dict[str, Any]]]]): ): move_target = None if move_target is None: + # A composite via_device_id resolves to the split owned by the + # entry being removed, i.e. this device, so it is a self-reference; + # reject it before deleting, atomically like the direct-id check + # before the ownership changes above. + if ( + via_device_id is not UNDEFINED + and via_device_id is not None + and via_device_id == old.composite_device_id + ): + raise HomeAssistantError( + "A device can not be its own via device" + ) self.async_remove_device(device_id) return None target_config_entry_id = move_target.config_entry_id @@ -2298,6 +2332,10 @@ class DeviceRegistry(BaseRegistry[dict[str, list[dict[str, Any]]]]): via_device_id = self._resolve_via_device_id( via_device_id, effective_config_entry_id ) + # Direct self-references were rejected before the ownership changes above; + # this catches a composite via_device_id that resolves to device_id. + if via_device_id == device_id: + raise HomeAssistantError("A device can not be its own via device") added_connections: set[tuple[str, str]] | None = None added_identifiers: set[tuple[str, str]] | None = None diff --git a/tests/components/hydrawise/test_init.py b/tests/components/hydrawise/test_init.py index fcd64d2a4bdf..15d24a53bce1 100644 --- a/tests/components/hydrawise/test_init.py +++ b/tests/components/hydrawise/test_init.py @@ -118,38 +118,6 @@ async def test_auto_add_devices( assert hass.states.get("sensor.zone_two_2_daily_active_watering_time") is not None -async def test_setup_clears_self_referential_via_device( - hass: HomeAssistant, - device_registry: DeviceRegistry, - mock_config_entry: MockConfigEntry, - mock_pydrawise: AsyncMock, -) -> None: - """Test setup clears a self-referential via_device left by older versions. - - Older versions linked the controller device to itself via its rain sensor - entity, which persists in the device registry across upgrades. - """ - mock_config_entry.add_to_hass(hass) - controller = device_registry.async_get_or_create( - config_entry_id=mock_config_entry.entry_id, - identifiers={(DOMAIN, "52496")}, - name="Home Controller", - ) - controller = device_registry.async_update_device( - controller.id, via_device_id=controller.id - ) - assert controller.via_device_id == controller.id - - await hass.config_entries.async_setup(mock_config_entry.entry_id) - await hass.async_block_till_done() - - controller = device_registry.async_get_device_by_identifier( - (DOMAIN, "52496"), mock_config_entry.entry_id - ) - assert controller is not None - assert controller.via_device_id is None - - async def test_auto_remove_devices( hass: HomeAssistant, device_registry: DeviceRegistry, diff --git a/tests/helpers/test_device_registry.py b/tests/helpers/test_device_registry.py index da97812e00d9..734afeb5dff5 100644 --- a/tests/helpers/test_device_registry.py +++ b/tests/helpers/test_device_registry.py @@ -2126,6 +2126,127 @@ async def test_migration_detaches_via_device_of_dropped_parent( assert child.via_device_id is None +@pytest.mark.parametrize("load_registries", [False]) +async def test_migration_clears_via_device_self_reference( + hass: HomeAssistant, hass_storage: dict[str, Any] +) -> None: + """The version 3.3 migration clears a device's via_device_id self-reference.""" + entry = MockConfigEntry() + entry.add_to_hass(hass) + device_id = "selfref00000000000000000000000" + hass_storage[dr.STORAGE_KEY] = { + "version": 3, + "minor_version": 2, + "key": dr.STORAGE_KEY, + "data": { + "devices": [ + { + "area_id": None, + "config_entry_id": entry.entry_id, + "config_subentry_id": None, + "composite_device_id": None, + "composite_primary_config_entry": None, + "split_at": None, + "has_composite_identifiers": False, + "configuration_url": None, + "connections": [], + "created_at": "1970-01-01T00:00:00+00:00", + "disabled_by": None, + "entry_type": None, + "hw_version": None, + "id": device_id, + "identifiers": [["test", "self"]], + "labels": [], + "manufacturer": None, + "model": None, + "name": None, + "model_id": None, + "modified_at": "1970-01-01T00:00:00+00:00", + "name_by_user": None, + "primary_config_entry": entry.entry_id, + "serial_number": None, + "sw_version": None, + # Buggy self-reference that the migration must clear + "via_device_id": device_id, + } + ], + "deleted_devices": [], + }, + } + + dr.async_setup(hass) + await dr.async_load(hass) + registry = dr.async_get(hass) + + device = registry.async_get(device_id) + assert device is not None + assert device.via_device_id is None + + +@pytest.mark.parametrize("load_registries", [False]) +async def test_migration_clears_composite_via_device_self_reference( + hass: HomeAssistant, hass_storage: dict[str, Any] +) -> None: + """A self-reference the 3.2 split remapping introduces is cleared. + + A pre-migration composite device linking to itself via via_device_id is split into + one device per config entry; the 3.2 remapping points each split's stale link at the + split owning its config entry, i.e. itself. The 3.3 step runs afterwards and clears + the resulting self-references. + """ + entry_1 = MockConfigEntry() + entry_1.add_to_hass(hass) + entry_2 = MockConfigEntry() + entry_2.add_to_hass(hass) + composite_id = "composite000000000000000000000" + hass_storage[dr.STORAGE_KEY] = { + "version": 1, + "minor_version": 12, + "key": dr.STORAGE_KEY, + "data": { + "devices": [ + { + "area_id": None, + "config_entries": [entry_1.entry_id, entry_2.entry_id], + "config_entries_subentries": { + entry_1.entry_id: [None], + entry_2.entry_id: [None], + }, + "configuration_url": None, + "connections": [], + "created_at": "1970-01-01T00:00:00+00:00", + "disabled_by": None, + "entry_type": None, + "hw_version": None, + "id": composite_id, + "identifiers": [["test", "composite"]], + "labels": [], + "manufacturer": None, + "model": None, + "name": None, + "model_id": None, + "modified_at": "1970-01-01T00:00:00+00:00", + "name_by_user": None, + "primary_config_entry": entry_1.entry_id, + "serial_number": None, + "sw_version": None, + # Buggy self-reference remapped to each split by the 3.2 migration + "via_device_id": composite_id, + } + ], + "deleted_devices": [], + }, + } + + dr.async_setup(hass) + await dr.async_load(hass) + registry = dr.async_get(hass) + + splits = registry.devices.get_devices_for_composite_device_id(composite_id) + assert len(splits) == 2 + assert all(split.via_device_id is None for split in splits) + + @pytest.mark.parametrize("load_registries", [False]) async def test_migration_collapses_multi_subentry_device( hass: HomeAssistant, hass_storage: dict[str, Any] @@ -4013,6 +4134,132 @@ async def test_update_device_unknown_via_device_id_raises_before_removal( assert device_registry.async_get(device.id) == device +async def test_get_or_create_via_device_self_reference_ignored( + hass: HomeAssistant, + device_registry: dr.DeviceRegistry, + caplog: pytest.LogCaptureFixture, +) -> None: + """A device referencing itself via the deprecated via_device is ignored and logged.""" + config_entry = MockConfigEntry() + config_entry.add_to_hass(hass) + device = device_registry.async_get_or_create( + config_entry_id=config_entry.entry_id, identifiers={("hue", "self")} + ) + + updated = device_registry.async_get_or_create( + config_entry_id=config_entry.entry_id, + identifiers={("hue", "self")}, + via_device=("hue", "self"), + ) + + assert updated.id == device.id + assert updated.via_device_id is None + assert ( + "calls `device_registry.async_get_or_create` with a `via_device` " + "referencing the device itself" in caplog.text + ) + + +async def test_get_or_create_via_device_id_self_reference_raises( + hass: HomeAssistant, device_registry: dr.DeviceRegistry +) -> None: + """A device referencing itself via via_device_id raises, leaving it unchanged.""" + config_entry = MockConfigEntry() + config_entry.add_to_hass(hass) + device = device_registry.async_get_or_create( + config_entry_id=config_entry.entry_id, identifiers={("hue", "self")} + ) + + with pytest.raises( + HomeAssistantError, match="A device can not be its own via device" + ): + device_registry.async_get_or_create( + config_entry_id=config_entry.entry_id, + identifiers={("hue", "self")}, + via_device_id=device.id, + ) + + assert device_registry.async_get(device.id).via_device_id is None + + +async def test_update_device_via_device_id_self_reference_raises( + hass: HomeAssistant, device_registry: dr.DeviceRegistry +) -> None: + """Updating a device to reference itself via via_device_id raises.""" + config_entry = MockConfigEntry() + config_entry.add_to_hass(hass) + device = device_registry.async_get_or_create( + config_entry_id=config_entry.entry_id, identifiers={("hue", "self")} + ) + + with pytest.raises( + HomeAssistantError, match="A device can not be its own via device" + ): + device_registry.async_update_device(device.id, via_device_id=device.id) + + assert device_registry.async_get(device.id).via_device_id is None + + +async def test_update_device_via_device_id_self_reference_raises_before_removal( + hass: HomeAssistant, device_registry: dr.DeviceRegistry +) -> None: + """A self-referencing via_device_id raises before a removal in the same call.""" + config_entry = MockConfigEntry() + config_entry.add_to_hass(hass) + device = device_registry.async_get_or_create( + config_entry_id=config_entry.entry_id, identifiers={("hue", "device")} + ) + + with pytest.raises( + HomeAssistantError, match="A device can not be its own via device" + ): + device_registry.async_update_device( + device.id, + remove_config_entry_id=config_entry.entry_id, + via_device_id=device.id, + ) + + # The device was not removed + assert device_registry.async_get(device.id) == device + + +async def test_update_device_composite_via_device_id_self_reference_raises_before_removal( + hass: HomeAssistant, device_registry: dr.DeviceRegistry +) -> None: + """A composite via_device_id self-reference raises before a removal in one call.""" + entry_1 = MockConfigEntry(domain="test") + entry_1.add_to_hass(hass) + entry_2 = MockConfigEntry(domain="test") + entry_2.add_to_hass(hass) + device_1 = device_registry.async_get_or_create( + config_entry_id=entry_1.entry_id, identifiers={("test", "1")} + ) + device_2 = device_registry.async_get_or_create( + config_entry_id=entry_2.entry_id, identifiers={("test", "2")} + ) + old_id = "composite00000000000000000000ab" + # Simulate a migration split: both devices carry the pre-migration composite id + device_registry.devices[device_1.id] = attr.evolve( + device_1, composite_device_id=old_id + ) + device_registry.devices[device_2.id] = attr.evolve( + device_2, composite_device_id=old_id + ) + + # old_id resolves to device_1 (the split owned by entry_1), so linking device_1 to + # it is a self-reference; it must raise before the removal deletes the device. + with pytest.raises( + HomeAssistantError, match="A device can not be its own via device" + ): + device_registry.async_update_device( + device_1.id, + remove_config_entry_id=entry_1.entry_id, + via_device_id=old_id, + ) + + assert device_1.id in device_registry.devices + + async def test_get_or_create_composite_via_device_id_resolved( hass: HomeAssistant, device_registry: dr.DeviceRegistry,