From ab8b750540bb11f9f523e6ceb67f19d220219a0c Mon Sep 17 00:00:00 2001 From: Mike Degatano Date: Wed, 19 Nov 2025 22:45:56 +0000 Subject: [PATCH] Fix stats test and add more for known errors --- supervisor/api/addons.py | 3 ++- supervisor/api/auth.py | 6 ++++-- supervisor/api/utils.py | 21 +++++++++++++------- supervisor/auth.py | 4 ++-- supervisor/exceptions.py | 37 +++++++++++++++++++++++++----------- tests/api/test_addons.py | 14 +++++++++++++- tests/api/test_auth.py | 31 ++++++++++++++++++++++++++++++ tests/api/test_store.py | 20 +++++++------------ tests/api/test_supervisor.py | 4 ++-- 9 files changed, 101 insertions(+), 39 deletions(-) diff --git a/supervisor/api/addons.py b/supervisor/api/addons.py index 535be1edc..202658e7e 100644 --- a/supervisor/api/addons.py +++ b/supervisor/api/addons.py @@ -102,6 +102,7 @@ from ..docker.stats import DockerStats from ..exceptions import ( AddonBootConfigCannotChangeError, AddonConfigurationInvalidError, + AddonNotSupportedWriteStdinError, APIAddonNotInstalled, APIError, APIForbidden, @@ -480,7 +481,7 @@ class APIAddons(CoreSysAttributes): """Write to stdin of add-on.""" addon = self.get_addon_for_request(request) if not addon.with_stdin: - raise APIError(f"STDIN not supported the {addon.slug} add-on") + raise AddonNotSupportedWriteStdinError(_LOGGER.error, addon=addon.slug) data = await request.read() await asyncio.shield(addon.write_stdin(data)) diff --git a/supervisor/api/auth.py b/supervisor/api/auth.py index 7cded9dbb..b68336384 100644 --- a/supervisor/api/auth.py +++ b/supervisor/api/auth.py @@ -15,7 +15,7 @@ import voluptuous as vol from ..addons.addon import Addon from ..const import ATTR_NAME, ATTR_PASSWORD, ATTR_USERNAME, REQUEST_FROM from ..coresys import CoreSysAttributes -from ..exceptions import APIForbidden +from ..exceptions import APIForbidden, AuthInvalidNonStringValueError from .const import ( ATTR_GROUP_IDS, ATTR_IS_ACTIVE, @@ -69,7 +69,9 @@ class APIAuth(CoreSysAttributes): try: _ = username.encode and password.encode # type: ignore except AttributeError: - raise HTTPUnauthorized(headers=REALM_HEADER) from None + raise AuthInvalidNonStringValueError( + _LOGGER.error, headers=REALM_HEADER + ) from None return self.sys_auth.check_login( addon, cast(str, username), cast(str, password) diff --git a/supervisor/api/utils.py b/supervisor/api/utils.py index bf9dfb024..ec0c0db51 100644 --- a/supervisor/api/utils.py +++ b/supervisor/api/utils.py @@ -1,7 +1,7 @@ """Init file for Supervisor util for RESTful API.""" import asyncio -from collections.abc import Callable +from collections.abc import Callable, Mapping import json from typing import Any, cast @@ -26,7 +26,7 @@ from ..const import ( RESULT_OK, ) from ..coresys import CoreSys, CoreSysAttributes -from ..exceptions import APIError, BackupFileNotFoundError, DockerAPIError, HassioError +from ..exceptions import APIError, DockerAPIError, HassioError from ..jobs import JobSchedulerOptions, SupervisorJob from ..utils import check_exception_chain, get_message_from_exception_chain from ..utils.json import json_dumps, json_loads as json_loads_util @@ -67,10 +67,10 @@ def api_process(method): """Return API information.""" try: answer = await method(*args, **kwargs) - except BackupFileNotFoundError as err: - return api_return_error(err, status=404) except APIError as err: - return api_return_error(err, status=err.status, job_id=err.job_id) + return api_return_error( + err, status=err.status, job_id=err.job_id, headers=err.headers + ) except HassioError as err: return api_return_error(err) @@ -139,6 +139,7 @@ def api_return_error( error_type: str | None = None, status: int = 400, *, + headers: Mapping[str, str] | None = None, job_id: str | None = None, ) -> web.Response: """Return an API error message.""" @@ -151,10 +152,15 @@ def api_return_error( match error_type: case const.CONTENT_TYPE_TEXT: - return web.Response(body=message, content_type=error_type, status=status) + return web.Response( + body=message, content_type=error_type, status=status, headers=headers + ) case const.CONTENT_TYPE_BINARY: return web.Response( - body=message.encode(), content_type=error_type, status=status + body=message.encode(), + content_type=error_type, + status=status, + headers=headers, ) case _: result: dict[str, Any] = { @@ -172,6 +178,7 @@ def api_return_error( result, status=status, dumps=json_dumps, + headers=headers, ) diff --git a/supervisor/auth.py b/supervisor/auth.py index dc6169b43..5116cc0bd 100644 --- a/supervisor/auth.py +++ b/supervisor/auth.py @@ -10,7 +10,7 @@ from .const import ATTR_PASSWORD, ATTR_TYPE, ATTR_USERNAME, FILE_HASSIO_AUTH from .coresys import CoreSys, CoreSysAttributes from .exceptions import ( AuthHomeAssistantAPIValidationError, - AuthInvalidNoneValueError, + AuthInvalidNonStringValueError, AuthListUsersError, AuthListUsersNoneResponseError, AuthPasswordResetError, @@ -86,7 +86,7 @@ class Auth(FileConfiguration, CoreSysAttributes): ) -> bool: """Check username login.""" if username is None or password is None: - raise AuthInvalidNoneValueError(_LOGGER.error) + raise AuthInvalidNonStringValueError(_LOGGER.error) _LOGGER.info("Auth request from '%s' for '%s'", addon.slug, username) diff --git a/supervisor/exceptions.py b/supervisor/exceptions.py index dd7cf0c79..def1d9ab4 100644 --- a/supervisor/exceptions.py +++ b/supervisor/exceptions.py @@ -1,6 +1,6 @@ """Core Exceptions.""" -from collections.abc import Callable +from collections.abc import Callable, Mapping from typing import Any MESSAGE_CHECK_SUPERVISOR_LOGS = ( @@ -48,16 +48,19 @@ class APIError(HassioError, RuntimeError): """API errors.""" status = 400 + headers: Mapping[str, str] | None = None def __init__( self, message: str | None = None, logger: Callable[..., None] | None = None, *, + headers: Mapping[str, str] | None = None, job_id: str | None = None, ) -> None: """Raise & log, optionally with job.""" super().__init__(message, logger) + self.headers = headers self.job_id = job_id @@ -113,7 +116,10 @@ class APIUnknownSupervisorError(APIError): status = 500 def __init__( - self, logger: Callable[..., None] | None = None, *, job_id: str | None = None + self, + logger: Callable[..., None] | None = None, + *, + job_id: str | None = None, ) -> None: """Initialize exception.""" self.message_template = ( @@ -556,7 +562,12 @@ class AuthPasswordResetError(AuthError, APIError): f"Unable to reset password for '{{user}}'. {MESSAGE_CHECK_SUPERVISOR_LOGS}" ) - def __init__(self, logger: Callable[..., None] | None = None, *, user: str) -> None: + def __init__( + self, + logger: Callable[..., None] | None = None, + *, + user: str, + ) -> None: """Initialize exception.""" self.extra_fields = {"user": user} | EXTRA_FIELDS_LOGS_COMMAND super().__init__(None, logger) @@ -581,16 +592,20 @@ class AuthListUsersNoneResponseError(AuthError, APIInternalServerError): super().__init__(None, logger) -class AuthInvalidNoneValueError(AuthError, APIUnauthorized): - """Auth error if None provided as username or password.""" +class AuthInvalidNonStringValueError(AuthError, APIUnauthorized): + """Auth error if something besides a string provided as username or password.""" - error_key = "auth_invalid_none_value_error" - message_template = "{none} as username or password is not supported" - extra_fields = {"none": "None"} + error_key = "auth_invalid_non_string_value_error" + message_template = "Username and password must be strings" - def __init__(self, logger: Callable[..., None] | None = None) -> None: + def __init__( + self, + logger: Callable[..., None] | None = None, + *, + headers: Mapping[str, str] | None = None, + ) -> None: """Initialize exception.""" - super().__init__(None, logger) + super().__init__(None, logger, headers=headers) class AuthHomeAssistantAPIValidationError(AuthError, APIUnknownSupervisorError): @@ -977,7 +992,7 @@ class BackupJobError(BackupError, JobException): """Raise on Backup job error.""" -class BackupFileNotFoundError(BackupError): +class BackupFileNotFoundError(BackupError, APINotFound): """Raise if the backup file hasn't been found.""" diff --git a/tests/api/test_addons.py b/tests/api/test_addons.py index 11dd38236..e01539acb 100644 --- a/tests/api/test_addons.py +++ b/tests/api/test_addons.py @@ -627,8 +627,9 @@ async def test_addon_start_options_error( @pytest.mark.parametrize(("method", "action"), [("get", "stats"), ("post", "stdin")]) +@pytest.mark.usefixtures("install_addon_example") async def test_addon_not_running_error( - api_client: TestClient, install_addon_example: Addon, method: str, action: str + api_client: TestClient, method: str, action: str ): """Test addon not running error for endpoints that require that.""" with patch.object( @@ -641,3 +642,14 @@ async def test_addon_not_running_error( assert body["message"] == "Add-on local_example is not running" assert body["error_key"] == "addon_not_running_error" assert body["extra_fields"] == {"addon": "local_example"} + + +@pytest.mark.usefixtures("install_addon_example") +async def test_addon_write_stdin_not_supported_error(api_client: TestClient): + """Test error when trying to write stdin to addon that does not support it.""" + resp = await api_client.post("/addons/local_example/stdin") + assert resp.status == 400 + body = await resp.json() + assert body["message"] == "Add-on local_example does not support writing to stdin" + assert body["error_key"] == "addon_not_supported_write_stdin_error" + assert body["extra_fields"] == {"addon": "local_example"} diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py index 4f981799d..bac993131 100644 --- a/tests/api/test_auth.py +++ b/tests/api/test_auth.py @@ -11,6 +11,7 @@ from securetar import Any from supervisor.addons.addon import Addon from supervisor.coresys import CoreSys from supervisor.exceptions import HomeAssistantAPIError, HomeAssistantWSError +from supervisor.homeassistant.api import HomeAssistantAPI from tests.common import MockResponse from tests.const import TEST_ADDON_SLUG @@ -246,6 +247,13 @@ async def test_auth_json_failure_none( mock_check_login.return_value = True resp = await api_client.post("/auth", json={"username": user, "password": password}) assert resp.status == 401 + assert ( + resp.headers["WWW-Authenticate"] + == 'Basic realm="Home Assistant Authentication"' + ) + body = await resp.json() + assert body["message"] == "Username and password must be strings" + assert body["error_key"] == "auth_invalid_non_string_value_error" @pytest.mark.parametrize("api_client", [TEST_ADDON_SLUG], indirect=True) @@ -357,3 +365,26 @@ async def test_non_addon_token_no_auth_access(api_client: TestClient): """Test auth where add-on is not allowed to access auth API.""" resp = await api_client.post("/auth", json={"username": "test", "password": "pass"}) assert resp.status == 403 + + +@pytest.mark.parametrize("api_client", [TEST_ADDON_SLUG], indirect=True) +@pytest.mark.usefixtures("install_addon_ssh") +async def test_auth_backend_login_failure(api_client: TestClient): + """Test backend login failure on auth.""" + with ( + patch.object(HomeAssistantAPI, "check_api_state", return_value=True), + patch.object( + HomeAssistantAPI, "make_request", side_effect=HomeAssistantAPIError("fail") + ), + ): + resp = await api_client.post( + "/auth", json={"username": "test", "password": "pass"} + ) + assert resp.status == 500 + body = await resp.json() + assert ( + body["message"] + == "Unable to validate authentication details with Home Assistant. Check supervisor logs for details (check with 'ha supervisor logs')" + ) + assert body["error_key"] == "auth_home_assistant_api_validation_error" + assert body["extra_fields"] == {"logs_command": "ha supervisor logs"} diff --git a/tests/api/test_store.py b/tests/api/test_store.py index 29927c344..cea281e1a 100644 --- a/tests/api/test_store.py +++ b/tests/api/test_store.py @@ -4,7 +4,6 @@ import asyncio from pathlib import Path from unittest.mock import AsyncMock, MagicMock, PropertyMock, patch -from aiohttp import ClientResponse from aiohttp.test_utils import TestClient from awesomeversion import AwesomeVersion import pytest @@ -292,14 +291,6 @@ async def test_api_detached_addon_documentation( assert result == "Addon local_ssh does not exist in the store" -async def get_message(resp: ClientResponse, json_expected: bool) -> str: - """Get message from response based on response type.""" - if json_expected: - body = await resp.json() - return body["message"] - return await resp.text() - - @pytest.mark.parametrize( ("method", "url", "json_expected"), [ @@ -325,10 +316,13 @@ async def test_store_addon_not_found( """Test store addon not found error.""" resp = await api_client.request(method, url) assert resp.status == 404 - assert ( - await get_message(resp, json_expected) - == "Addon bad does not exist in the store" - ) + if json_expected: + body = await resp.json() + assert body["message"] == "Addon bad does not exist in the store" + assert body["error_key"] == "store_addon_not_found_error" + assert body["extra_fields"] == {"addon": "bad"} + else: + assert await resp.text() == "Addon bad does not exist in the store" @pytest.mark.parametrize( diff --git a/tests/api/test_supervisor.py b/tests/api/test_supervisor.py index 1ece380b4..cfde5089a 100644 --- a/tests/api/test_supervisor.py +++ b/tests/api/test_supervisor.py @@ -437,8 +437,8 @@ async def test_supervisor_api_stats_failure( body = await resp.json() assert ( body["message"] - == "Can't get stats for Supervisor container. Check supervisor logs for details (check with 'ha supervisor logs')" + == "An unknown error occurred with Supervisor. Check supervisor logs for details (check with 'ha supervisor logs')" ) - assert body["error_key"] == "supervisor_stats_error" + assert body["error_key"] == "supervisor_unknown_error" assert body["extra_fields"] == {"logs_command": "ha supervisor logs"} assert "Could not inspect container 'hassio_supervisor': fail" in caplog.text