diff --git a/supervisor/addons/validate.py b/supervisor/addons/validate.py index e0a98489f..73d4af9ee 100644 --- a/supervisor/addons/validate.py +++ b/supervisor/addons/validate.py @@ -175,6 +175,20 @@ def _warn_addon_config(config: dict[str, Any]): name, ) + invalid_services: list[str] = [] + for service in config.get(ATTR_DISCOVERY, []): + try: + valid_discovery_service(service) + except vol.Invalid: + invalid_services.append(service) + + if invalid_services: + _LOGGER.warning( + "Add-on lists the following unknown services for discovery: %s. Please report this to the maintainer of %s", + ", ".join(invalid_services), + name, + ) + return config @@ -313,7 +327,7 @@ _SCHEMA_ADDON_CONFIG = vol.Schema( vol.Optional(ATTR_DOCKER_API, default=False): vol.Boolean(), vol.Optional(ATTR_AUTH_API, default=False): vol.Boolean(), vol.Optional(ATTR_SERVICES): [vol.Match(RE_SERVICE)], - vol.Optional(ATTR_DISCOVERY): [valid_discovery_service], + vol.Optional(ATTR_DISCOVERY): [str], vol.Optional(ATTR_BACKUP_EXCLUDE): [str], vol.Optional(ATTR_BACKUP_PRE): str, vol.Optional(ATTR_BACKUP_POST): str, diff --git a/supervisor/api/discovery.py b/supervisor/api/discovery.py index 1c6155150..a095ca14b 100644 --- a/supervisor/api/discovery.py +++ b/supervisor/api/discovery.py @@ -1,6 +1,9 @@ """Init file for Supervisor network RESTful API.""" +import logging + import voluptuous as vol +from ..addons.addon import Addon from ..const import ( ATTR_ADDON, ATTR_CONFIG, @@ -15,9 +18,11 @@ from ..discovery.validate import valid_discovery_service from ..exceptions import APIError, APIForbidden from .utils import api_process, api_validate, require_home_assistant +_LOGGER: logging.Logger = logging.getLogger(__name__) + SCHEMA_DISCOVERY = vol.Schema( { - vol.Required(ATTR_SERVICE): valid_discovery_service, + vol.Required(ATTR_SERVICE): str, vol.Optional(ATTR_CONFIG): vol.Maybe(dict), } ) @@ -62,11 +67,28 @@ class APIDiscovery(CoreSysAttributes): async def set_discovery(self, request): """Write data into a discovery pipeline.""" body = await api_validate(SCHEMA_DISCOVERY, request) - addon = request[REQUEST_FROM] + addon: Addon = request[REQUEST_FROM] + service = body[ATTR_SERVICE] + + try: + valid_discovery_service(service) + except vol.Invalid: + _LOGGER.warning( + "Received discovery message for unknown service %s from addon %s. Please report this to the maintainer of the add-on", + service, + addon.name, + ) # Access? if body[ATTR_SERVICE] not in addon.discovery: - raise APIForbidden("Can't use discovery!") + _LOGGER.error( + "Add-on %s attempted to send discovery for service %s which is not listed in its config. Please report this to the maintainer of the add-on", + addon.name, + service, + ) + raise APIForbidden( + "Add-ons must list services they provide via discovery in their config!" + ) # Process discovery message message = self.sys_discovery.send(addon, **body) diff --git a/supervisor/discovery/validate.py b/supervisor/discovery/validate.py index 0a61b0e44..17ada59b3 100644 --- a/supervisor/discovery/validate.py +++ b/supervisor/discovery/validate.py @@ -33,7 +33,7 @@ SCHEMA_DISCOVERY = vol.Schema( { vol.Required(ATTR_UUID): uuid_match, vol.Required(ATTR_ADDON): str, - vol.Required(ATTR_SERVICE): valid_discovery_service, + vol.Required(ATTR_SERVICE): str, vol.Required(ATTR_CONFIG): vol.Maybe(dict), }, extra=vol.REMOVE_EXTRA, diff --git a/supervisor/utils/sentry.py b/supervisor/utils/sentry.py index f1c303a31..5f0536df6 100644 --- a/supervisor/utils/sentry.py +++ b/supervisor/utils/sentry.py @@ -1,6 +1,7 @@ """Utilities for sentry.""" import logging +from typing import Any import sentry_sdk from sentry_sdk.integrations.aiohttp import AioHttpIntegration @@ -16,6 +17,8 @@ from ..misc.filter import filter_data _LOGGER: logging.Logger = logging.getLogger(__name__) +only_once_events: set[str] = set() + def sentry_connected() -> bool: """Is sentry connected.""" @@ -44,6 +47,14 @@ def init_sentry(coresys: CoreSys) -> None: ) +def capture_event(event: dict[str, Any], only_once: str | None = None): + """Capture an event and send to sentry.""" + if sentry_connected(): + if only_once and only_once not in only_once_events: + only_once_events.add(only_once) + sentry_sdk.capture_event(event) + + def capture_exception(err: Exception) -> None: """Capture an exception and send to sentry.""" if sentry_connected(): diff --git a/tests/addons/test_config.py b/tests/addons/test_config.py index a761d1e56..a83a9f995 100644 --- a/tests/addons/test_config.py +++ b/tests/addons/test_config.py @@ -1,5 +1,8 @@ """Validate Add-on configs.""" +import logging +from unittest.mock import Mock + import pytest import voluptuous as vol @@ -283,4 +286,13 @@ def test_valid_slug(): with pytest.raises(vol.Invalid): assert vd.SCHEMA_ADDON_CONFIG(config) - # + +def test_invalid_discovery(capture_event: Mock, caplog: pytest.LogCaptureFixture): + """Test invalid discovery.""" + config = load_json_fixture("basic-addon-config.json") + config["discovery"] = ["mqtt", "junk", "junk2"] + + assert vd.SCHEMA_ADDON_CONFIG(config) + + with caplog.at_level(logging.WARNING): + assert "unknown services for discovery: junk, junk2" in caplog.text diff --git a/tests/api/test_discovery.py b/tests/api/test_discovery.py new file mode 100644 index 000000000..106dffabb --- /dev/null +++ b/tests/api/test_discovery.py @@ -0,0 +1,53 @@ +"""Test discovery API.""" + +import logging +from unittest.mock import MagicMock, patch +from uuid import uuid4 + +from aiohttp.test_utils import TestClient +import pytest + +from supervisor.addons.addon import Addon +from supervisor.discovery import Discovery + + +@pytest.mark.parametrize("api_client", ["local_ssh"], indirect=True) +async def test_discovery_forbidden( + api_client: TestClient, caplog: pytest.LogCaptureFixture, install_addon_ssh +): + """Test addon sending discovery message for an unregistered service.""" + caplog.clear() + + with caplog.at_level(logging.ERROR): + resp = await api_client.post("/discovery", json={"service": "mqtt"}) + + assert resp.status == 400 + result = await resp.json() + assert result["result"] == "error" + assert ( + result["message"] + == "Add-ons must list services they provide via discovery in their config!" + ) + assert "Please report this to the maintainer of the add-on" in caplog.text + + +@pytest.mark.parametrize("api_client", ["local_ssh"], indirect=True) +async def test_discovery_unknown_service( + api_client: TestClient, caplog: pytest.LogCaptureFixture, install_addon_ssh: Addon +): + """Test addon sending discovery message for an unkown service.""" + caplog.clear() + install_addon_ssh.data["discovery"] = ["junk"] + + message = MagicMock() + message.uuid = uuid4().hex + + with caplog.at_level(logging.WARNING), patch.object( + Discovery, "send", return_value=message + ): + resp = await api_client.post("/discovery", json={"service": "junk"}) + + assert resp.status == 200 + result = await resp.json() + assert result["data"]["uuid"] == message.uuid + assert "Please report this to the maintainer of the add-on" in caplog.text diff --git a/tests/conftest.py b/tests/conftest.py index 7bc702d60..e922b6902 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -408,13 +408,21 @@ def sys_supervisor(): @pytest.fixture -async def api_client(aiohttp_client, coresys: CoreSys) -> TestClient: +async def api_client( + aiohttp_client, coresys: CoreSys, request: pytest.FixtureRequest +) -> TestClient: """Fixture for RestAPI client.""" + request_from: str | None = getattr(request, "param", None) + @web.middleware async def _security_middleware(request: web.Request, handler: web.RequestHandler): - """Make request are from Core.""" - request[REQUEST_FROM] = coresys.homeassistant + """Make request are from Core or specified add-on.""" + if request_from: + request[REQUEST_FROM] = coresys.addons.get(request_from, local_only=True) + else: + request[REQUEST_FROM] = coresys.homeassistant + return await handler(request) api = RestAPI(coresys) @@ -606,6 +614,15 @@ async def capture_exception() -> Mock: yield capture_exception +@pytest.fixture +async def capture_event() -> Mock: + """Mock capture event for testing.""" + with patch("supervisor.utils.sentry.sentry_connected", return_value=True), patch( + "supervisor.utils.sentry.sentry_sdk.capture_event" + ) as capture_event: + yield capture_event + + @pytest.fixture async def os_available(request: pytest.FixtureRequest) -> None: """Mock os as available."""