From ace2ec2cf5dd2943347ca8f51145e7d0930f85d3 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Fri, 28 Aug 2026 15:17:58 +0200 Subject: [PATCH] Guard restored discovery messages against uuid and service collisions Discovery messages are keyed by uuid across all apps, so restoring a message whose uuid is already taken silently dropped the message it collided with, including one belonging to another app. Skip such a message instead, and log it as it points at a corrupt backup. The set of services which already have a message was also built once before the loop, so a backup listing the same service twice ended up with two messages for it. Only one of the two can ever be updated or removed again, since send() and remove() match on app and service, while both are handed to Home Assistant. Grow the set while restoring so that the first message for a service wins. Co-Authored-By: Claude Opus 5 (1M context) --- supervisor/discovery/__init__.py | 19 +++++++++- tests/discovery/test_discovery.py | 61 +++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/supervisor/discovery/__init__.py b/supervisor/discovery/__init__.py index 1427950f6..e73cd4929 100644 --- a/supervisor/discovery/__init__.py +++ b/supervisor/discovery/__init__.py @@ -142,11 +142,15 @@ class Discovery(CoreSysAttributes, FileConfiguration): is not started again. Instead it is marked as unannounced so that the push happens once the app sends the message itself. """ - live_services = {message.service for message in self.messages_for_app(app.slug)} + # Services which already have a message, grown while restoring so that + # a backup listing a service twice does not end up with two messages + known_services = { + message.service for message in self.messages_for_app(app.slug) + } restored = False for message in messages: - if message.service in live_services: + if message.service in known_services: continue if message.service not in app.discovery: _LOGGER.info( @@ -155,9 +159,20 @@ class Discovery(CoreSysAttributes, FileConfiguration): app.slug, ) continue + if message.uuid in self.message_obj: + # Messages are keyed by uuid across all apps, so restoring this + # one would drop the message it collides with + _LOGGER.warning( + "Skipping discovery message for service %s of app %s, uuid %s is already in use", + message.service, + app.slug, + message.uuid, + ) + continue self.message_obj[message.uuid] = message self._unannounced.add(message.uuid) + known_services.add(message.service) restored = True _LOGGER.info( "Restored discovery %s for service %s from %s", diff --git a/tests/discovery/test_discovery.py b/tests/discovery/test_discovery.py index fa5c2c242..603908908 100644 --- a/tests/discovery/test_discovery.py +++ b/tests/discovery/test_discovery.py @@ -131,6 +131,67 @@ async def test_restore_app_messages_skips_dropped_service( assert "app local_ssh does not provide it anymore" in caplog.text +async def test_restore_app_messages_skips_uuid_in_use( + coresys: CoreSys, app_with_discovery: App, caplog: pytest.LogCaptureFixture +): + """Test a message does not overwrite the message another app owns. + + Messages are keyed by uuid across all apps, so restoring a uuid which is + already taken would drop the message it collides with. + """ + other = Message(app="core_mosquitto", service="mqtt", config={}, uuid=BACKUP_UUID) + coresys.discovery.message_obj[other.uuid] = other + + await coresys.discovery.restore_app_messages( + app_with_discovery, + [ + Message( + app=app_with_discovery.slug, + service="mcp", + config=MCP_CONFIG, + uuid=BACKUP_UUID, + ) + ], + ) + + assert coresys.discovery.get(BACKUP_UUID) is other + assert coresys.discovery.messages_for_app(app_with_discovery.slug) == [] + assert f"uuid {BACKUP_UUID} is already in use" in caplog.text + + +async def test_restore_app_messages_service_only_once( + coresys: CoreSys, app_with_discovery: App +): + """Test a service repeated in a backup only gets a single message. + + Two messages for one service would both be handed to Home Assistant, and + only one of them can ever be updated or removed again, as send() and + remove() match on app and service. + """ + await coresys.discovery.restore_app_messages( + app_with_discovery, + [ + Message( + app=app_with_discovery.slug, + service="mcp", + config=MCP_CONFIG, + uuid=BACKUP_UUID, + ), + Message( + app=app_with_discovery.slug, + service="mcp", + config={"url": "http://local-ssh:9999/mcp"}, + uuid="a1b2c3d4e5f60718293a4b5c6d7e8f90", + ), + ], + ) + + assert [ + message.uuid + for message in coresys.discovery.messages_for_app(app_with_discovery.slug) + ] == [BACKUP_UUID] + + async def test_messages_for_app(coresys: CoreSys, app_with_discovery: App): """Test listing the messages of a single app.""" message = await coresys.discovery.send(app_with_discovery, "mcp", dict(MCP_CONFIG))