fix(ha-diag): node_name from env + fail-fast on unknown — evt-unknown-* files poisoned observer checkpoint (see d5139c9)

An "unknown"/empty node_name silently produced evt-unknown-* event files. One
landed in events/piha/ and, being lexically greater than every evt-piha-* name,
pinned the observer's (then lexical) checkpoint and blocked PIHA for ~34 days.

Defense in depth, two layers, both refuse rather than emit poison:
- config.py: node_name field_validator rejects ""/"unknown" (case-insensitive,
  trimmed); validate_default=True so the "unknown" default itself is rejected
  when NODE_NAME never reaches the process. main() catches ValidationError and
  exits 1 with a clear FATAL message instead of a raw traceback.
- event_emitter.py: EventEmitter.__init__ guards node_name at the exact poison
  site (node_name is embedded in the filename), so no future call path can
  reintroduce evt-unknown-*.

Precedence unchanged and correct: Settings.load() uses os.environ.setdefault, so
env NODE_NAME wins over YAML; YAML only supplies node_name when env is absent.
api.py never writes event files (only /health, /trigger), so its _node_name
global cannot produce poison — verified, left as-is.

Tests (services/ha-diag-agent/tests/): new test_config.py (env load, env>YAML
precedence, fail-fast on unknown/empty/whitespace/default/YAML-unknown) and
EventEmitter guard tests. Full unit suite: 139 passed, 0 regressions.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
oskar 2026-07-15 19:23:00 +02:00
parent f57a01a20d
commit f2ba81bcc1
5 changed files with 215 additions and 5 deletions

View file

@ -4,11 +4,18 @@ import os
from pathlib import Path from pathlib import Path
import yaml import yaml
from pydantic import field_validator from pydantic import Field, field_validator
from pydantic_settings import BaseSettings from pydantic_settings import BaseSettings
_CONFIG_YAML = Path("/config/ha-diag-agent.yaml") _CONFIG_YAML = Path("/config/ha-diag-agent.yaml")
# node_name values that are unsafe to run with. A node_name of "unknown" (or
# empty) silently produces evt-unknown-* event files. Such a file once landed in
# events/piha/ and, being lexically greater than every evt-piha-* name, pinned
# the observer's (then lexical) checkpoint and blocked PIHA for ~34 days — see
# fix d5139c9. We refuse to start rather than ever emit that poison again.
_FORBIDDEN_NODE_NAMES = {"", "unknown"}
class Settings(BaseSettings): class Settings(BaseSettings):
# HA connection # HA connection
@ -16,8 +23,11 @@ class Settings(BaseSettings):
ha_token: str = "" ha_token: str = ""
ha_timeout: float = 10.0 ha_timeout: float = 10.0
# Node identity # Node identity.
node_name: str = "unknown" # validate_default=True so the "unknown" default itself is rejected: if
# NODE_NAME never reaches the process the agent must fail fast, not run as
# "unknown" and poison another node's event directory (see d5139c9).
node_name: str = Field(default="unknown", validate_default=True)
location_tag: str = "default" location_tag: str = "default"
# Intervals (seconds) # Intervals (seconds)
@ -69,6 +79,26 @@ class Settings(BaseSettings):
def strip_trailing_slash(cls, v: str) -> str: def strip_trailing_slash(cls, v: str) -> str:
return v.rstrip("/") return v.rstrip("/")
@field_validator("node_name")
@classmethod
def reject_unknown_node(cls, v: str) -> str:
"""Fail-fast: never let the agent run as "unknown"/empty.
A node_name of "unknown" makes EventEmitter write evt-unknown-* files.
Landing in another node's events dir, such a file poisoned the observer
checkpoint and blocked PIHA for ~34 days (d5139c9). Refusing to start is
strictly safer than silently emitting the poison.
"""
normalized = (v or "").strip()
if normalized.lower() in _FORBIDDEN_NODE_NAMES:
raise ValueError(
"NODE_NAME not set — refusing to start, would poison event "
"pipeline with evt-unknown-* files (see d5139c9). "
"Set NODE_NAME to the node's canonical inventory name "
"(e.g. piha, chelsty-infra)."
)
return normalized
@classmethod @classmethod
def load(cls) -> "Settings": def load(cls) -> "Settings":
"""Load settings: YAML file provides defaults; env vars override.""" """Load settings: YAML file provides defaults; env vars override."""

View file

@ -16,8 +16,19 @@ class EventEmitter:
def __init__( def __init__(
self, events_dir: Path, node_name: str, location_tag: str = "" self, events_dir: Path, node_name: str, location_tag: str = ""
) -> None: ) -> None:
# Defense in depth (config.py already fails fast on this): the event id
# embeds node_name into the filename, so "unknown"/empty here is exactly
# what produces evt-unknown-* poison files. Refuse at the emit boundary
# too, so no future call path can quietly reintroduce it (see d5139c9).
normalized = (node_name or "").strip()
if normalized.lower() in ("", "unknown"):
raise ValueError(
f"EventEmitter refuses node_name={node_name!r} — would write "
"evt-unknown-* files that poison the observer checkpoint "
"(see d5139c9). node_name must be a real node identity."
)
self._events_dir = events_dir self._events_dir = events_dir
self._node_name = node_name self._node_name = normalized
self._location_tag = location_tag self._location_tag = location_tag
self._seq = 0 self._seq = 0
events_dir.mkdir(parents=True, exist_ok=True) events_dir.mkdir(parents=True, exist_ok=True)

View file

@ -3,12 +3,14 @@ from __future__ import annotations
import asyncio import asyncio
import json import json
import logging import logging
import sys
import time import time
from datetime import datetime from datetime import datetime
import structlog import structlog
import uvicorn import uvicorn
from apscheduler.schedulers.asyncio import AsyncIOScheduler from apscheduler.schedulers.asyncio import AsyncIOScheduler
from pydantic import ValidationError
from .api import app, register_checks, register_ws_monitor from .api import app, register_checks, register_ws_monitor
from .checks.automation_failures import AutomationFailuresCheck from .checks.automation_failures import AutomationFailuresCheck
@ -196,7 +198,18 @@ async def run(settings: Settings) -> None:
def main() -> None: def main() -> None:
try:
settings = Settings.load() settings = Settings.load()
except ValidationError as exc:
# Fail fast and loud rather than run mis-identified. The most important
# case is node_name="unknown"/empty, which would emit evt-unknown-*
# poison files into another node's events dir (see d5139c9).
print(
"FATAL: refusing to start ha-diag-agent — invalid configuration:\n"
f"{exc}",
file=sys.stderr,
)
raise SystemExit(1)
asyncio.run(run(settings)) asyncio.run(run(settings))

View file

@ -0,0 +1,141 @@
"""Tests for Settings / node_name fail-fast.
Regression coverage for the evt-unknown-* poison bug: a node_name of "unknown"
(or empty) produced event files that pinned the observer checkpoint and blocked
PIHA for ~34 days (see fix d5139c9). The agent must now refuse to start rather
than ever emit such a file.
"""
from __future__ import annotations
import os
from pathlib import Path
import pytest
from pydantic import ValidationError
from ha_diag import config
from ha_diag.config import Settings
@pytest.fixture(autouse=True)
def _isolate_environ():
"""Snapshot os.environ — Settings.load() mutates it via setdefault()."""
saved = dict(os.environ)
yield
os.environ.clear()
os.environ.update(saved)
# ---------------------------------------------------------------------------
# node_name loaded from env NODE_NAME
# ---------------------------------------------------------------------------
def test_node_name_from_env(monkeypatch):
monkeypatch.setenv("NODE_NAME", "piha")
settings = Settings()
assert settings.node_name == "piha"
def test_node_name_from_env_case_insensitive_var(monkeypatch):
# BaseSettings is case-insensitive: lowercase env var maps to the field too.
monkeypatch.delenv("NODE_NAME", raising=False)
monkeypatch.setenv("node_name", "chelsty-infra")
settings = Settings()
assert settings.node_name == "chelsty-infra"
def test_node_name_whitespace_is_stripped(monkeypatch):
monkeypatch.setenv("NODE_NAME", " piha ")
settings = Settings()
assert settings.node_name == "piha"
# ---------------------------------------------------------------------------
# fail-fast: unknown / empty node_name refuses to start
# ---------------------------------------------------------------------------
def test_explicit_unknown_node_name_refuses(monkeypatch):
monkeypatch.delenv("NODE_NAME", raising=False)
with pytest.raises(ValidationError) as exc:
Settings(node_name="unknown")
assert "refusing to start" in str(exc.value)
def test_unknown_case_insensitive_refuses(monkeypatch):
monkeypatch.delenv("NODE_NAME", raising=False)
with pytest.raises(ValidationError):
Settings(node_name="UNKNOWN")
def test_empty_node_name_refuses(monkeypatch):
monkeypatch.delenv("NODE_NAME", raising=False)
with pytest.raises(ValidationError):
Settings(node_name="")
def test_whitespace_only_node_name_refuses(monkeypatch):
monkeypatch.delenv("NODE_NAME", raising=False)
with pytest.raises(ValidationError):
Settings(node_name=" ")
def test_env_unknown_refuses(monkeypatch):
monkeypatch.setenv("NODE_NAME", "unknown")
with pytest.raises(ValidationError):
Settings()
def test_default_without_env_refuses(monkeypatch):
# No NODE_NAME anywhere → the "unknown" default must be rejected, not used.
monkeypatch.delenv("NODE_NAME", raising=False)
monkeypatch.delenv("node_name", raising=False)
with pytest.raises(ValidationError):
Settings()
# ---------------------------------------------------------------------------
# precedence: env wins over YAML for node_name
# ---------------------------------------------------------------------------
def test_env_wins_over_yaml(monkeypatch, tmp_path: Path):
yaml_file = tmp_path / "ha-diag-agent.yaml"
yaml_file.write_text("node_name: solaria\n")
monkeypatch.setattr(config, "_CONFIG_YAML", yaml_file)
monkeypatch.setenv("NODE_NAME", "piha")
settings = Settings.load()
# env value must win; YAML only supplies a default when env is absent.
assert settings.node_name == "piha"
def test_yaml_supplies_node_name_when_env_absent(monkeypatch, tmp_path: Path):
yaml_file = tmp_path / "ha-diag-agent.yaml"
yaml_file.write_text("node_name: chelsty-infra\n")
monkeypatch.setattr(config, "_CONFIG_YAML", yaml_file)
monkeypatch.delenv("NODE_NAME", raising=False)
monkeypatch.delenv("node_name", raising=False)
settings = Settings.load()
assert settings.node_name == "chelsty-infra"
def test_yaml_unknown_still_refuses(monkeypatch, tmp_path: Path):
# Even if YAML carries the poison value, fail-fast must still fire.
yaml_file = tmp_path / "ha-diag-agent.yaml"
yaml_file.write_text("node_name: unknown\n")
monkeypatch.setattr(config, "_CONFIG_YAML", yaml_file)
monkeypatch.delenv("NODE_NAME", raising=False)
monkeypatch.delenv("node_name", raising=False)
with pytest.raises(ValidationError):
Settings.load()
def test_load_valid_node_name(monkeypatch, tmp_path: Path):
monkeypatch.setattr(config, "_CONFIG_YAML", tmp_path / "does-not-exist.yaml")
monkeypatch.setenv("NODE_NAME", "piha")
settings = Settings.load()
assert settings.node_name == "piha"

View file

@ -76,6 +76,21 @@ def test_location_tag_empty_not_in_payload(tmp_events_dir: Path):
assert "location_tag" not in data["payload"] assert "location_tag" not in data["payload"]
@pytest.mark.parametrize("bad_node", ["unknown", "UNKNOWN", "", " "])
def test_emitter_refuses_unknown_or_empty_node(tmp_events_dir: Path, bad_node: str):
# No emit path may ever produce evt-unknown-* poison files (see d5139c9).
with pytest.raises(ValueError):
EventEmitter(tmp_events_dir, node_name=bad_node)
def test_emitter_never_writes_unknown_node_in_filename(tmp_events_dir: Path):
emitter = EventEmitter(tmp_events_dir, node_name="piha")
event_id = emitter.emit("ha_websocket_dead", "error", "homeassistant", "msg")
assert "unknown" not in event_id
data = json.loads((tmp_events_dir / f"{event_id}.json").read_text())
assert data["node"] == "piha"
def test_location_tag_does_not_override_explicit_payload_key(tmp_events_dir: Path): def test_location_tag_does_not_override_explicit_payload_key(tmp_events_dir: Path):
emitter = EventEmitter(tmp_events_dir, node_name="piha", location_tag="ken") emitter = EventEmitter(tmp_events_dir, node_name="piha", location_tag="ken")
event_id = emitter.emit( event_id = emitter.emit(