Auditor1 R5 re-review

Scope and revision

Base SHA: ca28be6ae629d3d67936475ea5896b6f70f6e1e4
Reviewed SHA: c5ede46bd1c31b1e23434538fdb8a1cd925751a6
Scope: re-review of prior auditor1 findings in the HAProxy no-restart reload procedure and active/source backup integrity, plus staged HTTP-01 → Caddy auth → TLS route ordering.
Resulting code SHA: no code changes by reviewer.
Files changed by this review: unique assignment and report only; existing AGENTS.md modification preserved.

Prior findings

CLOSED — MEDIUM: HAProxy no-restart reload path was unspecified

The runbook now has a concrete, gated master-worker path in docs/STAGE1_DEPLOYMENT_RUNBOOK.md:414-477. It first requires PID 1 to be haproxy, an actual -W/-Ws argument, and manual confirmation that every live -f path matches the validated config set (:416-425). The function checks PID 1 and the master-worker argument again immediately before signaling (:435-440), sends USR2 specifically to the confirmed PID 1 (:441-442), waits for a new HAProxy child worker PID (:443-451), and requires unchanged container ID, start time, restart count, and running state (:452-455). Unknown process models stop without sending a signal (:416-420). Staged activation and failure/rollback instructions are now explicit (:459-477, :518-525).

Evidence: source inspection at those lines. The coordinator reports having tested the exact function against an official HAProxy 3.0 fixture: worker handoff occurred with ID/StartedAt/RestartCount unchanged, and non-master PID 1 was rejected before signal. That fixture result was not independently rerun in this review. The live VPS process model remains UNVERIFIED, so this closes the runbook gap only; operation on the VPS remains conditional on its documented preflight.

CLOSED — LOW: directory-tree backups lacked completion/integrity checks

The directory-mount branch now checks that all four active/source backup trees are nonempty, compares each source tree to its saved copy, and compares each active tree copy against its source copy before edits (docs/STAGE1_DEPLOYMENT_RUNBOOK.md:303-326). The single-file path also compares each host-source backup to its source (:289-300). The root shell is instructed to use set -euo pipefail (:148-155), so a failed copy command aborts before the later steps; explicit mismatch checks stop with a non-content error.

Evidence: source inspection at those lines. The coordinator reports directory backups and diff checks reviewed; no VPS backup commands were executed in this review.

Remaining finding

LOW — When adding a new HTTP-01-only route, the final config never routes ordinary HTTP requests for the hostname to Caddy

The new conditional route in deploy/randomayzer.haproxy.cfg:18-27 matches both the requested host and only /.well-known/acme-challenge/. The runbook says to add this challenge-only route when the hostname is not already sent to Caddy, then to add Caddy auth and later the TLS SNI route (docs/STAGE1_DEPLOYMENT_RUNBOOK.md:375-391, :459-477). It never adds or enables a general HTTP host route after the Caddy site and TLS route are ready. In that topology, ordinary http://random.ochenstarik.ru/ traffic continues to HAProxy's previous catch-all and cannot receive Caddy's automatic HTTP-to-HTTPS redirect. This is conditional on the HTTP host not already being routed to Caddy; the runbook's local/domain route baselines should include an explicit HTTP check for Randomayzer after activation.

Evidence: the official Caddy Automatic HTTPS documentation says it creates HTTP-to-HTTPS redirects on port 80, while the HAProxy example routes only the challenge path to Caddy: Caddy Automatic HTTPS and the cited source lines. This was not tested against the live topology. A post-TLS stage can route the full host to Caddy after the authenticated site is loaded, preserving the challenge path and other hosts; if broad HTTP forwarding is not intended, document the deliberate behavior instead.

Staged ordering assessment

The runbook now orders a needed HTTP-01 challenge-only route first, Caddy's Basic Auth site second, and the new TLS SNI route third (docs/STAGE1_DEPLOYMENT_RUNBOOK.md:375-391). The HTTP-01 route is limited to the challenge prefix and preserves port 80, while the TLS SNI route remains absent until after Caddy has loaded the protected site. The ordering is sound for avoiding a new public TLS route before the auth config is active. The remaining LOW finding above concerns ordinary HTTP requests after activation, not the ordering of challenge/auth/TLS stages.

Commands and evidence

git rev-parse HEAD — c5ede46bd1c31b1e23434538fdb8a1cd925751a6.
git diff --check ca28be6ae629d3d67936475ea5896b6f70f6e1e4 c5ede46bd1c31b1e23434538fdb8a1cd925751a6 — exit 0.
Source diff inspected with git diff --unified=5 ca28be6ae629d3d67936475ea5896b6f70f6e1e4 c5ede46bd1c31b1e23434538fdb8a1cd925751a6 -- deploy/randomayzer.haproxy.cfg docs/STAGE1_DEPLOYMENT_RUNBOOK.md.
No Docker, container, VPS, application test, or full-gate commands were run. Coordinator-reported checks: official HAProxy 3.0 fixture behavior and config validation passed; 25 Bash blocks passed syntax checks. These are not claimed as independently run.

UNVERIFIED and blockers

VPS PID 1, actual HAProxy arguments and complete -f config set, mounts, routing rules, Caddy admin endpoint, and HTTP path to Caddy remain UNVERIFIED. The new reload command must not run unless the live preflight proves its preconditions.
DNS, public certificate issuance, external HTTP/HTTPS behavior, login, WireGuard peer identity, firewall isolation, and preservation of sites on the actual VPS remain UNVERIFIED.
No R5 phase closure or deployment readiness is claimed.

5.7 KiB Raw Blame History