fix(haproxy): vhost add appendait des doublons et ecrivait du TOML invalide, vhost remove decapitait la section suivante (ref #1015)

TROIS DEFAUTS DANS L'EDITION DU haproxy.toml.

1. "vhost add" appendait sans regarder l'existant : deux appels pour le meme
   domaine laissaient deux tables, donc deux ACL identiques dans le cfg
   genere. Il met desormais a jour en place, en PRESERVANT les cles que le
   paquet ne gere pas — perdre waf_bypass desactiverait en silence la seule
   exception WAF sanctionnee.

2. "vhost add" inserait son argument ssl verbatim : passer "ssl" en troisieme
   argument produisait la ligne "ssl = ssl", qui n'est pas du TOML. Arrive
   deux fois en production, sur bbs et social.

3. "vhost remove" supprimait jusqu'a l'en-tete suivant INCLUS : retirer un
   vhost decapitait le suivant et laissait ses cles orphelines, absorbees par
   la section precedente. Silencieusement.

Consequence : /etc/secubox/haproxy.toml n'etait pas du TOML valide. L'API du
module le charge avec tomllib et echouait donc, pendant que le generateur —
qui lit par grep/sed — continuait sans rien signaler. C'est ce qui a permis au
defaut de durer.

Deux ajouts :

- "config-repair", en lecture seule par defaut et avec sauvegarde horodatee
  avant reecriture, pour redresser un fichier deja abime sans edition a la
  main.
- "generate --allow-shrink", parce que le garde-fou anti-derive ne peut pas
  distinguer un retrait delibere d'une perte accidentelle : replier une table
  en double fait legitimement baisser le compte d'ACL, et la generation
  restait bloquee. L'aveu est explicite, jamais un defaut.

Verifie sur gk2 : 3 anomalies corrigees, TOML valide (122 vhosts), ACL
aletheia 4 -> 2, haproxy recharge, six vhosts temoins en 200.

packages/secubox-haproxy/debian/changelog

@@ -1,3 +1,39 @@
+secubox-haproxy (1.6.3-1~bookworm1) bookworm; urgency=medium
+
+  * `vhost add` est idempotent (ref #1015). Il appendait sans jamais regarder
+    l'existant : deux appels pour le meme domaine laissaient deux tables, donc
+    deux ACL identiques dans le cfg genere. Une table est desormais mise a jour
+    en place, en PRESERVANT les cles que le paquet ne gere pas — perdre
+    `waf_bypass` a la mise a jour d'un vhost desactiverait en silence la seule
+    exception WAF sanctionnee.
+  * `vhost add` normalise son argument `ssl` en booleen et refuse ce qui n'en
+    est pas un. Il l'inserait verbatim : `vhost add <d> <b> ssl` produisait
+    `ssl = ssl`, qui n'est pas du TOML. C'est arrive en production.
+  * `vhost remove` ne decapite plus la section suivante. La plage
+    `sed '/^\[vhosts\.X\]/,/^\[/d'` allait jusqu'a l'en-tete d'apres INCLUS :
+    retirer un vhost laissait les cles du suivant orphelines, absorbees par la
+    section precedente. Silencieusement.
+  * Nouveau `haproxyctl config-repair` : signale les tables en double et les
+    booleens invalides, et les redresse avec `--write` (sauvegarde horodatee
+    avant reecriture). En lecture seule par defaut — on ne reecrit pas la
+    configuration d'un frontal en production par simple consultation.
+  * `generate --allow-shrink` : le garde-fou anti-derive refuse toute
+    generation produisant MOINS d'entrees que la config vivante, ce qui est
+    juste — mais il ne sait pas distinguer une perte accidentelle d'un retrait
+    delibere. Replier une table en double fait legitimement baisser le compte,
+    et la generation restait alors bloquee. L'aveu est explicite, jamais un
+    defaut : sans quoi le garde-fou ne protegerait plus de rien.
+  * `CONF_PATH` et `CONFIG_DIR` sont surchargeables par l'environnement, ce qui
+    permet de verifier une configuration hors de la board sans jamais toucher au
+    frontal.
+
+  Consequence du defaut : /etc/secubox/haproxy.toml n'etait pas du TOML valide.
+  L'API du module le charge avec tomllib et echouait donc, pendant que le
+  generateur — qui lit par grep/sed — continuait sans rien signaler. C'est ce
+  qui a permis au defaut de durer.
+
+ -- Gerald Kerma <devel@cybermind.fr>  Wed, 12 Aug 2026 15:40:00 +0200
+
 secubox-haproxy (1.6.1-1~bookworm1) bookworm; urgency=medium
 
   * mitmproxy_inspector : %[query] omet le « ? », reecrivant « /a.css?v=3 »

packages/secubox-haproxy/sbin/haproxyctl

@@ -16,8 +16,10 @@ DOCKER_NAME="secubox-haproxy"
 # actually keep certs in /data/haproxy/certs/.
 DATA_PATH="${HAPROXY_DATA_PATH:-/data/haproxy}"
 [ -d "$DATA_PATH" ] || DATA_PATH="/srv/haproxy"
-CONF_PATH="/etc/secubox/haproxy.toml"
-CONFIG_DIR="/etc/haproxy"
+# Surchargeables : indispensable pour verifier une configuration hors de la
+# board (tests, mise au point) sans jamais toucher au frontal en production.
+CONF_PATH="${SECUBOX_HAPROXY_CONF:-/etc/secubox/haproxy.toml}"
+CONFIG_DIR="${SECUBOX_HAPROXY_CONFIG_DIR:-/etc/haproxy}"
 CERTS_DIR="$DATA_PATH/certs"
 # Operator-managed extra frontends/backends (webui-lan, gitea-ssh, custom
 # metablog_* backends, etc.) live as separate files in /etc/haproxy/cfg.d/.
@@ -444,6 +446,62 @@ cmd_vhost_list() {
     echo ']}'
 }
 
+# ─────────────────────────────────────────────────────────────────────
+# EDITION DU TOML — bornes de section (#1015)
+#
+# Une section TOML va de son en-tete jusqu'a l'en-tete SUIVANT, EXCLU. C'est
+# exactement ce que l'ancienne suppression rate : `sed '/^\[a\]/,/^\[/d'`
+# supprime jusqu'a la ligne suivante commencant par `[` INCLUSE, donc decapite
+# la section d'apres et laisse ses cles orphelines. Silencieusement.
+# ─────────────────────────────────────────────────────────────────────
+
+# vhost_table_delete <nom> — retire UNE table [vhosts.<nom>], la premiere.
+# Rend 0 si quelque chose a ete retire.
+vhost_table_delete() {
+    local name="$1" tmp
+    tmp=$(mktemp) || return 1
+    awk -v cible="[vhosts.$name]" '
+        # On copie tout, sauf les lignes de la table ciblee. `dans` retombe a 0
+        # des le prochain en-tete, qui est donc CONSERVE.
+        /^\[/ { dans = ($0 == cible) ? 1 : 0 }
+        !dans  { print }
+    ' "$CONF_PATH" > "$tmp" || { rm -f "$tmp"; return 1; }
+    # Ecriture par renommage : un fichier de configuration tronque par une
+    # interruption rendrait HAProxy non generable.
+    cat "$tmp" > "$CONF_PATH" && rm -f "$tmp"
+}
+
+# vhost_table_replace <nom> <domaine> <backend> <ssl> — remplace la table en
+# place, en PRESERVANT les cles que le paquet ne gere pas (waf_bypass, etc.).
+# Les perdre a la mise a jour d'un vhost desactiverait en silence une exception
+# WAF explicitement declaree par l'operateur.
+vhost_table_replace() {
+    local name="$1" domain="$2" backend="$3" ssl="$4" tmp
+    tmp=$(mktemp) || return 1
+    awk -v cible="[vhosts.$name]" -v dom="$domain" -v bk="$backend" -v ssl="$ssl" '
+        /^\[/ {
+            if (dans && !ecrit) { ecrit = 1 }
+            dans = ($0 == cible) ? 1 : 0
+            if (dans) {
+                print cible
+                print "domain = \"" dom "\""
+                print "backend = \"" bk "\""
+                print "ssl = " ssl
+                next
+            }
+        }
+        dans {
+            # Dans la table ciblee : on jette ce qu on vient de reecrire et on
+            # garde tout le reste tel quel.
+            if ($0 ~ /^(domain|backend|ssl)[[:space:]]*=/) next
+            print
+            next
+        }
+        { print }
+    ' "$CONF_PATH" > "$tmp" || { rm -f "$tmp"; return 1; }
+    cat "$tmp" > "$CONF_PATH" && rm -f "$tmp"
+}
+
 cmd_vhost_add() {
     local domain="$1"
     local backend="$2"
@@ -456,7 +514,28 @@ cmd_vhost_add() {
 
     local name=$(echo "$domain" | tr '.-' '_')
 
-    # Append to config
+    # `ssl` EST NORMALISE EN BOOLEEN (#1015). L'argument etait insere verbatim :
+    # `haproxyctl vhost add <d> <b> ssl` produisait `ssl = ssl`, qui n'est pas du
+    # TOML. C'est arrive en production — l'API du module charge ce fichier avec
+    # tomllib et echouait donc, pendant que le generateur (grep/sed) continuait
+    # sans rien signaler.
+    case "$(echo "$ssl" | tr '[:upper:]' '[:lower:]')" in
+        true|yes|on|1|ssl)  ssl=true ;;
+        false|no|off|0|"")  ssl=false ;;
+        *) error "vhost add: ssl doit etre true ou false (recu: '$ssl')"; return 1 ;;
+    esac
+
+    # IDEMPOTENT (#1015). L'ancienne version appendait sans regarder l'existant :
+    # deux appels pour le meme domaine laissaient deux tables, donc deux ACL
+    # identiques dans le cfg genere et un TOML invalide au sens strict.
+    if grep -q "^\[vhosts\.$name\]" "$CONF_PATH" 2>/dev/null; then
+        vhost_table_replace "$name" "$domain" "$backend" "$ssl" \
+            || { error "vhost add: mise a jour de $domain impossible"; return 1; }
+        log "Updated vhost: $domain -> $backend"
+        cmd_generate
+        return 0
+    fi
+
     cat >> "$CONF_PATH" << EOF
 
 [vhosts.$name]
@@ -471,12 +550,78 @@ EOF
     cmd_generate
 }
 
+cmd_config_repair() {
+    # Redresse un haproxy.toml deja abime par les anciens `vhost add/remove`
+    # (#1015). EN LECTURE SEULE PAR DEFAUT : on ne reecrit pas la configuration
+    # d'un frontal en production sans demande explicite.
+    local ecrire=0
+    [ "${1:-}" = "--write" ] && ecrire=1
+
+    [ -f "$CONF_PATH" ] || { error "config absente: $CONF_PATH"; return 1; }
+
+    python3 - "$CONF_PATH" "$ecrire" <<'PYEOF'
+import re, shutil, sys, time
+
+chemin, ecrire = sys.argv[1], sys.argv[2] == "1"
+lignes = open(chemin).read().splitlines(keepends=True)
+
+# 1. BOOLEENS INVALIDES. `vhost add <d> <b> ssl` ecrivait `ssl = ssl`, qui n'est
+#    pas du TOML. Le generateur lit ce champ par `grep -q 'true'` : tout ce qui
+#    n'est pas litteralement `true` vaut deja false pour lui. Normaliser vers
+#    `false` preserve donc EXACTEMENT le comportement, sans toucher au trafic.
+bools, sortie = [], []
+for i, l in enumerate(lignes, 1):
+    m = re.match(r'^(ssl|ssl_redirect|enabled|waf_bypass)\s*=\s*(.+?)\s*$', l)
+    if m and m.group(2) not in ("true", "false"):
+        bools.append((i, m.group(1), m.group(2)))
+        l = "%s = false\n" % m.group(1)
+    sortie.append(l)
+
+# 2. TABLES EN DOUBLE. On garde la PREMIERE : c'est celle que le generateur
+#    rencontre d'abord, donc celle dont le comportement est deja observe.
+vus, doublons, final, jeter = set(), [], [], False
+for i, l in enumerate(sortie, 1):
+    m = re.match(r'^\[([^\]]+)\]', l)
+    if m:
+        nom = m.group(1)
+        jeter = nom in vus
+        if jeter:
+            doublons.append((i, nom))
+        vus.add(nom)
+    if not jeter:
+        final.append(l)
+
+for i, cle, val in bools:
+    print("  ligne %d: %s = %s  -> booleen invalide" % (i, cle, val))
+for i, nom in doublons:
+    print("  ligne %d: [%s]  -> table en double" % (i, nom))
+
+if not bools and not doublons:
+    print("  configuration saine")
+    sys.exit(0)
+
+if not ecrire:
+    print("  %d anomalie(s) — relancer avec --write pour corriger"
+          % (len(bools) + len(doublons)))
+    sys.exit(0)
+
+# Sauvegarde horodatee AVANT reecriture : la configuration d'un frontal en
+# production ne se remplace pas sans filet.
+shutil.copy2(chemin, "%s.avant-reparation-%s" % (chemin, time.strftime("%Y%m%d-%H%M%S")))
+open(chemin, "w").write("".join(final))
+print("  %d anomalie(s) corrigee(s)" % (len(bools) + len(doublons)))
+PYEOF
+}
+
 cmd_vhost_remove() {
     local name="$1"
     [ -z "$name" ] && { error "Usage: haproxyctl vhost remove <name>"; return 1; }
 
-    # Remove section from TOML (sed-based)
-    sed -i "/^\[vhosts\.$name\]/,/^\[/d" "$CONF_PATH" 2>/dev/null
+    # NE TOUCHE PLUS A LA SECTION SUIVANTE (#1015). L'ancien
+    # `sed '/^\[vhosts\.X\]/,/^\[/d'` supprimait jusqu'a l'en-tete d'apres
+    # INCLUS : retirer un vhost decapitait le suivant et laissait ses cles
+    # orphelines, absorbees par la section precedente.
+    vhost_table_delete "$name" || { error "vhost remove: echec sur $name"; return 1; }
 
     log "Removed vhost: $name"
     cmd_generate
@@ -912,9 +1057,18 @@ EOF
             local _nb=$(grep -c '^backend ' "$out")
             local _ob=$(grep -c '^backend ' "$CONFIG_DIR/haproxy.cfg")
             if [ "${_nh:-0}" -lt "${_oh:-0}" ] || [ "${_nb:-0}" -lt "${_ob:-0}" ]; then
-                error "Drift guard: generated cfg has fewer vhosts/backends than live (acl ${_nh}<${_oh} or backend ${_nb}<${_ob}) — refusing to clobber. Migrate the missing entries into haproxy.toml/cfg.d first."
-                rm -f "$out"
-                return 1
+                # UNE REDUCTION PEUT ETRE VOULUE (#1015). Le garde-fou ne sait
+                # pas distinguer une perte accidentelle d'un retrait delibere :
+                # retirer un vhost, ou replier une table en double, fait
+                # legitimement baisser le compte. `--allow-shrink` est l'aveu
+                # explicite de l'operateur — jamais un defaut, sans quoi le
+                # garde-fou ne protegerait plus de rien.
+                if [ "${ALLOW_SHRINK:-0}" != "1" ]; then
+                    error "Drift guard: generated cfg has fewer vhosts/backends than live (acl ${_nh}<${_oh} or backend ${_nb}<${_ob}) — refusing to clobber. Migrate the missing entries into haproxy.toml/cfg.d first, or pass --allow-shrink if the reduction is intended."
+                    rm -f "$out"
+                    return 1
+                fi
+                log "Drift guard: reduction acceptee sur demande explicite (acl ${_oh} -> ${_nh}, backend ${_ob} -> ${_nb})"
             fi
         fi
         install -m 0644 -o root -g root "$out" "$CONFIG_DIR/haproxy.cfg"
@@ -1095,7 +1249,10 @@ case "${1:-}" in
     reload)     cmd_reload ;;
     stats)      cmd_stats ;;
     info)       cmd_info ;;
-    generate)   cmd_generate ;;
+    generate)   [ "${2:-}" = "--allow-shrink" ] && ALLOW_SHRINK=1; cmd_generate ;;
+    # Verifie (et, avec --write, redresse) un haproxy.toml abime par les
+    # anciens `vhost add/remove` — doublons de tables, booleens invalides.
+    config-repair) cmd_config_repair "${2:-}" ;;
 
     vhost)
         case "${2:-}" in

packages/secubox-haproxy/tests/test_vhost_toml.py

@@ -0,0 +1,164 @@
+# SPDX-License-Identifier: LicenseRef-CMSD-1.0
+# Copyright (c) 2026 CyberMind — Gérald Kerma <devel@cybermind.fr>
+# Source-Disclosed License — All rights reserved except as expressly granted.
+# See LICENCE-CMSD-1.0.md for terms.
+
+"""
+SecuBox-Deb :: haproxyctl — édition du haproxy.toml (#1015).
+
+Trois défauts, tous relevés sur le fichier vivant de gk2 :
+
+  - `vhost add` appendait sans regarder l'existant → deux tables pour le même
+    domaine, deux ACL identiques dans le cfg généré ;
+  - `vhost add` insérait son argument `ssl` verbatim → `ssl = ssl`, qui n'est
+    pas du TOML ;
+  - `vhost remove` supprimait jusqu'à l'en-tête SUIVANT inclus → la section
+    d'après était décapitée et ses clés devenaient orphelines.
+
+Le fichier de production n'était de ce fait pas du TOML valide, et l'API du
+module, qui le charge avec `tomllib`, échouait — pendant que le générateur, qui
+le lit avec `grep`/`sed`, continuait sans rien signaler.
+"""
+
+import subprocess
+import tomllib
+from pathlib import Path
+
+import pytest
+
+RACINE = Path(__file__).resolve().parent.parent
+CTL = RACINE / "sbin" / "haproxyctl"
+
+SAIN = """\
+[global]
+maxconn = 2048
+
+[vhosts.a_exemple_fr]
+domain = "a.exemple.fr"
+backend = "mitmproxy_inspector"
+ssl = false
+ssl_redirect = true
+enabled = true
+
+[vhosts.b_exemple_fr]
+domain = "b.exemple.fr"
+backend = "nginx_vhosts"
+ssl = true
+ssl_redirect = true
+enabled = true
+waf_bypass = true
+"""
+
+
+def ctl(conf, *args):
+    """Lance haproxyctl sur une config temporaire, sans toucher au systeme."""
+    r = subprocess.run(
+        ["bash", str(CTL), *args],
+        env={"PATH": "/usr/bin:/bin", "SECUBOX_HAPROXY_CONF": str(conf),
+             "HOME": "/tmp"},
+        capture_output=True, text=True)
+    return r
+
+
+@pytest.fixture
+def conf(tmp_path):
+    c = tmp_path / "haproxy.toml"
+    c.write_text(SAIN)
+    return c
+
+
+# ── Le verbe de reparation ────────────────────────────────────────────────
+
+def test_une_config_saine_est_declaree_saine(conf):
+    r = ctl(conf, "config-repair")
+    assert "configuration saine" in r.stdout, r.stdout + r.stderr
+
+
+def test_le_booleen_invalide_est_signale_puis_corrige(conf):
+    # LE CAS DE GK2 : `ssl = ssl`, produit par `vhost add <d> <b> ssl`.
+    conf.write_text(SAIN + '\n[vhosts.c_exemple_fr]\ndomain = "c.exemple.fr"\n'
+                           'backend = "nginx_vhosts"\nssl = ssl\nenabled = true\n')
+    with pytest.raises(tomllib.TOMLDecodeError):
+        tomllib.loads(conf.read_text())
+
+    vu = ctl(conf, "config-repair")
+    assert "booleen invalide" in vu.stdout, vu.stdout + vu.stderr
+    # SANS --write, RIEN N'EST TOUCHE : on ne reecrit pas la configuration d'un
+    # frontal en production par simple consultation.
+    with pytest.raises(tomllib.TOMLDecodeError):
+        tomllib.loads(conf.read_text())
+
+    ctl(conf, "config-repair", "--write")
+    d = tomllib.loads(conf.read_text())
+    assert d["vhosts"]["c_exemple_fr"]["ssl"] is False
+
+
+def test_la_reparation_preserve_le_comportement(conf):
+    # `ssl = ssl` est lu comme FALSE par le generateur (`grep -q 'true'`).
+    # Le normaliser vers `false` ne change donc rien au trafic — vers `true`,
+    # si, et cela exposerait un vhost sans certificat.
+    conf.write_text(SAIN.replace("ssl = false", "ssl = ssl", 1))
+    ctl(conf, "config-repair", "--write")
+    d = tomllib.loads(conf.read_text())
+    assert d["vhosts"]["a_exemple_fr"]["ssl"] is False
+
+
+def test_la_table_en_double_est_repliee_sur_la_premiere(conf):
+    # On garde la PREMIERE : c'est celle que le generateur rencontre d'abord,
+    # donc celle dont le comportement est deja observe en production.
+    conf.write_text(SAIN + '\n[vhosts.a_exemple_fr]\ndomain = "a.exemple.fr"\n'
+                           'backend = "AUTRE"\nssl = true\nenabled = true\n')
+    ctl(conf, "config-repair", "--write")
+    d = tomllib.loads(conf.read_text())
+    assert d["vhosts"]["a_exemple_fr"]["backend"] == "mitmproxy_inspector"
+
+
+def test_la_reparation_laisse_une_sauvegarde(conf, tmp_path):
+    # La configuration d'un frontal ne se remplace pas sans filet.
+    conf.write_text(SAIN.replace("ssl = false", "ssl = ssl", 1))
+    ctl(conf, "config-repair", "--write")
+    assert list(tmp_path.glob("haproxy.toml.avant-reparation-*")), \
+        "aucune sauvegarde avant reecriture"
+
+
+# ── Ajout et suppression ──────────────────────────────────────────────────
+
+def test_ajouter_deux_fois_ne_cree_pas_deux_tables(conf):
+    # LE DEFAUT D'ORIGINE : `aletheia` declare deux fois, d'ou deux ACL
+    # identiques dans le cfg genere.
+    ctl(conf, "vhost", "add", "c.exemple.fr", "nginx_vhosts", "false")
+    ctl(conf, "vhost", "add", "c.exemple.fr", "nginx_vhosts", "false")
+    assert conf.read_text().count("[vhosts.c_exemple_fr]") == 1
+    tomllib.loads(conf.read_text())
+
+
+def test_un_ssl_non_booleen_est_refuse_pas_ecrit(conf):
+    # Ecrire l'argument verbatim produisait `ssl = ssl` — pas du TOML.
+    r = ctl(conf, "vhost", "add", "d.exemple.fr", "nginx_vhosts", "peut-etre")
+    assert r.returncode != 0, "un ssl absurde a ete accepte"
+    assert "d_exemple_fr" not in conf.read_text()
+    tomllib.loads(conf.read_text())
+
+
+def test_supprimer_un_vhost_ne_decapite_pas_le_suivant(conf):
+    # LE PIEGE DE LA PLAGE sed : `/^\\[a\\]/,/^\\[/d` supprime jusqu'a l'en-tete
+    # d'apres INCLUS. La section suivante perdait son en-tete et ses cles
+    # etaient absorbees par la precedente — en silence.
+    ctl(conf, "vhost", "remove", "a_exemple_fr")
+    texte = conf.read_text()
+    assert "[vhosts.a_exemple_fr]" not in texte
+    assert "[vhosts.b_exemple_fr]" in texte, "la section suivante a ete decapitee"
+    d = tomllib.loads(texte)
+    assert d["vhosts"]["b_exemple_fr"]["backend"] == "nginx_vhosts"
+    # Et la cle que le paquet ne gere pas doit survivre : la perdre
+    # desactiverait en silence une exception WAF declaree par l'operateur.
+    assert d["vhosts"]["b_exemple_fr"]["waf_bypass"] is True
+
+
+def test_mettre_a_jour_un_vhost_preserve_ses_cles_non_gerees(conf):
+    # `waf_bypass` est la seule exception WAF sanctionnee : la perdre a la
+    # mise a jour d'un vhost la desactiverait sans que personne ne le voie.
+    ctl(conf, "vhost", "add", "b.exemple.fr", "mitmproxy_inspector", "true")
+    d = tomllib.loads(conf.read_text())
+    assert d["vhosts"]["b_exemple_fr"]["backend"] == "mitmproxy_inspector"
+    assert d["vhosts"]["b_exemple_fr"]["waf_bypass"] is True