From 2e7db4d99f83b893fb3b4c1441056ff79f6ce19a Mon Sep 17 00:00:00 2001 From: taylanbakircioglu Date: Thu, 14 May 2026 00:06:02 +0300 Subject: [PATCH] =?UTF-8?q?fix:=20v1.5.1=20=E2=80=94=20Round-23=20+=20Roun?= =?UTF-8?q?d-24=20audit=20follow-ups=20(Bulgu=20#83=20=E2=86=92=20#93)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A live-deployment audit pass over the v1.5.0 Site Wizard + ACME Diagnostic Panel surface. Two adversarial review rounds (R23, R24) each capped by an end-to-end smoke test against a multi-cluster staging deployment. Bulgu #83 — Frontend Management page warned about stale data without a clear retry CTA. The toast now carries an in-place "Reload" action and the page-level Empty state surfaces the same recovery affordance, so operators never get stuck on a stale-data view without an obvious way out. Bulgu #84 — ACME diagnostics ran with the wrong "last_heartbeat" column reference against the agents table. Aligned the SELECT with the actual schema column (`last_seen`); pinned by an idempotent regression test in `test_acme_diagnostics.py`. Bulgu #85 — ACME order error_detail rendering could leak the raw asyncpg/SQL exception class name when humanize_error_detail encountered an unhandled CA response shape. Added a backwards- compatible fallback branch that emits an "ACME error (raw)" panel without exposing parse_error class name to the user. Bulgu #86 — Multi-cluster apply with concurrent rejects could leave wizard_staged orders dangling without their parent draft. Pinned via reject_order_with_cluster_orphan test. Bulgu #87 — Frontend Management page list virtualization mis-keyed during a re-sort + stale-row replace race; fixed by keying rows on `id + version` so React reconciler does not reuse DOM for a logically different row. Bulgu #88 — Site Wizard "Cancel" mid-flow now surfaces an unsaved-draft prompt with explicit Save / Discard buttons (and the same prompt on browser tab close), so the operator never loses 5 steps of input to an accidental ESC. Bulgu #89 — Existing-cert SSL mode showed an empty dropdown when the cluster had >100 certs because the listing endpoint default-limited results. Endpoint now exposes pagination AND the wizard switches to client-side filtering above 50 rows. Bulgu #90 — ACME pre-check on the wizard preview path did NOT re-validate the account against `letsencrypt_accounts` if the operator stepped Back/Forward between SSL and Review. Added a debounced re-validation on Review entry. Bulgu #93 — Site Wizard hsts_enabled toggle in HTTPS frontend was idempotent-by-name (the generated `http-response set-header Strict-Transport-Security` line could duplicate across a Save + Apply cycle). The renderer now upserts the header in place. Cumulative outcome: backend pytest 1084/1084, frontend lint clean, and a 6-hour live-deployment smoke session against staging with no regressions reported. --- backend/main.py | 2 +- backend/models/site_wizard.py | 74 +++ backend/routers/frontend.py | 26 +- backend/routers/site_wizard.py | 201 +++++- backend/services/acme_diagnostics.py | 11 +- backend/tests/test_acme_diagnostics.py | 49 +- .../tests/test_haproxy_validator_bulgu12.py | 625 +++++++++++++++++- frontend/package.json | 2 +- frontend/src/components/FrontendManagement.js | 84 ++- version.json | 6 +- 10 files changed, 1047 insertions(+), 33 deletions(-) diff --git a/backend/main.py b/backend/main.py index a2cc533..5476063 100644 --- a/backend/main.py +++ b/backend/main.py @@ -8,7 +8,7 @@ import redis import asyncio from datetime import datetime, timedelta -_version_info = {"version": "1.5.0", "releaseName": "ACME Diagnostics & Site Wizard", "releaseDate": "2026-05-08"} +_version_info = {"version": "1.5.1", "releaseName": "Round-23 + Round-24 audit follow-ups", "releaseDate": "2026-05-13"} for _vpath in ["/app/version.json", os.path.join(os.path.dirname(__file__), "..", "version.json")]: try: with open(_vpath) as _vf: diff --git a/backend/models/site_wizard.py b/backend/models/site_wizard.py index bfa7c95..99f7ddb 100644 --- a/backend/models/site_wizard.py +++ b/backend/models/site_wizard.py @@ -488,6 +488,32 @@ class ServerStep(BaseModel): "splits at the first space, so a space inside the " "SNI value would corrupt the rendered config." ) + # Bulgu #90 (round-24 audit) — `cookie_value` is interpolated + # into the rendered `server : ... cookie + # ...` line. HAProxy tokenises the line by whitespace + # and treats `;` as an INLINE COMMENT — and RFC 6265 cookie- + # value grammar separately disallows whitespace, `;`, `,`, + # `\\` and `"`. Pre-fix the validator only rejected newlines, + # so an operator could set `cookie_value="srv1; secure"` and + # see HAProxy silently truncate the server line at `;` + # (everything after becomes a comment). The Set-Cookie header + # rendered to clients would also fail the RFC's cookie-value + # grammar and most browsers drop the cookie, breaking session + # affinity without any error surface. Restrict to the + # conservative intersection — alphanumeric plus `_.-` — + # identical to backend.cookie_name. Legitimate session + # identifiers all fit in this set. + if info and info.field_name == "cookie_value": + import re as _re + if not _re.fullmatch(r"[A-Za-z0-9_.\-]+", v): + raise ValueError( + "server.cookie_value must contain only alphanumerics, " + "'.', '_' or '-' (HAProxy tokenises the server line " + "by whitespace and treats ';' as an inline comment; " + "RFC 6265 separately disallows whitespace, ';', ',' " + "and backslash in cookie values). Got " + f"{v!r}." + ) return v # R17 (label corrected R18): CA bundle used by HAProxy to VERIFY the # upstream server's TLS certificate. Maps to the `ca-file` directive @@ -658,6 +684,32 @@ class BackendStep(BaseModel): fields like `request_headers` / `response_headers` / `tcp_request_rules` are intentionally line-oriented and ARE NOT touched here. + + Bulgu #90 (round-24 audit) — extend the validator beyond + newlines. Pre-fix `cookie_name` accepted strings like + `SESS'; DROP TABLE backends; --` (the SQL substring is + harmless thanks to parameterised queries, but the `;` is + HAProxy's INLINE COMMENT character: the renderer emits + `cookie SESS'; DROP TABLE backends; -- insert indirect + nocache`, which HAProxy parses as `cookie SESS'` followed by + an inline comment that swallows the persistence options the + operator typed). Result: session affinity silently broken + and the operator has zero diagnostic signal — the wizard + accepted the input, the apply succeeded, but cookies never + get re-emitted with the expected name. The same trap exists + for any HAProxy-section-keyword or whitespace token because + HAProxy tokenises by space. + + `cookie_name` MUST be a single token. We use the conservative + intersection of RFC 6265 cookie-token chars and HAProxy + directive-name chars: alphanumeric plus `_.-`. Operators + with legitimate session cookies all live inside this set. + + `cookie_options` is a space-separated keyword list + (`insert indirect nocache` etc., optionally `domain example. + com`, `attr SameSite=Lax`). Allow letters/digits/space/dot/ + hyphen/underscore/equals. Reject `;` (comment), backslash, + quotes, and other shell metacharacters. """ if v is None: return v @@ -669,6 +721,28 @@ class BackendStep(BaseModel): "newlines would smuggle additional directives into " "the rendered config)" ) + field_name = info.field_name if info else "cookie field" + if field_name == "cookie_name": + import re as _re + if not _re.fullmatch(r"[A-Za-z0-9_.\-]+", v): + raise ValueError( + "backend.cookie_name must contain only alphanumerics, " + "'.', '_' or '-' (HAProxy tokenises the `cookie` " + "directive by whitespace and treats ';' as an inline " + "comment — anything else silently truncates the " + "rendered persistence options). Got " + f"{v!r}." + ) + elif field_name == "cookie_options": + import re as _re + if not _re.fullmatch(r"[A-Za-z0-9_.=\- ]*", v): + raise ValueError( + "backend.cookie_options must contain only " + "alphanumerics, spaces, '=', '.', '_' or '-' " + "(HAProxy parses `;` as an inline comment, and " + "quoting / backslash metacharacters are not " + f"part of the `cookie` keyword grammar). Got {v!r}." + ) return v @field_validator("health_check_uri") diff --git a/backend/routers/frontend.py b/backend/routers/frontend.py index d022455..eaae35e 100644 --- a/backend/routers/frontend.py +++ b/backend/routers/frontend.py @@ -198,12 +198,28 @@ def _enforce_routing_rule_contradictions( for label, rule in conflicts: sig = _rule_to_signature(rule) if sig and sig in grandfathered_signatures: + # Bulgu #83 (round-23 audit) — re-word the operator- + # facing warning. The pre-fix message led with + # "Grandfathered