test(haproxy): harden wp-admin gate suite against comment-collision, fix two false doc claims

Adversarial mutation audit found the wp-admin gate test suite (26 tests, all
green) did not actually test the feature: 14 of 26 assertions ran bare
str.index/assertIn/re.search over the full rendered config, so they matched
this file's own explanatory comment blocks (which quote ACL names and whole
rules) just as happily as the real rule. Deleting the entire redirect rule,
or `acl wp_admin_allowed`, or all five normalizers, left the old suite at
26/26 PASS. rule_lines() also only stripped whole-comment lines, so a
trailing " # decoy" comment on a surviving line could impersonate a deleted
one, and one ordering test used bare cfg.index() which still "finds" a
normalize-uri directive that has been fully commented out (the substring
survives after the '#').

Rewrites every rule-presence/content/ordering assertion to go through
rule_lines()/rule_positions(), now truncating each line at the first ' #'
before matching, and adds require_rule()/require_position() guards so a
missing rule raises a named AssertionError instead of IndexError or
"substring not found". Adds dedicated declared-ACL tests for wp_admin_path,
wp_admin_asset, wp_admin_allowed and wp_gate_exempt so each has its own
direct, comment-safe check. 29 tests now (was 26).

Proved via a mutation harness (copy templates to a scratch dir, mutate the
copy, run the suite via HAPROXY_MANAGER_DIR, restore): commenting out the
redirect rule, either deny rule, any of the four wp_admin_* ACLs, any one of
the five normalize-uri lines, or expose-experimental-directives now reddens
the suite -- 13/13 required mutations caught, plus the exact trailing-comment
decoy and "all five normalizers commented at once" cases from the audit.

Also corrects two doc claims the audit found factually wrong:

- hap_listener.tpl: normalize-uri's percent-to-uppercase and
  percent-decode-unreserved rewrite the WHOLE request-target, not just the
  path -- measured examples included, and the query-sort-by-name rejection
  reasoning ("every rule matches path") was a non-sequitur given that. Real
  reason to leave it off: reordering would break signed/cached URLs. Fleet
  checked: no .NET backends, no URL-in-path proxies, no known victim today.

- hap_header.tpl: dropping expose-experimental-directives does not
  crash-loop the container. do_initial_setup() swallows the `haproxy -c`
  failure and start_haproxy() returns without raising, so start-up.sh execs
  gunicorn as PID 1 anyway -- a silent total outage (ports 80/443 unbound,
  every site down) that ensure_haproxy.py retries forever without
  escalating, while GET /health keeps answering 200.

No HAProxy rule, ACL, or normalizer changed -- comments and tests only.
Verified: all 5 required suites green, and `haproxy -c` against the real
haproxy 3.0.11 (Debian package) still exits 0 with only the same pre-existing
warnings as before (wp_admin_asset path_reg advisory, stats file).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-14 09:38:49 -07:00
co-authored by Claude Opus 5
parent 8a8d9c5fe3
commit e545f3b6e0
3 changed files with 273 additions and 85 deletions
+17 -2
View File
@@ -35,8 +35,23 @@ global
# experimental, must be allowed via a global
# 'expose-experimental-directives'
# (verified against real haproxy 3.0.11-9e587df: `haproxy -c` exits 1).
# So this line and the normalize-uri rules must be added/removed together;
# dropping this one alone crash-loops every container on the fleet.
# So this line and the normalize-uri rules must be added/removed together.
#
# Dropping this one alone does NOT crash-loop the container -- the truth
# is worse: it is a SILENT TOTAL OUTAGE that nothing escalates. Container
# init (scripts/init.py -> haproxy_manager.do_initial_setup()) calls
# generate_config() (which still succeeds -- Jinja doesn't validate
# HAProxy semantics) and then start_haproxy(), which runs `haproxy -c`,
# sees it fail, logs an error, and RETURNS WITHOUT RAISING. init.py exits
# 0. scripts/start-up.sh then execs gunicorn as PID 1 regardless. Result:
# the container stays "Up", ports 80/443 are never bound, EVERY SITE ON
# THE HOST IS DOWN, and the in-container supervisor loop
# (ensure_haproxy.py, every HAPROXY_SUPERVISOR_INTERVAL seconds) retries
# the identical failing render forever without ever escalating. Worse
# still, GET /health keeps returning HTTP 200 -- health_check() only
# answers 500 on a database error; a dead haproxy just flips the JSON
# body's "haproxy_status" to "stopped" while the status code a naive
# monitor checks never changes. Do not trust /health alone to catch this.
#
# This exposes ONLY the experimental directives that are actually used --
# it does not change the behaviour of anything else in this file.
+30 -10
View File
@@ -75,19 +75,39 @@ frontend web
# leaves those in place and the vector
# survives -- measured, both forms tested.
#
# DELIBERATELY NOT ENABLED: query-sort-by-name. It reorders query-string
# parameters, which silently breaks anything that signs or caches on the
# DELIBERATELY NOT ENABLED: query-sort-by-name. Reordering query-string
# parameters would silently break anything that signs or caches on the
# exact query string (signed asset URLs, HMAC'd callbacks, CDN cache
# keys). It buys this gate nothing -- every rule here matches on `path`,
# which excludes the query string.
# keys). This is NOT because the enabled normalizers already leave the
# query alone -- see BLAST RADIUS just below, they don't -- it is a
# deliberate line drawn between "case-fold / decode", which RFC 3986
# defines as no-ops, and "reorder", which is not a no-op for a caller
# treating the query as an opaque signed string.
#
# BLAST RADIUS: this block applies to EVERY request for EVERY site on
# EVERY tier, so the decoding was kept minimal on purpose.
# percent-decode-unreserved touches only unreserved characters, so
# %20 (space), %2B, %C3%A9 and friends pass through byte-identical --
# verified. The only rewrite a normal site can notice is %7E -> ~ , which
# RFC 3986 defines as the same URI, plus the merge/dot resolution the
# backend would have performed anyway.
# EVERY tier, so the decoding was kept minimal on purpose -- and it is
# NOT scoped to the path. percent-to-uppercase and percent-decode-
# unreserved normalise the WHOLE request-target as HAProxy parses it --
# query string included, not just the path component the ACLs below
# match on -- and the BACKEND receives the rewritten query on the wire,
# not just an internal haproxy view of it. Measured against real HAProxy
# 3.0.11:
# /a?sig=%2babc%2fdef -> /a?sig=%2Babc%2Fdef (percent-hex upper-cased)
# /a?b=%41%42%43 -> /a?b=ABC (unreserved chars decoded)
# /a?tok=%7e%2d%5f%2e -> /a?tok=~-_. (unreserved chars decoded)
# Only parameter ORDER is preserved -- that guarantee is exactly why
# query-sort-by-name above is the one normalizer in this family left
# disabled. Per RFC 3986 both enabled rewrites are defined as the same
# URI (case in a percent-escape, and an unreserved character vs. its
# escape, carry no distinct meaning), but "the same URI" is not "the
# same bytes": an application that HMACs or otherwise signs the RAW
# query string, rather than parsing it first, could see a mutated value
# and fail to verify an otherwise-legitimate request. Checked against
# this fleet: no .NET backends (.NET's UrlEncode emits lowercase
# percent-hex, which percent-to-uppercase would rewrite) and no
# URL-in-path proxies, so there is no known victim today -- but do not
# assume "path only" from this block; that was the actual bug in an
# earlier draft of this comment.
#
# normalize-uri is EXPERIMENTAL in 3.0 and requires
# `expose-experimental-directives` in the global section