Merge branch 'pr-1126' into addon-repo

Author: kingpanther13

homeassistant-addon-dev/config.yaml

@@ -14,7 +14,10 @@ ingress: true
 ingress_port: 9583
 ingress_stream: true
 hassio_api: true
-hassio_role: default
+# `manager` (not `default`) is required so the Supervisor token grants access
+# to /addons/<slug>/logs and /<service>/logs (system service logs). Verified
+# via live test — `default` returns 403 for both. See #1116.
+hassio_role: manager
 homeassistant_api: true
 host_network: true
 image: "ghcr.io/homeassistant-ai/ha-mcp-addon-dev-{arch}"

homeassistant-addon/config.yaml

@@ -13,7 +13,10 @@ startup: application
 boot: manual
 # Enable access to Supervisor API for auto-discovery
 hassio_api: true
-hassio_role: default
+# `manager` (not `default`) is required so the Supervisor token grants access
+# to /addons/<slug>/logs and /<service>/logs (system service logs). Verified
+# via live test — `default` returns 403 for both. See #1116.
+hassio_role: manager
 # Enable access to Home Assistant API
 homeassistant_api: true
 # Enable host network mode for local network access

src/ha_mcp/client/rest_client.py

@@ -5,11 +5,13 @@
 import asyncio
 import json
 import logging
+import os
 import ssl
 from typing import Any
 
 import httpx
 
+from .._version import is_running_in_addon
 from ..config import get_global_settings
 
 
@@ -434,28 +436,184 @@ async def get_error_log(self) -> str:
         return response if isinstance(response, str) else str(response)
 
     async def get_addon_logs(self, slug: str) -> str:
-        """Fetch an add-on's container logs via HA Core's Supervisor REST proxy.
+        """Fetch an add-on's container logs.
 
-        Uses `/api/hassio/addons/{slug}/logs`, which HA Core proxies to
-        Supervisor and returns as `text/plain`. This avoids the
-        `supervisor/api` websocket path that tries to JSON-decode the text
-        body and always fails (see #950).
+        Branch on ``is_running_in_addon()`` (which keys off ``SUPERVISOR_TOKEN``
+        in env): inside the add-on container goes directly to the Supervisor
+        REST API at ``http://supervisor/addons/{slug}/logs`` with the
+        Supervisor token. The HA Core proxy at
+        ``/api/hassio/addons/{slug}/logs`` rejects this token+path combination
+        on current HA Core releases (see #1116) — the direct path bypasses
+        HA Core entirely and is the documented Supervisor contract.
+
+        On non-addon installs (Docker, pyinstaller, pip pointing at a normal
+        HA URL), falls back to the HA Core proxy path. That path requires an
+        admin LLA but works fine when not invoked from the add-on container.
+
+        Both branches return ``text/plain`` log content.
 
         Raises:
-            HomeAssistantAuthError: 401 from HA Core.
-            HomeAssistantAPIError: Non-2xx response (e.g. 404 unknown slug,
-                400 addon not installed). `status_code` is set so callers
-                can map to specific suggestions.
+            HomeAssistantAuthError: 401 response, or ``SUPERVISOR_TOKEN`` empty
+                at call time on the addon branch.
+            HomeAssistantAPIError: 403 (role too low — addon needs hassio_role
+                ``manager``), 404 (unknown slug), or other non-2xx. The
+                ``status_code`` attribute lets callers map to specific
+                suggestions.
             HomeAssistantConnectionError: Network, timeout, or transport error.
         """
-        logger.debug(f"Fetching addon logs for slug={slug}")
+        if is_running_in_addon():
+            return await self._get_addon_logs_via_supervisor(slug)
+
+        logger.debug(f"Fetching addon logs for slug={slug} via HA Core proxy")
         response = await self._raw_request(
             "GET",
             f"/hassio/addons/{slug}/logs",
             headers={"Accept": "text/plain"},
         )
         return response.text
 
+    async def _supervisor_logs_get(self, path: str) -> str:
+        """Fetch ``text/plain`` logs from a Supervisor REST endpoint.
+
+        ``path`` is everything between ``http://supervisor/`` and ``/logs``:
+
+        - ``"addons/<slug>"`` for add-on container logs
+        - ``"<service>"`` (where service ∈ {supervisor, host, core, dns, audio,
+          multicast, observer}) for system-service logs
+
+        Bypasses ``HomeAssistantClient.httpx_client`` because the Supervisor
+        endpoint uses a different base URL (``http://supervisor``) and a
+        different token (``SUPERVISOR_TOKEN``) than HA Core REST. Both
+        endpoints require the addon's ``hassio_role`` to be ``manager`` (not
+        ``default``); a ``default`` role gets a 403 here — see #1116.
+
+        Raises:
+            HomeAssistantAuthError: ``SUPERVISOR_TOKEN`` absent at call time,
+                or 401 from Supervisor.
+            HomeAssistantAPIError: 403 (role too low — distinct branch with
+                role hint), 404, other 4xx/5xx. Tries to parse Supervisor's
+                ``{"result":"error","message":"..."}`` JSON envelope before
+                falling back to text body / reason phrase / placeholder.
+            HomeAssistantConnectionError: Timeout or transport error, with
+                distinct messages so callers can tell them apart.
+        """
+        token = os.environ.get("SUPERVISOR_TOKEN", "")
+        if not token:
+            # The is_running_in_addon() gate already keys off SUPERVISOR_TOKEN
+            # being truthy, so a direct caller landing here without one is a
+            # detection/config mismatch — fail-fast with a distinct message
+            # so operators don't read it as "token rejected".
+            raise HomeAssistantAuthError(
+                f"Supervisor token absent at call time for /{path}/logs "
+                "(addon-mode gate fired but SUPERVISOR_TOKEN env var not set)"
+            )
+
+        url = f"http://supervisor/{path}/logs"
+        logger.debug("Fetching %s via Supervisor direct", url)
+
+        try:
+            async with httpx.AsyncClient(
+                timeout=httpx.Timeout(self.timeout),
+                # `verify` is a no-op for plain http://supervisor, but kept
+                # for symmetry with the other two direct-Supervisor httpx
+                # clients (#1128 establishes the 3-site convention).
+                verify=self.verify_ssl,
+            ) as client:
+                response = await client.get(
+                    url,
+                    headers={
+                        "Authorization": f"Bearer {token}",
+                        "Accept": "text/plain",
+                    },
+                )
+        except httpx.TimeoutException as e:
+            raise HomeAssistantConnectionError(
+                f"Timeout fetching /{path}/logs from Supervisor: {e}"
+            ) from e
+        except httpx.HTTPError as e:
+            raise HomeAssistantConnectionError(
+                f"Transport error fetching /{path}/logs from Supervisor: {e}"
+            ) from e
+
+        if response.status_code == 401:
+            raise HomeAssistantAuthError(
+                f"Invalid Supervisor token for /{path}/logs"
+            )
+        if response.status_code == 403:
+            # Distinct from 401: token is valid but addon's hassio_role isn't
+            # high enough. Most-likely cause for this exact endpoint at the
+            # time #1116 surfaced (default → manager bump in addon config.yaml
+            # is the same-PR companion fix).
+            logger.warning(
+                "Supervisor returned 403 for /%s/logs — addon hassio_role may "
+                "be too low (need 'manager')",
+                path,
+            )
+            raise HomeAssistantAPIError(
+                f"Supervisor forbids /{path}/logs (403) — addon's hassio_role "
+                "may be 'default'; need 'manager' or higher",
+                status_code=403,
+                response_data={"path": path},
+            )
+        if response.status_code >= 400:
+            text_body = response.text
+            # Supervisor returns {"result":"error","message":"..."} JSON on
+            # some 4xx paths. Try parsing that first so the user sees the
+            # human message instead of a JSON blob; then fall back to the
+            # text body, then reason_phrase, then a placeholder.
+            message = ""
+            try:
+                envelope = json.loads(text_body) if text_body else None
+                if isinstance(envelope, dict):
+                    msg = envelope.get("message")
+                    if isinstance(msg, str) and msg:
+                        message = msg
+            except json.JSONDecodeError:
+                pass
+            if not message:
+                message = (
+                    text_body.strip() or response.reason_phrase or "<empty body>"
+                )
+            logger.warning(
+                "Supervisor returned %s for /%s/logs: %s",
+                response.status_code, path, message,
+            )
+            raise HomeAssistantAPIError(
+                f"API error: {response.status_code} - {message}",
+                status_code=response.status_code,
+                response_data={"message": text_body, "path": path},
+            )
+        return response.text
+
+    async def _get_addon_logs_via_supervisor(self, slug: str) -> str:
+        """Fetch add-on container logs directly from Supervisor's REST API.
+
+        Distinct from ``tools_bug_report._fetch_addon_logs``: that helper is
+        hardcoded to ``/addons/self/logs`` and silently swallows failures
+        (it's an aux-data fetch for bug reports, fine to skip on error). This
+        helper takes arbitrary slugs and surfaces failures as exceptions
+        because callers (``ha_get_logs(source="supervisor", slug=...)``) need
+        them. Both endpoints require ``hassio_role: manager``.
+
+        Delegates to ``_supervisor_logs_get`` so error handling stays in
+        lockstep with ``_get_system_service_logs``.
+        """
+        return await self._supervisor_logs_get(f"addons/{slug}")
+
+    async def _get_system_service_logs(self, service: str) -> str:
+        """Fetch HA system-service logs directly from Supervisor's REST API.
+
+        Hits ``http://supervisor/{service}/logs``. ``service`` must be one of
+        the seven Supervisor-managed services: ``supervisor``, ``host``,
+        ``core``, ``dns``, ``audio``, ``multicast``, ``observer``. Caller is
+        responsible for validating ``service`` against the allowed set; this
+        helper does no validation and will raise ``HomeAssistantAPIError`` on
+        any unknown path (404 from Supervisor).
+
+        Requires ``hassio_role: manager`` like the addon-logs path.
+        """
+        return await self._supervisor_logs_get(service)
+
     async def test_connection(self) -> tuple[bool, str | None]:
         """
         Test connection to Home Assistant.