diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c88f63..04be4a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Changelog +## 2.5.1 — the address a client can actually verify us by + +An existing install corrects itself on the next start: a stored certificate that names the wrong address is already treated as unfit and re-signed, so nothing +needs clearing by hand. + +### Fixed + +- **The certificate SAN and the mDNS advertisement now carry this host's own address rather than its upstream router's**, which under `host_network: true` left + no address a client could reach the panel at and verify the certificate against. +- **Supervisor discovery entries are now removed when the add-on stops**, where the registration's identifier was silently discarded and stale entries + accumulated across restarts. +- **The add-on image builds again**, where a dependency bump had pinned a version of the eBus SDK that the emitter's own requirements exclude, leaving the + two unsatisfiable together. + ## 2.5.0 — a fixed certificate authority, so a firmware upgrade looks like one **The certificate authority is now shipped with the package rather than generated at startup**, and is identical to the one the simulator ships. The simulator diff --git a/panelbench/Dockerfile b/panelbench/Dockerfile index 2c73390..2b0d241 100644 --- a/panelbench/Dockerfile +++ b/panelbench/Dockerfile @@ -32,7 +32,7 @@ EXPOSE 18883 8081 18080 LABEL io.hass.name="SPAN PanelBench" \ io.hass.description="Simulates a SPAN electrical panel for testing and upgrade modeling" \ io.hass.type="addon" \ - io.hass.version="2.5.0" \ + io.hass.version="2.5.1" \ io.hass.arch="aarch64|amd64" CMD ["/run.sh"] diff --git a/panelbench/config.yaml b/panelbench/config.yaml index c79160f..5cd3fb1 100644 --- a/panelbench/config.yaml +++ b/panelbench/config.yaml @@ -1,6 +1,6 @@ name: "SPAN PanelBench" description: "Simulates a SPAN electrical panel for testing and upgrade modeling" -version: "2.5.0" +version: "2.5.1" slug: "panelbench" url: "https://github.com/SpanPanel/panelbench" image: "ghcr.io/spanpanel/panelbench/{arch}" diff --git a/panelbench/run.sh b/panelbench/run.sh index 03ea652..d4ba6ff 100755 --- a/panelbench/run.sh +++ b/panelbench/run.sh @@ -19,11 +19,39 @@ LOG_LEVEL=$(jq -r '.log_level' "${OPTIONS_FILE}") DASHBOARD_ENABLED=$(jq -r '.dashboard_enabled' "${OPTIONS_FILE}") BASE_HTTP_PORT=$(jq -r '.base_http_port // 8081' "${OPTIONS_FILE}") -# Auto-detect host IP for TLS cert SAN. -# Inside a bridge-networked container the default gateway is the host. -# Strip control characters — some container ip implementations emit trailing -# non-printables that would break Python string literals or cert generation. -ADVERTISE_ADDRESS=$(ip route | awk '/default/ { print $3 }' | tr -d '[:cntrl:]' || true) +# Auto-detect the address a client on the LAN reaches this add-on at. It goes +# into the leaf certificate's SAN and into the mDNS advertisement, so getting it +# wrong leaves no address a client can verify us by. +# +# This ran `ip route | awk '/default/ { print $3 }'` and took the *gateway*. The +# reasoning held for a bridge-networked container, where the default gateway is +# the host -- but this add-on sets `host_network: true` (config.yaml), so the +# container shares the host's network namespace and reads the host's routing +# table. `$3` of `default via 192.168.65.1 dev eth0` is then the upstream +# router: a neighbouring device, named in our certificate, that is not us. +# +# `ip route get` is a routing-table lookup rather than a probe -- it sends no +# packets and needs nothing at the far address to be reachable. It answers the +# question that actually matters, which source address the kernel would put on a +# reply, and stays right on an interface holding several addresses where taking +# the first one listed would not. +# +# $ ip -4 route get 1.1.1.1 +# 1.1.1.1 via 192.168.65.1 dev eth0 src 192.168.65.19 uid 0 +# ^^^^^^^^^^^^^ what we want +# +# Control characters are stripped because some container `ip` implementations +# emit trailing non-printables, which would break cert generation downstream. +detect_advertise_address() { + ip -4 route get 1.1.1.1 2>/dev/null \ + | awk '{ for (i = 1; i < NF; i++) if ($i == "src") { print $(i + 1); exit } }' \ + | tr -d '[:cntrl:]' || true +} + +# An address supplied by the environment wins over detection: scripts/run-local.sh +# sets one, and an operator on a multi-homed host may need to pick which of its +# addresses the panel is known by. +ADVERTISE_ADDRESS="${ADVERTISE_ADDRESS:-$(detect_advertise_address)}" export ADVERTISE_ADDRESS export CERT_DIR="/data/certs" export BROKER_USERNAME="span" diff --git a/pyproject.toml b/pyproject.toml index 6c97942..2997071 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "panelbench" -version = "2.5.0" +version = "2.5.1" description = "Standalone eBus simulator for SPAN panels" requires-python = ">=3.14" dependencies = [ diff --git a/src/panelbench/__init__.py b/src/panelbench/__init__.py index f187d87..d70b9e1 100644 --- a/src/panelbench/__init__.py +++ b/src/panelbench/__init__.py @@ -1,3 +1,3 @@ """Standalone eBus simulator for SPAN panels.""" -__version__ = "2.5.0" +__version__ = "2.5.1" diff --git a/src/panelbench/supervisor_discovery.py b/src/panelbench/supervisor_discovery.py index b8cb62b..345d996 100644 --- a/src/panelbench/supervisor_discovery.py +++ b/src/panelbench/supervisor_discovery.py @@ -32,6 +32,18 @@ def _container_hostname() -> str: _SERVICE_NAME = "span_panel" +def _payload(body: object) -> dict[str, object]: + """Unwrap a Supervisor API response envelope. + + Every Supervisor endpoint answers ``{"result": ..., "data": {...}}``; + the fields we want live under ``data``, never at the top level. + """ + if not isinstance(body, dict): + return {} + data = body.get("data") + return data if isinstance(data, dict) else {} + + class SupervisorDiscovery: """Manages Supervisor Discovery entries for simulated panels.""" @@ -61,9 +73,11 @@ async def cleanup_stale(self) -> None: async with session.get(_SUPERVISOR_DISCOVERY_URL, headers=self._headers()) as resp: if resp.status != 200: return - data = await resp.json() + body = await resp.json() - entries = data.get("discovery", []) + entries = _payload(body).get("discovery", []) + if not isinstance(entries, list): + return for entry in entries: if entry.get("service") != _SERVICE_NAME: continue @@ -123,8 +137,8 @@ async def register_panel( headers=self._headers(), ) as resp: if resp.status == 200: - data = await resp.json() - uuid = data.get("uuid") + body = await resp.json() + uuid = _payload(body).get("uuid") if isinstance(uuid, str) and uuid: self._entries[serial] = uuid _LOGGER.info( @@ -136,7 +150,7 @@ async def register_panel( _LOGGER.warning( "Supervisor discovery: register %s returned invalid uuid: %s", serial, - data, + body, ) else: text = await resp.text() diff --git a/tests/test_run_sh_advertise_address.py b/tests/test_run_sh_advertise_address.py new file mode 100644 index 0000000..2f1c7e9 --- /dev/null +++ b/tests/test_run_sh_advertise_address.py @@ -0,0 +1,102 @@ +"""Tests for the host-address detection in the add-on's run.sh. + +The function under test is extracted from the shipped ``run.sh`` and executed, +rather than restated here. A copy of the pipeline would keep passing after the +original drifted, and drift is precisely the failure this guards: the previous +implementation was correct for a bridge-networked container and silently became +wrong when the add-on moved to ``host_network: true``. + +``ip`` is replaced with a stub on PATH so the sample routing output is fixed and +the test says nothing about the machine it runs on. +""" + +from __future__ import annotations + +import re +import subprocess +from pathlib import Path + +_RUN_SH = Path(__file__).parent.parent / "panelbench" / "run.sh" + +# `host_network: true`, so this is the *host's* table: .19 is this machine and +# .1 is the upstream router. Taking the gateway is the bug being guarded. +_ROUTE_GET = "1.1.1.1 via 192.168.65.1 dev eth0 src 192.168.65.19 uid 0\n" + + +def _detect( + ip_stdout: str, + tmp_path: Path, + *, + ip_exit: int = 0, + env_value: str | None = None, +) -> str: + """Run run.sh's detection with a stubbed ``ip`` and return what it printed.""" + source = _RUN_SH.read_text() + match = re.search(r"^detect_advertise_address\(\) \{.*?^\}", source, re.MULTILINE | re.DOTALL) + assert match, "run.sh no longer defines detect_advertise_address()" + + stub_dir = tmp_path / "bin" + stub_dir.mkdir() + stub = stub_dir / "ip" + stub.write_text(f"#!/usr/bin/env bash\ncat <<'OUT'\n{ip_stdout}OUT\nexit {ip_exit}\n") + stub.chmod(0o755) + + # Mirrors run.sh: the same `set` flags, the function, then the assignment it + # actually feeds -- so the env-override precedence is covered too. + script = ( + "set -euo pipefail\n" + f"{match.group(0)}\n" + 'ADVERTISE_ADDRESS="${ADVERTISE_ADDRESS:-$(detect_advertise_address)}"\n' + 'printf "%s" "$ADVERTISE_ADDRESS"\n' + ) + env = {"PATH": f"{stub_dir}:/usr/bin:/bin"} + if env_value is not None: + env["ADVERTISE_ADDRESS"] = env_value + + result = subprocess.run( + ["bash", "-c", script], capture_output=True, text=True, env=env, check=True + ) + return result.stdout + + +def test_the_hosts_own_address_is_detected(tmp_path: Path) -> None: + """The source address is taken, not the gateway it routes via.""" + assert _detect(_ROUTE_GET, tmp_path) == "192.168.65.19" + + +def test_the_gateway_is_not_mistaken_for_the_host(tmp_path: Path) -> None: + """The original defect, stated directly. + + Named separately from the test above because this is the assertion that + would have failed before the fix, and the one worth reading in a report. + """ + assert _detect(_ROUTE_GET, tmp_path) != "192.168.65.1" + + +def test_an_on_link_route_with_no_gateway_still_yields_the_source(tmp_path: Path) -> None: + """A destination on the local segment has no `via`, so `src` shifts position. + + Guards against a parser that counted fields instead of finding the keyword. + """ + on_link = "10.0.0.7 dev eth0 src 10.0.0.3 uid 0\n" + assert _detect(on_link, tmp_path) == "10.0.0.3" + + +def test_control_characters_are_stripped(tmp_path: Path) -> None: + """Some container `ip` builds emit trailing non-printables.""" + noisy = "1.1.1.1 via 192.168.65.1 dev eth0 src 192.168.65.19\r uid 0\n" + assert _detect(noisy, tmp_path) == "192.168.65.19" + + +def test_no_route_leaves_the_address_empty(tmp_path: Path) -> None: + """A failing `ip` must not abort the script under `set -euo pipefail`. + + An empty address costs the leaf its IP SAN. Failing to boot costs the panel + entirely, so the empty answer is the right one. + """ + assert _detect("", tmp_path, ip_exit=2) == "" + + +def test_an_explicit_address_is_not_overridden(tmp_path: Path) -> None: + """run-local.sh sets this, and a multi-homed host may need to choose.""" + assert _detect(_ROUTE_GET, tmp_path, env_value="10.9.9.9") == "10.9.9.9" diff --git a/tests/test_supervisor_discovery.py b/tests/test_supervisor_discovery.py index 773b4a9..acab201 100644 --- a/tests/test_supervisor_discovery.py +++ b/tests/test_supervisor_discovery.py @@ -45,7 +45,7 @@ def _mock_session(response_status: int, response_json: dict | None = None) -> Ma async def test_register_panel_posts_to_supervisor(discovery: SupervisorDiscovery): """register_panel POSTs to /discovery and tracks the UUID.""" - mock_session = _mock_session(200, {"uuid": "disc-uuid-123"}) + mock_session = _mock_session(200, {"result": "ok", "data": {"uuid": "disc-uuid-123"}}) with ( patch("aiohttp.ClientSession", return_value=mock_session), patch( @@ -66,7 +66,7 @@ async def test_register_panel_publishes_both_ports(discovery: SupervisorDiscover against the certificate served on the TLS port. Publishing both is what lets it do that without asking a human for a number only this process knows. """ - mock_session = _mock_session(200, {"uuid": "disc-uuid-123"}) + mock_session = _mock_session(200, {"result": "ok", "data": {"uuid": "disc-uuid-123"}}) with ( patch("aiohttp.ClientSession", return_value=mock_session), patch( @@ -122,7 +122,9 @@ async def test_cleanup_stale_on_startup(discovery: SupervisorDiscovery): ] get_resp = AsyncMock() get_resp.status = 200 - get_resp.json = AsyncMock(return_value={"discovery": existing_entries}) + get_resp.json = AsyncMock( + return_value={"result": "ok", "data": {"discovery": existing_entries}} + ) del_resp = AsyncMock() del_resp.status = 200 diff --git a/uv.lock b/uv.lock index b4cde61..25f581f 100644 --- a/uv.lock +++ b/uv.lock @@ -677,7 +677,7 @@ wheels = [ [[package]] name = "panelbench" -version = "2.5.0" +version = "2.5.1" source = { editable = "." } dependencies = [ { name = "aiohttp" },