Skip to content

Fleet hardening: security, salt, agent loop, deploy fixes + VictoriaMetrics migration - #1053

Open
hellodk wants to merge 9 commits into
masterfrom
fix/fleet-hardening-vm-migration
Open

Fleet hardening: security, salt, agent loop, deploy fixes + VictoriaMetrics migration#1053
hellodk wants to merge 9 commits into
masterfrom
fix/fleet-hardening-vm-migration

Conversation

@hellodk

@hellodk hellodk commented Aug 23, 2026

Copy link
Copy Markdown
Owner

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)

  • WebSSH/VNC websockets required only authentication — any viewer could open interactive shells/VNC to any node. Now operator/admin via the same role hierarchy as the REST siblings.
  • /playbooks/tree accepted arbitrary absolute source_dir (traversal + arbitrary file read as viewer). Validated against configured playbook roots.
  • SSH host keys are now pinned pre-auth: stored node keys go into a temp known_hosts so a mismatch fails during the handshake, before credentials are transmitted. Probe gains host_key_mismatch state.
  • Pillar secret files written 0600 (were umask-world-readable on the master); salt-api TLS verify defaults true; git clones use StrictHostKeyChecking=accept-new; approval emails HTML-escape fleet-controlled strings; agent tool-call args redacted in query logs.

Salt / minion telemetry (#1047)

  • Heartbeat: node token out of the world-readable script into /etc/kri/node_token (0700/0600), os_family branches for group + restart, stable python3 path.
  • Process report: 30s full state.apply per minion replaced with a plain scheduled cmd.run (N×120 master-touching renders/hour → ~0).
  • sbom_scan double-prefix URL (404 every run) fixed + pillar guard.
  • salt_master_auto_accept defaults false — manual accept via Minion Keys UI, matching the pillar-hosting threat model.
  • Dead returner deleted; mobileconfig grain moved to salt/_grains/; kri_enroll fails loudly instead of reporting green bootstraps.

Agent loop (#1048)

  • Quadratic prompt growth bounded (rolling verbatim window + digest); overflow no longer poisons healthy endpoint health.
  • Client disconnects wired through should_stop with exactly-once finalize (cost/session/query-log recorded).
  • Anthropic non-streaming timeout 30s→180s; chat routes sanitize model output; usage estimated when providers omit it; per-provider cost rates; call bound matches reality.

Workers (#1049) + Observability (#1052)

  • Token-owned task locks (no cross-owner deletes) with per-site TTLs; drift engine now fed real package/service facts by a scheduled collector (+ exact-pin constraints); provision_master gets long-job time limits.
  • Request-context middleware (X-Request-ID + structlog bindings), celery task_id correlation, LOG_LEVEL env, realtime troubleshooting runbook in OBSERVABILITY_ARCHITECTURE.md.

Deploy (#1050)

  • Compose grace periods matched to acks_late (3700s/2200s); entrypoint execs uvicorn (SIGTERM finally reaches it); Redis noeviction; beat 2 replicas behind RedBeat election + real liveness probe; NetworkPolicy un-blocked (metrics scrape in, RFC1918 SSH/salt-api egress, OTLP out); METRICS_TOKEN wired; invalid configmap removed.

VictoriaMetrics (#1051)

  • vmagent scrape config w/ METRICS_TOKEN auth, optional compose profile (victoria-metrics/vmagent/vmalert pinned v1.150.0), k8s vmagent Deployment+ConfigMap replacing ServiceMonitor (no vmoperator deployed), Grafana datasource → VM Prometheus-compatible API. Alert rules unchanged and still in sync across portable/k8s forms.

Test evidence

  • Full unit suite: 4251 passed, 0 failed (pytest tests/unit/ -q)
  • ruff check/format clean; mypy clean under CI flags; bandit -ll exit 0
  • 9 new test files (~120 tests) covering every fix; 9 existing contract tests updated to the new behaviour they pin

Known follow-ups (deliberately not in this PR)

  • vmagent env-expansion of ${METRICS_TOKEN} should be smoke-tested against a live VM once deployed
  • deploy/k8s/README.md still mentions service-monitor.yaml
  • Cost-rate table prices should be revisited at next provider price change

Closes #1045, closes #1046, closes #1047, closes #1048, closes #1049, closes #1050, closes #1051, closes #1052

kri-agent 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.
@github-actions github-actions Bot added test Test coverage improvement docs Documentation infra CI/CD, Docker, deployment frontend Frontend / React changes labels Aug 23, 2026
@github-actions

Copy link
Copy Markdown

Coverage Report

fleet_platform/services/ 85.5% (gate: 80%)

fleet_platform/agent/ 95.0% (gate: 80%)

Gate passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment