From 42bc6c862642b68afd26518bbe388e8da1248a24 Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Wed, 22 Jul 2026 23:34:21 +0000 Subject: [PATCH] fix(monitoring): keep diagnostics content-free --- agent/monitoring/events.py | 1 - agent/monitoring/gateway_health.py | 5 ----- agent/monitoring/gateway_health_export.py | 5 ++++- agent/monitoring/otlp_exporter.py | 2 +- docs/observability/monitoring.md | 8 ++++---- hermes_cli/main.py | 4 ++-- .../monitoring/test_gateway_health_export.py | 20 +++++++++++-------- tests/monitoring/test_otlp_exporter.py | 9 ++++++--- 8 files changed, 29 insertions(+), 25 deletions(-) diff --git a/agent/monitoring/events.py b/agent/monitoring/events.py index 2151932b730..74b0aea2a7c 100644 --- a/agent/monitoring/events.py +++ b/agent/monitoring/events.py @@ -50,7 +50,6 @@ class GatewayDiagnosticEvent: subsystem: str error_class: str = "unknown" error_code: Optional[str] = None - redacted_message: Optional[str] = None platform: Optional[str] = None old_state: Optional[str] = None new_state: Optional[str] = None diff --git a/agent/monitoring/gateway_health.py b/agent/monitoring/gateway_health.py index ece881bca4e..cb02aac95c1 100644 --- a/agent/monitoring/gateway_health.py +++ b/agent/monitoring/gateway_health.py @@ -246,7 +246,6 @@ def build_gateway_health_snapshot( platform=str(platform), error_code=error_code, error_class=classify_gateway_error(error_code or pdata.get("error_message")), - redacted_message=redact_gateway_message(pdata.get("error_message")), profile=profile, version=version, severity="error" if state == "fatal" else "warning", @@ -325,7 +324,6 @@ def emit_runtime_status_transition(previous: Optional[dict[str, Any]], current: subsystem="gateway", error_class=classify_gateway_error(current.get("exit_reason") or "startup_failed"), error_code=classify_gateway_error(current.get("exit_reason") or "startup_failed"), - redacted_message=redact_gateway_message(current.get("exit_reason") or "startup failed"), profile=profile, version=version, severity="error", @@ -374,7 +372,6 @@ def emit_runtime_status_transition(previous: Optional[dict[str, Any]], current: new_state=new_state, error_code=error_code, error_class=error_code, - redacted_message=redact_gateway_message(pdata.get("error_message")), profile=profile, version=version, severity=severity, @@ -386,7 +383,6 @@ def emit_runtime_status_transition(previous: Optional[dict[str, Any]], current: platform=str(platform), error_code=error_code, error_class=error_code, - redacted_message=redact_gateway_message(pdata.get("error_message")), profile=profile, version=version, severity=severity, @@ -426,7 +422,6 @@ class GatewayDiagnosticLogHandler(logging.Handler): subsystem=subsystem, platform=platform_for_subsystem(subsystem), error_class=classify_gateway_error(message), - redacted_message=redact_gateway_message(message), profile=self.profile, version=self.version, severity=record.levelname.lower(), diff --git a/agent/monitoring/gateway_health_export.py b/agent/monitoring/gateway_health_export.py index efb7575c692..f8fefd24e2b 100644 --- a/agent/monitoring/gateway_health_export.py +++ b/agent/monitoring/gateway_health_export.py @@ -391,7 +391,10 @@ class GatewayDiagnosticLogStreamer: if ev.get("event") != "gateway_diagnostic": continue attrs = _diagnostic_log_attributes(ev) - body = ev.get("redacted_message") or ev.get("name") or "gateway diagnostic" + # Rendered Python log messages may contain arbitrary user IDs, names, + # paths, or configured strings. Keep the OTLP body content-free and + # carry only the structured, allowlisted attributes above. + body = "gateway diagnostic" record = self._LogRecord( timestamp=ev.get("ts_ns"), trace_id=self._sdk["INVALID_TRACE_ID"], diff --git a/agent/monitoring/otlp_exporter.py b/agent/monitoring/otlp_exporter.py index c9268b4ee5a..db584a3b098 100644 --- a/agent/monitoring/otlp_exporter.py +++ b/agent/monitoring/otlp_exporter.py @@ -147,7 +147,7 @@ def _span_attrs(ev: Dict[str, Any]) -> Dict[str, Any]: "fatal_platform_count", "version", "supervision_mode", "pid"), "gateway_diagnostic": ("name", "subsystem", "error_class", "error_code", - "redacted_message", "platform", "old_state", "new_state", + "platform", "old_state", "new_state", "version", "severity"), } for col in keep_by_kind.get(kind, ()): # type: ignore[arg-type] diff --git a/docs/observability/monitoring.md b/docs/observability/monitoring.md index 9ca8f2ce12a..bb5088929d4 100644 --- a/docs/observability/monitoring.md +++ b/docs/observability/monitoring.md @@ -1,11 +1,11 @@ # Gateway Monitoring -Service health monitoring plus redacted operational diagnostics for the +Service health monitoring plus structured operational diagnostics for the Hermes gateway daemon, exported over OTLP/HTTP to an operator-configured endpoint (OpenTelemetry Collector, DataDog, or any OTLP receiver). This plane is content-free by construction. It exports gateway lifecycle -state, platform connector health, and redacted warning/error diagnostics. +state, platform connector health, and content-free warning/error diagnostics. It never exports prompts, messages, tool arguments or results, session history, usage analytics, audit logs, or execution traces. Run/model/tool trajectory capture is a separate plane served by the NeMo Relay integration @@ -17,7 +17,7 @@ trajectory capture is a separate plane served by the NeMo Relay integration | --- | --- | --- | | Gateway gauges | `/v1/metrics` | `hermes.gateway.up/state/busy/drainable/active_agents/restart_requested`, `hermes.platform.up/degraded` with bounded `error_code` attributes | | Health/lifecycle events | `/v1/traces` | `gateway.lifecycle` state transitions (`starting -> running -> draining -> stopped`, `startup_failed`, exit), `gateway.health_snapshot`, platform state changes | -| Diagnostics | `/v1/logs` | Warning/error gateway log events with secrets AND PII scrubbed in-process before egress (`[redacted]` / `[email]`), bounded error classes | +| Diagnostics | `/v1/logs` | Warning/error gateway events with a constant body and bounded subsystem, severity, error class, and error code attributes; rendered log messages are never exported | Signals carry `service.name`, version, supervision mode, and a stable one-way hash of the install id so an operator can distinguish instances without @@ -85,7 +85,7 @@ python scripts/observability/otel_capture_collector.py \ --host 127.0.0.1 --port 4318 --log /tmp/hermes_otel_capture.jsonl # terminal 2: drive the real exporter through lifecycle transitions, -# a fatal platform, and a redacted warning log, then flush +# a fatal platform, and a structured warning event, then flush python scripts/observability/gateway_health_export_probe.py \ --endpoint http://127.0.0.1:4318/v1/traces \ --log /tmp/hermes_otel_capture.jsonl --wait 8 diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 3157d78870a..f3fb0fe581a 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -14340,8 +14340,8 @@ def cmd_monitoring(args): print(f" Diagnostic events: {'on' if gh.get('diagnostic_events_enabled', True) else 'off'}") print(f" Warning/error logs: {'on' if gh.get('warning_error_events_enabled', True) else 'off'} " f"(interval {gh.get('logs_export_interval_seconds', 5)}s)") - print(" Redaction: always on " - "(secrets/PII scrubbed in-process before egress; not configurable)") + print(" Content safety: always on " + "(rendered messages are never exported; not configurable)") endpoint = otlp.get("endpoint") or "" if otlp.get("enabled") and endpoint: print(f" OTLP endpoint: {endpoint}") diff --git a/tests/monitoring/test_gateway_health_export.py b/tests/monitoring/test_gateway_health_export.py index 8e144c9abf9..e926c5ba2b7 100644 --- a/tests/monitoring/test_gateway_health_export.py +++ b/tests/monitoring/test_gateway_health_export.py @@ -111,8 +111,8 @@ def test_gateway_health_snapshot_emits_content_free_diagnostic_event(): assert health["fatal_platform_count"] == 1 assert platform["platform"] == "slack" assert platform["error_code"] == "auth_failed" - assert "secret" not in platform["redacted_message"].lower() - assert "Bearer" not in platform["redacted_message"] + assert "redacted_message" not in platform + assert "Bearer" not in str(platform) def test_gateway_health_snapshot_preserves_real_bounded_platform_states(): @@ -147,7 +147,7 @@ def test_gateway_health_snapshot_preserves_real_bounded_platform_states(): assert observed == expected -def test_gateway_diagnostic_log_handler_redacts_and_filters(caplog): +def test_gateway_diagnostic_log_handler_never_carries_rendered_message(caplog): from agent.monitoring import emitter from agent.monitoring.gateway_health import GatewayDiagnosticLogHandler @@ -166,7 +166,10 @@ def test_gateway_diagnostic_log_handler_redacts_and_filters(caplog): logger.addHandler(handler) try: logger.info("ignore info token sk-live-secret") - logger.warning("Slack token sk-live-secret failed for user@example.com") + logger.warning( + "Unauthorized user: acct_7f3a (Alice Smith) on slack; " + "token «redacted:sk-…»" + ) finally: logger.removeHandler(handler) finally: @@ -178,8 +181,9 @@ def test_gateway_diagnostic_log_handler_redacts_and_filters(caplog): assert event["name"] == "gateway.log.warning" assert event["subsystem"] == "platform.slack" assert event["error_class"] == "auth_failed" - assert "***" not in event["redacted_message"] - assert "user@example.com" not in event["redacted_message"] + assert "redacted_message" not in event + assert "acct_7f3a" not in str(event) + assert "Alice Smith" not in str(event) def test_runtime_status_transition_emits_lifecycle_and_platform_events(monkeypatch): @@ -223,7 +227,7 @@ def test_runtime_status_transition_emits_lifecycle_and_platform_events(monkeypat assert platform["old_state"] == "running" assert platform["new_state"] == "fatal" assert platform["error_code"] == "auth_failed" - assert "Bearer" not in platform["redacted_message"] + assert "redacted_message" not in platform def test_runtime_status_transition_emits_startup_failed_and_exit(): @@ -256,7 +260,7 @@ def test_runtime_status_transition_emits_startup_failed_and_exit(): assert "gateway.startup_failed" in names assert "gateway.exit" in names failed = next(e for e in captured if e["name"] == "gateway.startup_failed") - assert "***" not in failed["redacted_message"] + assert "redacted_message" not in failed lifecycle = next(e for e in captured if e["name"] == "gateway.lifecycle") assert lifecycle["exit_reason"] == "auth_failed" exit_event = next(e for e in captured if e["name"] == "gateway.exit") diff --git a/tests/monitoring/test_otlp_exporter.py b/tests/monitoring/test_otlp_exporter.py index 91ba7ede553..93749fcbfbb 100644 --- a/tests/monitoring/test_otlp_exporter.py +++ b/tests/monitoring/test_otlp_exporter.py @@ -42,16 +42,19 @@ def test_gateway_health_event_maps_to_span_with_attrs(): assert attrs["hermes.active_agents"] == 2 -def test_gateway_diagnostic_event_maps_redacted_message(): +def test_gateway_diagnostic_event_drops_arbitrary_message_content(): provider, mem = _mem_provider() OE.export_batch(provider, [{ "event": "gateway_diagnostic", "name": "platform.fatal", "subsystem": "platform.slack", "error_class": "auth_failed", - "redacted_message": "token [redacted] rejected", "severity": "error", + "redacted_message": "Unauthorized user: acct_7f3a (Alice Smith)", + "severity": "error", }]) attrs = dict(mem.get_finished_spans()[0].attributes or {}) assert attrs["hermes.error_class"] == "auth_failed" - assert attrs["hermes.redacted_message"] == "token [redacted] rejected" + assert "hermes.redacted_message" not in attrs + assert "acct_7f3a" not in str(attrs) + assert "Alice Smith" not in str(attrs) def test_unknown_event_kind_exports_no_attrs_beyond_kind():