fix: sidebar settings panel 503s forever when webhook access is disabled (#1806)

* fix: keep the sidebar settings panel working when webhook access is disabled (#1803)

The panel proxy reads its loopback target from the webhook forwarding
config, which was only created when the remote-webhook option was on --
with it off the proxy returned 503 forever even though the server was
running. Store the forwarding config whenever the server starts and gate
only the public webhook endpoint (and ha_auth surface) on the option.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix: address review findings on the webhook-disabled forwarding path

- clear any leftover webhook registration even in local-only mode (off
  means off after a crashed unload); runs before the session opens so it
  cannot leak
- pin the producer->consumer seam: the panel proxy forwards using a cfg
  stored by async_register_webhook(register_endpoint=False)
- assert the stored session identity and the skipped endpoint in the
  ha_auth local-only test
- tighten the _active_resource_server and ui_panel docstrings for the
  decoupled forwarding config

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: kingpanther13 <kingpanther13@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: kingpanther13

custom_components/ha_mcp_tools/embedded_setup.py

@@ -106,15 +106,18 @@ async def async_bring_up_server(hass: HomeAssistant, entry: ConfigEntry) -> None
         auth_mode = str(entry.options.get(OPT_WEBHOOK_AUTH, WEBHOOK_AUTH_NONE))
         secret_path = str(entry.data[DATA_SECRET_PATH])
         webhook_enabled = bool(entry.options.get(OPT_ENABLE_WEBHOOK, True))
-        if webhook_enabled:
-            await async_register_webhook(
-                hass,
-                entry,
-                port=manager.port,
-                secret_path=secret_path,
-                auth_mode=auth_mode,
-            )
-        else:
+        # Always set up the loopback forwarding config — the sidebar settings
+        # panel proxies through it (#1803); the option gates only the public
+        # webhook endpoint.
+        await async_register_webhook(
+            hass,
+            entry,
+            port=manager.port,
+            secret_path=secret_path,
+            auth_mode=auth_mode,
+            register_endpoint=webhook_enabled,
+        )
+        if not webhook_enabled:
             _LOGGER.info(
                 "Webhook access disabled by option - the server is local-only "
                 "(direct port + sidebar panel)"

custom_components/ha_mcp_tools/manifest.json

@@ -19,5 +19,5 @@
     "requirements": [
         "ruamel.yaml>=0.18.0"
     ],
-    "version": "1.0.3"
+    "version": "1.0.4"
 }

custom_components/ha_mcp_tools/mcp_webhook.py

@@ -206,8 +206,9 @@ def _active_resource_server(hass: HomeAssistant) -> ResourceServer | None:
     at registration time: aiohttp can't drop a bound view until HA restarts, so
     a remove + re-add of the config entry (which mints a NEW webhook id in the
     same HA session) would otherwise leave the views advertising the old id.
-    Returns None when no entry is live or the webhook auth mode is not ha_auth
-    — the views then 404 like an unregistered route.
+    Returns None when no entry is live, the webhook auth mode is not ha_auth,
+    or the public endpoint is disabled (local-only mode constructs no resource
+    server even under ha_auth) — the views then 404 like an unregistered route.
     """
     domain_data = hass.data.get(DOMAIN)
     if not isinstance(domain_data, dict):
@@ -502,6 +503,7 @@ async def async_register_webhook(
     port: int,
     secret_path: str,
     auth_mode: str,
+    register_endpoint: bool = True,
 ) -> None:
     """Register the ingress webhook (and, for ha_auth, the discovery views).
 
@@ -510,6 +512,12 @@ async def async_register_webhook(
     already unregistered, so the caller never leaves a half-configured endpoint
     live. ``webhook`` is a manifest dependency, so HA guarantees it is set up
     before this runs.
+
+    With ``register_endpoint=False`` (remote webhook access disabled by option)
+    no public endpoint or ha_auth surface is created — and any leftover endpoint
+    from a crashed unload is cleared, so off means off; only the forwarding
+    config is stored, which same-host consumers — the sidebar settings panel
+    proxy — need to reach the loopback server (#1803).
     """
     if auth_mode not in (WEBHOOK_AUTH_NONE, WEBHOOK_AUTH_HA):
         # Fail CLOSED on an unknown mode (corrupt/migrated options): refusing
@@ -518,6 +526,11 @@ async def async_register_webhook(
         raise ValueError(f"Unknown webhook auth mode: {auth_mode!r}")
 
     webhook_id: str = entry.data[DATA_WEBHOOK_ID]
+    # Reload-safe and off-means-off: clear any leftover registration from a
+    # crashed unload before (re)registering — or before storing a local-only
+    # config (async_unregister is a no-op pop when nothing is registered).
+    # Runs before the session opens so a raise here cannot leak it.
+    async_unregister(hass, webhook_id)
     target_url = f"http://127.0.0.1:{port}{secret_path}"
     session = aiohttp.ClientSession(timeout=_CLIENT_TIMEOUT)
 
@@ -529,31 +542,29 @@ async def async_register_webhook(
         "resource_server": None,
     }
 
-    try:
-        # Reload-safe: clear any leftover registration from a crashed unload
-        # before (re)registering (async_unregister is a no-op pop).
-        async_unregister(hass, webhook_id)
-        async_register(
-            hass,
-            DOMAIN,
-            _WEBHOOK_NAME,
-            webhook_id,
-            _async_handle_webhook,
-            allowed_methods=["POST", "GET"],
-        )
-        if auth_mode == WEBHOOK_AUTH_HA:
-            provider = ResourceServer(hass, webhook_id)
-            _register_metadata_views(hass)
-            cfg["resource_server"] = provider
-    except Exception:
-        # Never leave a live endpoint (or a leaked session) behind a failed
-        # auth-setup path. suppress: the ORIGINAL error must be what
-        # propagates (review finding) - a raising cleanup would mask it.
-        with suppress(Exception):
-            async_unregister(hass, webhook_id)
-        with suppress(Exception):
-            await session.close()
-        raise
+    if register_endpoint:
+        try:
+            async_register(
+                hass,
+                DOMAIN,
+                _WEBHOOK_NAME,
+                webhook_id,
+                _async_handle_webhook,
+                allowed_methods=["POST", "GET"],
+            )
+            if auth_mode == WEBHOOK_AUTH_HA:
+                provider = ResourceServer(hass, webhook_id)
+                _register_metadata_views(hass)
+                cfg["resource_server"] = provider
+        except Exception:
+            # Never leave a live endpoint (or a leaked session) behind a failed
+            # auth-setup path. suppress: the ORIGINAL error must be what
+            # propagates (review finding) - a raising cleanup would mask it.
+            with suppress(Exception):
+                async_unregister(hass, webhook_id)
+            with suppress(Exception):
+                await session.close()
+            raise
 
     hass.data.setdefault(DOMAIN, {})[DATA_WEBHOOK] = cfg
 

custom_components/ha_mcp_tools/ui_panel.py

@@ -38,9 +38,10 @@
    bearer) validates that cookie against a live admin user on every request and
    forwards to the loopback settings server.
 
-The proxy reuses the ingress webhook's loopback target + aiohttp session
-(``hass.data[DOMAIN][DATA_WEBHOOK]``), so it is available exactly while the server
-is running and returns 503 otherwise.
+The proxy reuses the server's loopback forwarding config + aiohttp session
+(``hass.data[DOMAIN][DATA_WEBHOOK]`` — stored whenever the server is running,
+even when the public webhook endpoint is disabled), so it is available exactly
+while the server is running and returns 503 otherwise.
 """
 
 from __future__ import annotations

tests/src/unit/test_embedded_setup.py

@@ -149,10 +149,14 @@ async def test_success_clears_stale_repair_issues(self, fake_manager):
             esetup.ISSUE_UPDATE_HELD,
         }
 
-    async def test_local_only_skips_webhook_registration(self, fake_manager, caplog):
+    async def test_local_only_skips_endpoint_but_keeps_forwarding(
+        self, fake_manager, caplog
+    ):
         # Owner request: enable_webhook=False must never register the webhook
-        # (Nabu Casa path dead) while the server still starts; the log carries
-        # the local-only note.
+        # endpoint (Nabu Casa path dead) while the server still starts; the log
+        # carries the local-only note. The forwarding config must still be set
+        # up (register_endpoint=False) or the sidebar settings panel 503s
+        # forever (#1803).
         import logging
 
         hass = _make_hass()
@@ -162,7 +166,9 @@ async def test_local_only_skips_webhook_registration(self, fake_manager, caplog)
             await esetup.async_bring_up_server(hass, entry)
 
         fake_manager.async_start.assert_awaited_once()
-        esetup.async_register_webhook.assert_not_awaited()
+        esetup.async_register_webhook.assert_awaited_once()
+        kwargs = esetup.async_register_webhook.await_args.kwargs
+        assert kwargs["register_endpoint"] is False
         esetup._surface_connect_urls.assert_called_once()
         assert "local-only" in caplog.text
 
@@ -177,6 +183,7 @@ async def test_passes_auth_mode_port_and_secret_to_webhook(self, fake_manager):
         assert kwargs["auth_mode"] == WEBHOOK_AUTH_HA
         assert kwargs["port"] == 9584
         assert kwargs["secret_path"] == "/private_secret"
+        assert kwargs["register_endpoint"] is True
 
     async def test_package_failure_files_package_issue_and_skips_webhook(
         self, fake_manager

tests/src/unit/test_mcp_webhook.py

@@ -724,3 +724,56 @@ async def test_unregister_is_idempotent(self):
         # No cfg present — must be a clean no-op.
         await mw.async_unregister_webhook(hass)
         await mw.async_unregister_webhook(hass)
+
+    async def test_register_endpoint_false_stores_forwarding_only(self, monkeypatch):
+        # #1803: with remote webhook access disabled, the loopback forwarding
+        # config must still be stored (the sidebar settings panel proxies
+        # through it) while NO public endpoint is registered.
+        hass = _register_hass()
+        fake_session = FakeSession()
+        monkeypatch.setattr(mw.aiohttp, "ClientSession", lambda **kw: fake_session)
+
+        await mw.async_register_webhook(
+            hass,
+            _entry(),
+            port=9584,
+            secret_path="/private_x",
+            auth_mode=WEBHOOK_AUTH_NONE,
+            register_endpoint=False,
+        )
+
+        cfg = hass.data[DOMAIN][DATA_WEBHOOK]
+        assert cfg["target_url"] == "http://127.0.0.1:9584/private_x"
+        assert cfg["session"] is fake_session
+        assert cfg["resource_server"] is None
+        mw.async_register.assert_not_called()
+        # Off means off: a leftover endpoint from a crashed unload is cleared
+        # even though nothing gets (re)registered.
+        mw.async_unregister.assert_called_once_with(hass, WEBHOOK_ID)
+
+        # Teardown still drops the cfg and closes the session.
+        await mw.async_unregister_webhook(hass)
+        assert DATA_WEBHOOK not in hass.data[DOMAIN]
+        assert fake_session.closed is True
+
+    async def test_register_endpoint_false_skips_ha_auth_surface(self, monkeypatch):
+        # Even with ha_auth configured, a disabled endpoint must not construct
+        # the resource server or bind the discovery views — the per-request
+        # resolver would otherwise advertise a webhook that does not exist.
+        hass = _register_hass()
+        monkeypatch.setattr(mw.aiohttp, "ClientSession", lambda **kw: FakeSession())
+
+        await mw.async_register_webhook(
+            hass,
+            _entry(),
+            port=9584,
+            secret_path="/private_x",
+            auth_mode=WEBHOOK_AUTH_HA,
+            register_endpoint=False,
+        )
+
+        cfg = hass.data[DOMAIN][DATA_WEBHOOK]
+        assert cfg["resource_server"] is None
+        mw.async_register.assert_not_called()
+        hass.http.register_view.assert_not_called()
+        assert mw._active_resource_server(hass) is None

tests/src/unit/test_ui_panel.py

@@ -231,6 +231,41 @@ async def test_path_traversal_rejected(self):
         resp = await ui_panel._ProxyView().get(request, "../secrets")
         assert resp.status == 400
 
+    async def test_forwards_with_cfg_from_local_only_setup(self, monkeypatch):
+        # #1803 end-to-end at unit level: the forwarding config stored by
+        # async_register_webhook(register_endpoint=False) must be directly
+        # consumable by the panel proxy — a cfg key rename on either side of
+        # the seam would 503 the sidebar panel again with both halves' own
+        # tests still green.
+        from custom_components.ha_mcp_tools import mcp_webhook as mw
+        from custom_components.ha_mcp_tools.const import DATA_WEBHOOK_ID
+
+        upstream = FakeUpstream(
+            status=200,
+            headers={"Content-Type": "text/html; charset=utf-8"},
+            body=b"<html>settings</html>",
+        )
+        session = FakeSession(upstream=upstream)
+        monkeypatch.setattr(mw.aiohttp, "ClientSession", lambda **kw: session)
+        hass = _make_hass(user=_make_user())
+        entry = MagicMock()
+        entry.data = {DATA_WEBHOOK_ID: "wh-seam"}
+
+        await mw.async_register_webhook(
+            hass,
+            entry,
+            port=9584,
+            secret_path="/private_x",
+            auth_mode=mw.WEBHOOK_AUTH_NONE,
+            register_endpoint=False,
+        )
+        request = _make_request(hass=hass, cookies=_valid_cookie(hass))
+
+        resp = await ui_panel._ProxyView().get(request, "settings")
+
+        assert resp.status == 200
+        assert session.calls[0]["url"] == "http://127.0.0.1:9584/private_x/settings"
+
     async def test_forwards_page_and_passes_through_html(self):
         upstream = FakeUpstream(
             status=200,