fix(security): authenticate health endpoint and stop webhook-id leak

The /station/health route is registered directly on the aiohttp router, so HA's
auth middleware only flags requests without blocking them - the endpoint was
reachable unauthenticated and returned the full health snapshot.

- health_status now requires HA authentication (bearer token or signed request)
  via KEY_AUTHENTICATED and returns 401 otherwise.
- Mask the Ecowitt webhook id (the endpoint's only credential) in last_ingress
  paths via _sanitize_path, so it never enters the snapshot exposed by the health
  endpoint or the diagnostics download.
- Redact ECOWITT_WEBHOOK_ID in diagnostics.
- Compare the Ecowitt webhook id in constant time (hmac.compare_digest), matching
  the WU/WSLink credential checks.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ecowitt_support
SchiZzA 2026-06-21 13:42:56 +02:00
parent 526d5e4f6e
commit a467c4b6e5
No known key found for this signature in database
4 changed files with 77 additions and 8 deletions

View File

@ -115,7 +115,10 @@ class WeatherDataUpdateCoordinator(DataUpdateCoordinator):
expected_webhook = self.config.options.get(ECOWITT_WEBHOOK_ID, "")
actual_webhook = webdata.match_info.get("webhook_id", "")
if not expected_webhook or actual_webhook != expected_webhook:
# Constant-time comparison to avoid leaking the webhook id via timing.
if not expected_webhook or not hmac.compare_digest(
actual_webhook.encode("utf-8"), expected_webhook.encode("utf-8")
):
_LOGGER.error("Ecowitt: invalid webhook ID")
if health:
health.update_ingress_result(

View File

@ -8,12 +8,21 @@ from typing import Any
from homeassistant.components.diagnostics import async_redact_data # pyright: ignore[reportUnknownVariableType]
from homeassistant.core import HomeAssistant
from .const import API_ID, API_KEY, POCASI_CZ_API_ID, POCASI_CZ_API_KEY, WINDY_STATION_ID, WINDY_STATION_PW
from .const import (
API_ID,
API_KEY,
ECOWITT_WEBHOOK_ID,
POCASI_CZ_API_ID,
POCASI_CZ_API_KEY,
WINDY_STATION_ID,
WINDY_STATION_PW,
)
from .data import SWSConfigEntry
TO_REDACT = {
API_ID,
API_KEY,
ECOWITT_WEBHOOK_ID,
POCASI_CZ_API_ID,
POCASI_CZ_API_KEY,
WINDY_STATION_ID,

View File

@ -23,8 +23,10 @@ from typing import Any
import aiohttp
from aiohttp import ClientConnectionError
import aiohttp.web
from aiohttp.web_exceptions import HTTPUnauthorized
from py_typecheck import checked, checked_or
from homeassistant.components.http import KEY_AUTHENTICATED
from homeassistant.components.network import async_get_source_ip
from homeassistant.core import HomeAssistant
from homeassistant.helpers.aiohttp_client import async_get_clientsession
@ -69,6 +71,18 @@ def _protocol_from_path(path: str) -> str:
return "unknown"
def _sanitize_path(path: str) -> str:
"""Strip the secret Ecowitt webhook id from a path before storing/exposing it.
The Ecowitt endpoint is `/weatherhub/<webhook_id>` where the id is the only
credential. Keeping the raw path in the health snapshot would leak it via the
health endpoint and diagnostics, so mask the id segment.
"""
if path.startswith(ECOWITT_URL_PREFIX + "/"):
return ECOWITT_URL_PREFIX + "/***"
return path
def _empty_forwarding_state(enabled: bool) -> dict[str, Any]:
"""Build the default forwarding status payload."""
return {
@ -269,7 +283,7 @@ class HealthCoordinator(DataUpdateCoordinator):
data["last_ingress"] = {
"time": dt_util.utcnow().isoformat(),
"protocol": _protocol_from_path(request.path),
"path": request.path,
"path": _sanitize_path(request.path),
"method": request.method,
"route_enabled": route_enabled,
"accepted": False,
@ -294,7 +308,7 @@ class HealthCoordinator(DataUpdateCoordinator):
{
"time": dt_util.utcnow().isoformat(),
"protocol": _protocol_from_path(request.path),
"path": request.path,
"path": _sanitize_path(request.path),
"method": request.method,
"accepted": accepted,
"authorized": authorized,
@ -326,11 +340,23 @@ class HealthCoordinator(DataUpdateCoordinator):
self._refresh_summary(data)
self._commit(data)
async def health_status(self, _: aiohttp.web.Request) -> aiohttp.web.Response:
async def health_status(self, request: aiohttp.web.Request) -> aiohttp.web.Response:
"""Serve the current health snapshot over HTTP.
Requires Home Assistant authentication. The route is registered directly on
the aiohttp router (so it can share the dispatcher), which means HA's auth
middleware only *flags* the request - it does not block it. We therefore
enforce auth here so the snapshot (internal URLs/IPs, add-on status, last
ingress) is never exposed to unauthenticated callers.
Auth is satisfied by a valid bearer token or a signed request, the same as
any HomeAssistantView.
The endpoint forces one refresh before returning so that the caller sees
a reasonably fresh add-on status.
"""
if not request.get(KEY_AUTHENTICATED, False):
raise HTTPUnauthorized
await self.async_request_refresh()
return aiohttp.web.json_response(self.data, status=200)

View File

@ -24,6 +24,7 @@ from unittest.mock import AsyncMock, MagicMock
import aiohttp
from aiohttp import ClientConnectionError
from aiohttp.web_exceptions import HTTPUnauthorized
import pytest
from pytest_homeassistant_custom_component.common import MockConfigEntry
@ -42,6 +43,7 @@ from custom_components.sws12500.const import (
from custom_components.sws12500.data import SWSRuntimeData
from custom_components.sws12500.health_coordinator import HealthCoordinator
from custom_components.sws12500.routes import Routes
from homeassistant.components.http import KEY_AUTHENTICATED
# ---------------------------------------------------------------------------
# Helpers / fixtures
@ -441,6 +443,20 @@ def test_record_dispatch_records_with_reason(hass, entry) -> None:
assert coordinator.data["integration_status"] == "degraded"
def test_record_dispatch_masks_ecowitt_webhook_id(hass, entry) -> None:
coordinator = HealthCoordinator(hass, entry)
_attach_runtime_data(entry, coordinator)
request = SimpleNamespace(path=ECOWITT_URL_PREFIX + "/supersecretid", method="POST")
coordinator.record_dispatch(request, route_enabled=True, reason=None)
ingress = coordinator.data["last_ingress"]
assert ingress["protocol"] == "ecowitt"
# The secret webhook id must never reach the (potentially exposed) snapshot.
assert ingress["path"] == ECOWITT_URL_PREFIX + "/***"
assert "supersecretid" not in ingress["path"]
# ---------------------------------------------------------------------------
# update_ingress_result
# ---------------------------------------------------------------------------
@ -520,21 +536,36 @@ def test_update_forwarding(hass, entry) -> None:
# ---------------------------------------------------------------------------
async def test_health_status_endpoint(hass, entry, monkeypatch) -> None:
async def test_health_status_endpoint_authenticated(hass, entry, monkeypatch) -> None:
coordinator = HealthCoordinator(hass, entry)
_attach_runtime_data(entry, coordinator)
# Avoid network: stub the refresh that health_status awaits.
monkeypatch.setattr(coordinator, "async_request_refresh", AsyncMock(return_value=None))
request = SimpleNamespace(path=HEALTH_URL, method="GET")
response = await coordinator.health_status(request)
# aiohttp Request is dict-like; health_status only reads KEY_AUTHENTICATED.
request = {KEY_AUTHENTICATED: True}
response = await coordinator.health_status(request) # type: ignore[arg-type]
assert isinstance(response, aiohttp.web.Response)
assert response.status == 200
coordinator.async_request_refresh.assert_awaited_once()
async def test_health_status_endpoint_rejects_unauthenticated(hass, entry, monkeypatch) -> None:
coordinator = HealthCoordinator(hass, entry)
_attach_runtime_data(entry, coordinator)
refresh = AsyncMock(return_value=None)
monkeypatch.setattr(coordinator, "async_request_refresh", refresh)
# No KEY_AUTHENTICATED flag -> unauthenticated -> 401, no refresh triggered.
with pytest.raises(HTTPUnauthorized):
await coordinator.health_status({}) # type: ignore[arg-type]
refresh.assert_not_awaited()
# ---------------------------------------------------------------------------
# health_sensor.py module helpers
# ---------------------------------------------------------------------------