diff --git a/src/serving/api/main.py b/src/serving/api/main.py index 3a2e217..2325f19 100644 --- a/src/serving/api/main.py +++ b/src/serving/api/main.py @@ -740,12 +740,14 @@ def _public_health_view(payload: dict) -> dict: `/v1/health` is auth-exempt, yet each component's `message`/`metrics` carry recon: `f"Kafka unavailable: {exc}"` (internal hostnames/URLs), broker/topic - counts, cluster sizes, and the serving backend's identity. Keep the overall - status, each component's name/status/source (the documented contract), and - only the allowlisted operational gauges (pool utilization, data freshness); - drop `message` and every non-allowlisted metric. Agents still get the - per-component status they use to caveat freshness, and pool-utilization - observability (its only exposure — not on /metrics) is preserved. + counts, cluster sizes, and the serving backend's identity. Neutralise the + `message` text (it is where every `{exc}` and count lives) and keep only the + allowlisted operational gauges (pool utilization, data freshness); keep the + overall status and each component's name/status/source. The field *shape* is + unchanged — `message` stays a string and `metrics` a dict — because the SDK's + `HealthComponent` model requires them; only the sensitive *content* is + dropped. Agents still get per-component status to caveat freshness, and + pool-utilization observability (its only exposure — not on /metrics) is kept. """ components = [] for c in payload.get("components", []): @@ -754,6 +756,9 @@ def _public_health_view(payload: dict) -> dict: { "name": c.get("name"), "status": c.get("status"), + # Blanked, not dropped: the SDK HealthComponent requires a string + # `message`, but its content (error text, counts) is the leak. + "message": "", "source": c.get("source"), "metrics": metrics, } diff --git a/tests/unit/test_health_public_view.py b/tests/unit/test_health_public_view.py index 40c4612..4aa66ce 100644 --- a/tests/unit/test_health_public_view.py +++ b/tests/unit/test_health_public_view.py @@ -9,6 +9,8 @@ from __future__ import annotations +from agentflow.models import HealthStatus + from src.serving.api.main import _public_health_view _RAW_PAYLOAD = { @@ -40,10 +42,23 @@ } -def test_view_keeps_only_name_status_source_per_component() -> None: +def test_view_component_fields_match_sdk_contract() -> None: view = _public_health_view(_RAW_PAYLOAD) for component in view["components"]: - assert set(component) == {"name", "status", "source", "metrics"} + assert set(component) == {"name", "status", "message", "source", "metrics"} + + +def test_view_still_validates_against_sdk_health_model() -> None: + # The SDK's HealthComponent requires string `message`/`source` and the + # top-level HealthStatus requires `checked_at`. The push-only E2E lane + # validates the /v1/health payload through this model — dropping `message` + # broke it. Pin the contract here so it fails in unit tests, not only e2e. + HealthStatus.model_validate(_public_health_view(_RAW_PAYLOAD)) + + +def test_view_blanks_message_but_keeps_it_a_string() -> None: + for component in _public_health_view(_RAW_PAYLOAD)["components"]: + assert component["message"] == "" def test_view_drops_error_strings_counts_and_backend_identity() -> None: @@ -95,5 +110,5 @@ def test_view_tolerates_missing_optional_keys() -> None: view = _public_health_view({"status": "healthy", "components": [{"name": "x", "status": "ok"}]}) assert view == { "status": "healthy", - "components": [{"name": "x", "status": "ok", "source": None, "metrics": {}}], + "components": [{"name": "x", "status": "ok", "message": "", "source": None, "metrics": {}}], }