Fleet hardening: security, salt, agent loop, deploy fixes + VictoriaMetrics migration - #1053
Open
hellodk wants to merge 9 commits into
Open
Fleet hardening: security, salt, agent loop, deploy fixes + VictoriaMetrics migration#1053hellodk wants to merge 9 commits into
hellodk wants to merge 9 commits into
Conversation
added 9 commits
August 23, 2026 22:03
…ne gaps WebSSH/VNC websocket endpoints authenticated but never authorized, letting a viewer open interactive shells or VNC sessions to any fleet node; they now enforce operator/admin via the same role hierarchy as the REST siblings. /playbooks/tree accepted an arbitrary absolute source_dir and walked/read it; it is now validated against the configured playbook roots. Credentials reached the wire before host-key verification (known_hosts=None, TOFU check after auth). A stored node key is now pinned via a temp known_hosts file so a mismatch fails during the handshake; the probe gains a host_key_mismatch state. Decrypted secrets mirrored to pillar files are chmod 0600 instead of umask-world-readable. salt-api TLS verification falls back to True, git clones use StrictHostKeyChecking=accept-new instead of no, and approval emails HTML-escape fleet-controlled strings. Closes #1045 Closes #1046
The heartbeat state baked the node token into a world-readable 0755 script, hardcoded group wheel and /opt/salt/bin/python3.10, and restarted via launchctl only — breaking Linux minions outright. The token now lives in a root-owned 0600 file under /etc/kri, the script is 0750, groups branch on os_family, and restarts use systemctl on Linux. process_report_schedule ran a full state.apply every 30s per minion (N x120 master-touching renders/hour); it now schedules the collector as a plain cmd.run with the install states applied once during enrollment. sbom_scan built a double-prefixed ingest URL (404 on every run), read pillar unguarded, and wrote to predictable /tmp paths. salt_master role default auto_accept flips to false: keys are accepted via the Minion Keys UI, matching deploy/salt-master.conf and the pillar-hosting threat model. The never-wired custom returner is deleted and the mobileconfig grain moved to salt/_grains where sync_grains can actually see it. kri_enroll now fails loudly per step instead of reporting green bootstraps, and installs the process-report prerequisites before enabling its schedule. Closes #1047
The planner re-sent every accumulated tool observation on each iteration, growing the prompt quadratically; with 6 iterations of 4KB observations an 8k-context local model overflows around iteration 3-4. Older observations now collapse into a bounded digest while the last two stay verbatim. An overflow used to mark the endpoint unhealthy for 60s, poisoning a healthy planner tier; only genuine LLM endpoint errors trip the cooldown now. Client disconnects were dead code: the loop never learned about them and GeneratorExit skipped session finalization, losing token/cost records. A disconnect watcher now aborts the run through should_stop and finalize still records spend exactly once. Also: non-streaming Anthropic read timeout 30s -> 180s (it was acting as a total generation ceiling), chat routes sanitize model output like the agent route already did, providers that omit usage chunks get length-estimated tokens instead of silent zeros, cost tracking uses per-provider input/output rates instead of one blended rate, MAX_TOOL_CALLS now matches the real 1-call-per-iteration bound, and the dead history plumbing is removed. Closes #1048
Every API request now binds request_id/method/path into the structlog context (echoed as X-Request-ID), so any log line can be traced to its HTTP request without grepping timestamps. Workers bind task_id/task_name per task for the same correlation on the Celery side. Log level comes from LOG_LEVEL instead of a hardcoded INFO so incident response can turn on debug verbosity without a code change. OBSERVABILITY_ARCHITECTURE.md gains a realtime troubleshooting section with worked queries. Closes #1052
task_lock released its Redis lock unconditionally after 300s regardless of the guarded task's actual duration (global 2100s, playbook 7260s), letting a second runner start while the first still held the work. Locks now carry a uuid token released only by their owner, with per-site TTLs. The drift engine scored pkgs/brew_pkgs/services grains that nothing populated - brew_pkgs was pushed once at enrollment and stale forever, so drift scores could only under-report. A scheduled collector now pulls pkg.list_pkgs and service.get_all for online nodes into the fact storage compute_drift reads, and exact-pin (==) version constraints are supported next to >=. provision_master inherits the global 35-minute kill despite living on the long-job queue; it now carries the same soft/hard limits as playbook runs. Closes #1049
…olicy fixes Compose workers ran with the 10s default stop grace against acks_late=True + reject_on_worker_lost: every redeploy SIGKILLed in-flight tasks and re-executed them against live nodes. Grace periods now match the k8s/systemd modes (3700s ansible worker, 2200s default worker and beat). The API booted uvicorn under sh -c so SIGTERM never reached it; a proper entrypoint script execs uvicorn as PID 1 after migrations. Redis gains an eviction policy (noeviction - losing broker keys to OOM must never be silent), beat runs 2 replicas behind RedBeat's lock election instead of a singleton, and its healthcheck verifies scheduler liveness in Redis rather than import success. NetworkPolicy was blocking the platform's own control paths: metrics scrapes from the monitoring namespace are now allowed in, RFC1918 egress covers SSH/salt-api to LAN fleet nodes and the native master, and OTLP can reach the collector. METRICS_TOKEN is documented and wired into the k8s deployment env; the invalid-by-construction configmap is removed. Closes #1050
Scrape examples, k8s manifests, and docs now target a VictoriaMetrics stack: single-node VM for storage, vmagent for scraping (Prometheus config format, bearer METRICS_TOKEN auth on kri /metrics), vmalert for the existing rule files - all pinned to v1.150.0. An optional compose profile (deploy/monitoring/docker-compose.monitoring.yml) brings the trio up alongside the existing Grafana; in k8s a vmagent Deployment+ConfigMap replaces the ServiceMonitor since vmoperator is not deployed. The Grafana datasource points at VM's Prometheus-compatible API. Alert rules are unchanged syntax and stay in sync between portable and k8s forms. Closes #1051
Companion to the request-context logging change: correlation-field table, Loki/jq recipes by request_id/trace_id/task_id/minion_id, Tempo link flow, and a worked incident walkthrough.
Coverage Report✅ ✅ Gate passed |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fleet hardening + VictoriaMetrics migration
Fixes the full finding set from the deep platform review (security, salt, agent loop, workers, deploy) and replaces the Prometheus-flavoured monitoring artifacts with a VictoriaMetrics stack. Eight issues, one commit each, TDD throughout.
Security (#1045, #1046)
/playbooks/treeaccepted arbitrary absolutesource_dir(traversal + arbitrary file read as viewer). Validated against configured playbook roots.host_key_mismatchstate.StrictHostKeyChecking=accept-new; approval emails HTML-escape fleet-controlled strings; agent tool-call args redacted in query logs.Salt / minion telemetry (#1047)
/etc/kri/node_token(0700/0600), os_family branches for group + restart, stablepython3path.state.applyper minion replaced with a plain scheduledcmd.run(N×120 master-touching renders/hour → ~0).salt_master_auto_acceptdefaults false — manual accept via Minion Keys UI, matching the pillar-hosting threat model.salt/_grains/; kri_enroll fails loudly instead of reporting green bootstraps.Agent loop (#1048)
should_stopwith exactly-once finalize (cost/session/query-log recorded).Workers (#1049) + Observability (#1052)
Deploy (#1050)
VictoriaMetrics (#1051)
Test evidence
pytest tests/unit/ -q)-llexit 0Known follow-ups (deliberately not in this PR)
${METRICS_TOKEN}should be smoke-tested against a live VM once deployeddeploy/k8s/README.mdstill mentions service-monitor.yamlCloses #1045, closes #1046, closes #1047, closes #1048, closes #1049, closes #1050, closes #1051, closes #1052