fix(monitoring): address review — persist install_id, drop dead config, single redaction path

- Cut the leftover telemetry.* DEFAULT_CONFIG block (nothing reads it) and
  the legacy telemetry-key fallback in policy.py.
- install_id: persist the minted UUID back to config.yaml on first use so
  service.instance.id survives gateway restarts (fail-open when the write
  is not possible); regression test covers the restart path.
- Remove the no-op gateway_health_export.redaction config keys. Redaction
  is always-on by design and deliberately not configurable; status output
  now says so.
- Collapse redaction to one unconditional secrets+PII scrub: drop the
  none/pii content modes (they served the dropped trajectories plane) and
  fold gateway_health.py's duplicate bearer/token/email/phone regex layer
  into agent/monitoring/redaction.py.
This commit is contained in:
Victor Kyriazakos
2026-07-14 20:55:15 +00:00
committed by Victor Kyriazakos
parent 505d12f662
commit 87a15733c0
8 changed files with 137 additions and 163 deletions
+7 -16
View File
@@ -9,7 +9,6 @@ session history, audit records, or product analytics belong here.
from __future__ import annotations
import logging
import re
from dataclasses import dataclass
from typing import Any, Dict, List, Optional
@@ -29,12 +28,6 @@ class GatewayHealthSnapshot:
events: List[GatewayHealthEvent | GatewayDiagnosticEvent]
_EMAIL_RE = re.compile(r"[A-Za-z0-9._%+\-]+@[A-Za-z0-9.\-]+\.[A-Za-z]{2,}")
_BEARER_RE = re.compile(r"\bBearer\s+[A-Za-z0-9._~+\-/]+=*", re.IGNORECASE)
_TOKEN_RE = re.compile(r"\b(xox[baprs]-[A-Za-z0-9-]+|sk-[A-Za-z0-9_-]{8,}|gh[pousr]_[A-Za-z0-9_]{8,})\b")
_SECRET_LITERAL_RE = re.compile(r"\*{3,}")
_PHONE_RE = re.compile(r"(?<!\w)(?:\+?\d{1,3}[\s.\-]?)?(?:\(\d{2,4}\)[\s.\-]?)?\d{3}[\s.\-]?\d{3,4}(?:[\s.\-]?\d{2,4})?(?!\w)")
_RUNNING_PLATFORM_STATES = {"running", "connected", "ok", "ready"}
_FATAL_PLATFORM_STATES = {"fatal", "degraded", "error", "failed"}
@@ -44,19 +37,17 @@ def _allowed_logger(name: str) -> bool:
def redact_gateway_message(message: Any) -> str:
"""Redact gateway diagnostic free text for customer-owned export."""
text = str(message or "")
"""Redact gateway diagnostic free text for operator-owned export.
Single scrub path: everything goes through
``agent.monitoring.redaction.redact_for_export`` (unconditional
secrets + PII), then is length-bounded.
"""
try:
from agent.monitoring.redaction import redact_for_export
redacted = redact_for_export(text, content_mode="pii") or ""
redacted = redact_for_export(str(message or "")) or ""
except Exception:
redacted = "[redaction-unavailable]"
redacted = _BEARER_RE.sub("[redacted]", redacted)
redacted = _TOKEN_RE.sub("[redacted]", redacted)
redacted = _SECRET_LITERAL_RE.sub("[redacted]", redacted)
redacted = re.sub(r"\bBearer\s+\[[^\]]+\]", "[redacted]", redacted, flags=re.IGNORECASE)
redacted = _EMAIL_RE.sub("[email]", redacted)
redacted = _PHONE_RE.sub("[phone]", redacted)
return redacted[:500]
+32 -15
View File
@@ -8,31 +8,48 @@ collector. It carries no account identity and can be rotated by clearing
from __future__ import annotations
import logging
import uuid
from typing import Any, Dict
def _monitoring_cfg(config: Dict[str, Any]) -> Dict[str, Any]:
for key in ("monitoring", "telemetry"): # accept legacy telemetry.* keys
cfg = config.get(key) if isinstance(config, dict) else None
if isinstance(cfg, dict) and cfg.get("install_id"):
return cfg
cfg = config.get("monitoring") if isinstance(config, dict) else None
return cfg if isinstance(cfg, dict) else {}
logger = logging.getLogger(__name__)
def ensure_install_id(config: Dict[str, Any]) -> str:
"""Return a stable install id, minting one if the config slot is empty.
"""Return a stable install id, minting and persisting one when empty.
Does not persist — the caller writes the returned value back to
config.yaml. Clearing ``monitoring.install_id`` (e.g. with
``hermes config set monitoring.install_id ""``) mints anew on next call.
The id must survive gateway restarts (it becomes ``service.instance.id``
on exported signals), so a freshly minted UUID is written back to
config.yaml immediately. The write is fail-open: if persisting fails
(read-only home, managed scope), the ephemeral id is still returned and
a new one is minted next start.
Clearing ``monitoring.install_id`` (e.g. ``hermes config set
monitoring.install_id ""``) rotates the id on the next gateway start.
"""
cfg = _monitoring_cfg(config)
existing = cfg.get("install_id")
mon = config.get("monitoring") if isinstance(config, dict) else None
existing = (mon or {}).get("install_id") if isinstance(mon, dict) else None
if isinstance(existing, str) and existing.strip():
return existing
return str(uuid.uuid4())
minted = str(uuid.uuid4())
try:
from hermes_cli.config import load_config, save_config
fresh = load_config()
if isinstance(fresh, dict):
slot = fresh.setdefault("monitoring", {})
if isinstance(slot, dict) and not str(slot.get("install_id") or "").strip():
slot["install_id"] = minted
save_config(fresh)
except Exception:
logger.debug("install_id persist failed; using ephemeral id", exc_info=True)
# Keep the in-memory config consistent for this process either way.
if isinstance(config, dict):
config.setdefault("monitoring", {})
if isinstance(config["monitoring"], dict):
config["monitoring"]["install_id"] = minted
return minted
__all__ = [
+38 -45
View File
@@ -1,77 +1,70 @@
"""Redaction applied to monitoring data before egress.
Secrets are always redacted, on every export path; no setting disables this.
Wraps ``agent/redact.py::redact_sensitive_text(force=True)`` and fails CLOSED:
if the redactor cannot run, the raw string is never emitted.
One unconditional scrub, no modes, no knobs. Every string that leaves the
process passes through ``redact_for_export``:
``redact_for_export(text, content_mode="pii")`` additionally scrubs e-mail
addresses, phone numbers, and UUID-shaped identifiers — the gateway
diagnostics path always uses this mode, so log-derived messages leave the
process with secrets AND PII already removed.
* Secrets first — wraps ``agent/redact.py::redact_sensitive_text(force=True)``
plus bearer/token-shape patterns, and fails CLOSED: if the redactor cannot
run, the raw string is never emitted.
* PII second — e-mail addresses, phone numbers, and UUID-shaped identifiers
are rewritten to ``[email]`` / ``[phone]`` / ``[id]``.
There is deliberately no setting to weaken this. The monitoring plane is
content-free by design, so the only free text it carries (log-derived
diagnostic messages) is always fully scrubbed.
"""
from __future__ import annotations
import re
from typing import Any, Dict, List, Optional
from typing import Optional
# Content-redaction strengths for any content that IS exported.
CONTENT_NONE = "none" # drop content entirely (structural telemetry only)
CONTENT_PII = "pii" # codec-aware PII redaction on exported content
CONTENT_MODES = {CONTENT_NONE, CONTENT_PII}
# ── secret shapes (belt-and-suspenders on top of agent/redact.py) ───────────
_BEARER_RE = re.compile(r"\bBearer\s+[A-Za-z0-9._~+\-/]+=*", re.IGNORECASE)
_TOKEN_RE = re.compile(
r"\b(xox[baprs]-[A-Za-z0-9-]+|sk-[A-Za-z0-9_-]{8,}|gh[pousr]_[A-Za-z0-9_]{8,})\b"
)
_SECRET_LITERAL_RE = re.compile(r"\*{3,}")
_BEARER_RESIDUE_RE = re.compile(r"\bBearer\s+\[[^\]]+\]", re.IGNORECASE)
# ── PII patterns (applied only in CONTENT_PII mode, on content that is exported) ──
# ── PII shapes ───────────────────────────────────────────────────────────────
_EMAIL_RE = re.compile(r"[A-Za-z0-9._%+\-]+@[A-Za-z0-9.\-]+\.[A-Za-z]{2,}")
# E.164-ish and common separators; conservative to avoid nuking code/IDs.
_PHONE_RE = re.compile(
r"(?<!\w)(?:\+?\d{1,3}[\s.\-]?)?(?:\(\d{2,4}\)[\s.\-]?)?\d{3}[\s.\-]?\d{3,4}(?:[\s.\-]?\d{2,4})?(?!\w)"
)
# Long opaque hex/uuid-ish user identifiers.
_UUID_RE = re.compile(r"\b[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}\b")
_UUID_RE = re.compile(
r"\b[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}\b"
)
def _secret_redact(text: Optional[str]) -> Optional[str]:
def _secret_redact(text: str) -> str:
"""Always-on secret redaction. force=True so user config can't disable it."""
if text is None:
return None
try:
from agent.redact import redact_sensitive_text
return redact_sensitive_text(str(text), force=True)
out = redact_sensitive_text(text, force=True)
except Exception:
# Fail CLOSED: if the redactor can't run, do not emit the raw string.
return "[redaction-unavailable]"
out = _BEARER_RE.sub("[redacted]", out)
out = _TOKEN_RE.sub("[redacted]", out)
out = _SECRET_LITERAL_RE.sub("[redacted]", out)
out = _BEARER_RESIDUE_RE.sub("[redacted]", out)
return out
def _pii_redact(text: str) -> str:
text = _EMAIL_RE.sub("[email]", text)
text = _UUID_RE.sub("[id]", text)
text = _PHONE_RE.sub("[phone]", text)
return text
def redact_for_export(
text: Optional[str],
*,
content_mode: str = CONTENT_NONE,
) -> Optional[str]:
"""Redact a single content string for export.
Secrets are ALWAYS stripped. Then PII is stripped when content_mode is 'pii'.
Callers gate *whether content is exported at all* via telemetry.trajectories
(see ``content_export_enabled``); this function only scrubs content that the
caller has already decided to export.
"""
redacted = _secret_redact(text)
if redacted is None:
def redact_for_export(text: Optional[str]) -> Optional[str]:
"""Scrub a string for egress: secrets, then PII. Unconditional."""
if text is None:
return None
if content_mode == CONTENT_PII:
redacted = _pii_redact(redacted)
return redacted
out = _secret_redact(str(text))
out = _EMAIL_RE.sub("[email]", out)
out = _UUID_RE.sub("[id]", out)
out = _PHONE_RE.sub("[phone]", out)
return out
__all__ = [
"CONTENT_NONE",
"CONTENT_PII",
"CONTENT_MODES",
"redact_for_export",
]
-59
View File
@@ -2175,60 +2175,6 @@ DEFAULT_CONFIG = {
"redact_pii": False, # When True, hash user IDs and strip phone numbers from LLM context
},
# Telemetry & observability. Three settings, isolated from each other:
# local — full-fidelity local observability the user owns (real
# models, providers, tool names). Default ON.
# aggregate — opt-in metadata to Nous (no uploader ships yet). Default OFF.
# trajectories — content trajectories for training. Separate consent (not here yet).
#
# Enterprise locking: any telemetry.* key can be pinned by an administrator via
# the managed-scope layer (/etc/hermes/config.yaml), which wins over the user's
# value on a per-key basis. There is no telemetry-specific policy block — pin
# e.g. `telemetry.allow_aggregate: false` in managed scope to hard-forbid egress.
"telemetry": {
# Local telemetry: event log + SQLite index in state.db. The user's own data;
# never leaves the machine unless they export it or opt into aggregate metrics.
"local": True,
# Hard gate for aggregate metrics. When False, aggregate metrics are off
# regardless of consent_state. Intended to be pinned False by an administrator
# via managed scope on locked-down or air-gapped deployments. Default True
# (consent_state still governs the opt-in).
"allow_aggregate": True,
# Aggregate-metrics consent — the single source of truth for the opt-in:
# "unknown" (no choice made — never uploads), "local" (declined),
# "aggregate" (opted in). Set with `hermes config set telemetry.consent_state`
# or a managed-scope pin. Non-interactive installs sit at "unknown".
"consent_state": "unknown",
# Stable install identifier (aggregate metrics only). Empty string means "mint a
# fresh UUID on first use"; clear it to rotate. Never sent by local telemetry.
"install_id": "",
# Local event-log retention before rotation (days). Local telemetry only.
"retention_days": 90,
# Keep secret redaction on even at full local capture (a SIEM full of live
# credentials is a bigger attack target). Admin may override via managed scope.
"redact_secrets": True,
# Content redaction for exports / support bundles: "none" | "pii".
"content_redaction": "none",
# Trajectories: full message content / reasoning / raw tool args.
# Off by default. When enabled, full content becomes exportable to the
# configured destination — always secret-redacted, and PII-redacted per
# content_redaction. This is the consent gate for content export and is
# admin-pinnable via managed scope. It does not enable any upload.
"trajectories": {
"enabled": False,
},
# Exporters. The OTLP exporter sends to a configured Collector endpoint;
# endpoint headers reference environment variable names rather than inline
# secrets.
"export": {
"otlp": {
"enabled": False,
"endpoint": None,
"headers_env": {}, # {"Authorization": "MY_OTLP_TOKEN_ENVVAR"}
},
},
},
# Text-to-speech configuration
# Each provider supports an optional `max_text_length:` override for the
# per-request input-character cap. Omit it to use the provider's documented
@@ -3107,11 +3053,6 @@ DEFAULT_CONFIG = {
"service.name": "hermes-gateway",
"deployment.environment": "production",
},
"redaction": {
"enabled": True,
"include_stack_summary": True,
"include_raw_stack": False,
},
},
# OTLP destination. headers_env maps header names to ENVIRONMENT
# VARIABLE NAMES (never secret values); values are read from the
+2 -4
View File
@@ -14340,10 +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)")
red_raw = gh.get("redaction")
red: dict = red_raw if isinstance(red_raw, dict) else {}
print(f" Redaction: {'on' if red.get('enabled', True) else 'OFF'} "
f"(secrets/PII scrubbed in-process before egress)")
print(" Redaction: always on "
"(secrets/PII scrubbed in-process before egress; not configurable)")
endpoint = otlp.get("endpoint") or ""
if otlp.get("enabled") and endpoint:
print(f" OTLP endpoint: {endpoint}")
@@ -45,7 +45,6 @@ def main() -> None:
"service.name": "hermes-gateway-smoke",
"deployment.environment": "local-smoke",
},
"redaction": {"enabled": True, "include_raw_stack": False},
},
"export": {
"otlp": {
+34 -21
View File
@@ -1,56 +1,69 @@
"""Export redaction pipeline tests — the security-critical layer.
"""Export redaction tests — the security-critical layer.
Invariants:
* Secrets ALWAYS stripped, every export path, no flag disables it.
* One unconditional scrub: secrets AND PII, no modes, no knobs.
* Fails CLOSED: if the redactor can't run, the raw string is never emitted.
* PII (emails, phones, UUID-shaped ids) stripped in 'pii' mode — the mode
the gateway diagnostics path always uses.
* Structure (subsystem names, error codes) survives; free-text PII does not.
"""
from __future__ import annotations
from unittest import mock
import agent.monitoring.redaction as R
def test_secret_always_stripped_in_none_mode():
def test_secret_key_always_stripped():
fake_key = "sk-ant-api03-" + "A" * 24 # constructed to dodge literal-scrubbers
text = f"calling with key {fake_key} and moving on"
out = R.redact_for_export(text, content_mode=R.CONTENT_NONE)
out = R.redact_for_export(f"calling with key {fake_key} and moving on")
assert out is not None
assert fake_key not in out
def test_secret_always_stripped_in_pii_mode():
fake_token = "ghp_" + "0123456789abcdef" * 2 + "0123"
text = f"token {fake_token} leaked"
out = R.redact_for_export(text, content_mode=R.CONTENT_PII)
def test_token_shapes_stripped():
ghp = "ghp_" + "0123456789abcdef" * 2 + "0123"
slack = "xoxb-" + "123456789012-abcdefABCDEF"
out = R.redact_for_export(f"token {ghp} and {slack} leaked")
assert out is not None
assert fake_token not in out
assert ghp not in out
assert slack not in out
assert "[redacted]" in out
def test_bearer_header_stripped():
out = R.redact_for_export("Authorization: Bearer abc.def-ghi_jkl")
assert out is not None
assert "abc.def-ghi_jkl" not in out
def test_none_passthrough():
assert R.redact_for_export(None, content_mode=R.CONTENT_NONE) is None
assert R.redact_for_export(None) is None
def test_pii_mode_strips_email_phone_uuid():
def test_pii_always_stripped():
text = ("reach alice@example.com or +1 415 555 0100, "
"install 123e4567-e89b-12d3-a456-426614174000")
out = R.redact_for_export(text, content_mode=R.CONTENT_PII)
out = R.redact_for_export(text)
assert out is not None
assert "alice@example.com" not in out
assert "426614174000" not in out
assert "[email]" in out
assert "[id]" in out
assert "[phone]" in out
def test_none_mode_keeps_ordinary_words():
out = R.redact_for_export("just ordinary words", content_mode=R.CONTENT_NONE)
assert out == "just ordinary words"
def test_ordinary_words_survive():
assert R.redact_for_export("just ordinary words") == "just ordinary words"
def test_pii_mode_preserves_non_pii_structure():
text = "platform.slack entered fatal after auth_failed"
out = R.redact_for_export(text, content_mode=R.CONTENT_PII)
def test_structure_preserved():
out = R.redact_for_export("platform.slack entered fatal after auth_failed")
assert out is not None
assert "platform.slack" in out
assert "auth_failed" in out
def test_fails_closed_when_redactor_unavailable():
with mock.patch("agent.redact.redact_sensitive_text", side_effect=RuntimeError):
out = R.redact_for_export("secret sauce sk-live-key")
assert out == "[redaction-unavailable]"
+24 -2
View File
@@ -14,8 +14,8 @@ def test_default_config_keeps_gateway_health_export_disabled():
assert cfg["warning_error_events_enabled"] is True
assert cfg["export_interval_seconds"] == 60
assert cfg["logs_export_interval_seconds"] == 5
assert cfg["redaction"]["enabled"] is True
assert cfg["redaction"]["include_raw_stack"] is False
# Redaction is always-on and deliberately NOT configurable.
assert "redaction" not in cfg
def test_gateway_health_snapshot_maps_runtime_status_to_low_cardinality_metrics():
@@ -340,3 +340,25 @@ def test_gateway_diagnostic_log_handler_never_raises_on_malformed_record():
)
handler.emit(record)
def test_install_id_persists_across_calls(tmp_path, monkeypatch):
"""A minted install id must survive restarts (service.instance.id continuity)."""
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
(tmp_path / "config.yaml").write_text("{}\n")
import hermes_cli.config as cfg_mod
from agent.monitoring.policy import ensure_install_id
first = ensure_install_id(cfg_mod.load_config())
assert first and first != "unknown"
# Persisted: a fresh load (simulating a new gateway process) returns the same id.
second = ensure_install_id(cfg_mod.load_config())
assert second == first
assert first in (tmp_path / "config.yaml").read_text()
def test_install_id_existing_value_wins(monkeypatch):
from agent.monitoring.policy import ensure_install_id
assert ensure_install_id({"monitoring": {"install_id": "keep-me"}}) == "keep-me"