Merge pull request 'fix(daily-infra-report): resolve PVE token from env; make a dead probe non-silent' (#133) from fix/daily-health-digest-pve-token-20260925 into master

scripts/daily-infra-report.py

@@ -16,7 +16,20 @@ from email.mime.text import MIMEText
 from email.mime.multipart import MIMEMultipart
 
 PVE = "https://192.168.68.12:8006"
-AUTH = "Authorization: PVEAPIToken=«vault: infrastructure/production PVE_API_TOKEN»"
+
+
+def pve_auth():
+    """PVE API auth header, resolved at call time from the injected environment.
+
+    The token is injected by ``infisical run --env=prod`` as ``PVE_TOKEN``
+    (format ``user@realm!tokenid=secret``).  It must never be hardcoded: a
+    placeholder literal authenticates as nobody, which is how this probe
+    reported zero nodes while still exiting 0.  Raise loudly instead.
+    """
+    token = os.environ.get("PVE_TOKEN")
+    if not token:
+        raise RuntimeError("PVE_TOKEN is not set (run under `infisical run --env=prod`)")
+    return f"Authorization: PVEAPIToken={token}"
 
 # ── Shared credentials —─
 
@@ -29,6 +42,12 @@ ZULIP_EMAIL = "abiba-bot@chat.sysloggh.net"
 ZULIP_AUTH = None
 DEGRADED_LEGS = []
 
+# Probe failures are different from degraded legs.  A missing credential is an
+# expected, survivable state (stays exit 0).  A probe that cannot reach the API
+# means the report has NO data for that section, which is a monitoring loss and
+# must exit non-zero so it cannot pass unnoticed.
+PROBE_FAILURES = []
+
 LITELLM_PUBLIC = "https://litellm.sysloggh.net"
 LITELLM_BACKEND = "192.168.68.116"
 AUTH_HOST = "192.168.68.11"
@@ -48,9 +67,13 @@ TIME_STR = NOW.strftime("%Y-%m-%d %H:%M UTC")
 # ── Helpers ──
 
 def pve_get(path):
-    """Fetch PVE API data. Returns list on success, None on error (to distinguish from empty list)."""
+    """Fetch PVE API data. Returns list on success, None on error (to distinguish from empty list).
-    cmd = f'curl -sk --connect-timeout 10 "{PVE}{path}" -H "{AUTH}"'
+
+    A missing PVE_TOKEN is caught here and reported as ``None`` so the caller
+    records a probe failure; it must not escape as an unhandled exception.
+    """
     try:
+        cmd = f'curl -sk --connect-timeout 10 "{PVE}{path}" -H "{pve_auth()}"'
         r = subprocess.run(cmd, shell=True, capture_output=True, text=True, timeout=12)
         if r.returncode != 0:
             return None
@@ -118,6 +141,7 @@ def collect():
         report["node_count"] = 0
         report["nodes_online"] = 0
         report["pve_probe_status"] = "unreachable"
+        PROBE_FAILURES.append("proxmox: node list unreachable (PVE_TOKEN missing or API down)")
     else:
         report["nodes"] = {n["node"]: {
             "cpu_pct": round(n.get('cpu',0)*100, 1),
@@ -137,6 +161,7 @@ def collect():
     if resources is None:
         vms = []
         report["resources_probe_status"] = "unreachable"
+        PROBE_FAILURES.append("proxmox: cluster resources unreachable")
     else:
         vms = [r for r in resources if r.get("type") in ("qemu","lxc")]
         report["resources_probe_status"] = "ok"
@@ -700,6 +725,10 @@ if __name__ == "__main__":
     
     if "--json" in sys.argv:
         print(json.dumps(report, indent=2, default=str))
+        if PROBE_FAILURES:
+            for leg in PROBE_FAILURES:
+                print(f"PROBE FAILURE: {leg}", file=sys.stderr)
+            sys.exit(1)
         sys.exit(0)
     
     print("   Building dashboard...")
@@ -737,3 +766,9 @@ if __name__ == "__main__":
     for k,v in report.get('agents',{}).items():
         agent_parts.append(f"{k}:{v.get('gateway_state',v.get('pm2_status','?'))}")
     print(f"   Agents: {', '.join(agent_parts)}")
+
+    if PROBE_FAILURES:
+        print(f"\n❌ Probe failures ({len(PROBE_FAILURES)}):")
+        for leg in PROBE_FAILURES:
+            print(f"   - {leg}")
+        sys.exit(1)

tests/test_daily_infra_report.py

@@ -6,6 +6,7 @@ Tests:
 (b) Asserts an unreachable pve_get renders labelled-unreachable, not "0/0"
 """
 import json
+import os
 import subprocess
 import sys
 from pathlib import Path
@@ -24,6 +25,50 @@ def load_script():
     return module
 
 
+def test_pve_token_is_read_from_the_environment():
+    """The PVE token must come from the injected environment, never a literal.
+
+    Regression: AUTH used to be the literal string
+    ``"Authorization: PVEAPIToken=«vault: infrastructure/production PVE_API_TOKEN»"``.
+    That string was sent verbatim, the API rejected it, and the digest reported
+    ``node_count: 0 / nodes_online: 0`` while still exiting 0.
+    """
+    mod = load_script()
+    assert hasattr(mod, "pve_auth"), "pve_auth() must exist to resolve the token at call time"
+    with patch.dict("os.environ", {"PVE_TOKEN": "user@pve!tokid=secretvalue"}, clear=False):
+        assert mod.pve_auth() == "Authorization: PVEAPIToken=user@pve!tokid=secretvalue"
+
+
+def test_missing_pve_token_is_degraded_not_a_placeholder():
+    """With no PVE_TOKEN, pve_get must return None (probe failure), not send a placeholder."""
+    mod = load_script()
+    env = {k: v for k, v in os.environ.items() if k != "PVE_TOKEN"}
+    with patch.dict("os.environ", env, clear=True):
+        assert mod.pve_get("/api2/json/nodes") is None, (
+            "a missing PVE_TOKEN must degrade to None so the caller records a probe failure"
+        )
+
+
+def test_unreachable_probe_is_recorded_as_a_failure():
+    """An unreachable probe must be recorded, so the run cannot pass silently."""
+    mod = load_script()
+    assert hasattr(mod, "PROBE_FAILURES"), "PROBE_FAILURES must exist"
+    mod.PROBE_FAILURES.clear()
+    with patch.object(mod, "pve_get", return_value=None):
+        report = mod.collect()
+    assert report["pve_probe_status"] == "unreachable"
+    assert any("unreachable" in f for f in mod.PROBE_FAILURES), (
+        f"unreachable probe must be recorded in PROBE_FAILURES, got {mod.PROBE_FAILURES}"
+    )
+
+
+def test_pve_token_placeholder_is_gone():
+    """The literal placeholder must no longer appear anywhere in the script."""
+    src = (Path(__file__).parent.parent / "scripts" / "daily-infra-report.py").read_text()
+    assert "«vault:" not in src, "the unresolved vault placeholder must not remain in the script"
+    assert "AUTH = \"Authorization" not in src, "the hardcoded AUTH literal must be gone"
+
+
 def test_nested_zulip_read_feeds_agent_card():
     """Test that Zulip state is read from the nested 'zulip' key and feeds agent-card fields."""
     # Mock the http_get_body response with nested structure
@@ -135,4 +180,32 @@ if __name__ == "__main__":
         print(f"✗ test_unreachable_resources_renders_labelled_unreachable failed: {e}")
         sys.exit(1)
     
+    try:
+        test_pve_token_is_read_from_the_environment()
+        print("✓ test_pve_token_is_read_from_the_environment passed")
+    except AssertionError as e:
+        print(f"✗ test_pve_token_is_read_from_the_environment failed: {e}")
+        sys.exit(1)
+
+    try:
+        test_missing_pve_token_is_degraded_not_a_placeholder()
+        print("✓ test_missing_pve_token_is_degraded_not_a_placeholder passed")
+    except AssertionError as e:
+        print(f"✗ test_missing_pve_token_is_degraded_not_a_placeholder failed: {e}")
+        sys.exit(1)
+
+    try:
+        test_unreachable_probe_is_recorded_as_a_failure()
+        print("✓ test_unreachable_probe_is_recorded_as_a_failure passed")
+    except AssertionError as e:
+        print(f"✗ test_unreachable_probe_is_recorded_as_a_failure failed: {e}")
+        sys.exit(1)
+
+    try:
+        test_pve_token_placeholder_is_gone()
+        print("✓ test_pve_token_placeholder_is_gone passed")
+    except AssertionError as e:
+        print(f"✗ test_pve_token_placeholder_is_gone failed: {e}")
+        sys.exit(1)
+
     print("All tests passed!")