Browse Source

fix(smart-plugs): HA entity search bypassed the schema's domain whitelist (#1388)

  Reporter MartinNYHC opened the Add Smart Plug dialog in HA mode, typed
  a search prefix matching a multi-entity device (one switch.* plus
  several sensor.*/binary_sensor.* siblings under the same friendly
  name), clicked one of the non-switch siblings, and got a 422 on Save:

    String should match pattern
    '^(switch|light|input_boolean|script)\.[a-z0-9_]+$'

  The screenshot confirms the bug shape — the X button next to the
  "empty-looking" Select Entity field only renders when haEntityId is
  truthy. So haEntityId was set, but selectedEntity (haEntities.find by
  that id) returned undefined, so the input rendered the placeholder
  text instead of the friendly-name display. That can only happen when
  the user had earlier picked an entity whose domain is NOT in the
  schema's allowed list, then the search cleared, the entity-list
  refetched without a search param, and the refreshed list (filtered to
  the default domains) no longer contained the user's pick.

  Root cause was in HomeAssistantService.list_entities: when a search
  query was present, the function bypassed the domain filter entirely
  and returned matches across every HA domain. Offering a clickable
  choice the schema can't accept is broken UX, and the cryptic Pydantic
  pattern echo on save made it look like a backend/schema problem
  rather than a search-permissiveness problem. Confirmed via git diff
  that the smart-plug code path is unchanged between v0.2.4 and
  0.2.4.1 — this has been latent since the script-domain commit in
  February 2026, only noticed now because the reporter hadn't reopened
  the modal in months.

  Fix: always apply the allowed-domains filter ({switch, light,
  input_boolean, script} — kept in sync with the regex in
  backend/app/schemas/smart_plug.py:17). Search composes on top as a
  substring match against entity_id or friendly_name, instead of
  replacing the domain filter. Whitespace-only search strings now
  fall back to the no-search behavior.
maziggy 1 week ago
parent
commit
135b8fd93b

File diff suppressed because it is too large
+ 0 - 0
CHANGELOG.md


+ 19 - 12
backend/app/services/homeassistant.py

@@ -237,8 +237,15 @@ class HomeAssistantService:
     async def list_entities(self, url: str, token: str, search: str | None = None) -> list[dict]:
     async def list_entities(self, url: str, token: str, search: str | None = None) -> list[dict]:
         """List available entities from HA.
         """List available entities from HA.
 
 
-        By default, returns switch/light/input_boolean domains.
-        When search is provided, searches ALL entities by entity_id or friendly_name.
+        Always filters to switch/light/input_boolean/script — the only domains
+        the SmartPlugBase.ha_entity_id pattern accepts. When a search query is
+        provided it narrows the same domain-filtered list by entity_id or
+        friendly_name substring (case-insensitive).
+
+        Previously search bypassed the domain filter, which let users pick a
+        sensor.* or binary_sensor.* entity from the dropdown that the backend
+        schema would then reject with the cryptic Pydantic pattern error
+        (#1388). Picking what you can't save isn't a useful UX.
 
 
         Returns list of entity dicts with:
         Returns list of entity dicts with:
             - entity_id: str
             - entity_id: str
@@ -246,8 +253,9 @@ class HomeAssistantService:
             - state: str
             - state: str
             - domain: str
             - domain: str
         """
         """
-        # Default domains for smart plug control
-        default_domains = {"switch", "light", "input_boolean", "script"}
+        # Allowed domains for smart plug control — must mirror the regex in
+        # backend/app/schemas/smart_plug.py:17 (SmartPlugBase.ha_entity_id).
+        allowed_domains = {"switch", "light", "input_boolean", "script"}
 
 
         try:
         try:
             async with httpx.AsyncClient(timeout=self.timeout) as client:
             async with httpx.AsyncClient(timeout=self.timeout) as client:
@@ -265,14 +273,13 @@ class HomeAssistantService:
                     domain = entity_id.split(".")[0] if "." in entity_id else ""
                     domain = entity_id.split(".")[0] if "." in entity_id else ""
                     friendly_name = entity.get("attributes", {}).get("friendly_name", entity_id)
                     friendly_name = entity.get("attributes", {}).get("friendly_name", entity_id)
 
 
-                    # If searching, match against entity_id or friendly_name
-                    if search_lower:
-                        if search_lower not in entity_id.lower() and search_lower not in friendly_name.lower():
-                            continue
-                    else:
-                        # No search: filter to default domains only
-                        if domain not in default_domains:
-                            continue
+                    if domain not in allowed_domains:
+                        continue
+
+                    if search_lower and (
+                        search_lower not in entity_id.lower() and search_lower not in friendly_name.lower()
+                    ):
+                        continue
 
 
                     entities.append(
                     entities.append(
                         {
                         {

+ 148 - 0
backend/tests/unit/services/test_homeassistant_list_entities.py

@@ -0,0 +1,148 @@
+"""Regression tests for HomeAssistantService.list_entities domain filtering (#1388).
+
+Reporter MartinNYHC opened the Add Smart Plug modal in HA mode, typed a search
+matching a multi-entity device (one Shelly outlet exposed as switch + several
+sensor.* and binary_sensor.* siblings), and clicked a non-switch entity. The
+schema regex for ha_entity_id only accepts switch/light/input_boolean/script,
+so the Save round-trip came back 422 with the raw Pydantic pattern string —
+the same regex shown in the bug report screenshot.
+
+Root cause: before this fix, the search path bypassed the domain filter
+entirely, so the dropdown showed every entity whose entity_id or friendly_name
+matched the query, regardless of whether the schema would later accept it.
+Users could click an entity they had no way to actually save.
+
+Fix: always apply the allowed-domains filter, and apply the search filter on
+top of it. The two filters now compose instead of branching.
+"""
+
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+
+from backend.app.services.homeassistant import HomeAssistantService
+
+
+def _ha_response(entities: list[dict]) -> MagicMock:
+    response = MagicMock()
+    response.raise_for_status = MagicMock()
+    response.json = MagicMock(return_value=entities)
+    return response
+
+
+def _mock_get(entities: list[dict]):
+    async_client = MagicMock()
+    async_client.get = AsyncMock(return_value=_ha_response(entities))
+    async_client.__aenter__ = AsyncMock(return_value=async_client)
+    async_client.__aexit__ = AsyncMock(return_value=None)
+    return async_client
+
+
+@pytest.mark.asyncio
+async def test_no_search_returns_only_allowed_domains():
+    """Without a search query, only switch/light/input_boolean/script appear."""
+    entities = [
+        {"entity_id": "switch.printer", "attributes": {"friendly_name": "Printer"}, "state": "on"},
+        {"entity_id": "light.lamp", "attributes": {"friendly_name": "Lamp"}, "state": "off"},
+        {"entity_id": "input_boolean.flag", "attributes": {"friendly_name": "Flag"}, "state": "on"},
+        {"entity_id": "script.morning", "attributes": {"friendly_name": "Morning"}, "state": "off"},
+        {"entity_id": "sensor.power", "attributes": {"friendly_name": "Power"}, "state": "12.3"},
+        {"entity_id": "binary_sensor.status", "attributes": {"friendly_name": "Status"}, "state": "on"},
+        {"entity_id": "media_player.tv", "attributes": {"friendly_name": "TV"}, "state": "idle"},
+    ]
+    service = HomeAssistantService()
+
+    with patch("httpx.AsyncClient", return_value=_mock_get(entities)):
+        result = await service.list_entities("http://ha.local", "tok")
+
+    domains = sorted({e["domain"] for e in result})
+    assert domains == ["input_boolean", "light", "script", "switch"]
+    assert len(result) == 4
+
+
+@pytest.mark.asyncio
+async def test_search_still_filters_to_allowed_domains():
+    """#1388: search must compose with the domain filter, not replace it.
+
+    Reporter's setup: a Shelly outlet device generates one switch.* entity
+    and several sensor.*/binary_sensor.* siblings, all sharing a common
+    friendly-name prefix. The user searched the prefix and was offered the
+    non-switch siblings as clickable options — picking one led to the 422
+    pattern error. After the fix, the search-narrowed list excludes them.
+    """
+    entities = [
+        {
+            "entity_id": "switch.prise_imprimante_3d_bambu_output_1",
+            "attributes": {"friendly_name": "Prise imprimante 3D Bambu Output 1"},
+            "state": "on",
+        },
+        {
+            "entity_id": "sensor.prise_imprimante_3d_bambu_output_1_power",
+            "attributes": {"friendly_name": "Prise imprimante 3D Bambu Output 1 Puissance"},
+            "state": "12.5",
+        },
+        {
+            "entity_id": "binary_sensor.prise_imprimante_3d_bambu_output_1_status",
+            "attributes": {"friendly_name": "Prise imprimante 3D Bambu Output 1 Status"},
+            "state": "on",
+        },
+        {
+            "entity_id": "sensor.prise_imprimante_3d_bambu_output_1_energy",
+            "attributes": {"friendly_name": "Prise imprimante 3D Bambu Output 1 Énergie"},
+            "state": "0.42",
+        },
+    ]
+    service = HomeAssistantService()
+
+    with patch("httpx.AsyncClient", return_value=_mock_get(entities)):
+        result = await service.list_entities("http://ha.local", "tok", search="Prise imprimante")
+
+    assert len(result) == 1
+    assert result[0]["entity_id"] == "switch.prise_imprimante_3d_bambu_output_1"
+
+
+@pytest.mark.asyncio
+async def test_search_matches_by_entity_id_or_friendly_name():
+    """Search still matches across both fields, just within the allowed set."""
+    entities = [
+        {"entity_id": "switch.printer_a", "attributes": {"friendly_name": "Living Room Plug"}, "state": "on"},
+        {"entity_id": "switch.printer_b", "attributes": {"friendly_name": "Office Plug"}, "state": "off"},
+        {"entity_id": "light.living_room", "attributes": {"friendly_name": "Ceiling"}, "state": "off"},
+    ]
+    service = HomeAssistantService()
+
+    with patch("httpx.AsyncClient", return_value=_mock_get(entities)):
+        result = await service.list_entities("http://ha.local", "tok", search="living")
+
+    ids = sorted(e["entity_id"] for e in result)
+    assert ids == ["light.living_room", "switch.printer_a"]
+
+
+@pytest.mark.asyncio
+async def test_search_is_case_insensitive():
+    entities = [
+        {"entity_id": "switch.PRINTER", "attributes": {"friendly_name": "Printer"}, "state": "on"},
+    ]
+    service = HomeAssistantService()
+
+    with patch("httpx.AsyncClient", return_value=_mock_get(entities)):
+        result = await service.list_entities("http://ha.local", "tok", search="PRINTER")
+
+    assert len(result) == 1
+
+
+@pytest.mark.asyncio
+async def test_empty_search_treated_as_no_search():
+    """A whitespace-only search string should fall back to the full allowed-
+    domain list rather than matching everything that contains an empty string."""
+    entities = [
+        {"entity_id": "switch.foo", "attributes": {"friendly_name": "Foo"}, "state": "on"},
+        {"entity_id": "sensor.bar", "attributes": {"friendly_name": "Bar"}, "state": "1"},
+    ]
+    service = HomeAssistantService()
+
+    with patch("httpx.AsyncClient", return_value=_mock_get(entities)):
+        result = await service.list_entities("http://ha.local", "tok", search="   ")
+
+    assert len(result) == 1
+    assert result[0]["entity_id"] == "switch.foo"

Some files were not shown because too many files changed in this diff