docs: add June 2026 security review - #43
Open
Mapika wants to merge 2 commits into
Open
Conversation
Full-repo security review covering auth & secrets, the HTTP API surface, the container/exec layer, persistence/CLI/packaging/CI, and the web UI. One High finding (admin API key leaks into a world-readable install log in scripts/setup-leader-vps.sh), a cluster of Medium hardening items, and Low/Info defense-in-depth notes. No Critical issues. Documents verified-sound practices and a recommended remediation order.
Addresses the findings documented in docs/security-review-2026-06.md. High: - setup-leader-vps.sh: create install log 0600 and redact sk-... secrets so the bootstrap admin key never lands in a world-readable file. Medium: - /metrics now requires admin tier (was any valid key), preventing low-tier tenants from enumerating engine addresses / cluster topology. - Admin auth bypass now keys solely on the explicit local_control_surface flag; dropped the fragile 'scope[client] is None' heuristic. - Engine image digest pinning: optional pinned_digest in backends.yaml, verify_image_digest in DockerClient, enforced at launch in the manager. - Drop default ipc_mode=host for engine containers (rely on private shm_size). - net_guard blocks adopted deployments from dialing link-local/metadata/ multicast addresses (SSRF), applied in dispatch and adapter load/unload. Low/Info: - UI admin token moved from localStorage to sessionStorage. - /deployments/current/logs streams async with client-disconnect handling. - Stop echoing raw engine response bodies to clients (log server-side). - wipe --home refuses dirs without a berth marker; backup snapshot 0600; enrollment URI off argv (hidden prompt / env); docs use curl --cacert; hardened sample systemd unit; pinned uv installer version. Cluster CERT_OPTIONAL is intentionally retained (the listener also serves the certless enrollment/CA endpoints; certless WS is rejected at the app layer). Tests: full suite green except 5 pre-existing root-in-sandbox environment failures (TLS listener bind, owner-only perms as root, doctor writable-as-root) present before these changes. New tests cover net_guard, digest pinning, the metrics tier gate, wipe marker, backup perms, and enrollment input.
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
2 participants
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.
Full-repo security review covering auth & secrets, the HTTP API surface,
the container/exec layer, persistence/CLI/packaging/CI, and the web UI.
One High finding (admin API key leaks into a world-readable install log in
scripts/setup-leader-vps.sh), a cluster of Medium hardening items, and
Low/Info defense-in-depth notes. No Critical issues. Documents verified-sound
practices and a recommended remediation order.