fix(webhook-proxy): surface webhook registration failures instead of silently loading (#1101)

* fix(webhook-proxy): surface webhook registration failures instead of silently loading

Defect 2 of #1020: when /config/.mcp_proxy_config.json contains a corrupted
target_url (e.g. the secret-path truncation triggered by Defect 1), the
mcp_proxy custom integration would proceed past its empty-string guard,
register with the truncated URL, and report state="loaded" while every
webhook request returned 404 from HA core.

Two changes in async_setup_entry:

- Validate target_url shape before registering. The path must match
  /private_<token> with token >= 16 chars (real ha-mcp generates a
  22-char base64url token; the truncation bug yielded ~7). On failure,
  raise ConfigEntryError(reason) so HA surfaces the reason in the UI
  and config-entries API instead of returning False.
- Wrap async_register() in try/except. On failure, close the aiohttp
  session and re-raise as ConfigEntryError with the original error as
  cause. Move hass.data[DOMAIN] assignment to after a successful
  register so a failed setup doesn't leave half-state.

Tests cover the issue's exact reproducer (/private_ZZZZZZZ truncation),
all rejection paths, async_register raising, and the happy path. They
stub homeassistant.* and aiohttp via sys.modules so the integration can
be imported under our existing pytest harness without pulling HA Core
into dev deps.

Defect 1 (the parser truncation itself) is addressed separately via
PR #952, which persists the secret path to addon options so the regex
fallback isn't reached in practice.

* fix(webhook-proxy): close additional silent-failure gaps and tighten validator

Address PR-review findings on top of the original Defect 2 fix:

- _read_config now lets OSError/JSONDecodeError propagate instead of
  swallowing them. async_setup_entry catches them and raises
  ConfigEntryError so a corrupted /config/.mcp_proxy_config.json no
  longer masquerades as a "no config yet, fresh install" state.
- _validate_target_url now also rejects URLs containing query strings,
  fragments, or path parameters. urlparse strips these from parsed.path,
  so previously something like /private_<token>?x=y validated clean but
  the addon would 404 on the forwarded request.
- Drop the dead try/except ValueError around urlparse — it doesn't raise
  on any string input.
- Validator failure reasons no longer echo parsed.path verbatim, which
  was leaking the raw secret token into ERROR-level logs even when the
  caller's masked_target was correct. Caught by the new caplog test.
- Reconcile the regex-vs-comment mismatch: comment said "22-char token",
  regex enforced >=16. Reworded as "real tokens are 22 chars; we accept
  >=16 as a sanity floor" to match what the code actually does.

Tests:

- TestUnloadEntry — async_unload_entry was untested. Two cases now pin
  that unload after a failed setup is a no-op (no async_unregister, no
  double-close) and that unload after a successful setup unregisters and
  closes the session.
- test_truncated_url_does_not_log_full_token — caplog assertion that
  catches the secret-leak in validator reasons.
- test_corrupted_json_raises_config_entry_error /
  test_unreadable_config_raises_config_entry_error — pin the new behavior
  that distinguishes "no file" from "broken file".
- test_register_failure_closes_session_and_raises is now parametrized
  over RuntimeError/ValueError/KeyError (HA's documented failure modes
  for async_register), documenting that the broad except catch is
  intentional.
- TestTargetUrlValidation gains 15-char boundary, query, fragment, and
  path-params rejection tests.
- All early-fail paths now also assert ClientSession is never constructed
  so a future "early connect probing" refactor can't silently leak
  resources.

Bump both the addon (config.yaml: 1.0.1 -> 1.0.2) and the bundled
mcp_proxy integration (manifest.json: 1.0.0 -> 1.0.2). The two had
diverged because an earlier addon-only bump didn't touch the integration;
aligning both to 1.0.2 going forward since they always ship together.

---------

Co-authored-by: kingpanther13 <kingpanther13@users.noreply.github.com>

Author: kingpanther13

homeassistant-addon-webhook-proxy/config.yaml

@@ -1,6 +1,6 @@
 name: "Nabu Casa / Webhook Proxy for HA MCP"
 description: "Remote access proxy via Nabu Casa or any reverse proxy (Cloudflare, DuckDNS, nginx)"
-version: "1.0.1"
+version: "1.0.2"
 slug: "ha_mcp_webhook_proxy"
 url: "https://github.com/homeassistant-ai/ha-mcp"
 arch:

homeassistant-addon-webhook-proxy/mcp_proxy/__init__.py

@@ -12,7 +12,9 @@
 
 import json
 import logging
+import re
 from pathlib import Path
+from urllib.parse import urlparse
 
 import aiohttp
 from aiohttp import web
@@ -22,13 +24,44 @@
 )
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import ConfigEntryError
 from homeassistant.helpers.typing import ConfigType
 
 _LOGGER = logging.getLogger(__name__)
 
 DOMAIN = "mcp_proxy"
 CONFIG_FILE = Path("/config/.mcp_proxy_config.json")
 
+# ha-mcp generates a 22-char base64url token after `/private_`. We accept >=16
+# as a sanity floor — a truncated/corrupted ha-mcp config yields a shorter
+# token, which is the failure mode this length check exists to catch.
+_SECRET_PATH_RE = re.compile(r"^/private_[A-Za-z0-9_-]{16,}$")
+
+
+def _validate_target_url(target_url: str) -> tuple[bool, str]:
+    """Check that target_url is a well-formed http(s) URL.
+
+    When the path starts with `/private_` we additionally enforce the
+    ha-mcp secret-path shape so a truncated token (the issue we're guarding
+    against) is rejected. Other paths are accepted as-is — users with a
+    custom MCP server pointed at a different path are not constrained.
+    """
+    parsed = urlparse(target_url)
+
+    if parsed.scheme not in ("http", "https"):
+        return False, f"scheme must be http or https, got {parsed.scheme!r}"
+    if not parsed.netloc:
+        return False, "URL is missing host"
+    if parsed.params or parsed.query or parsed.fragment:
+        return False, "URL must not contain query, fragment, or path parameters"
+    if parsed.path.startswith("/private_") and not _SECRET_PATH_RE.match(parsed.path):
+        # Don't echo parsed.path — it contains the (truncated) secret token.
+        return False, (
+            "secret path is too short or malformed "
+            "(expected /private_<token> with token of at least 16 characters)"
+        )
+    return True, ""
+
 
 async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
     """Set up the MCP Webhook Proxy from configuration.yaml (migration only).
@@ -51,7 +84,15 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
 
 async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     """Set up MCP Webhook Proxy from a config entry."""
-    proxy_config = await hass.async_add_executor_job(_read_config)
+    try:
+        proxy_config = await hass.async_add_executor_job(_read_config)
+    except (OSError, json.JSONDecodeError) as err:
+        _LOGGER.error("MCP Proxy: Failed to read %s: %s", CONFIG_FILE, err)
+        raise ConfigEntryError(
+            f"Failed to read {CONFIG_FILE}: {err}. Restart the Webhook Proxy "
+            "addon to regenerate the config file."
+        ) from err
+
     if proxy_config is None:
         _LOGGER.info(
             "MCP Proxy: No config found at %s. "
@@ -65,47 +106,78 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
 
     if not target_url or not webhook_id:
         _LOGGER.error("MCP Proxy: Invalid config - missing target_url or webhook_id")
-        return False
+        raise ConfigEntryError(
+            "Missing target_url or webhook_id in /config/.mcp_proxy_config.json. "
+            "Restart the Webhook Proxy addon to regenerate it."
+        )
 
     # Mask sensitive values in logs to avoid leaking secrets
     if "/private_" in target_url:
         masked_target = target_url.split("/private_")[0] + "/private_********"
     else:
         masked_target = target_url
     masked_wh = webhook_id[:6] + "..." if len(webhook_id) > 6 else "***"
+
+    # Validate target_url shape before registering. Without this, a corrupted
+    # URL (e.g. a truncated secret-path) propagates silently and the config
+    # entry reports `loaded` while every webhook request returns 404.
+    is_valid, reason = _validate_target_url(target_url)
+    if not is_valid:
+        _LOGGER.error(
+            "MCP Proxy: target_url validation failed for %s: %s",
+            masked_target,
+            reason,
+        )
+        raise ConfigEntryError(
+            f"Invalid target_url ({reason}). Restart the Webhook Proxy addon "
+            "to regenerate /config/.mcp_proxy_config.json."
+        )
+
     _LOGGER.info("MCP Proxy: target = %s", masked_target)
     _LOGGER.info("MCP Proxy: webhook endpoint = /api/webhook/%s", masked_wh)
 
     session = aiohttp.ClientSession(
         timeout=aiohttp.ClientTimeout(total=300, sock_connect=10, sock_read=300),
     )
+
+    try:
+        async_register(
+            hass,
+            DOMAIN,
+            "MCP Proxy",
+            webhook_id,
+            _handle_webhook,
+            allowed_methods=["POST", "GET"],
+        )
+    except Exception as err:
+        _LOGGER.exception(
+            "MCP Proxy: failed to register webhook endpoint /api/webhook/%s",
+            masked_wh,
+        )
+        await session.close()
+        raise ConfigEntryError(
+            f"Failed to register webhook endpoint: {err}"
+        ) from err
+
     hass.data[DOMAIN] = {
         "target_url": target_url,
         "webhook_id": webhook_id,
         "session": session,
     }
 
-    async_register(
-        hass,
-        DOMAIN,
-        "MCP Proxy",
-        webhook_id,
-        _handle_webhook,
-        allowed_methods=["POST", "GET"],
-    )
-
     return True
 
 
 def _read_config() -> dict | None:
-    """Read proxy config from JSON file (blocking I/O)."""
+    """Read proxy config from JSON file (blocking I/O).
+
+    Returns None only when the file does not exist (fresh install). Read or
+    parse errors propagate as OSError/JSONDecodeError so the caller can
+    distinguish "no config yet" from "config is corrupted".
+    """
     if not CONFIG_FILE.exists():
         return None
-    try:
-        return json.loads(CONFIG_FILE.read_text())
-    except (OSError, json.JSONDecodeError) as e:
-        _LOGGER.error("MCP Proxy: Failed to read %s: %s", CONFIG_FILE, e)
-        return None
+    return json.loads(CONFIG_FILE.read_text())
 
 
 async def _handle_webhook(

homeassistant-addon-webhook-proxy/mcp_proxy/manifest.json

@@ -2,7 +2,7 @@
   "domain": "mcp_proxy",
   "name": "MCP Webhook Proxy",
   "documentation": "https://github.com/homeassistant-ai/ha-mcp",
-  "version": "1.0.0",
+  "version": "1.0.2",
   "codeowners": [],
   "config_flow": true,
   "dependencies": ["webhook"],