Don't allow devices setting the via device link to itself (#178194)

Co-authored-by: Artur Pragacz <49985303+arturpragacz@users.noreply.github.com>
This commit is contained in:
Erik Montnemery
2026-08-08 18:05:57 +02:00
committed by GitHub
co-authored by Artur Pragacz
parent 177810d00c
commit f5dc5aa9ec
3 changed files with 287 additions and 34 deletions
+40 -2
View File
@@ -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
-32
View File
@@ -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,
+247
View File
@@ -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,