From cdc7ad2c79f93a25ff93528438f537ce1b5c0f50 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 25 Sep 2026 10:34:07 +0000 Subject: [PATCH] fix(daily-infra-report): resolve PVE token from env; make a dead probe non-silent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Proxmox leg of the daily digest has been reporting NOTHING while exiting 0. Root cause: the auth header was a literal placeholder string, AUTH = "Authorization: PVEAPIToken=«vault: infrastructure/production PVE_API_TOKEN»" which was sent verbatim. The API rejected it, pve_get() returned None, and the report rendered node_count=0 / nodes_online=0 with pve_probe_status='unreachable' while still exiting 0. A monitoring gap that looks like a healthy run. Also: the vault key is PVE_TOKEN, not PVE_API_TOKEN, so even reading os.environ by the old name would not have found it. Fixes: * pve_auth() resolves the token at call time from PVE_TOKEN (injected by 'infisical run --env=prod'). Nothing is hardcoded; a missing token raises. * pve_get() builds the command inside its try block, so a missing token degrades to None instead of escaping as an unhandled exception. * PROBE_FAILURES records an unreachable node/resources probe. Probe failures are deliberately separate from DEGRADED_LEGS: a missing credential stays exit 0 (existing intent), but a probe with no data now exits 1 in both the report and --json paths, so it cannot pass unnoticed. Measured effect on the live host: pve_probe_status unreachable -> ok, node_count 0 -> 5, nodes_online 0 -> 5, total_vms 0 -> 22, running_vms 0 -> 22. Tests: 4 new regression tests; all 4 fail against the pre-fix script and pass after, and the 3 pre-existing tests still pass (7/7). Not fixed here (needs the captain): the email leg fails with '534 5.7.9 Application-specific password required' - EMAIL_PASSWORD in the vault is not a valid Gmail app password for jtabiri@gmail.com. That is a credential action, not a code change. --- scripts/daily-infra-report.py | 41 ++++++++++++++++-- tests/test_daily_infra_report.py | 73 ++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 3 deletions(-) diff --git a/scripts/daily-infra-report.py b/scripts/daily-infra-report.py index 4045ea9..92b4c53 100755 --- a/scripts/daily-infra-report.py +++ b/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).""" - cmd = f'curl -sk --connect-timeout 10 "{PVE}{path}" -H "{AUTH}"' + """Fetch PVE API data. Returns list on success, None on error (to distinguish from empty list). + + 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) diff --git a/tests/test_daily_infra_report.py b/tests/test_daily_infra_report.py index 7d9e271..324d68e 100644 --- a/tests/test_daily_infra_report.py +++ b/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!") -- 2.54.0