fix(domain-removal): stop deleting certificates other live sites are served from #9

2026-08-23T11:48:07Z

jknapp commented

2026-08-23 11:48:07 +00:00

Verdict: the bug is real, unguarded, and the shared-path shape is the majority case in production.

What I verified in the code

remove_domain() (haproxy_manager.py, DELETE /api/domain) ran, for any row with ssl_enabled:

os.remove(ssl_cert_path)                                  # gated only on: ssl_enabled and ssl_cert_path
certbot delete --cert-name <domain> --non-interactive     # gated only on: ssl_enabled

Schema permits sharing. init_db() declares domain TEXT UNIQUE NOT NULL but ssl_cert_path TEXT — no UNIQUE, no index, no constraint anywhere.
Nothing guarded it. No refcount, no "is anyone else using this", no quarantine. (Contrast _quarantine_superseded_certs(), which moves files to a backup dir instead of unlinking, and defers certbot delete to _delete_superseded_lineages() — that hardening exists on the bundle-issuance path only, never on the removal path.)
The sharing is created deliberately, by design. request_ssl_bundle() issues one SAN cert as --cert-name <primary>, publishes it once to /etc/haproxy/certs/<primary>.pem, then, in its own words, "Mark every name in the bundle as ssl_enabled, all pointing at the same combined .pem." One row per name, one file.
HAProxy binds the whole crt directory (bind ... ssl crt /etc/haproxy/certs), so an unlinked file is not noticed until the next reload/restart — at which point the listener refuses to start or those names get the wrong certificate.
certbot delete is the worse hazard, and separately wrong. It destroys archive + live symlinks + renewal config; recovery is a fresh, rate-limited ACME order. And --cert-name <domain> is the wrong lineage for a SAN member — on whp01, 81 of 150 SSL-enabled rows have a cert path whose basename is not their own domain, so for those the call was a silent no-op; but for a bundle primary it deleted the one lineage still renewing the cert every other name in that bundle is served with.

The report's note that the destructive branch is gated on the haproxy-manager's own SQLite (not WHP's MySQL) is confirmed — DB_FILE = /etc/haproxy/haproxy_config.db, read inside the container.

Measured in production (read-only, 2026-08-23)

host	domain rows	ssl_enabled	distinct cert paths	paths shared by >1 row	rows affected
whp01	157	150	71	39	118 / 150
whp02	33	31	15	11	27 / 31

Worst offenders on whp01: brain-jar.com.pem and arclightcourt.com.pem (10 domains each), hackerpublicradio.org.pem (6), anhonesthost.com.pem (5).
On whp02, threeworldsoneheart.org.pem is referenced by the apex, its www, and mail.threeworldsoneheart.org — exactly the row that stopped a cleanup short. Removing that mail alias through the API would have unlinked the live site's PEM.

So this is not a rare race: on whp01, 79% of SSL-enabled domain rows point at a PEM that at least one other domain also depends on.

Also checked: zero rows on either host have ssl_enabled=1 with an empty ssl_cert_path, so the new "no path → no attributable lineage" branch changes nothing on the current fleet.

What changed

Three helpers plus a refcount in remove_domain(), evaluated after the row is deleted so the query answers "who else still needs this":

lineage_name_for_cert_path() — the lineage is the published bundle's basename minus .pem (the same derivation _quarantine_superseded_certs() already uses), not the domain being removed.
domains_referencing_cert_path() / domains_referencing_lineage() — remaining rows naming that file / that lineage.
remove_domain() unlinks only when nothing else references the path, certbot deletes only when nothing else references the lineage, logs the retained domain names explicitly when it skips, and returns them as certificate_retained_for / lineage_retained_for.

ssl_enabled is deliberately not filtered on in the refcount: the two mistakes are not symmetric. A stale PEM costs kilobytes; an unlinked live one is HTTPS down for every name it serves. Cleanup is deferred, not cancelled — removing the last name on a bundle still unlinks the file and deletes the lineage.

Tests

9 new tests in scripts/test-cert-write-safety.py (suite 22 → 31, all offline, existing conventions: stdlib unittest, stub certbot/haproxy/socat on PATH, HAPROXY_MANAGER_DIR override). Assertions are on os.path.exists, file contents, edge_would_start() and the recorded certbot argv — never on which branch ran.

Covered: last reference → file unlinked and lineage deleted; the lineage targeted is the bundle's, not the removed name; shared file survives removal of a SAN member and of the bundle primary, byte-for-byte, edge still starts; shared lineage not certbot-deleted; the production mail.* shape; removing every name eventually cleans up; an unrelated bundle is never collateral damage.

Mutation results — 5 mutants, 5 killed

#	mutant	result	a killing assertion
1	guard absent entirely (suite run against `main` via `HAPROXY_MANAGER_DIR`)	6 failures	`AssertionError: False is not true : example.com is still configured and still served from this file`
2	refcount taken before the row is deleted (off-by-one)	5 failures	`AssertionError: True is not false : the last reference is gone - now it may be removed`
3	certbot guard removed, file guard kept	3 failures	`Lists differ: [] != ['delete --cert-name example.com --non-interactive']`
4	lineage taken from the domain name, not the cert path	3 failures	`Lists differ: ['delete --cert-name example.com ...'] != ['delete --cert-name www.example.com ...']`
5	file-unlink guard removed, certbot guard kept	4 failures	`AssertionError: False is not true : two sites are still served from this bundle`

Mutant 1 is the honest end-to-end check: the new tests run unmodified against the pre-fix tree and fail there for the right reasons, while test_unrelated_domains_certificate_is_untouched correctly still passes (nothing is shared in that scenario).

Other suites re-run and green: test-config-rollback.py, test-cert-scripts.py, test-stick-table-contract.py, test-runtime-map-contract.py.

Not deployed

No push to main, no build triggered, no container recreated — this repo builds on push to main, so merging is the deploy. Nothing was changed on whp01/whp02; the only production access was read-only SQLite counting.

🤖 Generated with Claude Code

## Verdict: the bug is real, unguarded, and the shared-path shape is the *majority* case in production. ### What I verified in the code `remove_domain()` (`haproxy_manager.py`, `DELETE /api/domain`) ran, for any row with `ssl_enabled`: ```python os.remove(ssl_cert_path) # gated only on: ssl_enabled and ssl_cert_path certbot delete --cert-name <domain> --non-interactive # gated only on: ssl_enabled ``` - **Schema permits sharing.** `init_db()` declares `domain TEXT UNIQUE NOT NULL` but `ssl_cert_path TEXT` — no UNIQUE, no index, no constraint anywhere. - **Nothing guarded it.** No refcount, no "is anyone else using this", no quarantine. (Contrast `_quarantine_superseded_certs()`, which *moves* files to a backup dir instead of unlinking, and defers `certbot delete` to `_delete_superseded_lineages()` — that hardening exists on the bundle-issuance path only, never on the removal path.) - **The sharing is created deliberately, by design.** `request_ssl_bundle()` issues one SAN cert as `--cert-name <primary>`, publishes it once to `/etc/haproxy/certs/<primary>.pem`, then, in its own words, *"Mark every name in the bundle as ssl_enabled, all pointing at the same combined .pem."* One row per name, one file. - **HAProxy binds the whole crt directory** (`bind ... ssl crt /etc/haproxy/certs`), so an unlinked file is not noticed until the next reload/restart — at which point the listener refuses to start or those names get the wrong certificate. - **`certbot delete` is the worse hazard, and separately wrong.** It destroys archive + live symlinks + renewal config; recovery is a fresh, rate-limited ACME order. And `--cert-name <domain>` is the wrong lineage for a SAN member — on whp01, **81 of 150** SSL-enabled rows have a cert path whose basename is not their own domain, so for those the call was a silent no-op; but for a bundle **primary** it deleted the one lineage still renewing the cert every other name in that bundle is served with. The report's note that the destructive branch is gated on the haproxy-manager's own SQLite (not WHP's MySQL) is confirmed — `DB_FILE = /etc/haproxy/haproxy_config.db`, read inside the container. ### Measured in production (read-only, 2026-08-23) | host | domain rows | ssl_enabled | distinct cert paths | **paths shared by >1 row** | rows affected | |---|---|---|---|---|---| | whp01 | 157 | 150 | 71 | **39** | **118 / 150** | | whp02 | 33 | 31 | 15 | **11** | **27 / 31** | Worst offenders on whp01: `brain-jar.com.pem` and `arclightcourt.com.pem` (10 domains each), `hackerpublicradio.org.pem` (6), `anhonesthost.com.pem` (5). On whp02, `threeworldsoneheart.org.pem` is referenced by the apex, its `www`, **and `mail.threeworldsoneheart.org`** — exactly the row that stopped a cleanup short. Removing that mail alias through the API would have unlinked the live site's PEM. So this is not a rare race: on whp01, **79% of SSL-enabled domain rows point at a PEM that at least one other domain also depends on.** Also checked: **zero** rows on either host have `ssl_enabled=1` with an empty `ssl_cert_path`, so the new "no path → no attributable lineage" branch changes nothing on the current fleet. ### What changed Three helpers plus a refcount in `remove_domain()`, evaluated **after** the row is deleted so the query answers *"who else still needs this"*: - `lineage_name_for_cert_path()` — the lineage is the published bundle's basename minus `.pem` (the same derivation `_quarantine_superseded_certs()` already uses), **not** the domain being removed. - `domains_referencing_cert_path()` / `domains_referencing_lineage()` — remaining rows naming that file / that lineage. - `remove_domain()` unlinks only when nothing else references the path, `certbot delete`s only when nothing else references the lineage, **logs the retained domain names explicitly** when it skips, and returns them as `certificate_retained_for` / `lineage_retained_for`. `ssl_enabled` is deliberately **not** filtered on in the refcount: the two mistakes are not symmetric. A stale PEM costs kilobytes; an unlinked live one is HTTPS down for every name it serves. Cleanup is **deferred, not cancelled** — removing the last name on a bundle still unlinks the file and deletes the lineage. ### Tests 9 new tests in `scripts/test-cert-write-safety.py` (suite 22 → 31, all offline, existing conventions: stdlib `unittest`, stub `certbot`/`haproxy`/`socat` on PATH, `HAPROXY_MANAGER_DIR` override). Assertions are on `os.path.exists`, file contents, `edge_would_start()` and the recorded certbot argv — never on which branch ran. Covered: last reference → file unlinked **and** lineage deleted; the lineage targeted is the bundle's, not the removed name; shared file survives removal of a SAN member **and** of the bundle primary, byte-for-byte, edge still starts; shared lineage not certbot-deleted; the production `mail.*` shape; removing every name eventually cleans up; an unrelated bundle is never collateral damage. ### Mutation results — 5 mutants, 5 killed | # | mutant | result | a killing assertion | |---|---|---|---| | 1 | guard absent entirely (suite run against `main` via `HAPROXY_MANAGER_DIR`) | **6 failures** | `AssertionError: False is not true : example.com is still configured and still served from this file` | | 2 | refcount taken **before** the row is deleted (off-by-one) | **5 failures** | `AssertionError: True is not false : the last reference is gone - now it may be removed` | | 3 | certbot guard removed, file guard kept | **3 failures** | `Lists differ: [] != ['delete --cert-name example.com --non-interactive']` | | 4 | lineage taken from the domain name, not the cert path | **3 failures** | `Lists differ: ['delete --cert-name example.com ...'] != ['delete --cert-name www.example.com ...']` | | 5 | file-unlink guard removed, certbot guard kept | **4 failures** | `AssertionError: False is not true : two sites are still served from this bundle` | Mutant 1 is the honest end-to-end check: the new tests run unmodified against the pre-fix tree and fail there for the right reasons, while `test_unrelated_domains_certificate_is_untouched` correctly still passes (nothing is shared in that scenario). Other suites re-run and green: `test-config-rollback.py`, `test-cert-scripts.py`, `test-stick-table-contract.py`, `test-runtime-map-contract.py`. ### Not deployed No push to `main`, no build triggered, no container recreated — this repo builds on push to `main`, so merging is the deploy. Nothing was changed on whp01/whp02; the only production access was read-only SQLite counting. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jknapp added 1 commit 2026-08-23 11:48:07 +00:00

fix(domain-removal): stop deleting certificates other live sites are served from b072786192

DELETE /api/domain ran, unconditionally, for any ssl_enabled row:

    os.remove(ssl_cert_path)
    certbot delete --cert-name <domain> --non-interactive

`domains.domain` is UNIQUE. `domains.ssl_cert_path` is not, and nothing
anywhere guarded against two rows naming the same file. Sharing is not an
edge case -- it is the normal shape of the table, because
request_ssl_bundle() deliberately creates it: one SAN certificate is issued
as `--cert-name <primary>`, published once to
/etc/haproxy/certs/<primary>.pem, and then EVERY included name's row is
pointed at that same path ("Mark every name in the bundle as ssl_enabled,
all pointing at the same combined .pem").

Measured read-only on the live SQLite in the haproxy-manager containers on
2026-08-23:

  * whp01: 157 domain rows, 150 ssl_enabled, 71 distinct cert paths.
    39 of those paths are referenced by MORE THAN ONE row, covering 118 of
    the 150 SSL-enabled rows. Worst cases: brain-jar.com.pem and
    arclightcourt.com.pem with 10 domains each, hackerpublicradio.org.pem
    with 6, anhonesthost.com.pem with 5.
  * whp02: 33 rows, 31 ssl_enabled, 15 distinct paths, 11 shared across 27
    rows -- including threeworldsoneheart.org.pem, referenced by the apex,
    its www, and mail.threeworldsoneheart.org. A production cleanup of that
    mail.* row was stopped short precisely because removing it would have
    unlinked the PEM the serving site is using.

So removing one domain unlinked a file up to nine other configured domains
were being served from. HAProxy binds the crt directory
(`bind ... ssl crt /etc/haproxy/certs`), so the loss is not noticed until
the next reload or restart, at which point the listener refuses to come up
or those names fall back to the wrong certificate.

`certbot delete` is the worse half. It destroys the lineage's archive, live
symlinks and renewal config; recovery is a fresh, rate-limited ACME order.
The old code passed `--cert-name <domain>`, which is also simply the wrong
lineage for a SAN member: 81 of whp01's 150 SSL-enabled rows have a cert
path whose basename is not their own domain, so for those the call was a
silent no-op -- while for a bundle PRIMARY it deleted the one lineage still
renewing the certificate every other name in the bundle is served with.

The fix refcounts, after the row is deleted so the query answers "who else
still needs this":

  * lineage_name_for_cert_path() -- the lineage is the published bundle's
    basename minus .pem, the same derivation _quarantine_superseded_certs()
    already uses, not the domain being removed.
  * domains_referencing_cert_path() / domains_referencing_lineage() -- the
    remaining rows that name that file, and that lineage.
  * remove_domain() unlinks only when the list is empty, `certbot delete`s
    only when the list is empty, logs the retained names explicitly when it
    skips, and reports them as certificate_retained_for /
    lineage_retained_for in the API response.

ssl_enabled is deliberately not filtered on in the refcount: the two
mistakes are not symmetric. A stale PEM left in the crt directory costs a
few kilobytes; an unlinked live one is HTTPS down for every name it serves.
Cleanup is deferred, not cancelled -- removing the last name on a bundle
still unlinks the file and deletes the lineage.

No row on either production host has ssl_enabled=1 with an empty
ssl_cert_path, so the "no path, no attributable lineage" branch changes
nothing on the current fleet.

Tests (scripts/test-cert-write-safety.py, +9, suite now 31, all offline):
last reference -> file unlinked and lineage deleted; shared file survives
removal of a SAN member AND of the bundle primary, byte-for-byte, with the
edge still starting; shared lineage is not certbot-deleted; the production
mail.* shape; removing every name eventually cleans up; an unrelated
bundle is never collateral damage. Assertions are on os.path.exists, file
contents and the recorded certbot argv, never on which branch ran.

Mutation-tested, all five mutants killed:
  1. guard absent entirely (suite run with HAPROXY_MANAGER_DIR pointed at
     main) -> 6 failures, incl. "example.com is still configured and still
     served from this file".
  2. refcount taken before the row is deleted -> 5 failures, incl. "the
     last reference is gone - now it may be removed".
  3. certbot guard removed, file guard kept -> 3 failures, incl.
     [] != ['delete --cert-name example.com --non-interactive'].
  4. lineage taken from the domain name instead of the cert path -> 3
     failures, incl. 'delete --cert-name example.com' != 'delete
     --cert-name www.example.com'.
  5. file-unlink guard removed, certbot guard kept -> 4 failures, incl.
     "two sites are still served from this bundle".

Other suites unchanged and green: test-config-rollback, test-cert-scripts,
test-stick-table-contract, test-runtime-map-contract.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

jknapp merged commit 43e522104b into main

2026-08-23 14:52:03 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: cloud-hosting-platform/haproxy-manager-base#9