From e8c24527948bc627c14ac0035fbc49657b233eba Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Wed, 26 Aug 2026 20:53:15 -0700 Subject: [PATCH 01/11] fix(auth): redact a rejected passphrase echoed as a scalar A pydantic-v2 field-level 422 reports the failure by pointing `loc` at the field and putting the rejected value under `input`, where it is a bare string. The key-based walk had nothing to judge it by, so `_redact` returned that body byte-identical and the passphrase went into the DEBUG log. A scalar in an error object whose `loc` names a credential field is now redacted on the strength of that `loc`; a dict or list under the same key is still walked, so the surrounding diagnostic survives. And `register_v2` now passes the passphrase it sent into the redaction, which closes the case structure cannot reach at all -- a panel quoting the credential back in free prose. --- src/span_panel_api/auth.py | 94 ++++++++++++++++++++++++++++++------ tests/test_auth_redaction.py | 85 ++++++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+), 16 deletions(-) diff --git a/src/span_panel_api/auth.py b/src/span_panel_api/auth.py index 2d4d502..a724ab6 100644 --- a/src/span_panel_api/auth.py +++ b/src/span_panel_api/auth.py @@ -9,6 +9,7 @@ from __future__ import annotations import asyncio +from collections.abc import Collection import hashlib import json import logging @@ -45,28 +46,82 @@ _REDACTED = "***" +#: The key a pydantic-style validation error puts the field path under. Its value +#: is a list -- ``["body", "hopPassphrase"]`` -- and it is the only thing in a +#: field-level error object that says what was being validated. +_LOCATION_KEY = "loc" -def _redact(value: object) -> object: - """Replace every credential-valued key in a JSON-decoded body, at any depth. +#: The members of a validation error object that describe the failure rather than +#: repeat what was submitted. In an error object whose ``loc`` names a credential, +#: every *other* scalar is the rejected value under one name or another -- +#: ``input`` is the name FastAPI uses, and a validator is free to add its own. +_ERROR_DESCRIPTION_KEYS = frozenset({"type", "loc", "msg", "url"}) - A top-level key scan is not enough, and the case that matters is the exact - one this exists for: a FastAPI-style 422 echoes the request that failed - validation back under ``detail[].input``, so the passphrase that was just - rejected reappears nested two levels down. Scanning only the outermost - object would redact nothing in precisely the response most likely to carry a - secret. - Walks dicts and lists; anything else is returned as-is, because a scalar - reached here is a value whose key has already been judged. +def _names_a_credential(location: object) -> bool: + """Whether a validation error's ``loc`` points at one of the credential fields.""" + if not isinstance(location, list): + return False + return any(isinstance(part, str) and part.lower() in _CREDENTIAL_KEYS for part in location) + + +def _scrub(text: str, secrets: Collection[str]) -> str: + """Remove every occurrence of a credential the caller knows it sent. + + Empty secrets are skipped rather than matched: a door-bypass registration + sends no passphrase, and ``"".replace("", ...)`` would rewrite every gap + between characters in the body. + """ + for secret in secrets: + if secret: + text = text.replace(secret, _REDACTED) + return text + + +def _redact_member(key: object, item: object, rejected_here: bool, secrets: Collection[str]) -> object: + """Decide one member of a decoded body, by its key and by what sits beside it.""" + name = str(key).lower() + if name in _CREDENTIAL_KEYS: + return _REDACTED + if rejected_here and name not in _ERROR_DESCRIPTION_KEYS and not isinstance(item, dict | list): + # A scalar in an error object whose `loc` names a credential. Its own key + # says nothing -- `input` is not a secret-sounding word -- so the `loc` + # beside it is the only evidence there is, and it is conclusive. A dict or + # a list is walked instead, because there the keys inside can be judged + # one by one and flattening it would throw away the diagnostic. + return _REDACTED + return _redact(item, secrets) + + +def _redact(value: object, secrets: Collection[str] = ()) -> object: + """Strip credentials out of a JSON-decoded body, at any depth, three ways. + + **By key**, wherever a credential-named key appears. A top-level scan is not + enough: a FastAPI-style 422 echoes the whole rejected request back under + ``detail[].input``, so the passphrase reappears two levels down. + + **By the key beside it.** The same validation layer reports a *field-level* + failure differently — ``loc`` points at the field and ``input`` carries the + bare value — and there the credential is a scalar under an innocuous key. Only + the sibling ``loc`` marks it, so that is what is read. + + **By value**, for the secrets the caller passes in. Structure runs out: a + validator may fold the rejected value into its own prose, and nothing marks + that. The one call that sends a credential knows exactly what it sent. + + Walks dicts and lists; any other value is returned as-is once scrubbed. """ if isinstance(value, dict): - return {key: _REDACTED if str(key).lower() in _CREDENTIAL_KEYS else _redact(item) for key, item in value.items()} + rejected_here = _names_a_credential(value.get(_LOCATION_KEY)) + return {_scrub(str(key), secrets): _redact_member(key, item, rejected_here, secrets) for key, item in value.items()} if isinstance(value, list): - return [_redact(item) for item in value] + return [_redact(item, secrets) for item in value] + if isinstance(value, str): + return _scrub(value, secrets) return value -def _log_auth_failure(endpoint: str, response: httpx.Response) -> None: +def _log_auth_failure(endpoint: str, response: httpx.Response, secrets: Collection[str] = ()) -> None: """Record why an auth call failed, at DEBUG and with credentials removed. The body is worth keeping — a 422's validation detail is the only thing that @@ -75,6 +130,10 @@ def _log_auth_failure(endpoint: str, response: httpx.Response) -> None: config-flow log, and carries into a diagnostics download. DEBUG is opt-in and is where a user chasing a registration failure is already looking. + ``secrets`` is what the caller sent on this request. Passing it closes the + gap that key-based redaction cannot: a panel is free to quote the credential + back in free prose, and no key or ``loc`` marks that. + A body that is not JSON is described rather than shown. It could be an HTML error page from a proxy, and it could equally be an echo of the request; with no structure to walk there is no way to redact it, so only its shape is @@ -95,7 +154,7 @@ def _log_auth_failure(endpoint: str, response: httpx.Response) -> None: "%s failed with HTTP %d; body (credentials redacted): %s", endpoint, response.status_code, - json.dumps(_redact(parsed), sort_keys=True), + json.dumps(_redact(parsed, secrets), sort_keys=True), ) @@ -203,8 +262,11 @@ async def register_v2( # is logged at DEBUG instead: a 422 from the panel's validation layer # echoes the submitted `hopPassphrase` straight back, and interpolating # `response.text` here put that secret into an exception message that - # Home Assistant shows in the UI and captures in diagnostics. - _log_auth_failure("/api/v2/auth/register", response) + # Home Assistant shows in the UI and captures in diagnostics. The + # passphrase goes with it because this is the one place in the library + # that knows what was sent, and the panel is under no obligation to + # quote it back under a key that names it. + _log_auth_failure("/api/v2/auth/register", response, () if passphrase is None else (passphrase,)) raise SpanPanelAuthError(f"Authentication failed (HTTP {response.status_code})") if response.status_code != 200: diff --git a/tests/test_auth_redaction.py b/tests/test_auth_redaction.py index b281b18..a06c365 100644 --- a/tests/test_auth_redaction.py +++ b/tests/test_auth_redaction.py @@ -34,6 +34,23 @@ ] } +# The other shape the same validation layer produces, and the one a key scan +# cannot see at all: pydantic v2 reports a *field-level* failure by pointing +# `loc` at the field and putting the rejected value itself under `input`. The +# credential is a bare string there, so nothing about the key it arrived under +# says "secret" — only the sibling `loc` does. +FIELD_LEVEL_422 = { + "detail": [ + { + "type": "string_too_short", + "loc": ["body", "hopPassphrase"], + "msg": "String should have at least 8 characters", + "input": SECRET, + "ctx": {"min_length": 8}, + } + ] +} + def _response(status_code: int, *, json_data: object | None = None, text: str = "") -> httpx.Response: if json_data is not None: @@ -88,6 +105,56 @@ def test_scalars_pass_through(self) -> None: assert _redact("plain") == "plain" +class TestRedactBySiblingLocation: + """A scalar is judged by the `loc` beside it when its own key says nothing.""" + + def test_redacts_a_scalar_input_named_by_loc(self) -> None: + redacted = _redact(FIELD_LEVEL_422) + assert SECRET not in json.dumps(redacted) + + def test_keeps_the_reason_the_field_was_rejected(self) -> None: + rendered = json.dumps(_redact(FIELD_LEVEL_422)) + assert "string_too_short" in rendered + assert "at least 8 characters" in rendered + assert "hopPassphrase" in rendered + + def test_a_loc_naming_no_credential_leaves_its_input_alone(self) -> None: + """The rule is scoped to credential fields — an ordinary 422 stays readable.""" + body = {"detail": [{"type": "missing", "loc": ["body", "name"], "msg": "Field required", "input": "ha"}]} + assert _redact(body) == body + + def test_a_container_input_is_still_walked_not_flattened(self) -> None: + """A nested echo keeps everything the key scan can clear individually.""" + rendered = json.dumps(_redact(VALIDATION_422)) + assert SECRET not in rendered + assert "home-assistant-0badcafe" in rendered + + +class TestRedactByValue: + """What the caller already knows is a secret is removed wherever it appears. + + Structure runs out: a validator is free to fold the rejected value into its + own prose, and no key or `loc` marks that. The one call that sends a + credential knows exactly what it sent, so it can say so. + """ + + def test_removes_the_secret_from_free_prose(self) -> None: + body = {"detail": f"passphrase {SECRET} was rejected"} + rendered = json.dumps(_redact(body, (SECRET,))) + assert SECRET not in rendered + assert "was rejected" in rendered + + def test_removes_the_secret_from_an_unnamed_scalar(self) -> None: + assert _redact({"echo": SECRET}, (SECRET,)) == {"echo": "***"} + + def test_an_empty_secret_does_not_shred_the_body(self) -> None: + """A door-bypass registration sends no passphrase; `""` must be inert.""" + assert _redact({"msg": "fine"}, ("",)) == {"msg": "fine"} + + def test_no_secrets_leaves_the_body_alone(self) -> None: + assert _redact({"msg": "fine"}, ()) == {"msg": "fine"} + + class TestRegisterV2AuthFailure: @pytest.mark.asyncio async def test_secret_is_not_in_the_exception(self) -> None: @@ -120,6 +187,24 @@ async def test_non_json_body_is_described_not_shown(self, caplog: pytest.LogCapt assert str(len(body.encode())) in caplog.text assert "text/html" in caplog.text + @pytest.mark.asyncio + async def test_a_field_level_422_does_not_log_the_rejected_passphrase(self, caplog: pytest.LogCaptureFixture) -> None: + """The scalar echo, end to end: nothing about `input`'s key marks it secret.""" + with caplog.at_level(logging.DEBUG, logger="span_panel_api.auth"): + exc = await _register_against(_response(422, json_data=FIELD_LEVEL_422)) + assert SECRET not in caplog.text + assert SECRET not in str(exc) + assert "at least 8 characters" in caplog.text + + @pytest.mark.asyncio + async def test_the_passphrase_is_removed_wherever_the_panel_put_it(self, caplog: pytest.LogCaptureFixture) -> None: + """No key and no `loc` names it — only the caller knows what it sent.""" + body = {"error": f"the passphrase {SECRET} is not the one on the label"} + with caplog.at_level(logging.DEBUG, logger="span_panel_api.auth"): + await _register_against(_response(401, json_data=body)) + assert SECRET not in caplog.text + assert "not the one on the label" in caplog.text + @pytest.mark.asyncio async def test_403_takes_the_same_path(self) -> None: exc = await _register_against(_response(403, json_data=VALIDATION_422)) From f8d969446fd846957f22f66544c02591244d9c03 Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Wed, 26 Aug 2026 21:00:37 -0700 Subject: [PATCH 02/11] fix(http): one request helper for every bootstrap REST call Seven of the eight caught `httpx.ConnectError` and `httpx.TimeoutException` and nothing else, so a `ReadError` from a panel resetting its listener mid-response, or a `RemoteProtocolError` from a proxy closing without answering, came back as a raw httpx exception -- not a `SpanPanelError`, and so caught by none of the retry clauses built on that contract. The bodies were unguarded the same way: a 200 missing a field raised `KeyError` and one that was not JSON raised `JSONDecodeError`. `_request` performs the call and translates the whole `TransportError` family; the `_Reply` it returns decodes the body and names the field the panel left out. Status classification stays with each caller, because it is genuinely per-endpoint -- 412 means "no passphrase is set" on one path and nothing anywhere else. `get_homie_schema` keeps its retryable reading of an unparseable body by asking for `SpanPanelServerError`, and the detector now catches this library's own two classes instead of a hand-listed tuple of httpx ones that had let `ReadError` out of a function documented to return a result. --- src/span_panel_api/_http.py | 132 ++++++++++- src/span_panel_api/auth.py | 306 +++++++++++++------------- src/span_panel_api/detection.py | 33 ++- tests/test_rest_transport_contract.py | 164 ++++++++++++++ 4 files changed, 469 insertions(+), 166 deletions(-) create mode 100644 tests/test_rest_transport_contract.py diff --git a/src/span_panel_api/_http.py b/src/span_panel_api/_http.py index f020165..15be558 100644 --- a/src/span_panel_api/_http.py +++ b/src/span_panel_api/_http.py @@ -6,11 +6,15 @@ from collections.abc import AsyncIterator from contextlib import asynccontextmanager from dataclasses import dataclass, field +import logging import ssl +from typing import Literal import httpx -from .exceptions import SpanPanelValidationError +from .exceptions import SpanPanelAPIError, SpanPanelConnectionError, SpanPanelTimeoutError, SpanPanelValidationError + +_LOGGER = logging.getLogger(__name__) #: What a bootstrap URL resolves to when the caller names no port. HTTP without a #: context, HTTPS with one -- so a caller that pins the panel CA and leaves the @@ -18,6 +22,17 @@ DEFAULT_HTTP_PORT = 80 DEFAULT_HTTPS_PORT = 443 +#: The one bootstrap path two modules request: the detector probes it to decide +#: whether the panel speaks v2 at all, and `get_v2_status` reads the same answer +#: for a caller that already knows it does. Named here rather than spelled out in +#: each, so the two cannot drift apart the way their parsers had. +V2_STATUS_PATH = "/api/v2/status" + +#: The verbs the bootstrap API uses. Spelled as a `Literal` rather than passed +#: through to `client.request()` so the dispatch below stays exhaustive and each +#: call still reaches the named httpx method. +type _Method = Literal["GET", "POST", "PUT", "DELETE"] + @dataclass class _SSLCache: @@ -122,3 +137,118 @@ async def _get_client( ctx = await _create_ssl_context() async with httpx.AsyncClient(timeout=timeout, verify=ctx) as client: yield client + + +@dataclass(frozen=True, slots=True) +class _Reply: + """One panel answer, with the decoding every caller of it needs. + + Status classification stays with the caller, because it is genuinely + per-endpoint: 412 means "no passphrase is set" on the rotation path and + nothing anywhere else, and 404 means "no FQDN configured" on one call and + "not a v2 panel" on another. The two steps *around* that classification are + the same everywhere and had been written out once per endpoint -- translating + a failed connection, and turning a body into an object with the fields the + caller is about to read. Both live here. + """ + + host: str + endpoint: str + response: httpx.Response + + @property + def status_code(self) -> int: + """The status the panel answered with.""" + return self.response.status_code + + @property + def text(self) -> str: + """The body as text, for the one endpoint that answers with a PEM.""" + return self.response.text + + @property + def headers(self) -> httpx.Headers: + """The response headers, for ``Retry-After`` and content-type.""" + return self.response.headers + + def json_object(self, *required: str, on_malformed: type[SpanPanelAPIError] = SpanPanelAPIError) -> dict[str, object]: + """Decode the body as a JSON object and confirm the fields about to be read. + + A 200 is not a promise of a body. A panel part-way through starting + answers one with nothing in it; a proxy in front of one answers with an + HTML error page under a 200; and firmware is free to omit a field this + library treats as mandatory. Untranslated those surfaced as + ``JSONDecodeError`` and ``KeyError`` -- neither of them a + ``SpanPanelError``, so neither caught by a caller holding this library's + contract, and both escaping the retry clauses built on it. + + ``on_malformed`` exists for the one endpoint where an unreadable body is + "not ready yet" rather than "wrong": the schema fetch, whose caller + retries a booting panel. Every other endpoint here is asked once. + """ + try: + parsed = self.response.json() + except ValueError as exc: + raise on_malformed( + f"{self.host} answered HTTP {self.status_code} for {self.endpoint} with a body that is not JSON", + status_code=self.status_code, + ) from exc + if not isinstance(parsed, dict): + raise on_malformed( + f"{self.host} answered HTTP {self.status_code} for {self.endpoint} " + f"with {type(parsed).__name__}, not a JSON object", + status_code=self.status_code, + ) + body: dict[str, object] = parsed + missing = sorted(key for key in required if key not in body) + if missing: + raise on_malformed( + f"{self.host} answered HTTP {self.status_code} for {self.endpoint} " + f"without the required field(s) {', '.join(missing)}", + status_code=self.status_code, + ) + return body + + +async def _request( + method: _Method, + host: str, + port: int | None, + path: str, + *, + timeout: float, + httpx_client: httpx.AsyncClient | None = None, + ssl_context: ssl.SSLContext | None = None, + json: dict[str, str] | None = None, + headers: dict[str, str] | None = None, +) -> _Reply: + """Make one bootstrap request and translate everything that is not an answer. + + The whole ``httpx.TransportError`` family, not just a refused connect: + ``ReadError`` and ``WriteError`` when a rebooting panel resets mid-request, + and ``RemoteProtocolError`` when its proxy closes without answering, which is + what a proxy restarting under load produces. ``TimeoutException`` is itself a + ``TransportError``, so it has to be caught first to keep its own class. + + The verb is dispatched to the named httpx method rather than handed to + ``client.request()``: the URL stays the first positional argument of a + recognisable call, which is what makes an injected client inspectable by the + caller that supplied it. + """ + url = _build_url(host, port, path, ssl_context) + try: + async with _get_client(httpx_client, timeout, ssl_context) as client: + match method: + case "GET": + response = await client.get(url, headers=headers) + case "POST": + response = await client.post(url, json=json, headers=headers) + case "PUT": + response = await client.put(url, json=json, headers=headers) + case "DELETE": + response = await client.delete(url, headers=headers) + except httpx.TimeoutException as exc: + raise SpanPanelTimeoutError(f"Timed out connecting to {host}") from exc + except httpx.TransportError as exc: + raise SpanPanelConnectionError(f"Cannot reach panel at {host}: {exc}") from exc + return _Reply(host=host, endpoint=path, response=response) diff --git a/src/span_panel_api/auth.py b/src/span_panel_api/auth.py index a724ab6..f7dc259 100644 --- a/src/span_panel_api/auth.py +++ b/src/span_panel_api/auth.py @@ -18,18 +18,21 @@ import httpx -from ._http import _build_url, _get_client -from .exceptions import ( - SpanPanelAPIError, - SpanPanelAuthError, - SpanPanelConnectionError, - SpanPanelServerError, - SpanPanelTimeoutError, -) +from ._http import V2_STATUS_PATH, _request +from .exceptions import SpanPanelAPIError, SpanPanelAuthError, SpanPanelServerError from .models import HomieSchemaTypes, V2AuthResponse, V2HomieSchema, V2StatusInfo _LOGGER = logging.getLogger(__name__) +#: Read, written and deleted by three calls, which is three chances to mistype it. +_FQDN_PATH = "/api/v2/dns/fqdn" + + +def _bearer(token: str) -> dict[str, str]: + """The Authorization header every token-bearing call sends.""" + return {"Authorization": f"Bearer {token}"} + + #: Keys whose value is a credential wherever it appears in a response body, #: compared case-folded because the panel spells them lowerCamelCase and a #: proxy or validation layer in between may not. @@ -240,7 +243,6 @@ async def register_v2( SpanPanelTimeoutError: Request timed out SpanPanelAPIError: Unexpected response """ - url = _build_url(host, port, "/api/v2/auth/register", ssl_context) # The panel requires unique client names — append a random suffix. # The passphrase field must be "hopPassphrase" per the SPAN v2 API spec. suffix = uuid.uuid4().hex[:8] @@ -249,15 +251,18 @@ async def register_v2( if passphrase: payload["hopPassphrase"] = passphrase - try: - async with _get_client(httpx_client, timeout, ssl_context) as client: - response = await client.post(url, json=payload) - except httpx.ConnectError as exc: - raise SpanPanelConnectionError(f"Cannot reach panel at {host}") from exc - except httpx.TimeoutException as exc: - raise SpanPanelTimeoutError(f"Timed out connecting to {host}") from exc - - if response.status_code in (401, 403, 422): + reply = await _request( + "POST", + host, + port, + "/api/v2/auth/register", + timeout=timeout, + httpx_client=httpx_client, + ssl_context=ssl_context, + json=payload, + ) + + if reply.status_code in (401, 403, 422): # Status only, matching the shape the branch below already uses. The body # is logged at DEBUG instead: a 422 from the panel's validation layer # echoes the submitted `hopPassphrase` straight back, and interpolating @@ -266,13 +271,26 @@ async def register_v2( # passphrase goes with it because this is the one place in the library # that knows what was sent, and the panel is under no obligation to # quote it back under a key that names it. - _log_auth_failure("/api/v2/auth/register", response, () if passphrase is None else (passphrase,)) - raise SpanPanelAuthError(f"Authentication failed (HTTP {response.status_code})") - - if response.status_code != 200: - raise SpanPanelAPIError(f"Unexpected response from /api/v2/auth/register: HTTP {response.status_code}") - - data: dict[str, object] = response.json() + _log_auth_failure(reply.endpoint, reply.response, () if passphrase is None else (passphrase,)) + raise SpanPanelAuthError(f"Authentication failed (HTTP {reply.status_code})") + + if reply.status_code != 200: + raise SpanPanelAPIError(f"Unexpected response from /api/v2/auth/register: HTTP {reply.status_code}") + + data = reply.json_object( + "accessToken", + "tokenType", + "iatMs", + "ebusBrokerUsername", + "ebusBrokerPassword", + "ebusBrokerHost", + "ebusBrokerMqttsPort", + "ebusBrokerWsPort", + "ebusBrokerWssPort", + "hostname", + "serialNumber", + "hopPassphrase", + ) return V2AuthResponse( access_token=_str(data["accessToken"]), token_type=_str(data["tokenType"]), @@ -343,28 +361,29 @@ async def download_ca_cert( SpanPanelTimeoutError: Request timed out SpanPanelAPIError: Unexpected response or invalid PEM """ - url = _build_url(host, port, "/api/v2/certificate/ca", ssl_context) last_status: int | None = None for attempt in range(1, max_attempts + 1): - try: - async with _get_client(httpx_client, timeout, ssl_context) as client: - response = await client.get(url) - except httpx.ConnectError as exc: - raise SpanPanelConnectionError(f"Cannot reach panel at {host}") from exc - except httpx.TimeoutException as exc: - raise SpanPanelTimeoutError(f"Timed out connecting to {host}") from exc - - if response.status_code == 200: - pem = response.text + reply = await _request( + "GET", + host, + port, + "/api/v2/certificate/ca", + timeout=timeout, + httpx_client=httpx_client, + ssl_context=ssl_context, + ) + + if reply.status_code == 200: + pem = reply.text if not pem.startswith("-----BEGIN"): raise SpanPanelAPIError("Response is not a valid PEM certificate") return pem - last_status = response.status_code + last_status = reply.status_code - if response.status_code == HTTP_TOO_MANY_REQUESTS and attempt < max_attempts: - await asyncio.sleep(_retry_delay(response.headers.get("retry-after"), attempt, backoff_s)) + if reply.status_code == HTTP_TOO_MANY_REQUESTS and attempt < max_attempts: + await asyncio.sleep(_retry_delay(reply.headers.get("retry-after"), attempt, backoff_s)) continue break @@ -402,25 +421,17 @@ async def get_homie_schema( SpanPanelTimeoutError: Request timed out SpanPanelAPIError: Unexpected response """ - url = _build_url(host, port, "/api/v2/homie/schema", ssl_context) + reply = await _request( + "GET", + host, + port, + "/api/v2/homie/schema", + timeout=timeout, + httpx_client=httpx_client, + ssl_context=ssl_context, + ) - try: - async with _get_client(httpx_client, timeout, ssl_context) as client: - response = await client.get(url) - except httpx.TimeoutException as exc: - raise SpanPanelTimeoutError(f"Timed out connecting to {host}") from exc - except httpx.TransportError as exc: - # Every way the connection itself can fail, not just a refused connect: - # `ReadError` and `WriteError` when a rebooting panel resets mid-request, - # and `RemoteProtocolError` when its proxy closes without answering -- - # which is exactly what a proxy restarting under load produces. Catching - # only `ConnectError` meant those escaped this function untranslated, - # skipped the caller's retry clause entirely, and stranded the parser the - # same way a 502 used to. `TimeoutException` is itself a `TransportError`, - # so it has to be caught first. - raise SpanPanelConnectionError(f"Cannot reach panel at {host}: {exc}") from exc - - if response.status_code >= 500: + if reply.status_code >= 500: # A rebooting panel answers 502 from its front end while the application # behind it is still starting. That is "not ready yet", not "wrong" -- # and it is the ordinary shape of a firmware upgrade, because a device @@ -428,33 +439,22 @@ async def get_homie_schema( # a distinct class so a caller can retry it and fail fast on a 4xx, which # will not fix itself. raise SpanPanelServerError( - f"Panel not ready: HTTP {response.status_code} fetching the Homie schema", - status_code=response.status_code, + f"Panel not ready: HTTP {reply.status_code} fetching the Homie schema", + status_code=reply.status_code, ) - if response.status_code != 200: + if reply.status_code != 200: raise SpanPanelAPIError( - f"Failed to fetch Homie schema: HTTP {response.status_code}", - status_code=response.status_code, + f"Failed to fetch Homie schema: HTTP {reply.status_code}", + status_code=reply.status_code, ) - try: - parsed = response.json() - except ValueError as exc: - # A panel part-way through starting can answer 200 with a truncated or - # empty body. Retryable for the same reason a 502 is -- it is "not ready - # yet" wearing a different status -- and untranslated this had precisely - # the 502's old character: raised out of the caller's retry loop on the - # first attempt and left the parser where it was. - raise SpanPanelServerError( - f"Panel not ready: {host} answered 200 with a body that is not JSON", - status_code=response.status_code, - ) from exc - if not isinstance(parsed, dict): - raise SpanPanelServerError( - f"Panel not ready: {host} answered 200 with {type(parsed).__name__}, not an object", - status_code=response.status_code, - ) - data: dict[str, object] = parsed + # A panel part-way through starting can answer 200 with a truncated or empty + # body. Retryable for the same reason a 502 is -- it is "not ready yet" + # wearing a different status -- which is why this is the one endpoint that + # asks for its malformed bodies as `SpanPanelServerError`. Untranslated it + # had precisely the 502's old character: raised out of the caller's retry + # loop on the first attempt and leaving the parser where it was. + data = reply.json_object(on_malformed=SpanPanelServerError) # Extract types — each value is a dict of property definitions raw_types = data.get("types", {}) @@ -521,25 +521,24 @@ async def regenerate_passphrase( SpanPanelTimeoutError: Request timed out SpanPanelAPIError: Unexpected response """ - url = _build_url(host, port, "/api/v2/auth/passphrase", ssl_context) - headers = {"Authorization": f"Bearer {token}"} - - try: - async with _get_client(httpx_client, timeout, ssl_context) as client: - response = await client.put(url, headers=headers) - except httpx.ConnectError as exc: - raise SpanPanelConnectionError(f"Cannot reach panel at {host}") from exc - except httpx.TimeoutException as exc: - raise SpanPanelTimeoutError(f"Timed out connecting to {host}") from exc + reply = await _request( + "PUT", + host, + port, + "/api/v2/auth/passphrase", + timeout=timeout, + httpx_client=httpx_client, + ssl_context=ssl_context, + headers=_bearer(token), + ) - if response.status_code in (401, 403, 412): - raise SpanPanelAuthError(f"Authentication failed (HTTP {response.status_code})") + if reply.status_code in (401, 403, 412): + raise SpanPanelAuthError(f"Authentication failed (HTTP {reply.status_code})") - if response.status_code != 200: - raise SpanPanelAPIError(f"Failed to regenerate passphrase: HTTP {response.status_code}") + if reply.status_code != 200: + raise SpanPanelAPIError(f"Failed to regenerate passphrase: HTTP {reply.status_code}") - data: dict[str, object] = response.json() - return _str(data["ebusBrokerPassword"]) + return _str(reply.json_object("ebusBrokerPassword")["ebusBrokerPassword"]) async def register_fqdn( @@ -576,23 +575,23 @@ async def register_fqdn( SpanPanelTimeoutError: Request timed out SpanPanelAPIError: Unexpected response (including 404 if unsupported) """ - url = _build_url(host, port, "/api/v2/dns/fqdn", ssl_context) - headers = {"Authorization": f"Bearer {token}"} - payload = {"ebusTlsFqdn": fqdn} - - try: - async with _get_client(httpx_client, timeout, ssl_context) as client: - response = await client.post(url, json=payload, headers=headers) - except httpx.ConnectError as exc: - raise SpanPanelConnectionError(f"Cannot reach panel at {host}") from exc - except httpx.TimeoutException as exc: - raise SpanPanelTimeoutError(f"Timed out connecting to {host}") from exc + reply = await _request( + "POST", + host, + port, + _FQDN_PATH, + timeout=timeout, + httpx_client=httpx_client, + ssl_context=ssl_context, + json={"ebusTlsFqdn": fqdn}, + headers=_bearer(token), + ) - if response.status_code in (401, 403): - raise SpanPanelAuthError(f"Authentication failed (HTTP {response.status_code})") + if reply.status_code in (401, 403): + raise SpanPanelAuthError(f"Authentication failed (HTTP {reply.status_code})") - if response.status_code not in (200, 201, 204): - raise SpanPanelAPIError(f"Failed to register FQDN: HTTP {response.status_code}") + if reply.status_code not in (200, 201, 204): + raise SpanPanelAPIError(f"Failed to register FQDN: HTTP {reply.status_code}") async def get_fqdn( @@ -628,28 +627,27 @@ async def get_fqdn( SpanPanelTimeoutError: Request timed out SpanPanelAPIError: Unexpected response """ - url = _build_url(host, port, "/api/v2/dns/fqdn", ssl_context) - headers = {"Authorization": f"Bearer {token}"} - - try: - async with _get_client(httpx_client, timeout, ssl_context) as client: - response = await client.get(url, headers=headers) - except httpx.ConnectError as exc: - raise SpanPanelConnectionError(f"Cannot reach panel at {host}") from exc - except httpx.TimeoutException as exc: - raise SpanPanelTimeoutError(f"Timed out connecting to {host}") from exc + reply = await _request( + "GET", + host, + port, + _FQDN_PATH, + timeout=timeout, + httpx_client=httpx_client, + ssl_context=ssl_context, + headers=_bearer(token), + ) - if response.status_code in (401, 403): - raise SpanPanelAuthError(f"Authentication failed (HTTP {response.status_code})") + if reply.status_code in (401, 403): + raise SpanPanelAuthError(f"Authentication failed (HTTP {reply.status_code})") - if response.status_code == 404: + if reply.status_code == 404: return None - if response.status_code != 200: - raise SpanPanelAPIError(f"Failed to get FQDN: HTTP {response.status_code}") + if reply.status_code != 200: + raise SpanPanelAPIError(f"Failed to get FQDN: HTTP {reply.status_code}") - data: dict[str, object] = response.json() - raw = data.get("ebusTlsFqdn") + raw = reply.json_object().get("ebusTlsFqdn") if raw is None: return None return str(raw) @@ -686,22 +684,22 @@ async def delete_fqdn( SpanPanelTimeoutError: Request timed out SpanPanelAPIError: Unexpected response """ - url = _build_url(host, port, "/api/v2/dns/fqdn", ssl_context) - headers = {"Authorization": f"Bearer {token}"} - - try: - async with _get_client(httpx_client, timeout, ssl_context) as client: - response = await client.delete(url, headers=headers) - except httpx.ConnectError as exc: - raise SpanPanelConnectionError(f"Cannot reach panel at {host}") from exc - except httpx.TimeoutException as exc: - raise SpanPanelTimeoutError(f"Timed out connecting to {host}") from exc + reply = await _request( + "DELETE", + host, + port, + _FQDN_PATH, + timeout=timeout, + httpx_client=httpx_client, + ssl_context=ssl_context, + headers=_bearer(token), + ) - if response.status_code in (401, 403): - raise SpanPanelAuthError(f"Authentication failed (HTTP {response.status_code})") + if reply.status_code in (401, 403): + raise SpanPanelAuthError(f"Authentication failed (HTTP {reply.status_code})") - if response.status_code not in (200, 204): - raise SpanPanelAPIError(f"Failed to delete FQDN: HTTP {response.status_code}") + if reply.status_code not in (200, 204): + raise SpanPanelAPIError(f"Failed to delete FQDN: HTTP {reply.status_code}") async def get_v2_status( @@ -732,20 +730,20 @@ async def get_v2_status( SpanPanelTimeoutError: Request timed out SpanPanelAPIError: Unexpected response or non-v2 panel """ - url = _build_url(host, port, "/api/v2/status", ssl_context) - - try: - async with _get_client(httpx_client, timeout, ssl_context) as client: - response = await client.get(url) - except httpx.ConnectError as exc: - raise SpanPanelConnectionError(f"Cannot reach panel at {host}") from exc - except httpx.TimeoutException as exc: - raise SpanPanelTimeoutError(f"Timed out connecting to {host}") from exc + reply = await _request( + "GET", + host, + port, + V2_STATUS_PATH, + timeout=timeout, + httpx_client=httpx_client, + ssl_context=ssl_context, + ) - if response.status_code != 200: - raise SpanPanelAPIError(f"Panel does not support v2 API: HTTP {response.status_code}") + if reply.status_code != 200: + raise SpanPanelAPIError(f"Panel does not support v2 API: HTTP {reply.status_code}") - data: dict[str, object] = response.json() + data = reply.json_object() return V2StatusInfo( serial_number=str(data.get("serialNumber", "")), firmware_version=str(data.get("firmwareVersion", "")), diff --git a/src/span_panel_api/detection.py b/src/span_panel_api/detection.py index 3949c12..69bf4e0 100644 --- a/src/span_panel_api/detection.py +++ b/src/span_panel_api/detection.py @@ -12,7 +12,8 @@ import httpx -from ._http import _build_url, _get_client +from ._http import V2_STATUS_PATH, _request +from .exceptions import SpanPanelConnectionError, SpanPanelTimeoutError from .models import V2StatusInfo @@ -58,19 +59,29 @@ async def detect_api_version( DetectionResult indicating which API version is available. On transport failures, ``api_version`` is ``"v1"`` and ``probe_failed`` is True. """ - url = _build_url(host, port, "/api/v2/status", ssl_context) try: - async with _get_client(httpx_client, timeout, ssl_context) as client: - response = await client.get(url) - except (httpx.ConnectError, httpx.TimeoutException, httpx.RemoteProtocolError): + reply = await _request( + "GET", + host, + port, + V2_STATUS_PATH, + timeout=timeout, + httpx_client=httpx_client, + ssl_context=ssl_context, + ) + except (SpanPanelConnectionError, SpanPanelTimeoutError): + # Every way the connection can fail, which is the point of catching this + # library's own two classes rather than a hand-listed tuple of httpx + # ones: the tuple named `ConnectError`, `TimeoutException` and + # `RemoteProtocolError` and therefore let a `ReadError` -- a panel + # resetting its listener mid-probe -- out of a function documented to + # return a result rather than raise. return DetectionResult(api_version="v1", probe_failed=True) - if response.status_code != 200: + if reply.status_code != 200: return DetectionResult(api_version="v1") - data: dict[str, object] = response.json() - serial = str(data.get("serialNumber", "")) - firmware = str(data.get("firmwareVersion", "")) + data = reply.json_object() raw_proximity = data.get("proximityProven") proximity_proven: bool | None = None if isinstance(raw_proximity, bool): @@ -78,8 +89,8 @@ async def detect_api_version( return DetectionResult( api_version="v2", status_info=V2StatusInfo( - serial_number=serial, - firmware_version=firmware, + serial_number=str(data.get("serialNumber", "")), + firmware_version=str(data.get("firmwareVersion", "")), proximity_proven=proximity_proven, ), ) diff --git a/tests/test_rest_transport_contract.py b/tests/test_rest_transport_contract.py new file mode 100644 index 0000000..25bf5da --- /dev/null +++ b/tests/test_rest_transport_contract.py @@ -0,0 +1,164 @@ +"""Every bootstrap REST call answers in this library's vocabulary. + +Seven of them caught `httpx.ConnectError` and `httpx.TimeoutException` and +nothing else, so a connection that failed any other way — a panel resetting its +listener mid-response, a proxy closing without answering, which is what a proxy +restarting under load produces — came back as a raw httpx exception. A caller +holding this library's contract catches `SpanPanelError` and does not catch +that, so it escaped every retry clause built on it. + +The bodies were unguarded in the same way. A 200 is not a promise of a body: a +panel part-way through starting answers one with nothing in it, and a proxy in +front of one answers with an HTML error page under a 200. Those reached the +caller as a bare `JSONDecodeError`, and a 200 missing a field as a bare +`KeyError`, neither of them something this library says it can raise. + +`get_homie_schema` already did all of this correctly and is here to hold that +line while the other seven move onto the same helper. +""" + +from __future__ import annotations + +import json +from collections.abc import Awaitable, Callable +from unittest.mock import AsyncMock + +import httpx +import pytest + +from span_panel_api.auth import ( + delete_fqdn, + download_ca_cert, + get_fqdn, + get_homie_schema, + get_v2_status, + regenerate_passphrase, + register_fqdn, + register_v2, +) +from span_panel_api.detection import detect_api_version +from span_panel_api.exceptions import SpanPanelAPIError, SpanPanelConnectionError, SpanPanelError + +HOST = "panel.invalid" + +#: Every REST call this library makes, with the httpx method it reaches for. +#: `download_ca_cert` reads a PEM rather than JSON, so it appears only where the +#: transport is under test. +CALLS: list[tuple[str, Callable[[httpx.AsyncClient], Awaitable[object]], str]] = [ + ("register_v2", lambda c: register_v2(HOST, "home-assistant", "pass", httpx_client=c), "post"), + ("download_ca_cert", lambda c: download_ca_cert(HOST, httpx_client=c), "get"), + ("regenerate_passphrase", lambda c: regenerate_passphrase(HOST, "jwt", httpx_client=c), "put"), + ("register_fqdn", lambda c: register_fqdn(HOST, "jwt", "panel.example.com", httpx_client=c), "post"), + ("get_fqdn", lambda c: get_fqdn(HOST, "jwt", httpx_client=c), "get"), + ("delete_fqdn", lambda c: delete_fqdn(HOST, "jwt", httpx_client=c), "delete"), + ("get_v2_status", lambda c: get_v2_status(HOST, httpx_client=c), "get"), + ("get_homie_schema", lambda c: get_homie_schema(HOST, httpx_client=c), "get"), +] + +#: The subset that decodes a JSON object out of a 200. The other four either +#: read a PEM or read nothing but the status line. +DECODERS: list[tuple[str, Callable[[httpx.AsyncClient], Awaitable[object]], str]] = [ + ("register_v2", lambda c: register_v2(HOST, "home-assistant", "pass", httpx_client=c), "post"), + ("regenerate_passphrase", lambda c: regenerate_passphrase(HOST, "jwt", httpx_client=c), "put"), + ("get_fqdn", lambda c: get_fqdn(HOST, "jwt", httpx_client=c), "get"), + ("get_v2_status", lambda c: get_v2_status(HOST, httpx_client=c), "get"), +] + + +def _client(method: str, answer: httpx.Response | Exception) -> AsyncMock: + """An injected client whose one method returns, or raises, `answer`.""" + injected = AsyncMock(spec=httpx.AsyncClient) + if isinstance(answer, Exception): + setattr(injected, method, AsyncMock(side_effect=answer)) + else: + setattr(injected, method, AsyncMock(return_value=answer)) + return injected + + +def _response(status_code: int, *, body: bytes, content_type: str) -> httpx.Response: + return httpx.Response( + status_code=status_code, + content=body, + headers={"content-type": content_type}, + request=httpx.Request("GET", f"http://{HOST}/"), + ) + + +def _json_response(payload: object, status_code: int = 200) -> httpx.Response: + return _response(status_code, body=json.dumps(payload).encode(), content_type="application/json") + + +class TestTransportFailuresAreTranslated: + """The whole `httpx.TransportError` family, not just a refused connect.""" + + @pytest.mark.asyncio + @pytest.mark.parametrize(("name", "call", "method"), CALLS, ids=[row[0] for row in CALLS]) + @pytest.mark.parametrize( + "failure", + [ + httpx.ReadError("connection reset"), + httpx.WriteError("broken pipe"), + httpx.RemoteProtocolError("server closed connection without sending a response"), + ], + ids=["read-reset", "write-reset", "proxy-closed-without-answering"], + ) + async def test_a_transport_failure_is_a_connection_error( + self, + name: str, + call: Callable[[httpx.AsyncClient], Awaitable[object]], + method: str, + failure: Exception, + ) -> None: + with pytest.raises(SpanPanelConnectionError): + await call(_client(method, failure)) + + @pytest.mark.asyncio + async def test_detection_reports_a_read_reset_as_a_failed_probe(self) -> None: + """The detector answers rather than raising, and this was not among the + failures it recognised — so a panel resetting mid-probe raised + `httpx.ReadError` out of a function documented to return a result.""" + result = await detect_api_version(HOST, httpx_client=_client("get", httpx.ReadError("connection reset"))) + assert result.api_version == "v1" + assert result.probe_failed is True + + +class TestMalformedBodies: + """A 200 whose body cannot be read is this library's error, not httpx's.""" + + @pytest.mark.asyncio + @pytest.mark.parametrize(("name", "call", "method"), DECODERS, ids=[row[0] for row in DECODERS]) + async def test_a_200_that_is_not_json_is_an_api_error( + self, name: str, call: Callable[[httpx.AsyncClient], Awaitable[object]], method: str + ) -> None: + page = _response(200, body=b"Bad Gateway", content_type="text/html") + with pytest.raises(SpanPanelAPIError): + await call(_client(method, page)) + + @pytest.mark.asyncio + @pytest.mark.parametrize(("name", "call", "method"), DECODERS, ids=[row[0] for row in DECODERS]) + async def test_a_200_that_is_not_an_object_is_an_api_error( + self, name: str, call: Callable[[httpx.AsyncClient], Awaitable[object]], method: str + ) -> None: + with pytest.raises(SpanPanelAPIError): + await call(_client(method, _json_response([1, 2, 3]))) + + @pytest.mark.asyncio + async def test_register_names_the_field_the_panel_left_out(self) -> None: + """A 200 without `accessToken` used to raise `KeyError('accessToken')`.""" + with pytest.raises(SpanPanelAPIError) as caught: + await register_v2( + HOST, "home-assistant", "pass", httpx_client=_client("post", _json_response({"tokenType": "Bearer"})) + ) + assert "accessToken" in str(caught.value) + + @pytest.mark.asyncio + async def test_regenerate_names_the_field_the_panel_left_out(self) -> None: + with pytest.raises(SpanPanelAPIError) as caught: + await regenerate_passphrase(HOST, "jwt", httpx_client=_client("put", _json_response({}))) + assert "ebusBrokerPassword" in str(caught.value) + + @pytest.mark.asyncio + async def test_every_malformed_body_stays_inside_the_error_hierarchy(self) -> None: + """The point of the translation: one `except SpanPanelError` catches it.""" + with pytest.raises(SpanPanelError): + await get_v2_status(HOST, httpx_client=_client("get", _response(200, body=b"", content_type="text/plain"))) From 54a22a9c5248745f3c4eeb41311d9f2cdea0a559 Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Wed, 26 Aug 2026 21:02:54 -0700 Subject: [PATCH 03/11] feat(http): warn when the panel's bootstrap traffic is plaintext The MQTT bridge already says once per bridge that an unpinned trust anchor is fetched over plaintext HTTP and whatever answers is trusted. The REST side said nothing, though registration is the request that carries the panel passphrase up and brings the broker password back -- so anything on the path reads both, and an operator had no way to tell the encryption was off. Warned by the two calls that bootstrap a client, not from inside `_request`: one line per client is a line somebody reads, one per request is a line somebody filters out. `register_v2` warns for itself and the factory warns only on the branch that never reaches it, so a caller never hears it twice. Plaintext remains the default -- requiring a pin would break every install on upgrade -- but it is now a default the log names. --- src/span_panel_api/_http.py | 31 +++++++ src/span_panel_api/auth.py | 6 +- src/span_panel_api/factory.py | 8 ++ tests/test_plaintext_warning.py | 139 ++++++++++++++++++++++++++++++++ 4 files changed, 183 insertions(+), 1 deletion(-) create mode 100644 tests/test_plaintext_warning.py diff --git a/src/span_panel_api/_http.py b/src/span_panel_api/_http.py index 15be558..eefdf5a 100644 --- a/src/span_panel_api/_http.py +++ b/src/span_panel_api/_http.py @@ -139,6 +139,37 @@ async def _get_client( yield client +def _warn_plaintext_transport(host: str, what: str, ssl_context: ssl.SSLContext | None) -> None: + """Say out loud that the panel's bootstrap traffic is not encrypted. + + In the same voice as the MQTT bridge's unpinned-CA warning, and for the same + reason: a security property that is off by default is only a decision if the + operator can tell it is off. ``ssl_context=None`` puts every bootstrap + request on plaintext ``http://``, and registration is the one that carries + the panel passphrase up and brings the broker password back -- so anything on + the path reads both, and nothing said so. + + Warned by the two calls that *bootstrap* a client rather than from inside + ``_request``. These are made a handful of times per config entry, so one line + per client is a line somebody reads; one per request -- registration, + detection, schema, status, FQDN -- is a line somebody filters out. The two + warn on mutually exclusive paths, so a caller never hears it twice. + + The credential itself is never named here. This is a warning *about* a + secret, not a place to put one. + """ + if ssl_context is not None: + return + _LOGGER.warning( + "%s for %s is being sent over plaintext HTTP: no ssl_context was supplied, so the request " + "and its response -- including any credential either one carries -- are readable by anything " + "on the path between here and the panel. Pin the panel's CA certificate and pass it as " + "ssl_context.", + what, + host, + ) + + @dataclass(frozen=True, slots=True) class _Reply: """One panel answer, with the decoding every caller of it needs. diff --git a/src/span_panel_api/auth.py b/src/span_panel_api/auth.py index f7dc259..9199bcf 100644 --- a/src/span_panel_api/auth.py +++ b/src/span_panel_api/auth.py @@ -18,7 +18,7 @@ import httpx -from ._http import V2_STATUS_PATH, _request +from ._http import V2_STATUS_PATH, _request, _warn_plaintext_transport from .exceptions import SpanPanelAPIError, SpanPanelAuthError, SpanPanelServerError from .models import HomieSchemaTypes, V2AuthResponse, V2HomieSchema, V2StatusInfo @@ -243,6 +243,10 @@ async def register_v2( SpanPanelTimeoutError: Request timed out SpanPanelAPIError: Unexpected response """ + # This is the request whose plaintext exposure is a credential exposure: the + # passphrase goes up in it and the broker password comes back in it. + _warn_plaintext_transport(host, "SPAN Panel v2 registration", ssl_context) + # The panel requires unique client names — append a random suffix. # The passphrase field must be "hopPassphrase" per the SPAN v2 API spec. suffix = uuid.uuid4().hex[:8] diff --git a/src/span_panel_api/factory.py b/src/span_panel_api/factory.py index 5ab4464..ac72e09 100644 --- a/src/span_panel_api/factory.py +++ b/src/span_panel_api/factory.py @@ -11,6 +11,7 @@ import ssl from typing import TYPE_CHECKING +from ._http import _warn_plaintext_transport from .adapters import resolve_adapter from .auth import get_homie_schema, register_v2 from .detection import detect_api_version @@ -93,6 +94,13 @@ async def create_span_client( ) if serial_number is None: serial_number = auth_response.serial_number + else: + # `register_v2` warns for itself, so this branch is the only one that + # bootstraps a client without ever reaching it. No passphrase travels + # here, but detection and the schema fetch still go out in the clear and + # still hand an observer the panel's topology -- and one HTTP path left + # open is where the next one gets added. + _warn_plaintext_transport(host, "Panel bootstrap traffic", ssl_context) if serial_number is None: # Try to detect from panel status diff --git a/tests/test_plaintext_warning.py b/tests/test_plaintext_warning.py new file mode 100644 index 0000000..16ffd8e --- /dev/null +++ b/tests/test_plaintext_warning.py @@ -0,0 +1,139 @@ +"""Bootstrapping a client over plaintext HTTP says so. + +Without an `ssl_context` every bootstrap request is plain `http://`, and +registration is the one that carries the panel passphrase up and brings the +broker password back — so anything on the path between the host and the panel +reads both. That is the default, and it stays the default, because requiring a +pin would break every install on upgrade. But the MQTT bridge already warns once +when its trust anchor is unpinned, and the REST side said nothing at all: a +security property that is off by default is only a decision if the operator can +tell it is off. +""" + +from __future__ import annotations + +import json +import logging +import ssl +from unittest.mock import AsyncMock, patch + +import httpx +import pytest + +from span_panel_api.auth import register_v2 +from span_panel_api.mqtt.models import MqttClientConfig + +from conftest import flat_schema + +HOST = "panel.invalid" + +V2_AUTH_JSON = { + "accessToken": "jwt", + "tokenType": "Bearer", + "iatMs": 1700000000000, + "ebusBrokerUsername": "user", + "ebusBrokerPassword": "pass", + "ebusBrokerHost": HOST, + "ebusBrokerMqttsPort": 8883, + "ebusBrokerWsPort": 9001, + "ebusBrokerWssPort": 9002, + "hostname": "spanpanel", + "serialNumber": "SYN-0000-0001", + "hopPassphrase": "hop", +} + + +def _auth_client() -> AsyncMock: + response = httpx.Response( + 200, + content=json.dumps(V2_AUTH_JSON).encode(), + headers={"content-type": "application/json"}, + request=httpx.Request("POST", f"http://{HOST}/api/v2/auth/register"), + ) + injected = AsyncMock(spec=httpx.AsyncClient) + injected.post = AsyncMock(return_value=response) + return injected + + +def _warnings(caplog: pytest.LogCaptureFixture) -> list[str]: + return [record.getMessage() for record in caplog.records if record.levelno >= logging.WARNING] + + +class TestRegisterV2: + @pytest.mark.asyncio + async def test_plaintext_registration_warns_once(self, caplog: pytest.LogCaptureFixture) -> None: + with caplog.at_level(logging.WARNING): + await register_v2(HOST, "home-assistant", "secret", httpx_client=_auth_client()) + warnings = _warnings(caplog) + assert len(warnings) == 1 + assert HOST in warnings[0] + assert "plaintext" in warnings[0] + assert "ssl_context" in warnings[0] + + @pytest.mark.asyncio + async def test_the_warning_never_repeats_the_credential(self, caplog: pytest.LogCaptureFixture) -> None: + """It is a warning about a credential, not a place to put one.""" + with caplog.at_level(logging.WARNING): + await register_v2(HOST, "home-assistant", "correct-horse-battery-staple", httpx_client=_auth_client()) + assert "correct-horse-battery-staple" not in caplog.text + + @pytest.mark.asyncio + async def test_a_pinned_registration_is_silent(self, caplog: pytest.LogCaptureFixture) -> None: + context = ssl.create_default_context() + with patch("span_panel_api._http.httpx.AsyncClient") as mock_cls: + dedicated = AsyncMock() + dedicated.__aenter__ = AsyncMock(return_value=dedicated) + dedicated.__aexit__ = AsyncMock(return_value=False) + dedicated.post = AsyncMock( + return_value=httpx.Response( + 200, + content=json.dumps(V2_AUTH_JSON).encode(), + headers={"content-type": "application/json"}, + request=httpx.Request("POST", f"https://{HOST}/api/v2/auth/register"), + ) + ) + mock_cls.return_value = dedicated + with caplog.at_level(logging.WARNING): + await register_v2(HOST, "home-assistant", "secret", ssl_context=context) + assert _warnings(caplog) == [] + + +class TestCreateSpanClient: + """The factory's other path bootstraps without ever calling `register_v2`.""" + + @pytest.mark.asyncio + async def test_a_prebuilt_config_still_warns_about_its_plaintext_bootstrap( + self, caplog: pytest.LogCaptureFixture + ) -> None: + """A caller supplying `mqtt_config` sends no passphrase, but detection + and the schema fetch are still plaintext and still expose the panel's + topology — and nothing on this path had a warning in it.""" + from span_panel_api.factory import create_span_client + + config = MqttClientConfig(broker_host="broker.local", username="user", password="pass") + with ( + patch("span_panel_api.factory.SpanMqttClient") as mock_cls, + patch("span_panel_api.factory.get_homie_schema", return_value=flat_schema(32)), + ): + mock_cls.return_value.connect = AsyncMock() + with caplog.at_level(logging.WARNING): + await create_span_client(HOST, mqtt_config=config, serial_number="SYN-0000-0001") + + warnings = _warnings(caplog) + assert len(warnings) == 1 + assert HOST in warnings[0] + + @pytest.mark.asyncio + async def test_the_registering_path_warns_exactly_once(self, caplog: pytest.LogCaptureFixture) -> None: + """`register_v2` already warned; the factory must not say it again.""" + from span_panel_api.factory import create_span_client + + with ( + patch("span_panel_api.factory.SpanMqttClient") as mock_cls, + patch("span_panel_api.factory.get_homie_schema", return_value=flat_schema(32)), + ): + mock_cls.return_value.connect = AsyncMock() + with caplog.at_level(logging.WARNING): + await create_span_client(HOST, passphrase="secret", httpx_client=_auth_client()) + + assert len(_warnings(caplog)) == 1 From 27b8685e436cece0bf6dbcd13b21f668523baf9b Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Wed, 26 Aug 2026 21:04:27 -0700 Subject: [PATCH 04/11] refactor(models): one reader for the v2 status endpoint `/api/v2/status` was parsed in two places -- the detector's, deciding whether the panel speaks v2 at all, and `get_v2_status`'s, reading the same answer for a caller that already knows it does -- and the two had drifted. Only the detector read `proximityProven`, so the same panel reported "proximity unknown" or "proximity proven" depending on which call had asked. `V2StatusInfo.from_status_payload` is now the only reader, so the field a caller gets no longer depends on the route it took. Absence still reads as `None` rather than false, because firmware below 202609 does not report proximity at all and that is not the same fact as not proven. --- src/span_panel_api/auth.py | 6 +-- src/span_panel_api/detection.py | 14 +------ src/span_panel_api/models.py | 29 +++++++++++++ tests/test_v2_status_parser.py | 74 +++++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 18 deletions(-) create mode 100644 tests/test_v2_status_parser.py diff --git a/src/span_panel_api/auth.py b/src/span_panel_api/auth.py index 9199bcf..261adb0 100644 --- a/src/span_panel_api/auth.py +++ b/src/span_panel_api/auth.py @@ -747,8 +747,4 @@ async def get_v2_status( if reply.status_code != 200: raise SpanPanelAPIError(f"Panel does not support v2 API: HTTP {reply.status_code}") - data = reply.json_object() - return V2StatusInfo( - serial_number=str(data.get("serialNumber", "")), - firmware_version=str(data.get("firmwareVersion", "")), - ) + return V2StatusInfo.from_status_payload(reply.json_object()) diff --git a/src/span_panel_api/detection.py b/src/span_panel_api/detection.py index 69bf4e0..f324f74 100644 --- a/src/span_panel_api/detection.py +++ b/src/span_panel_api/detection.py @@ -81,16 +81,4 @@ async def detect_api_version( if reply.status_code != 200: return DetectionResult(api_version="v1") - data = reply.json_object() - raw_proximity = data.get("proximityProven") - proximity_proven: bool | None = None - if isinstance(raw_proximity, bool): - proximity_proven = raw_proximity - return DetectionResult( - api_version="v2", - status_info=V2StatusInfo( - serial_number=str(data.get("serialNumber", "")), - firmware_version=str(data.get("firmwareVersion", "")), - proximity_proven=proximity_proven, - ), - ) + return DetectionResult(api_version="v2", status_info=V2StatusInfo.from_status_payload(reply.json_object())) diff --git a/src/span_panel_api/models.py b/src/span_panel_api/models.py index 4436423..10135b1 100644 --- a/src/span_panel_api/models.py +++ b/src/span_panel_api/models.py @@ -9,6 +9,7 @@ from __future__ import annotations +from collections.abc import Mapping from dataclasses import dataclass, field # Homie schema type: {type_name: {property_name: {attribute: value}}} @@ -536,6 +537,34 @@ class V2StatusInfo: firmware_version: str proximity_proven: bool | None = None # Added in firmware 202609; None on older panels + @classmethod + def from_status_payload(cls, payload: Mapping[str, object]) -> V2StatusInfo: + """Read one decoded ``/api/v2/status`` body. + + One reader, because there were two — the detector's, deciding whether the + panel speaks v2 at all, and ``get_v2_status``'s, reading the same answer + for a caller that already knows it does. They had already drifted: only + the detector read ``proximityProven``, so the same panel reported + "proximity unknown" or "proximity proven" depending on which of the two + had asked. + + A field the panel omits reads as the empty string rather than as an + error. This endpoint's whole job on the detection path is to answer for a + panel that may not fully support it, so a partial body is information, + not a failure. + + ``proximity_proven`` is the exception and stays ``None`` unless the panel + published a real boolean. Absent and false are different facts there — + firmware below 202609 does not report it at all — and coercing whatever + arrived would turn a string ``"false"`` into ``True``. + """ + raw_proximity = payload.get("proximityProven") + return cls( + serial_number=str(payload.get("serialNumber", "")), + firmware_version=str(payload.get("firmwareVersion", "")), + proximity_proven=raw_proximity if isinstance(raw_proximity, bool) else None, + ) + _CIRCUIT_TYPE_KEY = "energy.ebus.device.circuit" diff --git a/tests/test_v2_status_parser.py b/tests/test_v2_status_parser.py new file mode 100644 index 0000000..b6a1bc2 --- /dev/null +++ b/tests/test_v2_status_parser.py @@ -0,0 +1,74 @@ +"""`/api/v2/status` is read once, by one reader. + +It was read twice — by the detector, deciding whether the panel speaks v2 at +all, and by `get_v2_status`, reading the same answer for a caller that already +knows it does. The two had already drifted: only the detector read +`proximityProven`, so the same panel answered "proximity unknown" or "proximity +proven" depending on which call had asked. +""" + +from __future__ import annotations + +import json +from unittest.mock import AsyncMock + +import httpx +import pytest + +from span_panel_api.auth import get_v2_status +from span_panel_api.detection import detect_api_version +from span_panel_api.models import V2StatusInfo + +HOST = "panel.invalid" + +STATUS_JSON = { + "serialNumber": "SYN-0000-0001", + "firmwareVersion": "spanos2/r202609/01", + "proximityProven": True, +} + + +def _client(payload: object) -> AsyncMock: + response = httpx.Response( + 200, + content=json.dumps(payload).encode(), + headers={"content-type": "application/json"}, + request=httpx.Request("GET", f"http://{HOST}/api/v2/status"), + ) + injected = AsyncMock(spec=httpx.AsyncClient) + injected.get = AsyncMock(return_value=response) + return injected + + +class TestBothCallersAgree: + @pytest.mark.asyncio + async def test_the_same_body_produces_the_same_status(self) -> None: + """The regression: `get_v2_status` dropped `proximityProven` on the floor.""" + probed = await detect_api_version(HOST, httpx_client=_client(STATUS_JSON)) + fetched = await get_v2_status(HOST, httpx_client=_client(STATUS_JSON)) + assert probed.status_info == fetched + assert fetched.proximity_proven is True + + +class TestStatusPayload: + def test_reads_every_field(self) -> None: + status = V2StatusInfo.from_status_payload(STATUS_JSON) + assert status.serial_number == "SYN-0000-0001" + assert status.firmware_version == "spanos2/r202609/01" + assert status.proximity_proven is True + + def test_an_omitted_field_reads_as_empty_rather_than_failing(self) -> None: + """This endpoint answers for panels that may not fully support it.""" + assert V2StatusInfo.from_status_payload({}) == V2StatusInfo(serial_number="", firmware_version="") + + def test_absent_proximity_stays_none_rather_than_false(self) -> None: + """Firmware below 202609 does not report it, and that is not "not proven".""" + assert V2StatusInfo.from_status_payload({"serialNumber": "SYN-0000-0001"}).proximity_proven is None + + @pytest.mark.parametrize("raw", ["true", 1, None, {}], ids=["string", "int", "null", "object"]) + def test_a_non_boolean_proximity_is_not_believed(self, raw: object) -> None: + """Coercing `"false"` to True is the failure mode this guards against.""" + assert V2StatusInfo.from_status_payload({"proximityProven": raw}).proximity_proven is None + + def test_a_false_proximity_is_kept(self) -> None: + assert V2StatusInfo.from_status_payload({"proximityProven": False}).proximity_proven is False From eff7e5b2adf7bce971b07fa71fb4a4c9a96d1819 Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Wed, 26 Aug 2026 21:05:43 -0700 Subject: [PATCH 05/11] refactor(schema-0): drop the unused circuit-id denormaliser `denormalize_circuit_id` restored dashes to a dashless UUID, and nothing asked it to. The adapter normalises ids on the way in for entity stability and never converts them back -- no parser, no snapshot, no topic builder and no consumer called it, and the only two references anywhere were the tests written for it. It is not part of the `SchemaAdapter` protocol and was never re-exported from the package, so nothing outside could reach it either. --- .../schema-0/src/span_panel_api_schema_0/const.py | 7 ------- tests/test_mqtt_homie.py | 14 -------------- 2 files changed, 21 deletions(-) diff --git a/packages/schema-0/src/span_panel_api_schema_0/const.py b/packages/schema-0/src/span_panel_api_schema_0/const.py index e94dc5c..400da6e 100644 --- a/packages/schema-0/src/span_panel_api_schema_0/const.py +++ b/packages/schema-0/src/span_panel_api_schema_0/const.py @@ -74,10 +74,3 @@ def normalize_circuit_id(node_id: str) -> str: """Strip dashes from Homie UUID for entity stability.""" return node_id.replace("-", "") - - -def denormalize_circuit_id(circuit_id: str) -> str: - """Restore dashes to a 32-char dashless UUID (8-4-4-4-12 format).""" - if len(circuit_id) == 32 and "-" not in circuit_id: - return f"{circuit_id[:8]}-{circuit_id[8:12]}-{circuit_id[12:16]}-{circuit_id[16:20]}-{circuit_id[20:]}" - return circuit_id diff --git a/tests/test_mqtt_homie.py b/tests/test_mqtt_homie.py index 324dec6..1814086 100644 --- a/tests/test_mqtt_homie.py +++ b/tests/test_mqtt_homie.py @@ -184,20 +184,6 @@ def test_circuit_id_normalization(self): assert normalize_circuit_id("aabbccdd-1122-3344-5566-778899001122") == "aabbccdd11223344556677889900112" + "2" - def test_circuit_id_denormalization(self): - from span_panel_api_schema_0.const import denormalize_circuit_id - - result = denormalize_circuit_id("aabbccdd11223344556677889900112" + "2") - assert result == "aabbccdd-1122-3344-5566-778899001122" - - def test_denormalize_non_uuid(self): - from span_panel_api_schema_0.const import denormalize_circuit_id - - # Non-32-char strings pass through unchanged - assert denormalize_circuit_id("short") == "short" - # Already dashed passes through - assert denormalize_circuit_id("aabbccdd-1122-3344-5566-778899001122") == "aabbccdd-1122-3344-5566-778899001122" - def test_circuit_power_negation(self): """active-power in W, negative=consumption → positive=consumption in snapshot.""" acc, consumer = _build_ready_consumer() From 31a86e09e476f6248b853c1093b72269bd1c7c4a Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Wed, 26 Aug 2026 21:07:56 -0700 Subject: [PATCH 06/11] chore: span-panel-api 3.1.1, span-panel-api-schema-0 1.1.1 The adapter's floor on the bootstrap stays at 3.1.0 and the bootstrap's extras stay at 1.1.0: nothing in either release asks anything new of the other, so neither pairing that resolved before stops resolving now. --- CHANGELOG.md | 15 +++++++++++++++ packages/schema-0/CHANGELOG.md | 8 ++++++++ packages/schema-0/pyproject.toml | 2 +- pyproject.toml | 2 +- uv.lock | 4 ++-- 5 files changed, 27 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 482f681..6d7baf7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,21 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), Pre-releases are not listed separately. A beta is a step towards the next public version, so its changes are folded into that version's entry as they land and are described against the **last public release**, never against the beta before it. What one beta corrected in an earlier beta does not appear at all: from the point of view of somebody upgrading between released versions, it never happened. +## [3.1.1] + +A follow-up to 3.1.0's security work, with no API change and no adapter move required. + +### Fixed + +- **A rejected passphrase no longer reaches the debug log**, closing the shape of validation response that reports the field that failed in one place and the value it rejected in another, under a key that says nothing about what it holds. +- **A panel that fails part-way through answering is reported as unreachable**, rather than as an error from the HTTP layer that a consumer catching this library's own errors would not catch. +- **A response this library cannot read is reported as an API error naming the endpoint and the missing field**, instead of a raw parsing error raised out of the call. +- **`get_v2_status` reports whether the panel proved proximity**, which until now only the detection path had read, so the same panel answered differently depending on which call had asked. + +### Added + +- **A warning when the panel's bootstrap traffic is unencrypted**, logged once per client, never repeating the credential it warns about, and leaving plaintext the default it has always been. + ## [3.1.0] A security release. Three things a caller could not previously find out — whether a control command was delivered, whether the panel's bootstrap traffic was encrypted, and whether the CA behind the MQTT broker is still the one that was there yesterday — diff --git a/packages/schema-0/CHANGELOG.md b/packages/schema-0/CHANGELOG.md index 1fb8f19..a15fda3 100644 --- a/packages/schema-0/CHANGELOG.md +++ b/packages/schema-0/CHANGELOG.md @@ -9,6 +9,14 @@ rather than by this version number. A release here means this parser changed, ne Pre-releases are not listed separately. A beta is a step towards the next public version, so its changes are folded into that version's entry as they land and are described against the last public release, never against the beta before it. +## [1.1.1] + +Still requires `span-panel-api` **3.1.0 or newer**, unchanged. + +### Removed + +- **`denormalize_circuit_id`, which nothing called** — it restored the dashes a circuit's UUID is stripped of on the way in, a form no snapshot, command topic or consumer has ever asked for. + ## [1.1.0] Requires `span-panel-api` **3.1.0 or newer**, and the two must be upgraded together in both directions: this wheel is rejected at discovery by a 3.0.x bootstrap, and a 1.0.0 wheel is rejected by 3.1.0. diff --git a/packages/schema-0/pyproject.toml b/packages/schema-0/pyproject.toml index 31e3aec..6edad03 100644 --- a/packages/schema-0/pyproject.toml +++ b/packages/schema-0/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "span-panel-api-schema-0" -version = "1.1.0" +version = "1.1.1" description = "Flat-schema (data-model-version absent) parser for span-panel-api" authors = [ {name = "SpanPanel"} diff --git a/pyproject.toml b/pyproject.toml index 378b0b1..cecf23a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "span-panel-api" -version = "3.1.0" +version = "3.1.1" description = "A client library for SPAN Panel API" authors = [ {name = "SpanPanel"} diff --git a/uv.lock b/uv.lock index dbb9c06..78b62aa 100644 --- a/uv.lock +++ b/uv.lock @@ -962,7 +962,7 @@ wheels = [ [[package]] name = "span-panel-api" -version = "3.1.0" +version = "3.1.1" source = { editable = "." } dependencies = [ { name = "httpx" }, @@ -1032,7 +1032,7 @@ dev = [ [[package]] name = "span-panel-api-schema-0" -version = "1.1.0" +version = "1.1.1" source = { editable = "packages/schema-0" } dependencies = [ { name = "span-panel-api" }, From 8757fab7475bab20691660e4f155b571470e9d78 Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Wed, 26 Aug 2026 21:30:35 -0700 Subject: [PATCH 07/11] fix(detection): answer rather than raise on an unreadable status body `detect_api_version` is documented to return a result, and every caller reads one rather than guarding the call. Since the body validation went in, a 200 carrying something that is not a status object raised out of it instead -- and the case that produces that is a proxy in front of a v1 panel answering its own HTML error page under a 200, which is a panel the caller was only asking about. Reported as a failed probe rather than a v1 verdict: no answer was read, so the honest report is that the question is still open. `probe_failed` now documents that reading, which covers both a request that did not complete and a response that could not be understood. --- src/span_panel_api/detection.py | 32 ++++++++++++++++++--------- tests/test_rest_transport_contract.py | 24 ++++++++++++++++++++ 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/span_panel_api/detection.py b/src/span_panel_api/detection.py index f324f74..53eb75b 100644 --- a/src/span_panel_api/detection.py +++ b/src/span_panel_api/detection.py @@ -13,7 +13,7 @@ import httpx from ._http import V2_STATUS_PATH, _request -from .exceptions import SpanPanelConnectionError, SpanPanelTimeoutError +from .exceptions import SpanPanelAPIError, SpanPanelConnectionError, SpanPanelTimeoutError from .models import V2StatusInfo @@ -21,9 +21,11 @@ class DetectionResult: """Result of probing a SPAN Panel for API version support. - ``probe_failed`` is True when the HTTP request did not complete (for example - connection refused, timeout, or protocol error). It is False when any HTTP - response was received, including non-200 statuses that imply a v1-only panel. + ``probe_failed`` is True when the probe produced no usable answer: the HTTP + request did not complete (connection refused, timeout, a reset mid-response), + or the panel answered 200 with a body that could not be read. It is False + when a readable answer arrived, including non-200 statuses that imply a + v1-only panel — those are a verdict, not a failed probe. """ api_version: str # "v1" | "v2" @@ -56,8 +58,10 @@ async def detect_api_version( this call to ``https://``; ``None`` is byte-identical to 3.0.1. Returns: - DetectionResult indicating which API version is available. On transport - failures, ``api_version`` is ``"v1"`` and ``probe_failed`` is True. + DetectionResult indicating which API version is available. This function + answers rather than raising: anything that leaves the question + unanswered comes back as ``api_version="v1"`` with ``probe_failed`` + True. """ try: reply = await _request( @@ -69,6 +73,9 @@ async def detect_api_version( httpx_client=httpx_client, ssl_context=ssl_context, ) + if reply.status_code != 200: + return DetectionResult(api_version="v1") + status = V2StatusInfo.from_status_payload(reply.json_object()) except (SpanPanelConnectionError, SpanPanelTimeoutError): # Every way the connection can fail, which is the point of catching this # library's own two classes rather than a hand-listed tuple of httpx @@ -77,8 +84,13 @@ async def detect_api_version( # resetting its listener mid-probe -- out of a function documented to # return a result rather than raise. return DetectionResult(api_version="v1", probe_failed=True) + except SpanPanelAPIError: + # A 200 carrying something that is not a status object. The case that + # matters is a proxy in front of a v1 panel answering its own HTML error + # page under a 200: unreadable, so nothing in it says "v2", and raising + # would take setup down over a panel the caller was only asking about. + # Reported as a failed probe rather than a v1 verdict, because no answer + # was read -- the caller is told the question is still open. + return DetectionResult(api_version="v1", probe_failed=True) - if reply.status_code != 200: - return DetectionResult(api_version="v1") - - return DetectionResult(api_version="v2", status_info=V2StatusInfo.from_status_payload(reply.json_object())) + return DetectionResult(api_version="v2", status_info=status) diff --git a/tests/test_rest_transport_contract.py b/tests/test_rest_transport_contract.py index 25bf5da..d0d1a98 100644 --- a/tests/test_rest_transport_contract.py +++ b/tests/test_rest_transport_contract.py @@ -157,6 +157,30 @@ async def test_regenerate_names_the_field_the_panel_left_out(self) -> None: await regenerate_passphrase(HOST, "jwt", httpx_client=_client("put", _json_response({}))) assert "ebusBrokerPassword" in str(caught.value) + @pytest.mark.asyncio + @pytest.mark.parametrize( + "answer", + [ + _response(200, body=b"Bad Gateway", content_type="text/html"), + _response(200, body=b"", content_type="text/plain"), + _json_response([1, 2, 3]), + ], + ids=["proxy-html", "empty", "not-an-object"], + ) + async def test_the_detector_answers_rather_than_raising_on_an_unreadable_body(self, answer: httpx.Response) -> None: + """`detect_api_version` is documented to return a result, never to raise. + + A proxy in front of a v1 panel answering its own HTML page under a 200 + is the case that matters: the body is unreadable, so nothing about it + says "v2", and the honest report is a probe that produced no answer. + Raising here would take setup down on a panel the caller was only + asking about. + """ + result = await detect_api_version(HOST, httpx_client=_client("get", answer)) + assert result.api_version == "v1" + assert result.probe_failed is True + assert result.status_info is None + @pytest.mark.asyncio async def test_every_malformed_body_stays_inside_the_error_hierarchy(self) -> None: """The point of the translation: one `except SpanPanelError` catches it.""" From 2fe2a7b0bb5b9a4592df3632b2a2a13cc8ba8f63 Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Wed, 26 Aug 2026 21:34:09 -0700 Subject: [PATCH 08/11] fix(http): warn about plaintext from the transport, once per panel The warning was raised by two call sites, which meant every new call site had to remember to raise it -- and `regenerate_passphrase` did not. That is the call a consumer reaches for when reauthenticating: it sends a bearer token up and brings the new broker password back over the same plaintext transport registration uses, and it said nothing at all. Moved into `_request`, so there is one mechanism and nothing to remember, and the changelog's "once per client" is now literally true of every call rather than of two of them. Scoped to the panel rather than the client object, which looks tighter and is worse: the CA download runs on every MQTT reconnect and builds a fresh client each time, so that key would produce a warning per reconnect -- exactly what the bridge's own once-per-bridge warning exists to avoid. --- CHANGELOG.md | 3 +- src/span_panel_api/_http.py | 62 ++++++--- src/span_panel_api/auth.py | 6 +- src/span_panel_api/factory.py | 8 -- tests/test_plaintext_warning.py | 216 ++++++++++++++++++++++++-------- 5 files changed, 209 insertions(+), 86 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d7baf7..72b7996 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,8 @@ A follow-up to 3.1.0's security work, with no API change and no adapter move req ### Added -- **A warning when the panel's bootstrap traffic is unencrypted**, logged once per client, never repeating the credential it warns about, and leaving plaintext the default it has always been. +- **A warning when a panel's bootstrap traffic is unencrypted**, raised by the transport so that every call carrying a credential is covered, logged once per panel, never repeating the credential it warns about, and leaving plaintext the default it has + always been. ## [3.1.0] diff --git a/src/span_panel_api/_http.py b/src/span_panel_api/_http.py index eefdf5a..0cffd1e 100644 --- a/src/span_panel_api/_http.py +++ b/src/span_panel_api/_http.py @@ -139,33 +139,54 @@ async def _get_client( yield client -def _warn_plaintext_transport(host: str, what: str, ssl_context: ssl.SSLContext | None) -> None: - """Say out loud that the panel's bootstrap traffic is not encrypted. +#: Panels already warned about over plaintext, so the warning is said once each. +#: Module state for the life of the process, which is the same scope the MQTT +#: bridge gives its own unpinned-CA warning. +_warned_plaintext_hosts: set[str] = set() + + +def _reset_plaintext_warnings() -> None: + """Test hook. Not public API.""" + _warned_plaintext_hosts.clear() + + +def _warn_plaintext_transport(host: str, ssl_context: ssl.SSLContext | None) -> None: + """Say out loud, once per panel, that its bootstrap traffic is not encrypted. In the same voice as the MQTT bridge's unpinned-CA warning, and for the same reason: a security property that is off by default is only a decision if the - operator can tell it is off. ``ssl_context=None`` puts every bootstrap - request on plaintext ``http://``, and registration is the one that carries - the panel passphrase up and brings the broker password back -- so anything on - the path reads both, and nothing said so. - - Warned by the two calls that *bootstrap* a client rather than from inside - ``_request``. These are made a handful of times per config entry, so one line - per client is a line somebody reads; one per request -- registration, - detection, schema, status, FQDN -- is a line somebody filters out. The two - warn on mutually exclusive paths, so a caller never hears it twice. - - The credential itself is never named here. This is a warning *about* a - secret, not a place to put one. + operator can tell it is off. ``ssl_context=None`` puts the request on + plaintext ``http://``, and two of these calls carry credentials -- + registration sends the panel passphrase and brings the broker password back, + and passphrase rotation sends a bearer token and brings the new broker + password back -- so anything on the path reads all of it. + + **Called from the transport, not from the calls that bootstrap a client.** + Warning at the call sites meant each new call site had to remember to, and + passphrase rotation did not: the one call a consumer reaches for when + reauthenticating went out in the clear and said nothing. There is one + mechanism here so there is nothing to remember. + + **Scoped to the panel, not to the request or the client object.** Per + request is a line somebody filters out. Per client object looks tighter and + is worse, because the CA download runs on every MQTT reconnect and builds a + fresh client each time -- so that key would produce a warning per reconnect, + which is precisely what the bridge's own once-per-bridge warning exists to + avoid. The panel is the thing the warning is actually about. + + The credential itself is never named. This is a warning *about* a secret, + not a place to put one. """ if ssl_context is not None: return + if host in _warned_plaintext_hosts: + return + _warned_plaintext_hosts.add(host) _LOGGER.warning( - "%s for %s is being sent over plaintext HTTP: no ssl_context was supplied, so the request " - "and its response -- including any credential either one carries -- are readable by anything " - "on the path between here and the panel. Pin the panel's CA certificate and pass it as " - "ssl_context.", - what, + "Bootstrap traffic for %s is being sent over plaintext HTTP: no ssl_context was supplied, so " + "these requests and their responses -- including any credential they carry, such as the panel " + "passphrase and the broker password -- are readable by anything on the path between here and " + "the panel. Pin the panel's CA certificate and pass it as ssl_context.", host, ) @@ -267,6 +288,7 @@ async def _request( caller that supplied it. """ url = _build_url(host, port, path, ssl_context) + _warn_plaintext_transport(host, ssl_context) try: async with _get_client(httpx_client, timeout, ssl_context) as client: match method: diff --git a/src/span_panel_api/auth.py b/src/span_panel_api/auth.py index 261adb0..1e6c0bb 100644 --- a/src/span_panel_api/auth.py +++ b/src/span_panel_api/auth.py @@ -18,7 +18,7 @@ import httpx -from ._http import V2_STATUS_PATH, _request, _warn_plaintext_transport +from ._http import V2_STATUS_PATH, _request from .exceptions import SpanPanelAPIError, SpanPanelAuthError, SpanPanelServerError from .models import HomieSchemaTypes, V2AuthResponse, V2HomieSchema, V2StatusInfo @@ -243,10 +243,6 @@ async def register_v2( SpanPanelTimeoutError: Request timed out SpanPanelAPIError: Unexpected response """ - # This is the request whose plaintext exposure is a credential exposure: the - # passphrase goes up in it and the broker password comes back in it. - _warn_plaintext_transport(host, "SPAN Panel v2 registration", ssl_context) - # The panel requires unique client names — append a random suffix. # The passphrase field must be "hopPassphrase" per the SPAN v2 API spec. suffix = uuid.uuid4().hex[:8] diff --git a/src/span_panel_api/factory.py b/src/span_panel_api/factory.py index ac72e09..5ab4464 100644 --- a/src/span_panel_api/factory.py +++ b/src/span_panel_api/factory.py @@ -11,7 +11,6 @@ import ssl from typing import TYPE_CHECKING -from ._http import _warn_plaintext_transport from .adapters import resolve_adapter from .auth import get_homie_schema, register_v2 from .detection import detect_api_version @@ -94,13 +93,6 @@ async def create_span_client( ) if serial_number is None: serial_number = auth_response.serial_number - else: - # `register_v2` warns for itself, so this branch is the only one that - # bootstraps a client without ever reaching it. No passphrase travels - # here, but detection and the schema fetch still go out in the clear and - # still hand an observer the panel's topology -- and one HTTP path left - # open is where the next one gets added. - _warn_plaintext_transport(host, "Panel bootstrap traffic", ssl_context) if serial_number is None: # Try to detect from panel status diff --git a/tests/test_plaintext_warning.py b/tests/test_plaintext_warning.py index 16ffd8e..f010b92 100644 --- a/tests/test_plaintext_warning.py +++ b/tests/test_plaintext_warning.py @@ -1,13 +1,19 @@ -"""Bootstrapping a client over plaintext HTTP says so. - -Without an `ssl_context` every bootstrap request is plain `http://`, and -registration is the one that carries the panel passphrase up and brings the -broker password back — so anything on the path between the host and the panel -reads both. That is the default, and it stays the default, because requiring a -pin would break every install on upgrade. But the MQTT bridge already warns once -when its trust anchor is unpinned, and the REST side said nothing at all: a -security property that is off by default is only a decision if the operator can -tell it is off. +"""Bootstrapping a panel over plaintext HTTP says so, once. + +Without an `ssl_context` every bootstrap request is plain `http://`, and two of +them carry credentials: registration sends the panel passphrase and brings the +broker password back, and passphrase rotation sends a bearer token and brings +the new broker password back. Anything on the path reads all of it. That stays +the default, because requiring a pin would break every install on upgrade — but +the MQTT bridge has warned about its unpinned trust anchor since 3.1.0, and the +REST side said nothing at all. + +The warning lives in the transport, so no call site can be added later that +quietly skips it, and it is scoped to the panel rather than to a request or a +client object. Those are the same two constraints the MQTT bridge already +resolved the same way: the CA is refetched on every reconnect, from a fresh +client each time, so anything narrower than "once per panel" is a warning per +reconnect — a line nobody reads by the second hour of an outage. """ from __future__ import annotations @@ -15,16 +21,16 @@ import json import logging import ssl +from collections.abc import Iterator from unittest.mock import AsyncMock, patch import httpx import pytest -from span_panel_api.auth import register_v2 +from span_panel_api._http import _reset_plaintext_warnings +from span_panel_api.auth import download_ca_cert, regenerate_passphrase, register_v2 from span_panel_api.mqtt.models import MqttClientConfig -from conftest import flat_schema - HOST = "panel.invalid" V2_AUTH_JSON = { @@ -42,16 +48,45 @@ "hopPassphrase": "hop", } +PEM = "-----BEGIN CERTIFICATE-----\nsynthetic\n-----END CERTIFICATE-----" + +#: A flat-schema fetch: no `dataModelVersion`, which is what routes the panel +#: to the schema_0 adapter the dev environment has installed. +SCHEMA_JSON = { + "firmwareVersion": "spanos2/r202603/05", + "types": {"energy.ebus.device.circuit": {"space": {"datatype": "integer", "format": "1:32:1"}}}, +} + -def _auth_client() -> AsyncMock: - response = httpx.Response( +@pytest.fixture(autouse=True) +def _forget_warned_hosts() -> Iterator[None]: + """The warning fires once per panel per process, so each test starts fresh.""" + _reset_plaintext_warnings() + yield + _reset_plaintext_warnings() + + +def _json_response(payload: object, method: str = "POST") -> httpx.Response: + return httpx.Response( 200, - content=json.dumps(V2_AUTH_JSON).encode(), + content=json.dumps(payload).encode(), headers={"content-type": "application/json"}, - request=httpx.Request("POST", f"http://{HOST}/api/v2/auth/register"), + request=httpx.Request(method, f"http://{HOST}/"), + ) + + +def _text_response(body: str) -> httpx.Response: + return httpx.Response( + 200, + content=body.encode(), + headers={"content-type": "text/plain"}, + request=httpx.Request("GET", f"http://{HOST}/"), ) + + +def _client(method: str, answer: httpx.Response) -> AsyncMock: injected = AsyncMock(spec=httpx.AsyncClient) - injected.post = AsyncMock(return_value=response) + setattr(injected, method, AsyncMock(return_value=answer)) return injected @@ -59,81 +94,158 @@ def _warnings(caplog: pytest.LogCaptureFixture) -> list[str]: return [record.getMessage() for record in caplog.records if record.levelno >= logging.WARNING] -class TestRegisterV2: +class TestEveryCredentialBearingCallWarns: + """Both of the calls that put a secret on the wire, not just registration.""" + @pytest.mark.asyncio - async def test_plaintext_registration_warns_once(self, caplog: pytest.LogCaptureFixture) -> None: + async def test_registration_warns(self, caplog: pytest.LogCaptureFixture) -> None: with caplog.at_level(logging.WARNING): - await register_v2(HOST, "home-assistant", "secret", httpx_client=_auth_client()) + await register_v2(HOST, "home-assistant", "secret", httpx_client=_client("post", _json_response(V2_AUTH_JSON))) warnings = _warnings(caplog) assert len(warnings) == 1 assert HOST in warnings[0] assert "plaintext" in warnings[0] assert "ssl_context" in warnings[0] + @pytest.mark.asyncio + async def test_passphrase_rotation_warns(self, caplog: pytest.LogCaptureFixture) -> None: + """The reauth path calls this one directly, and it was silent. + + It sends the bearer token up and brings the new broker password back + over the same plaintext transport registration uses. + """ + answer = _json_response({"ebusBrokerPassword": "new-pass"}, method="PUT") + with caplog.at_level(logging.WARNING): + await regenerate_passphrase(HOST, "jwt", httpx_client=_client("put", answer)) + warnings = _warnings(caplog) + assert len(warnings) == 1 + assert HOST in warnings[0] + @pytest.mark.asyncio async def test_the_warning_never_repeats_the_credential(self, caplog: pytest.LogCaptureFixture) -> None: """It is a warning about a credential, not a place to put one.""" with caplog.at_level(logging.WARNING): - await register_v2(HOST, "home-assistant", "correct-horse-battery-staple", httpx_client=_auth_client()) + await register_v2( + HOST, + "home-assistant", + "correct-horse-battery-staple", + httpx_client=_client("post", _json_response(V2_AUTH_JSON)), + ) assert "correct-horse-battery-staple" not in caplog.text + +class TestItIsSaidOnce: + @pytest.mark.asyncio + async def test_two_calls_on_one_client_warn_once(self, caplog: pytest.LogCaptureFixture) -> None: + answer = _json_response({"ebusBrokerPassword": "new-pass"}, method="PUT") + injected = _client("put", answer) + with caplog.at_level(logging.WARNING): + await regenerate_passphrase(HOST, "jwt", httpx_client=injected) + await regenerate_passphrase(HOST, "jwt", httpx_client=injected) + assert len(_warnings(caplog)) == 1 + + @pytest.mark.asyncio + async def test_repeated_ca_fetches_on_fresh_clients_warn_once(self, caplog: pytest.LogCaptureFixture) -> None: + """The reconnect path, and the reason this is scoped to the panel. + + `download_ca_cert` is called on every MQTT reconnect with no injected + client, so each call builds its own. Anything keyed on the client object + would warn once per reconnect — exactly the log the bridge's own + once-per-bridge warning exists to avoid. + """ + with patch("span_panel_api._http.httpx.AsyncClient") as mock_cls: + dedicated = AsyncMock() + dedicated.__aenter__ = AsyncMock(return_value=dedicated) + dedicated.__aexit__ = AsyncMock(return_value=False) + dedicated.get = AsyncMock(return_value=_text_response(PEM)) + mock_cls.return_value = dedicated + + with caplog.at_level(logging.WARNING): + for _ in range(5): + await download_ca_cert(HOST) + + assert len(_warnings(caplog)) == 1 + + @pytest.mark.asyncio + async def test_a_second_panel_is_warned_about_separately(self, caplog: pytest.LogCaptureFixture) -> None: + """Scoped to the panel, so a second unpinned panel is not swallowed.""" + answer = _json_response({"ebusBrokerPassword": "new-pass"}, method="PUT") + with caplog.at_level(logging.WARNING): + await regenerate_passphrase(HOST, "jwt", httpx_client=_client("put", answer)) + await regenerate_passphrase("other-panel.invalid", "jwt", httpx_client=_client("put", answer)) + assert len(_warnings(caplog)) == 2 + + +class TestTlsIsSilent: @pytest.mark.asyncio - async def test_a_pinned_registration_is_silent(self, caplog: pytest.LogCaptureFixture) -> None: + async def test_a_pinned_call_never_warns(self, caplog: pytest.LogCaptureFixture) -> None: context = ssl.create_default_context() with patch("span_panel_api._http.httpx.AsyncClient") as mock_cls: dedicated = AsyncMock() dedicated.__aenter__ = AsyncMock(return_value=dedicated) dedicated.__aexit__ = AsyncMock(return_value=False) - dedicated.post = AsyncMock( - return_value=httpx.Response( - 200, - content=json.dumps(V2_AUTH_JSON).encode(), - headers={"content-type": "application/json"}, - request=httpx.Request("POST", f"https://{HOST}/api/v2/auth/register"), - ) - ) + dedicated.post = AsyncMock(return_value=_json_response(V2_AUTH_JSON)) mock_cls.return_value = dedicated + with caplog.at_level(logging.WARNING): await register_v2(HOST, "home-assistant", "secret", ssl_context=context) + assert _warnings(caplog) == [] + @pytest.mark.asyncio + async def test_pinning_one_panel_does_not_silence_an_unpinned_one(self, caplog: pytest.LogCaptureFixture) -> None: + """A silent TLS call must not register the host as already warned.""" + context = ssl.create_default_context() + # Built before the patch: specced against the real class, not the mock. + plaintext = _client("post", _json_response(V2_AUTH_JSON)) + with patch("span_panel_api._http.httpx.AsyncClient") as mock_cls: + dedicated = AsyncMock() + dedicated.__aenter__ = AsyncMock(return_value=dedicated) + dedicated.__aexit__ = AsyncMock(return_value=False) + dedicated.post = AsyncMock(return_value=_json_response(V2_AUTH_JSON)) + mock_cls.return_value = dedicated + + with caplog.at_level(logging.WARNING): + await register_v2(HOST, "home-assistant", "secret", ssl_context=context) + await register_v2(HOST, "home-assistant", "secret", httpx_client=plaintext) + + assert len(_warnings(caplog)) == 1 + class TestCreateSpanClient: - """The factory's other path bootstraps without ever calling `register_v2`.""" + """Both factory paths bootstrap over the same transport. + + Neither patches the bootstrap calls out, because the warning now comes from + the request itself -- patching them would leave the test asserting nothing. + """ @pytest.mark.asyncio - async def test_a_prebuilt_config_still_warns_about_its_plaintext_bootstrap( - self, caplog: pytest.LogCaptureFixture - ) -> None: - """A caller supplying `mqtt_config` sends no passphrase, but detection - and the schema fetch are still plaintext and still expose the panel's - topology — and nothing on this path had a warning in it.""" + async def test_a_prebuilt_config_still_warns(self, caplog: pytest.LogCaptureFixture) -> None: + """This path sends no passphrase, but the schema fetch is still plaintext.""" from span_panel_api.factory import create_span_client config = MqttClientConfig(broker_host="broker.local", username="user", password="pass") - with ( - patch("span_panel_api.factory.SpanMqttClient") as mock_cls, - patch("span_panel_api.factory.get_homie_schema", return_value=flat_schema(32)), - ): + injected = _client("get", _json_response(SCHEMA_JSON, method="GET")) + with patch("span_panel_api.factory.SpanMqttClient") as mock_cls: mock_cls.return_value.connect = AsyncMock() with caplog.at_level(logging.WARNING): - await create_span_client(HOST, mqtt_config=config, serial_number="SYN-0000-0001") + await create_span_client(HOST, mqtt_config=config, serial_number="SYN-0000-0001", httpx_client=injected) - warnings = _warnings(caplog) - assert len(warnings) == 1 - assert HOST in warnings[0] + assert len(_warnings(caplog)) == 1 @pytest.mark.asyncio async def test_the_registering_path_warns_exactly_once(self, caplog: pytest.LogCaptureFixture) -> None: - """`register_v2` already warned; the factory must not say it again.""" + """Registration and the schema fetch are two requests to one panel.""" from span_panel_api.factory import create_span_client - with ( - patch("span_panel_api.factory.SpanMqttClient") as mock_cls, - patch("span_panel_api.factory.get_homie_schema", return_value=flat_schema(32)), - ): + injected = AsyncMock(spec=httpx.AsyncClient) + injected.post = AsyncMock(return_value=_json_response(V2_AUTH_JSON)) + injected.get = AsyncMock(return_value=_json_response(SCHEMA_JSON, method="GET")) + with patch("span_panel_api.factory.SpanMqttClient") as mock_cls: mock_cls.return_value.connect = AsyncMock() with caplog.at_level(logging.WARNING): - await create_span_client(HOST, passphrase="secret", httpx_client=_auth_client()) + await create_span_client(HOST, passphrase="secret", httpx_client=injected) + injected.post.assert_awaited_once() + injected.get.assert_awaited_once() assert len(_warnings(caplog)) == 1 From 694bdf8c35a7b90f303e579d2b7878165dde0a97 Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Wed, 26 Aug 2026 21:35:21 -0700 Subject: [PATCH 09/11] fix(http): return the context that was built, not the field it was stored in `_create_ssl_context` is annotated to return an `SSLContext` and its last statement returned `_ssl_cache.context`, which is typed `SSLContext | None`. The assignment on the line above does not make the read that follows safe: the field is shared mutable state, so another task may clear or replace it in between, and a strict checker is right to object. Handing back the local the executor just produced says what the function actually means and needs no narrowing to be true. --- src/span_panel_api/_http.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/span_panel_api/_http.py b/src/span_panel_api/_http.py index 0cffd1e..d67c41f 100644 --- a/src/span_panel_api/_http.py +++ b/src/span_panel_api/_http.py @@ -91,15 +91,24 @@ async def _create_ssl_context() -> ssl.SSLContext: performs blocking file I/O on the system CA bundle. The resulting context is thread-safe and reusable, so we cache it for the lifetime of the process. """ - if _ssl_cache.context is not None: - return _ssl_cache.context + cached = _ssl_cache.context + if cached is not None: + return cached async with _ssl_cache.get_lock(): # Double-check after acquiring the lock. - if _ssl_cache.context is not None: - return _ssl_cache.context + cached = _ssl_cache.context + if cached is not None: + return cached + # Read back through a local rather than returning the field again. The + # field is `SSLContext | None` and another task may clear or replace it + # between the assignment and the return, so returning it a second time + # is a read this function cannot promise is non-None -- which is what a + # strict checker objects to, correctly. The value that was just built is + # the value to hand back. loop = asyncio.get_running_loop() - _ssl_cache.context = await loop.run_in_executor(None, ssl.create_default_context) - return _ssl_cache.context + context = await loop.run_in_executor(None, ssl.create_default_context) + _ssl_cache.context = context + return context @asynccontextmanager From fb3dcc4df54c0c8962d29315bb1af1dbcff3470a Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Wed, 26 Aug 2026 21:36:24 -0700 Subject: [PATCH 10/11] docs(auth): say that the loc rule is completed at the call site Two loose ends from review. `_redact` reads as three independent rules, so the residual case -- a scalar the walk cannot place -- looks unhandled when read on its own; it is covered because the sole call site always passes the passphrase it sent, and the docstring now says so rather than leaving the reader to find the call site. And the redaction tests carried an import of `unittest.mock.patch` that nothing used, which the test tree's blanket lint exclusion had been hiding. --- src/span_panel_api/auth.py | 5 +++++ tests/test_auth_redaction.py | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/span_panel_api/auth.py b/src/span_panel_api/auth.py index 1e6c0bb..a7dfc90 100644 --- a/src/span_panel_api/auth.py +++ b/src/span_panel_api/auth.py @@ -112,6 +112,11 @@ def _redact(value: object, secrets: Collection[str] = ()) -> object: validator may fold the rejected value into its own prose, and nothing marks that. The one call that sends a credential knows exactly what it sent. + The three are meant to be read together rather than each judged alone: the + ``loc``-sibling rule covers the credential's *position* and is completed by + the value scrubbing the sole call site always supplies, so a scalar this + walk cannot place is still removed when the caller knows what it sent. + Walks dicts and lists; any other value is returned as-is once scrubbed. """ if isinstance(value, dict): diff --git a/tests/test_auth_redaction.py b/tests/test_auth_redaction.py index a06c365..dc8d878 100644 --- a/tests/test_auth_redaction.py +++ b/tests/test_auth_redaction.py @@ -11,7 +11,7 @@ import json import logging -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock import httpx import pytest From 89dc008c81eadcfe1db01592a4a1094bbe244ba7 Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Wed, 26 Aug 2026 21:40:51 -0700 Subject: [PATCH 11/11] fix(http): say the plaintext warning's scope plainly, and reset it with the other module state The comment claimed the once-per-host set had the same scope as the MQTT bridge's unpinned-CA warning. It does not: the bridge's flag is per instance and repeats on a config-entry reload; this set is process-wide. The per-host choice stands -- the CA download builds a fresh client per reconnect -- but the comment now says what the scope is rather than what it resembles. The test reset moves to conftest beside the SSL-cache reset, so any test that drives a plaintext request starts unwarned, not only the ones in the file that happens to assert on it. --- src/span_panel_api/_http.py | 6 ++++-- tests/conftest.py | 6 ++++++ tests/test_plaintext_warning.py | 10 ---------- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/span_panel_api/_http.py b/src/span_panel_api/_http.py index d67c41f..bed455e 100644 --- a/src/span_panel_api/_http.py +++ b/src/span_panel_api/_http.py @@ -149,8 +149,10 @@ async def _get_client( #: Panels already warned about over plaintext, so the warning is said once each. -#: Module state for the life of the process, which is the same scope the MQTT -#: bridge gives its own unpinned-CA warning. +#: Process-wide, once per panel host: wider than the MQTT bridge's unpinned-CA +#: warning, which is per bridge instance and so repeats when a config entry is +#: reloaded. See `_warn_plaintext_transport` for why the client object is the +#: wrong key. _warned_plaintext_hosts: set[str] = set() diff --git a/tests/conftest.py b/tests/conftest.py index afb7400..84fe134 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -56,6 +56,12 @@ def _reset_ssl_cache() -> None: _http_mod._ssl_cache.lock = None +@pytest.fixture(autouse=True) +def _reset_plaintext_warnings() -> None: + """The plaintext warning is said once per host per process; each test starts unwarned.""" + _http_mod._reset_plaintext_warnings() + + # --------------------------------------------------------------------------- # Constants shared across MQTT tests # --------------------------------------------------------------------------- diff --git a/tests/test_plaintext_warning.py b/tests/test_plaintext_warning.py index f010b92..782d371 100644 --- a/tests/test_plaintext_warning.py +++ b/tests/test_plaintext_warning.py @@ -21,13 +21,11 @@ import json import logging import ssl -from collections.abc import Iterator from unittest.mock import AsyncMock, patch import httpx import pytest -from span_panel_api._http import _reset_plaintext_warnings from span_panel_api.auth import download_ca_cert, regenerate_passphrase, register_v2 from span_panel_api.mqtt.models import MqttClientConfig @@ -58,14 +56,6 @@ } -@pytest.fixture(autouse=True) -def _forget_warned_hosts() -> Iterator[None]: - """The warning fires once per panel per process, so each test starts fresh.""" - _reset_plaintext_warnings() - yield - _reset_plaintext_warnings() - - def _json_response(payload: object, method: str = "POST") -> httpx.Response: return httpx.Response( 200,