From d55c590c5a603033f68488260a4d53da6bb2aa1e Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Fri, 9 Oct 2026 12:37:28 +0200 Subject: [PATCH] =?UTF-8?q?hub=20(unreleased):=20R-922=20option=20A=20?= =?UTF-8?q?=E2=80=94=20a=20household's=20clear=20deletes=20its=20notificat?= =?UTF-8?q?ion=20address=20(email=5Fcleared);=20MAIL-HOLD=20=E2=80=94=20a?= =?UTF-8?q?=20restored=20hub=20sends=20no=20mail=20until=20released;=20two?= =?UTF-8?q?=20log=20lines=20drop=20the=20address;=20runbooks:=20mail=20hol?= =?UTF-8?q?d=20is=20restore=20step=201;=2007=20=C2=A76.4=20R-921=20pre-che?= =?UTF-8?q?ck;=20R-921/R-922=20narrowed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- REUSE.md | 1 + .../architecture/07-backup-architecture.md | 6 + documentation/backlog/OPEN-ITEMS.md | 4 +- .../legal/DRAFT-adatkezelesi-tajekoztato.md | 1 + .../runbooks/RUNBOOK-hub-db-offsite-backup.md | 19 ++- .../runbooks/total-loss-of-dooplex.md | 2 +- hub/CHANGELOG.md | 35 +++++ hub/cmd/hub/main.go | 14 ++ hub/internal/api/handler.go | 36 ++++- hub/internal/api/mail.go | 11 ++ hub/internal/api/mailhold_test.go | 87 +++++++++++ hub/internal/api/preferences_guard_test.go | 61 ++++++++ hub/internal/mailhold/mailhold.go | 88 +++++++++++ hub/internal/mailhold/mailhold_test.go | 59 ++++++++ hub/internal/notify/dispatcher.go | 89 +++++++++-- hub/internal/notify/mailhold_test.go | 143 ++++++++++++++++++ hub/internal/web/funcmap_test.go | 1 + hub/internal/web/mailhold_test.go | 135 +++++++++++++++++ hub/internal/web/r135_csrf_test.go | 1 + hub/internal/web/server.go | 40 ++++- hub/internal/web/templates/app_detail.html | 1 + hub/internal/web/templates/apps.html | 1 + hub/internal/web/templates/config_form.html | 1 + hub/internal/web/templates/configs.html | 1 + hub/internal/web/templates/configuration.html | 17 +++ .../web/templates/customer_unified.html | 1 + hub/internal/web/templates/dashboard.html | 1 + hub/internal/web/templates/host_detail.html | 1 + hub/internal/web/templates/hosts.html | 1 + hub/internal/web/templates/log_tail.html | 1 + hub/internal/web/templates/mail_hold.html | 8 + hub/internal/web/templates/offsite.html | 1 + hub/internal/web/templates/style.css | 11 ++ hub/internal/web/templates/system.html | 1 + scripts/wire_contract_gate.py | 4 + 35 files changed, 861 insertions(+), 23 deletions(-) create mode 100644 hub/internal/api/mailhold_test.go create mode 100644 hub/internal/mailhold/mailhold.go create mode 100644 hub/internal/mailhold/mailhold_test.go create mode 100644 hub/internal/notify/mailhold_test.go create mode 100644 hub/internal/web/mailhold_test.go create mode 100644 hub/internal/web/templates/mail_hold.html diff --git a/REUSE.md b/REUSE.md index 29025dd9..f4b62247 100644 --- a/REUSE.md +++ b/REUSE.md @@ -31,6 +31,7 @@ | `severityNotifies` | hub/internal/notify/dispatcher.go (~L77) | `(severity string) bool` | Deciding whether a severity emails | warning/error/critical notify; info intentionally doesn't; anything else is logged as unrecognized (v0.24.0 fix — do not regress). Recovery mails exist DESPITE this gate (eventType branch), not through it. | | `priorityHeaders` | hub/internal/notify/dispatcher.go (~L56) | `(severity string) map[string]string` | High-priority mail-client nudge (audit F14-light) | error/critical → `X-Priority: 1` + `Importance: high`; everything else nil — a warning/info mail must NOT masquerade as urgent (red-proofed). | | `sendEmailFn` seam / `sendEmail` | hub/internal/notify/dispatcher.go (~L33 / ~L300) | `func(to, subject, textBody string, headers map[string]string) error` | Test seam for all sends; Resend POST | Signature grew a `headers` param in v0.71.0 — payload carries `"headers"` only when non-empty. Tests capture recipient+subject+headers through the seam. | +| `mailhold.Hold` (`Check` / `Held` / `Release`) / `(*Dispatcher).deliver` | hub/internal/mailhold/mailhold.go; hub/internal/notify/dispatcher.go | `Check(kind, who string) error` (ErrHeld when held) | THE mail gate: while `/MAIL-HOLD` exists the hub sends nothing | Every send goes through it — dispatcher sends ONLY via `deliver()` (pinned by `TestMailHold_EverySendPathIsGated`), `/notify` and the app-mail relay call `Check` first. Held mail is DROPPED, never queued. A new send path that skips it re-opens the restored-hub mail leak. | | `FormatOperatorEmail` / `FormatCustomerEmail` | hub/internal/notify/templates.go (~L24 / ~L118) | `(...) (subject, body)` | Operator (English) / customer (Hungarian) email bodies | Customer messages come from the `customerMessages` map — add the Hungarian text when adding an event type. Budapest TZ via package `init()`. Operator icon is eventType-aware: `*_recovered` → ✅ (severity is the fallback). | | `monitor.EventNotifyFunc` | hub/internal/monitor/staleness.go (~L14) | `func(customerID, eventType, severity, message, detailsJSON, source)` | Decoupling checkers from notify; wired to `dispatcher.ProcessEvent` in main | May be nil — always nil-check before calling (all checkers do). | | `(*Store).LogNotification` | hub/internal/store/store.go (~L433) | `(customerID, eventType, severity, message, status, errorMsg, channel)` | Audit trail of every send attempt (sent/failed, per channel) | Log BOTH success and failure (dispatcher does). Since v0.71.0 these rows are also the recovery PAIRING evidence — never prune them casually. | diff --git a/documentation/architecture/07-backup-architecture.md b/documentation/architecture/07-backup-architecture.md index fa464642..4ca43a03 100644 --- a/documentation/architecture/07-backup-architecture.md +++ b/documentation/architecture/07-backup-architecture.md @@ -751,6 +751,12 @@ the apps' own stop and start (demo-hp 2026-10-05: stop 21 s, start 46 s). **Not rule (the scratch guest has no agent connection; the demo boxes take only deliveries and read-backs) — the first night run on the demo boxes is the proof owed (R-518). The page states about 1–1.5 minutes (an estimate from the parts above, not a measured press). +**Check first, stop second (R-921, built 2026-10-09, ships with the next controller release).** Measured 2026-10-08 on +demo-hp: the off-site tier stopped every app for ~1 min and the agent then refused it (BUSY: the night OS step held the +agent's host-wide lock). Before stopping anything the controller now asks the agent which tiers have a job in flight +(`GET /backup/status`); another tier's job running or snapshotted → no stop, the tier stays due. A refusal after the +stop still resumes the apps at once (pinned by test). **Not covered:** the host-wide busy lock (OS step, restore test, +fstrim) — the agent serves it on no endpoint, so the measured case needs an agent change too (R-921, LEFT). **Measured 2026-10-05 on demo-hp (9 apps, controller v0.295.0):** „Mentés most" stopped the apps at 09:19:08Z, the local tier ran 09:19:29–09:24:09, the PBS tier was busy (the controller logged a retry in 15 min; no second stop was seen in the next 55 min), the last app was back at 09:24:55Z — the longest stop **5 min 47 s**, for the local tier alone. The button text and its confirm (v0.296.0) give both diff --git a/documentation/backlog/OPEN-ITEMS.md b/documentation/backlog/OPEN-ITEMS.md index f45af33a..6892e730 100644 --- a/documentation/backlog/OPEN-ITEMS.md +++ b/documentation/backlog/OPEN-ITEMS.md @@ -166,7 +166,7 @@ stopping line that lies. | **R-540** | Backup & restore | P3 | **[P3-LOW] The hub knows exactly ONE off-site pool box, so there is no rule for what happens when it fills.** Read from source 2026-09-16 while making off-site the default: `HETZNER_POOL_BOX_ID` is a single value, and every shared customer becomes a sub-account on that box. With off-site now ON for every new customer (hub v0.116.0) the box fills faster, and the fill warning (80%/90% of the box, `monitor/offsite.go`) tells the operator it is filling but nothing says which box a new customer should land on. **Needs a selection rule** (least-full, or explicit per-customer), not a bigger box. No customer is at risk today: the pool box read 0.3% full (2.7 GB of 1 TB), Σ shared quota 150 GB, oversub 0.15x. | **READY — rank P3-LOW; owner: CC (hub)** **2026-10-05 (burn-down night): NEEDS A DESIGN (and money).** A selection rule needs a multi-box configuration and a second box. Next: an operator pick of the rule and its trigger (e.g. add box 2 at 70 %). | — | — | CC | | **R-548** | Backup & restore | P3 | **[P3-LOW] The whole-guest backup’s LOCAL tier cannot fit on a small-system-disk box, and will retry on that tier for ever.** MEASURED 2026-09-17 (chaos night) on `tester-1-022354`: a whole-guest backup wrote a **~29 GB source** (`mp0` = `local-lvm:vm-9201-disk-1`, 70 G provisioned, 40.58 % used, `backup=1`) into `pve-root`, which on a 32 GB system disk is **14 GB total with ~4.9 GB free**. Two samples thirty seconds apart showed the archive growing ~497 MB while free space fell ~475 MB — **~16 MB/s, i.e. under four minutes to a full `/`** on the nested PVE. **The product’s behaviour is correct and legible throughout:** it failed the tier and said which one — `whole_guest_backup_failed` (error, operator-only): „Whole-guest backup FAILED on the local tier — retrying with backoff (next attempt in 15m0s)” — its status surface agreed (`target_id:"local"`, `success:false`, `size_bytes:0`), and the **off-site tier then ran from the same snapshot and succeeded in ~8½ minutes, encrypted to ep0, consuming no local disk at all**. So the data still left the house. What is filed is the loop: on a box shaped like this the local tier can **never** succeed, and it keeps retrying on a backoff for ever, burning I/O and risking `/` each time. **Honest caveat:** the 32 GB system disk is this drill’s fixture choice, so the row is conditional on disk size — but nothing in the product checks whether the local target could ever hold the source before trying. **Fix shape:** compare the source size against the target’s free space before starting the local tier and skip it with a clear reason, rather than discovering it at ~16 MB/s. Evidence: `audits/evidence-chaos-night-2026-09-17/round-6.txt`. **-- 2026-09-30, the class on a real box: demo-hp's whole-guest LOCAL tier was refused by the space preflight (R-685) 10 times since 2026-09-27** („local has 12.1 GiB free; the last archive of guest 9201 was 8.9 GiB, so a new one needs about 12.1 GiB") — its newest local archive was 2026-09-29 04:42, 30 h old at the day's read (the weekly PBS tier, last 2026-09-24, is its whole copy meanwhile). The refusal is correct and named; what is missing is that nothing gives the box the room back (the host's `local` holds the 8.9 GiB archive of the only guest plus templates on a 39 GB root). `audits/pg-last-six-2026-09-30/C/`. | **READY — rank P3-LOW; owner: CC** | — | — | CC | | **R-698** | Backup & restore | P3 | **[P3-LOW] A backup stores the image's NAME, not the image — a restore of a version its maker has deleted cannot start.** `RecoveryManifest.image_pins` ("image NOT stored — re-pulled on restore"); since controller v0.275.0 each data file also records its running `ref@digest`, and a restore brings the data back AT ITS OWN VERSION (`07` §6.6) — so a restore asks for exactly the old image. **Measured 2026-09-26** (`audits/version-travel-2026-09-26/A7/`, registry HEADs, no pulls): the catalog's 42 ladder `ref@digest` pairs all resolve (200); an invented digest answers 404 on Docker Hub and ghcr.io (negative control). Not measured: the digests recorded on boxes (older than any ladder entry), how often makers delete versions, the catalog's 66 digest-less compose lines. **Options (decide nothing yet):** (a) keep — a restore of a deleted version fails at the pull and the household uses the next copy or a newer version; (b) mirror every INSTALLED image into the DooPlex registry, restore falls back to it — storage + bandwidth on DooPlex, a new part on the recovery path; (c) mirror only ladder-named versions — bounded, misses pre-ladder boxes; (d) `docker save` into the unit — hundreds of MB per app per copy on every tier. **-- 2026-09-30 late (decision 53):** a box now keeps only an app's running and previous image; a restore to an older version re-pulls it — as every restore already did. The limit above is unchanged. | **OPEN — P3; owner: operator (a decision), CC measures** **Operator ruling 2026-10-05 18:23: kept OPEN as a known risk to a household's restore; owner the operator; not worked on in the burn-down.** | — | — | operator | -| **R-921** | Backup & restore | P3 | **When the off-site tier answers BUSY, the household still gets a short stop of every app — with no copy.** MEASURED 2026-10-08 night on demo-hp (read-back 2026-10-09, `audits/dooplex-survival-2026-10-09/partE/R-518.txt`): local tier 20:20–20:25Z (own stop, < 2 min), then at 20:26:14Z the agent refused the off-site request (`backup refused — a heavy operation is already in flight`, `busy=backup:local` — the night OS step ran 20:27–20:29Z), yet the controller's metrics show all 15 app containers down at 20:26:19Z and back at 20:27:19Z; the retry ran the off-site tier at 20:34Z with its own short stop. So a two-tier night costs THREE stops, one of them for nothing. This was R-518's noted "unmeasured" risk. Fix shape: ask the agent whether it is free (or reserve the slot) BEFORE quiescing, and resume at once on BUSY. | **READY** — needs a controller (and maybe agent) release; this session had no release budget. | — | Build the pre-quiesce check; read back a two-tier night | CC | +| **R-921** | Backup & restore | P3 | **When the off-site tier answers BUSY, the household still gets a short stop of every app — with no copy.** MEASURED 2026-10-08 night on demo-hp (read-back 2026-10-09, `audits/dooplex-survival-2026-10-09/partE/R-518.txt`): local tier 20:20–20:25Z (own stop, < 2 min), then at 20:26:14Z the agent refused the off-site request (`backup refused — a heavy operation is already in flight`, `busy=backup:local` — the night OS step ran 20:27–20:29Z), yet the controller's metrics show all 15 app containers down at 20:26:19Z and back at 20:27:19Z; the retry ran the off-site tier at 20:34Z with its own short stop. So a two-tier night costs THREE stops, one of them for nothing. This was R-518's noted "unmeasured" risk. Fix shape: ask the agent whether it is free (or reserve the slot) BEFORE quiescing, and resume at once on BUSY. | **NARROWED 2026-10-09 — built, not released (controller):** before stopping apps the controller asks the agent which tiers have a job in flight (`GET /backup/status`); another tier's job in flight → no stop, the tier stays due (red-proved: 3 apps stopped for a refused tier before); a refusal after the stop still resumes the apps at once (pinned). Also fixes a stop after a long upload or a restart mid-upload. **LEFT: the measured case itself** — the agent's host-wide busy lock (the night OS step, restore test, fstrim) is served on NO endpoint, so the controller cannot see it before stopping; needs one agent field (e.g. `busy` on `GET /backup/status`) + a controller pre-check behind a feature probe. `07` §6.4. | next controller release; an agent change | Release the controller; build the agent `busy` field + its controller check; read back a two-tier night | CC | | **R-91** | Backup & restore | P4 | Old 13 GB datastore copy at `/srv/pbs-felhom` on ep0's root disk **Checked from source 2026-10-05 (burn-down round 2):** Gate is long past (row waits on demo-felhom's first post-migration PBS backup, migration 2026-07-27). Last positive record of the copy: audits/CAMPAIGN-9-restore-proof-2026-07-28.md:759 'ep0 : /srv/pbs-felhom rollback copy intact (13G)'; CONTEXT.md:3666 still says it is 13 G of dead weight awaiting R-91. No later record of deletion found (grep srv/pbs-felhom across felhom.eu). Deleting is on ep0 (protected) and needs an operator word. | WATCHING | demo-felhom's first **post-migration** PBS backup | Delete once it lands; fix `CONTEXT.md:1018` same commit | CC | | **R-164** | Backup & restore | P4 | **C2's chain: the DB volume tar cannot be dropped until a SOUND dump predicate exists.** The unit carries both a volume tar and a SQL dump; the restore uses **both** — the dump is authoritative and replayed *after* the tar so it WINS (F17), with only the DB service up (R-47) — `internal/backup/restore_unit.go:262-266`. Dropping the DB container's tar would halve DB-app units **and** close the R-127(b) initdb-skip password trap (restored PGDATA ⇒ `POSTGRES_PASSWORD` ignored). | **BLOCKED** — on the predicate | a dump-validity predicate that is not `accounts has rows` | **The obvious gate is DEAD, measured:** `ValidateDump` warns when the `accounts` table is empty, and that warning was **correct** — the live DB genuinely had 0 accounts, and seeding one stopped the warning and put the row in the dump. But **a fresh appliance legitimately has zero accounts**, so promoting that predicate to a gate would **block every new customer's first backup**. Order: (1) a sound predicate — dump vs **live** per-table counts, not an absolute expectation; (2) warn→gate; (3) tar-drop. **Until (1), the tar is load-bearing** — not because dumps are bad, but because nothing can yet prove one is good. Pairs with **R-127** | CC | | **R-213** | Backup & restore | P4 | **Putting files back in place — the half the recovery screen deliberately does not do.** The screen (R-193, controller v0.200.0) unlocks the repository and LISTS what is in it; restoring is per-app and lives in the backups area, and the operator ruled the two separate on 2026-08-05: *a screen that unlocks and then offers to overwrite is two decisions wearing one button*. What is missing is the step after the listing — a customer who can now SEE their files still has to work out, per app, which restore to choose. **The operator named its requirement: a live-versus-backup comparison** — the customer must be able to see what would change before anything is overwritten | **OPEN — not started, deliberately** | the comparison design (nothing exists for it yet) | Design the live-vs-backup comparison, then the put-back flow on top of it. Do NOT fold it into the recovery screen | Operator + CC | @@ -248,7 +248,7 @@ stopping line that lies. | ID | Category | Sev | What | State | Blocked on | Next action | Owner | |---|---|---|---|---|---|---|---| -| **R-922** | Hub & operator | P2 | **A household that clears its mail address on the dashboard does not get it cleared on the hub — the hub keeps the old address.** SEEN 2026-10-09 on Tester 1 (`audits/release-2026-10-09/proofs/D3/d3.txt`): the push with an empty address answered 200 and the hub row kept the old address (with no events, so no mail is sent). Cause: the empty-email no-clobber guard (`hub/internal/api/handler.go`, v0.71.0, audit F12) protects against an unconfigured box wiping a seeded address, and cannot tell that from a household's deliberate clear. Personal data the household removed stays on our side with no stated end — the same family as R-901's deletion rules. | **READY — operator ruling 2026-10-09 11:19: option A** (the hub deletes the address when the household clears it). CC builds it; it ships with the next release. | — | Build A (controller + hub), red tests, privacy-notice line | CC | +| **R-922** | Hub & operator | P2 | **A household that clears its mail address on the dashboard does not get it cleared on the hub — the hub keeps the old address.** SEEN 2026-10-09 on Tester 1 (`audits/release-2026-10-09/proofs/D3/d3.txt`): the push with an empty address answered 200 and the hub row kept the old address (with no events, so no mail is sent). Cause: the empty-email no-clobber guard (`hub/internal/api/handler.go`, v0.71.0, audit F12) protects against an unconfigured box wiping a seeded address, and cannot tell that from a household's deliberate clear. Personal data the household removed stays on our side with no stated end — the same family as R-901's deletion rules. | **NARROWED 2026-10-09 — option A BUILT, not released** (operator ruling 11:19): the controller sends `email_cleared: true` while a household-cleared address stays empty; the hub deletes its stored notification address on that flag and keeps the F12 guard for every other empty push (red-proved both sides; the wire-contract gate now checks this push). Privacy-notice draft: one retention row. Ships with the next hub + controller release (hub first). **LEFT:** the operator-registered address `customer_configs.email` is a separate copy and stays — and the kernel notice, claim codes and self-bind mails read THAT one (`hub/internal/notify/dispatcher.go` `SendKernelNotice`), so a household that cleared its address still gets kernel notices; which address counts is a rule for the operator. | operator rule for the second copy | Release; then the operator: does a household clear also stop mails to the registered address? | CC (release), operator (rule) | | **R-31** | Hub & operator | P3 | **[P2-HIGH] Offsite provisioning is synchronous with no status affordance.** Save runs the Hetzner sync in-request, so the request can hit the nginx 504 **while succeeding server-side**: the operator cannot tell failed from slow, and a retry races the first attempt. **MIGRATED FROM `ROADMAP.md` 2026-08-22 (R-369) — originally filed 2026-07-21, size M, roadmap state `idea`.** Moved verbatim; nothing added or reinterpreted. The roadmap keeps its copy as history, marked moved. | **OPEN — migrated from ROADMAP 2026-08-22, rank unchanged** **Re-ranked 2026-10-03: P2→P3: operator-only; a known workaround (click once, wait, verify) exists.** **2026-10-06 night: the race half fixed on felhom.eu main (hub, unreleased):** a second Save while the first still provisions is refused with 409 („already running — wait about a minute, then reload; do not save again"); nothing saved, nothing created; per customer, in memory. `TestProvision_R31_*`, red-proof `audits/night-burndown-2026-10-06/hub/R-31-red.txt`. LEFT: the async save with a status card (the escrow-card idiom). | — | Direction: make it async + a status card, reusing the proven **awaiting-card/poll idiom** (v0.138.0 escrow card). **Interim mitigation belongs in R-3 as an operator note: click once, wait, verify — do not re-click.** | CC | | **R-244** | Hub & operator | P3 | **The customer DELETE cascade leaves `app_log_issues` behind, and it is systematic across every venue ever torn down.** Found **2026-08-07** while verifying the `finalwalk` teardown with a **full census** (every table, every column) rather than a per-table query. After a cascade that logged `COMPLETE … full teardown`, **61 rows still matched `finalwalk`**. Four of the five sources are **deliberate and correct** — the cascade's own header states *"Provenance/events are NEVER wiped — audit outlives every tier"*: `events` 16, `notification_log` 14, `host_deletions` 1, `customer_resets` 1. **The fifth is a gap:** `app_log_issues` 29 rows, which the residue purge does not touch (its logged leg covers `reports`/`app_telemetry`/`app_log_tails`/`log_tail_requests`/`notif_prefs`/`selfbind_tokens`/`appliance_registrations` — not this table). **It is not a `finalwalk` quirk:** rows still reference **`c11` 40, `rewalk` 20, `part4` 24** — all three torn down 2026-08-06, whose ledger recorded *"0 occurrences"*. **That prior claim was measured with a narrower query and does not survive a full census; the correction is recorded rather than the measurement quietly redone.** **Why it was probably never written, established rather than assumed:** the table is a **fleet-wide aggregate** keyed on `app_name`+`fingerprint` with an `affected_customers` JSON list — of the 29 `finalwalk` rows, **12 reference only `finalwalk`** (orphans, safely deletable) and **17 are shared with LIVE customers** (`demo-felhom`, `peti-felhom`, …) and **must not be deleted, only de-referenced.** A naive `DELETE … WHERE customer LIKE` would destroy a live customer's issue history — which is very likely why the leg does not exist, and is the reason this is not a one-line fix. **Severity is LOW and stated plainly: no secret material is involved** — app name, fingerprint, message text, counts, timestamps. What survives is a deleted customer's *identifier* inside an aggregate row. **Proposed shape:** a residue leg that (a) removes the customer id from `affected_customers`/`context_customer`, and (b) deletes rows whose `affected_customers` becomes empty; plus a one-off sweep for the four already-torn-down venues. **The general lesson is the reusable part:** *a per-table absence query is not a census.* The teardown verification is now a full-schema sweep, and that is what found this. **Not fixed** — a cascade change needs its own red-proof and this session was scoped as a spike plus two operations. Evidence: `tests/teardown-finalwalk-2026-08-07.md`. **⚠ STILL OWED, AND NOW MEASURED RATHER THAN ESTIMATED (2026-08-08 census, read-only, no truncation).** `app_log_issues` holds **1309 rows**; **71 reference a torn-down venue** (`finalwalk`, `c11`, `rewalk`, `part4`); of those **44 are ORPHANS** — they name only torn-down customers and are safely deletable — and **27 are SHARED with a live customer** (`demo-felhom`, `peti-felhom`, …) and **must be de-referenced, never deleted**. 1238 rows are untouched. **The 27 are exactly why the leg was never written**, and why a `DELETE … WHERE customer LIKE` would destroy a live customer's issue history. **What it needs, precisely:** a cascade leg that (a) removes the customer id from `affected_customers` / `context_customer`, and (b) deletes only rows whose `affected_customers` becomes empty; plus a one-off sweep for the four venues already gone. **Why it was NOT done on 2026-08-08:** the fix is hub code, and that session's scope forbade a hub version bump; a hand-run SQL mutation over 71 rows — 27 of them needing surgical de-referencing — with no tested code path and no red-proof is precisely the shape that goes wrong on a live database. **It accumulates one venue at a time, so the next walk adds to it**; the numbers above mean the next session starts from data rather than a guess. **⚠ IT GREW AGAIN, AS PREDICTED — walk5 teardown, 2026-08-08.** The fifth walk's venue was torn down with a full-schema census taken **before and after**: **168 rows → 67**. Of the 67, **37 are by design** (`events` 21, `notification_log` 14, `host_deletions` 1, `customer_resets` 1) and **30 are `app_log_issues`** — this row's gap, and the count was **predicted in the pre-run enumeration rather than discovered afterwards**, which is the difference from the ledger that once recorded *"0 occurrences"* from a narrower query. **The running total across torn-down venues therefore rises from 71 to ~101 rows** (`finalwalk`, `c11`, `rewalk`, `part4`, now `walk5`) — the shared-with-a-live-customer subset must still be **de-referenced, never deleted**. **It accumulates one venue at a time and it did so again.** Evidence: `tests/walk5-r201-2026-08-07/teardown-walk5-2026-08-08.md`. **2026-09-25:** `peti-felhom` is no longer a live customer (deleted through the cascade, journal #20); 8 `app_log_issues` rows still name it — the same gap. | **READY** — owner Viktor | — | — | operator | | **R-882** | Hub & operator | P3 | **Longhorn on DooPlex could not grow a volume online: its `instance-manager` (116 days up) called a host process that no longer existed** — `nsenter: cannot open /host/proc/196610/ns/mnt` on every expansion retry, and an offline growth was blocked by the expansion's own attachment ticket (found 2026-10-05 growing `hub-data` to 2 Gi). A restart of the instance-manager (operator-approved) fixed it: 77/77 volumes back `attached/healthy` in 110 s. **Why the cached PID went stale was not established** (likely a containerd/k3s or iscsid restart after the instance-manager started), so it will recur after the next such restart and stay invisible until a volume needs to grow. `audits/hub-db-offsite-2026-10-05/partA/step1-*.txt` | **OPEN** | — | Find which host process the PID was and whether Longhorn 1.10.x re-resolves it; until then, before growing any volume, check the instance-manager's age against the last k3s/containerd restart | operator | diff --git a/documentation/legal/DRAFT-adatkezelesi-tajekoztato.md b/documentation/legal/DRAFT-adatkezelesi-tajekoztato.md index e63c34c9..ce511eb3 100644 --- a/documentation/legal/DRAFT-adatkezelesi-tajekoztato.md +++ b/documentation/legal/DRAFT-adatkezelesi-tajekoztato.md @@ -113,6 +113,7 @@ A központi rendszer (`hub.felhom.eu`) a Felhom saját szerverén fut (k3s fürt | Adat | Mire kell | Meddig marad meg | |---|---|---| | Ügyfél azonosítója, neve, domainje, e-mail-címe, nyelve | szerződés teljesítése, értesítések | az ügyfél törléséig | +| Értesítési e-mail-cím (amelyre a háztartás a figyelmeztetéseket kéri) | értesítések | amíg a háztartás meg nem változtatja; ha a háztartás a dashboardon kitörli, a központi rendszer a következő szinkronizáláskor törli; az ügyfél törlésekor törlődik | | A szerver állapotjelentései (gépnév, processzor-, memória-, lemezhasználat, hőmérséklet, a telepített alkalmazások neve és állapota, mentések állapota, a dashboard nyelve) | felügyelet, hibajelzés | **90 nap** | | Események (pl. „mentés sikertelen", „lemez megtelt") | felügyelet, ügyfélnek látható napló | **90 nap** | | Alkalmazásonkénti erőforrás-statisztika | kapacitástervezés | **90 nap** | diff --git a/documentation/runbooks/RUNBOOK-hub-db-offsite-backup.md b/documentation/runbooks/RUNBOOK-hub-db-offsite-backup.md index 351c4178..b627593c 100644 --- a/documentation/runbooks/RUNBOOK-hub-db-offsite-backup.md +++ b/documentation/runbooks/RUNBOOK-hub-db-offsite-backup.md @@ -158,7 +158,7 @@ Run the unit by hand; read the snapshot on ep0 (`proxmox-backup-client snapshot test by hand; stop the timer for a day on purpose and see `HubDBBackupStale` mail arrive (positive observable), then start it again. -## 3. Bringing the hub back from this copy (the procedure the plan exists for) — TESTED 2026-10-05 (steps 1–3) and 2026-10-09 (steps 4–5) +## 3. Bringing the hub back from this copy (the procedure the plan exists for) — TESTED 2026-10-05 (steps 1–3) and 2026-10-09 (steps 4–5) — *steps renumbered 2026-10-09: the mail hold became step 1, so the old 1–3 are now 2–4 and the old 4–5 are 5–6* Steps 1–3 were run on 2026-10-05 against the real copy on ep0 (`audits/hub-db-offsite-2026-10-05/partD/restore-procedure/drill.txt`): 4 hosts, **4 of 4 console passwords opened with the saved seal key, 0 of 4 with a random key**. **Steps 4–5 were run on @@ -175,10 +175,17 @@ copy, the customer list and host list equal live (4/4, 4/4), all 4 console passw What you need, from the break-glass sheet (`break-glass-sheet.md`; the password manager only if it survived or was restored first — `total-loss-of-dooplex.md` step 5): the seal key (`OFFSITE_SECRET_KEY`), the backup key's `data` field, and the read-only token (or ep0 root to mint a new one: Step 2). -1. **The backup key file.** On the machine doing the restore, as root, `umask 077`, write +1. **Mail held — before the copy is ever started (from the next hub release, R-923 follow-up).** Create the empty + marker `MAIL-HOLD` in the hub's data directory (`/data/MAIL-HOLD`, in the PVC, by the helper pod of step 6) BEFORE + the restored hub first starts. While it exists the hub sends no e-mail at all (households and operator), logs each + dropped mail as `[WARN] MAIL-HOLD: not sending to `, and every operator page shows a banner. Held mails + are dropped, not queued (they were decided from a snapshot). Release it on the Configuration page (or `POST + /configuration/mail-hold/release`) once the restored hub is the only hub and its pending notices are understood. + Until that release is deployed: start a test restore with no network at all. +2. **The backup key file.** On the machine doing the restore, as root, `umask 077`, write `{"kdf": null, "created": "2026-01-01T00:00:00+00:00", "modified": "2026-01-01T00:00:00+00:00", "data": ""}` to `enc.key` (Step 0 note). A copy of DooPlex's `/etc/felhom-hub-backup/enc.key` works as is. -2. **Restore the newest copy** (from any machine that reaches ep0's PBS on 8007 — DooPlex uses the tunnel 127.0.0.1:18007): +3. **Restore the newest copy** (from any machine that reaches ep0's PBS on 8007 — DooPlex uses the tunnel 127.0.0.1:18007): ```bash export PBS_PASSWORD_FILE= PBS_FINGERPRINT= R='dooplex-hub@pbs!restore@:8007:felhom-offsite' @@ -187,14 +194,14 @@ the read-only token (or ep0 root to mint a new one: Step 2). sqlite3 -readonly out/hub.db 'PRAGMA integrity_check' # must print: ok ``` If DooPlex's `ep0-copy` datastore survived, the same copy is there too (pulled nightly). -3. **Prove the seal key matches BEFORE putting the copy in place** — on a COPY of `out/hub.db` (the check migrates it): +4. **Prove the seal key matches BEFORE putting the copy in place** — on a COPY of `out/hub.db` (the check migrates it): ```bash cd felhom.eu/hub && go build -o hubdb-check ./cmd/hubdb-check printf '%s' "" > k; chmod 600 k # from the password manager — not on a command line in a shared shell ./hubdb-check copy-of-hub.db k # want: hosts=N console_passwords_opened=N failed=0; exit 0 ``` `failed>0` means the wrong seal key: the hub would start but could open no console password (`05` §16.2). -4. **A k3s with the `felhom` ArgoCD app**, and `Secret/offsite-secret-key` recreated with the SAME value: +5. **A k3s with the `felhom` ArgoCD app**, and `Secret/offsite-secret-key` recreated with the SAME value: `kubectl -n felhom-system create secret generic offsite-secret-key --from-file=OFFSITE_SECRET_KEY=k`. **Corrected 2026-10-09:** the Deployment also needs, NOT optional, `Secret/resend-api` (`RESEND_API_KEY`), `Secret/report-api` (`REPORT_API_KEY`) and `Secret/gitea-creds` (`username`, `password`); without them the pod does @@ -202,7 +209,7 @@ the read-only token (or ep0 root to mint a new one: Step 2). off-site (`runbooks/gitea-restore.md`, `secrets/*.gpg`, opened with DooPlex's restic passphrase). A test uses dummies. The hub's image is pulled from Gitea's registry — on a rebuild with no registry, `docker save` it from any machine that has it, or build it from the restored code. -5. **Into the PVC:** scale `deploy/hub` to 0; put `out/hub.db` into the volume as `/data/hub.db` (a helper pod mounting +6. **Into the PVC:** scale `deploy/hub` to 0; put `out/hub.db` into the volume as `/data/hub.db` (a helper pod mounting `hub-data`; delete any `hub.db-wal`/`-shm` there — the snapshot is a whole database); scale to 1. The start-up log line `console passwords sealed at rest (0 legacy plaintext row(s) sealed now)` and one reveal on a host page confirm it. As run 2026-10-09: `kubectl scale deploy/hub --replicas=0`; a `busybox` pod mounting `hub-data` at `/data`; diff --git a/documentation/runbooks/total-loss-of-dooplex.md b/documentation/runbooks/total-loss-of-dooplex.md index f0b3b2a6..4caebb65 100644 --- a/documentation/runbooks/total-loss-of-dooplex.md +++ b/documentation/runbooks/total-loss-of-dooplex.md @@ -69,7 +69,7 @@ local restic repos; Prometheus history; DooPlex's other homelab apps (not Felhom 7. **Gitea:** `gitea-restore.md` (on a normal network this time). Then rebuild the images from the code into the new registry: hub, controller, agent (`RUNBOOK-manual-build.md`). 8. **The hub:** k3s, then `RUNBOOK-hub-db-offsite-backup.md` §3 with S2 and S4 — **start it with mail held** - (`MAIL-HOLD`, from the next hub release; until then: no network until the pending notices are understood). + (§3 step 1: the `MAIL-HOLD` marker, from the next hub release; until then: no network until the pending notices are understood). 9. **DNS:** in Cloudflare (S11) point `hub.felhom.eu` (today a CNAME to `dooplex.hopto.org`) and `gitea.dooplex.hu` at the new place. The boxes reconnect by themselves: their API keys are in the restored hub DB. 10. **Signing:** with S6 on paper, put the keys back and sign as before. Without it, no box accepts an agent update or a diff --git a/hub/CHANGELOG.md b/hub/CHANGELOG.md index c576321f..ca17a95d 100644 --- a/hub/CHANGELOG.md +++ b/hub/CHANGELOG.md @@ -1,3 +1,38 @@ +## Unreleased — a household's cleared mail address is deleted (R-922) and a restored hub starts quiet (MAIL-HOLD) (2026-10-09) + +Not released: ships with the next hub release. No version bump here. + +- **Two log lines no longer print a household's mail address** (found while building R-922, fixed without a row): the + preferences push logs `address set=` and the dispatcher logs `Customer email sent for /`. + +- **R-922 — the household clears its mail address, the hub deletes it** (operator ruling 2026-10-09 11:19, option A). + `POST /api/v1/preferences` decodes a new boolean `email_cleared`, which the controller sends true ONLY when the + household deliberately cleared the address on its dashboard. `email_cleared: true` + empty `email` → the stored + notification address is deleted (INFO `household cleared its mail address — deleted`). Every other empty push keeps + today's F12 no-clobber guard (v0.71.0) unchanged; the flag beside a non-empty address is an ordinary update. Older + controllers never send the flag, so nothing changes for them. `TestSavePreferences_DeliberateClearDeletesAddress` + (red-proved: the address stayed), `_ClearFlagWithAddressIsAnUpdate`, `_ClearFlagFalseKeepsAddress`; the F12 test + re-proved red. The operator-registered address in `customer_configs.email` is a separate copy and is NOT touched + (the kernel notice, claim and self-bind mails read it). `scripts/wire_contract_gate.py` gains the root + `controller -> hub (POST /preferences)` (seen convicting a planted controller `email_cleared` against a hub without + it). Privacy-notice draft: one retention row for the notification address. +- **MAIL-HOLD — a restored hub starts quiet.** Evidence: the 2026-10-09 restore drill (`documentation/audits/ + dooplex-survival-2026-10-09/partD-05-checks.txt`) — a hub started on a restored DB mailed households pending kernel + notices within a minute; `notifications.operator_enabled: false` did not stop household mail. New package + `internal/mailhold`: while `/MAIL-HOLD` exists the hub sends **no e-mail on any path** — the dispatcher + (household + operator events, recovery, test mail, kernel notice, claim codes, self-bind link; all through ONE + `deliver()` gate), the legacy `/api/v1/notify`, and the app-mail relay `/api/v1/mail` (503 while held). Each held mail + is logged once, `[WARN] MAIL-HOLD: not sending to ` (no address, no body), and recorded + `held` in the notification log. **Held mails are dropped, not re-sent** — a restored copy's pending mails were decided + from a snapshot of the past; the cooldown a dropped mail armed is given back, so the first real alarm after the + release is not silenced. A held kernel notice returns an error, so the kernel step does not run (no mail, no step). + An unreadable marker state counts as held (fail-closed). The operator lifts it with `POST + /configuration/mail-hold/release` (operator auth + the R-135 CSRF gate; a button on Configuration); every operator page + shows an amber banner while held. Tests: `TestMailHold_*` (notify, api, web), `TestHold_*`, + `TestMailHoldBanner_*` (both branches), `TestMailHoldRelease_*`; the route joined `r135PostRoutes`. Red-proved: gate + skipped → 7 mails sent; cooldown not given back → 0 mails after the release; banner func false → no banner; release + without `Release()` → marker stays; CSRF gate open → a Basic-only POST released the hold. + ## v0.144.0 — the operator's decision sheet (D1, D2, D3, D6, D7) and the morning's R-243, R-901, R-304, R-415 (2026-10-09) Release together with controller 0.304.0 (D1's operator actions need both). No wire change an older box cannot read. diff --git a/hub/cmd/hub/main.go b/hub/cmd/hub/main.go index b75ff9d8..50b8fa36 100644 --- a/hub/cmd/hub/main.go +++ b/hub/cmd/hub/main.go @@ -21,6 +21,7 @@ import ( "gitea.dooplex.hu/admin/felhom-hub/internal/gitea" "gitea.dooplex.hu/admin/felhom-hub/internal/hetznerapi" "gitea.dooplex.hu/admin/felhom-hub/internal/intent" + "gitea.dooplex.hu/admin/felhom-hub/internal/mailhold" "gitea.dooplex.hu/admin/felhom-hub/internal/mailrelay" "gitea.dooplex.hu/admin/felhom-hub/internal/monitor" "gitea.dooplex.hu/admin/felhom-hub/internal/notify" @@ -319,6 +320,17 @@ func main() { ) apiHandler.SetDispatcher(dispatcher) + // MAIL-HOLD (internal/mailhold): while /MAIL-HOLD exists the hub sends NO e-mail on any path — + // dispatcher, /notify, app-mail relay. A hub started on a restored database mailed households within a minute + // (2026-10-09 restore drill); the marker makes a restored copy start quiet. Lifted by the operator with + // POST /configuration/mail-hold/release; held mails are dropped, not re-sent. + mailHold := mailhold.New(cfg.Server.DataDir, logger) + if mailHold.Held() { + logger.Printf("[WARN] MAIL-HOLD: marker present at start (%s) — the hub sends no e-mail until the operator releases it on the Configuration page", mailHold.Path) + } + dispatcher.SetMailHold(mailHold) + apiHandler.SetMailHold(mailHold) + // Customer-claim password arc (v0.50.0, F-4): the code engine — the dispatcher delivers the // Hungarian emails, the store holds bcrypt(code) only. Wired into the API (Day-0 issue at // config retrieve, live-box issue + claimed ingest + ACK on report, reset endpoint) and the @@ -341,6 +353,8 @@ func main() { webServer := web.New(dataStore, cfg.Auth.PasswordHash, cfg.API.ReportAPIKey, Version, staleThreshold, logger) webServer.SetTemplateFetcher(templateFetcher) webServer.SetAssetManager(assetsMgr) + // MAIL-HOLD banner on every operator page + the release button on Configuration. + webServer.SetMailHold(mailHold) webServer.SetClaimEngine(claimEngine) // v0.50.0 — Setup-tab claim chip + resend button webServer.SetSelfBindMailer(dispatcher) // v0.66.0 (R-27) — customer self-bind link button (sibling of claim mailer) webServer.SetEventEmitter(dispatcher.ProcessEvent) // v0.135.0 (R-604) — the "floor raise skipped boxes" mail diff --git a/hub/internal/api/handler.go b/hub/internal/api/handler.go index 54080d9a..e1626919 100644 --- a/hub/internal/api/handler.go +++ b/hub/internal/api/handler.go @@ -20,6 +20,7 @@ import ( "gitea.dooplex.hu/admin/felhom-hub/internal/claim" "gitea.dooplex.hu/admin/felhom-hub/internal/configgen" "gitea.dooplex.hu/admin/felhom-hub/internal/intent" + "gitea.dooplex.hu/admin/felhom-hub/internal/mailhold" "gitea.dooplex.hu/admin/felhom-hub/internal/mailrelay" "gitea.dooplex.hu/admin/felhom-hub/internal/notify" "gitea.dooplex.hu/admin/felhom-hub/internal/store" @@ -67,7 +68,9 @@ type Handler struct { floorNotes sync.Map // App-email passthrough (POST /api/v1/mail). nil sender = endpoint returns 503. - mailSender mailrelay.Sender + mailSender mailrelay.Sender + // mailHold is the hub-wide mail gate (MAIL-HOLD marker; internal/mailhold). nil never holds. + mailHold *mailhold.Hold mailLimiter *mailRateLimiter mailFromAllow map[string]bool @@ -156,6 +159,12 @@ func New(store *store.Store, apiKey, resendAPIKey, fromEmail string, templatePro } } +// SetMailHold wires the hub-wide mail gate: while the MAIL-HOLD marker exists, /notify and the app-mail relay send +// nothing (internal/mailhold). +func (h *Handler) SetMailHold(m *mailhold.Hold) { + h.mailHold = m +} + // SetDispatcher sets the notification dispatcher for event-triggered emails. func (h *Handler) SetDispatcher(d *notify.Dispatcher) { h.dispatcher = d @@ -2583,6 +2592,14 @@ func (h *Handler) handleNotify(w http.ResponseWriter, r *http.Request) { return } + // MAIL-HOLD (internal/mailhold): a restored or quarantined hub sends nothing. The mail is dropped, not queued. + if err := h.mailHold.Check("customer event "+payload.EventType, payload.CustomerID); err != nil { + h.store.LogNotification(payload.CustomerID, payload.EventType, payload.Severity, payload.Message, "held", "MAIL-HOLD marker present — dropped", "customer") + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"status":"ok","sent":false,"reason":"mail_hold"}`)) + return + } + subject, emailBody := formatNotificationEmail(payload.CustomerID, payload.EventType, payload.Severity, payload.Message, payload.Details) sendErr := h.sendResendEmail(prefs.Email, subject, emailBody) if sendErr != nil { @@ -2617,6 +2634,10 @@ func (h *Handler) handleSavePreferences(w http.ResponseWriter, r *http.Request) Email string `json:"email"` EnabledEvents []string `json:"enabled_events"` CooldownHours int `json:"cooldown_hours"` + // EmailCleared (R-922, operator ruling 2026-10-09 option A) is sent true by the controller ONLY when the + // household deliberately cleared its address on the dashboard. Omitted (false) on every other push — + // including every push from a controller older than the one that emits it. + EmailCleared bool `json:"email_cleared"` } if err := json.Unmarshal(body, &payload); err != nil || payload.CustomerID == "" { http.Error(w, "Invalid payload: customer_id required", http.StatusBadRequest) @@ -2627,8 +2648,17 @@ func (h *Handler) handleSavePreferences(w http.ResponseWriter, r *http.Request) // (e.g. an unconfigured box) must never wipe a stored non-empty address — the seeded/edited // email is the customer's alert lifeline. Events + cooldown from the push still apply; a push // with a non-empty email updates everything (customer edits keep working). + // + // R-922: the ONE exception is a deliberate clear — `email_cleared: true` with an empty address. The household + // removed its address, so the hub deletes it (personal data the household withdrew does not stay on our side). + // The guard cannot tell a clear from an unconfigured box by the address alone, which is why the clear travels + // as its own flag. A flag beside a NON-empty address is an ordinary update (the address wins). Pinned by + // TestSavePreferences_DeliberateClearDeletesAddress, TestSavePreferences_EmptyEmailCannotClobber and + // TestSavePreferences_ClearFlagWithAddressIsAnUpdate. saveEmail := payload.Email - if saveEmail == "" { + if saveEmail == "" && payload.EmailCleared { + h.logger.Printf("[INFO] Notification prefs push for %s: household cleared its mail address — deleted", payload.CustomerID) + } else if saveEmail == "" { if existing, err := h.store.GetNotificationPrefs(payload.CustomerID); err == nil && existing != nil && existing.Email != "" { saveEmail = existing.Email h.logger.Printf("[INFO] Notification prefs push for %s had empty email — preserving stored address", payload.CustomerID) @@ -2641,7 +2671,7 @@ func (h *Handler) handleSavePreferences(w http.ResponseWriter, r *http.Request) return } - h.logger.Printf("[INFO] Notification preferences updated for %s: email=%s, events=%v", payload.CustomerID, saveEmail, payload.EnabledEvents) + h.logger.Printf("[INFO] Notification preferences updated for %s: address set=%t, events=%v", payload.CustomerID, saveEmail != "", payload.EnabledEvents) // never the address (R-922) w.WriteHeader(http.StatusOK) w.Write([]byte(`{"status":"ok"}`)) } diff --git a/hub/internal/api/mail.go b/hub/internal/api/mail.go index 66167a42..60d7e688 100644 --- a/hub/internal/api/mail.go +++ b/hub/internal/api/mail.go @@ -144,6 +144,17 @@ func (h *Handler) handleMail(w http.ResponseWriter, r *http.Request) { return } + // MAIL-HOLD (internal/mailhold): the relay sends nothing while the hub holds mail. 503 — a temporary refusal the + // box's shim surfaces to the app; the hub keeps no copy (held mail is dropped, never queued). + holdWho := custID + if holdWho == "" { + holdWho = "a global-key caller" + } + if err := h.mailHold.Check("app mail", holdWho); err != nil { + http.Error(w, "Mail is on hold at the hub", http.StatusServiceUnavailable) + return + } + ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second) defer cancel() if err := h.mailSender.Send(ctx, req.RawMIME, req.MailFrom, req.RcptTo); err != nil { diff --git a/hub/internal/api/mailhold_test.go b/hub/internal/api/mailhold_test.go new file mode 100644 index 00000000..571af06f --- /dev/null +++ b/hub/internal/api/mailhold_test.go @@ -0,0 +1,87 @@ +package api + +import ( + "io" + "net/http" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "gitea.dooplex.hu/admin/felhom-hub/internal/mailhold" +) + +// MAIL-HOLD on the two API send paths (the dispatcher's paths: internal/notify/mailhold_test.go). +// COMPANION RED-PROOF (REPORT): remove the h.mailHold.Check block from handleNotify / handleMail → these fail with the +// mail handed to Resend (the recorded transport / the fake SMTP sender). + +func heldHold(t *testing.T) *mailhold.Hold { + t.Helper() + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, mailhold.FileName), nil, 0o644); err != nil { + t.Fatal(err) + } + return mailhold.New(dir, nil) +} + +// recordingTransport stands in for api.resend.com: it records each request and never leaves the process. +type recordingTransport struct { + mu sync.Mutex + calls int +} + +func (rt *recordingTransport) RoundTrip(r *http.Request) (*http.Response, error) { + rt.mu.Lock() + rt.calls++ + rt.mu.Unlock() + return &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader(`{"id":"x"}`)), Header: http.Header{}}, nil +} + +func TestMailHold_NotifySendsNothingWhileHeld(t *testing.T) { + h, st := newEventTestHandler(t) + rt := &recordingTransport{} + h.httpClient = &http.Client{Transport: rt} + h.resendAPIKey = "test-key" + if err := st.SaveNotificationPrefs("c1", "household@example.hu", []string{"backup_failed"}, 6); err != nil { + t.Fatal(err) + } + h.SetMailHold(heldHold(t)) + + rr := do(h, http.MethodPost, "/notify", "ckey", `{"customer_id":"c1","event_type":"backup_failed","severity":"error","message":"m"}`) + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, body=%s", rr.Code, rr.Body.String()) + } + if rt.calls != 0 { + t.Fatalf("MAIL-HOLD present but /notify handed %d mail(s) to Resend", rt.calls) + } + if !strings.Contains(rr.Body.String(), `"reason":"mail_hold"`) { + t.Fatalf("the answer must say the mail was held: %s", rr.Body.String()) + } +} + +func TestMailHold_AppMailRelayRefusesWhileHeld(t *testing.T) { + h, st, _ := newTestHandler(t) + withCustomer(t, st, "c1", "ckey") + fake := &fakeSender{} + h.SetMailRelay(fake, 30, []string{"felhom.eu"}) + hold := heldHold(t) + h.SetMailHold(hold) + + rr := do(h, "POST", "/mail", "ckey", mailBody(t, "vaultwarden@felhom.eu")) + if fake.callCount() != 0 { + t.Fatalf("MAIL-HOLD present but the relay sent %d mail(s)", fake.callCount()) + } + if rr.Code != http.StatusServiceUnavailable { + t.Fatalf("held relay: status %d, want 503", rr.Code) + } + + // Released: the relay sends again. + if err := hold.Release(); err != nil { + t.Fatal(err) + } + rr = do(h, "POST", "/mail", "ckey", mailBody(t, "vaultwarden@felhom.eu")) + if rr.Code != http.StatusOK || fake.callCount() != 1 { + t.Fatalf("after release: status %d, sends %d — want 200 and 1", rr.Code, fake.callCount()) + } +} diff --git a/hub/internal/api/preferences_guard_test.go b/hub/internal/api/preferences_guard_test.go index e2dc0786..7e48a945 100644 --- a/hub/internal/api/preferences_guard_test.go +++ b/hub/internal/api/preferences_guard_test.go @@ -65,3 +65,64 @@ func TestSavePreferences_EmptyEmailNoStoredRow(t *testing.T) { t.Fatalf("all-off push must store the empty row unchanged, got %+v", prefs) } } + +// TestSavePreferences_DeliberateClearDeletesAddress (R-922, operator ruling 2026-10-09 option A): when the household +// clears its address on the dashboard, the controller pushes `email_cleared: true` with an empty address, and the +// hub DELETES the stored address. Seen on Tester 1 before the fix: the push answered 200 and the old address stayed. +// Red-proof: with the `payload.EmailCleared` branch removed, this fails with email="seeded@example.hu". +func TestSavePreferences_DeliberateClearDeletesAddress(t *testing.T) { + h, st := newEventTestHandler(t) + if err := st.SaveNotificationPrefs("c1", "seeded@example.hu", []string{"node_down"}, 6); err != nil { + t.Fatalf("stored prefs: %v", err) + } + + rr := do(h, http.MethodPost, "/preferences", "ckey", + `{"customer_id":"c1","email":"","email_cleared":true,"enabled_events":[],"cooldown_hours":6}`) + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, body=%s", rr.Code, rr.Body.String()) + } + prefs, err := st.GetNotificationPrefs("c1") + if err != nil || prefs == nil { + t.Fatalf("prefs: %+v err=%v", prefs, err) + } + if prefs.Email != "" { + t.Fatalf("a deliberate clear must delete the stored address, still stored: email=%q", prefs.Email) + } +} + +// TestSavePreferences_ClearFlagWithAddressIsAnUpdate (R-922): `email_cleared: true` beside a NON-empty address is an +// ordinary update — the address in the push wins, it is never deleted. +func TestSavePreferences_ClearFlagWithAddressIsAnUpdate(t *testing.T) { + h, st := newEventTestHandler(t) + if err := st.SaveNotificationPrefs("c1", "old@example.hu", []string{"node_down"}, 6); err != nil { + t.Fatalf("stored prefs: %v", err) + } + + rr := do(h, http.MethodPost, "/preferences", "ckey", + `{"customer_id":"c1","email":"new@example.hu","email_cleared":true,"enabled_events":["node_down"],"cooldown_hours":6}`) + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, body=%s", rr.Code, rr.Body.String()) + } + prefs, _ := st.GetNotificationPrefs("c1") + if prefs == nil || prefs.Email != "new@example.hu" { + t.Fatalf("flag + non-empty address must store the pushed address, got %+v", prefs) + } +} + +// TestSavePreferences_ClearFlagFalseKeepsAddress (R-922): an explicit `email_cleared: false` with an empty address is +// NOT a clear — the F12 no-clobber guard still holds. +func TestSavePreferences_ClearFlagFalseKeepsAddress(t *testing.T) { + h, st := newEventTestHandler(t) + if err := st.SaveNotificationPrefs("c1", "seeded@example.hu", []string{"node_down"}, 6); err != nil { + t.Fatalf("stored prefs: %v", err) + } + rr := do(h, http.MethodPost, "/preferences", "ckey", + `{"customer_id":"c1","email":"","email_cleared":false,"enabled_events":["node_down"],"cooldown_hours":6}`) + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, body=%s", rr.Code, rr.Body.String()) + } + prefs, _ := st.GetNotificationPrefs("c1") + if prefs == nil || prefs.Email != "seeded@example.hu" { + t.Fatalf("email_cleared:false with an empty address must keep the stored address, got %+v", prefs) + } +} diff --git a/hub/internal/mailhold/mailhold.go b/hub/internal/mailhold/mailhold.go new file mode 100644 index 00000000..acdebf08 --- /dev/null +++ b/hub/internal/mailhold/mailhold.go @@ -0,0 +1,88 @@ +// Package mailhold is the hub's one mail gate: while the marker file `/MAIL-HOLD` exists, the hub sends +// NO e-mail at all — household or operator, on every path that reaches Resend (the dispatcher's HTTP-API sends, the +// legacy /notify endpoint, the app-mail SMTP relay). +// +// WHY. The 2026-10-09 restore drill (documentation/audits/dooplex-survival-2026-10-09/partD-05-checks.txt) started a +// hub on a restored database, and within a minute it mailed households the pending "kernel notice" mails. +// `notifications.operator_enabled: false` did not stop it, because that switch covers the operator channel only. A +// restored copy is a second hub speaking with the first one's voice; it must start quiet until the operator says so. +// The marker is created (an empty file is enough) before a restored copy's first start. +// +// HELD MAILS ARE DROPPED, NOT QUEUED, and that is deliberate: every mail a restored copy wants to send was decided +// from a snapshot of the past — a kernel notice for a step the real hub already ran or will run, an alarm about a +// state that has since changed. Re-sending them after the hold is lifted would deliver stale news as if it were +// current. After the release, the hub's checkers and boxes produce fresh mails from live state on their own. Each +// dropped mail is logged once (`[WARN] MAIL-HOLD: not sending to ` — never the address, never the body). +// +// The marker is read on every send (one stat of a file in the data dir; no wait on a device the hub does not +// already depend on for its database). A stat error other than "does not exist" counts as HELD: the gate fails +// closed, and the operator pages show the banner, so the state is never invisible. +package mailhold + +import ( + "errors" + "log" + "os" + "path/filepath" +) + +// FileName is the marker's name inside the hub's data directory. +const FileName = "MAIL-HOLD" + +// ErrHeld is returned by Check for a mail the hold stopped. Callers treat it as "not sent" (never as sent). +var ErrHeld = errors.New("mail hold: the hub is not sending e-mail (MAIL-HOLD marker present)") + +// Hold reads and removes the marker. The zero value and a nil *Hold never hold (tests and wiring without a data dir). +type Hold struct { + Path string + Logger *log.Logger +} + +// New returns the hold for a data directory. +func New(dataDir string, logger *log.Logger) *Hold { + return &Hold{Path: filepath.Join(dataDir, FileName), Logger: logger} +} + +// Held reports whether the marker is present (fail-closed on an unreadable state). +func (h *Hold) Held() bool { + if h == nil || h.Path == "" { + return false + } + _, err := os.Stat(h.Path) + if err == nil { + return true + } + if errors.Is(err, os.ErrNotExist) { + return false + } + h.logf("[WARN] MAIL-HOLD: cannot read the marker state (%v) — treating mail as HELD", err) + return true +} + +// Check is the gate every sender calls immediately before handing a mail to Resend. When held it logs the one WARN +// line for this mail and returns ErrHeld; otherwise nil. kind names the mail ("customer event backup_failed", +// "kernel notice", …); who is a customer ID or "operator" — never an address. +func (h *Hold) Check(kind, who string) error { + if !h.Held() { + return nil + } + h.logf("[WARN] MAIL-HOLD: not sending %s to %s", kind, who) + return ErrHeld +} + +// Release removes the marker. Removing a marker that is not there is not an error (the release is idempotent). +func (h *Hold) Release() error { + if h == nil || h.Path == "" { + return nil + } + if err := os.Remove(h.Path); err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + return nil +} + +func (h *Hold) logf(format string, args ...interface{}) { + if h != nil && h.Logger != nil { + h.Logger.Printf(format, args...) + } +} diff --git a/hub/internal/mailhold/mailhold_test.go b/hub/internal/mailhold/mailhold_test.go new file mode 100644 index 00000000..62f8a68c --- /dev/null +++ b/hub/internal/mailhold/mailhold_test.go @@ -0,0 +1,59 @@ +package mailhold + +import ( + "bytes" + "errors" + "log" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestHold_MarkerDecidesAndReleaseRemovesIt(t *testing.T) { + dir := t.TempDir() + buf := &bytes.Buffer{} + h := New(dir, log.New(buf, "", 0)) + if h.Held() { + t.Fatal("no marker: must not hold") + } + if err := h.Check("kernel notice", "c1"); err != nil { + t.Fatalf("no marker: Check = %v", err) + } + if err := os.WriteFile(filepath.Join(dir, FileName), nil, 0o644); err != nil { + t.Fatal(err) + } + if err := h.Check("kernel notice", "c1"); !errors.Is(err, ErrHeld) { + t.Fatalf("marker present: Check = %v, want ErrHeld", err) + } + if got := buf.String(); !strings.Contains(got, "[WARN] MAIL-HOLD: not sending kernel notice to c1") { + t.Fatalf("held mail log line missing: %q", got) + } + if err := h.Release(); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(dir, FileName)); !os.IsNotExist(err) { + t.Fatalf("Release left the marker: %v", err) + } + if err := h.Release(); err != nil { + t.Fatalf("a second Release must be a no-op, got %v", err) + } +} + +// Fail-closed: a marker state that cannot be read (here: the data dir is a FILE, so stat says ENOTDIR) holds mail. +func TestHold_UnreadableStateHolds(t *testing.T) { + f := filepath.Join(t.TempDir(), "not-a-dir") + if err := os.WriteFile(f, nil, 0o644); err != nil { + t.Fatal(err) + } + if !New(f, nil).Held() { + t.Fatal("an unreadable marker state must count as HELD") + } +} + +func TestHold_NilNeverHolds(t *testing.T) { + var h *Hold + if h.Held() || h.Check("x", "y") != nil || h.Release() != nil { + t.Fatal("a nil hold must never hold and never fail") + } +} diff --git a/hub/internal/notify/dispatcher.go b/hub/internal/notify/dispatcher.go index f998701f..de2df761 100644 --- a/hub/internal/notify/dispatcher.go +++ b/hub/internal/notify/dispatcher.go @@ -3,6 +3,7 @@ package notify import ( "bytes" "encoding/json" + "errors" "fmt" "io" "log" @@ -11,6 +12,7 @@ import ( "sync" "time" + "gitea.dooplex.hu/admin/felhom-hub/internal/mailhold" "gitea.dooplex.hu/admin/felhom-hub/internal/store" "gitea.dooplex.hu/admin/felhom-hub/internal/i18n" @@ -40,8 +42,32 @@ type Dispatcher struct { // afterFn schedules a delayed call (time.AfterFunc; a seam so tests run the retries at once). Used by // retryOperatorEmail (the lost alarm of 2026-10-05). afterFn func(time.Duration, func()) + + // mailHold is the hub-wide mail gate (internal/mailhold): while its marker exists, deliver() hands nothing to + // sendEmailFn. nil never holds. + mailHold *mailhold.Hold } +// SetMailHold wires the hub-wide mail gate (the MAIL-HOLD marker in the data dir). +func (d *Dispatcher) SetMailHold(m *mailhold.Hold) { + d.mailHold = m +} + +// deliver is the ONE way the dispatcher sends a mail: the MAIL-HOLD gate, then sendEmailFn. Every send in this file +// goes through it, so the hold cannot be bypassed by a path someone adds later without calling sendEmailFn directly +// (pinned by TestMailHold_EverySendPathIsGated). kind names the mail; who is a customer ID or "operator" — the hold's +// log line carries those two and never the address. A held mail returns mailhold.ErrHeld and is DROPPED, never +// queued: see the package comment for why. +func (d *Dispatcher) deliver(kind, who, to, subject, body string, headers map[string]string) error { + if err := d.mailHold.Check(kind, who); err != nil { + return err + } + return d.sendEmailFn(to, subject, body, headers) +} + +// isHeld reports whether a send error is the mail hold (not a delivery failure). +func isHeld(err error) bool { return errors.Is(err, mailhold.ErrHeld) } + // operatorRetryDelays are the waits before each further try of an operator mail whose send FAILED (the lost alarm of 2026-10-05). Measured // 2026-10-05: demo-hp's whole_guest_backup_failed (error) hit a 10 s Resend client timeout, was logged `failed`, and was // never sent — the only operator mail of 692 that failed, and the one alarm the operator needed that night. The @@ -77,7 +103,12 @@ func (d *Dispatcher) retryOperatorEmail(customerID, eventType, severity, message } d.afterFn(operatorRetryDelays[attempt], func() { n := attempt + 1 - if err := d.sendEmailFn(d.operatorEmail, subject, body, headers); err != nil { + if err := d.deliver("operator mail "+eventType+" (retry)", "operator", d.operatorEmail, subject, body, headers); err != nil { + if isHeld(err) { + // The hold began between tries: the mail is dropped like every other held mail, and the retries stop. + d.store.LogNotification(customerID, eventType, severity, message, "held", "MAIL-HOLD marker present — dropped", "operator") + return + } d.logger.Printf("[ERROR] Operator email retry %d failed for %s/%s: %v", n, customerID, eventType, err) d.store.LogNotification(customerID, eventType, severity, message, "failed", fmt.Sprintf("retry %d: %v", n, err), "operator") d.retryOperatorEmail(customerID, eventType, severity, message, subject, body, headers, n) @@ -246,7 +277,9 @@ func (d *Dispatcher) sendTestEmail(customerID string) { subject := b.Msg(lang, "mail.test.subject") body := b.Msg(lang, "mail.test.body") - if err := d.sendEmailFn(prefs.Email, subject, body, nil); err != nil { + if err := d.deliver("test mail", customerID, prefs.Email, subject, body, nil); isHeld(err) { + d.store.LogNotification(customerID, "test", "info", "Teszt értesítés", "held", "MAIL-HOLD marker present — dropped", "customer") + } else if err != nil { d.logger.Printf("[ERROR] Test email to %s failed: %v", prefs.Email, err) d.store.LogNotification(customerID, "test", "info", "Teszt értesítés", "failed", err.Error(), "customer") } else { @@ -268,7 +301,10 @@ headers render correctly. The customer test mail result is recorded in the notification log. Dashboard: https://hub.felhom.eu/customers/%s`, customerID, customerID) - if err := d.sendEmailFn(d.operatorEmail, opSubject, opBody, priorityHeaders("critical")); err != nil { + if err := d.deliver("operator test mail", "operator", d.operatorEmail, opSubject, opBody, priorityHeaders("critical")); isHeld(err) { + d.store.LogNotification(customerID, "test", "info", "operator test copy", "held", "MAIL-HOLD marker present — dropped", "operator") + return + } else if err != nil { d.logger.Printf("[ERROR] Operator test email failed for %s: %v", customerID, err) d.store.LogNotification(customerID, "test", "info", "operator test copy", "failed", err.Error(), "operator") return @@ -330,7 +366,11 @@ func (d *Dispatcher) processRecovery(customerID, eventType, severity, message, m subject, body := FormatCustomerEmail(d.store.CustomerLanguage(customerID), customerID, eventType, severity, message, messageCustomer, detailsJSON) - if err := d.sendEmailFn(prefs.Email, subject, body, priorityHeaders(severity)); err != nil { + if err := d.deliver("customer recovery mail "+eventType, customerID, prefs.Email, subject, body, priorityHeaders(severity)); isHeld(err) { + d.forgetCustomerCooldown(cooldownKey) + d.store.LogNotification(customerID, eventType, severity, message, "held", "MAIL-HOLD marker present — dropped", "customer") + return + } else if err != nil { d.logger.Printf("[ERROR] Customer recovery email failed for %s/%s: %v", customerID, eventType, err) d.store.LogNotification(customerID, eventType, severity, message, "failed", err.Error(), "customer") return @@ -607,7 +647,15 @@ func (d *Dispatcher) processOperator(customerID, eventType, severity, message, d subject, body := FormatOperatorEmail(customerID, eventType, severity, message, detailsJSON) hdrs := priorityHeaders(severity) - if err := d.sendEmailFn(d.operatorEmail, subject, body, hdrs); err != nil { + if err := d.deliver("operator mail "+eventType, "operator", d.operatorEmail, subject, body, hdrs); isHeld(err) { + // Dropped, and the cooldown it armed is given back: the first real alarm after the release must not be + // silenced by a mail that was never sent. + d.mu.Lock() + delete(d.opCooldowns, cooldownKey) + d.mu.Unlock() + d.store.LogNotification(customerID, eventType, severity, message, "held", "MAIL-HOLD marker present — dropped", "operator") + return + } else if err != nil { d.logger.Printf("[ERROR] Operator email failed for %s/%s: %v — retrying in %v", customerID, eventType, err, operatorRetryDelays[0]) d.store.LogNotification(customerID, eventType, severity, message, "failed", err.Error(), "operator") d.retryOperatorEmail(customerID, eventType, severity, message, subject, body, hdrs, 0) @@ -883,12 +931,16 @@ func (d *Dispatcher) processCustomer(customerID, eventType, severity, message, m subject, body := FormatCustomerEmail(d.store.CustomerLanguage(customerID), customerID, eventType, severity, message, messageCustomer, detailsJSON) - if err := d.sendEmailFn(prefs.Email, subject, body, priorityHeaders(severity)); err != nil { + if err := d.deliver("customer mail "+eventType, customerID, prefs.Email, subject, body, priorityHeaders(severity)); isHeld(err) { + d.forgetCustomerCooldown(cooldownKey) + d.store.LogNotification(customerID, eventType, severity, message, "held", "MAIL-HOLD marker present — dropped", "customer") + return + } else if err != nil { d.logger.Printf("[ERROR] Customer email failed for %s/%s: %v", customerID, eventType, err) d.store.LogNotification(customerID, eventType, severity, message, "failed", err.Error(), "customer") return } - d.logger.Printf("[INFO] Customer email sent to %s for %s/%s", prefs.Email, customerID, eventType) + d.logger.Printf("[INFO] Customer email sent for %s/%s", customerID, eventType) // never the address (R-922) d.store.LogNotification(customerID, eventType, severity, message, "sent", "", "customer") } @@ -944,6 +996,13 @@ func (d *Dispatcher) replyToFor(to string) string { return op } +// forgetCustomerCooldown gives back a customer cooldown armed for a mail the hold dropped. +func (d *Dispatcher) forgetCustomerCooldown(key string) { + d.mu.Lock() + delete(d.custCooldowns, key) + d.mu.Unlock() +} + func isEventEnabled(enabledEvents []string, eventType string) bool { for _, e := range enabledEvents { if e == eventType { @@ -964,7 +1023,10 @@ func (d *Dispatcher) SendClaimEmail(kind, customerID, email, domain, code string } subject, body := FormatClaimEmail(d.store.CustomerLanguage(customerID), kind, customerID, domain, code) eventType := "claim_" + kind - if err := d.sendEmailFn(email, subject, body, nil); err != nil { + if err := d.deliver("claim "+kind+" mail", customerID, email, subject, body, nil); isHeld(err) { + d.store.LogNotification(customerID, eventType, "info", subject, "held", "MAIL-HOLD marker present — dropped", "customer") + return err + } else if err != nil { d.logger.Printf("[ERROR] claim %s email to customer %s failed: %v", kind, customerID, err) d.store.LogNotification(customerID, eventType, "info", subject, "failed", err.Error(), "customer") return err @@ -985,7 +1047,10 @@ func (d *Dispatcher) SendSelfBindEmail(customerID, email, link string) error { return fmt.Errorf("notify: no resend api key") } subject, body := FormatSelfBindEmail(d.store.CustomerLanguage(customerID), customerID, link) - if err := d.sendEmailFn(email, subject, body, nil); err != nil { + if err := d.deliver("self-bind link mail", customerID, email, subject, body, nil); isHeld(err) { + d.store.LogNotification(customerID, "selfbind_link", "info", subject, "held", "MAIL-HOLD marker present — dropped", "customer") + return err + } else if err != nil { d.logger.Printf("[ERROR] self-bind link email to customer %s failed: %v", customerID, err) d.store.LogNotification(customerID, "selfbind_link", "info", subject, "failed", err.Error(), "customer") return err @@ -1018,7 +1083,11 @@ func (d *Dispatcher) SendKernelNotice(customerID, kver string) (string, error) { } lang := d.store.CustomerLanguage(customerID) subject, body := FormatKernelNoticeEmail(lang) - if err := d.sendEmailFn(to, subject, body, nil); err != nil { + if err := d.deliver("kernel notice", customerID, to, subject, body, nil); isHeld(err) { + // The caller runs no kernel step on an error (no mail, no step) — exactly right for a held hub. + d.store.LogNotification(customerID, "kernel_notice", "info", subject, "held", "MAIL-HOLD marker present — dropped", "customer") + return lang, err + } else if err != nil { d.logger.Printf("[ERROR] kernel notice (%s) to customer %s failed: %v", kver, customerID, err) d.store.LogNotification(customerID, "kernel_notice", "info", subject, "failed", err.Error(), "customer") return lang, err diff --git a/hub/internal/notify/mailhold_test.go b/hub/internal/notify/mailhold_test.go new file mode 100644 index 00000000..54aa7431 --- /dev/null +++ b/hub/internal/notify/mailhold_test.go @@ -0,0 +1,143 @@ +package notify + +import ( + "bytes" + "log" + "os" + "path/filepath" + "regexp" + "strings" + "sync" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-hub/internal/mailhold" + "gitea.dooplex.hu/admin/felhom-hub/internal/store" +) + +// MAIL-HOLD — a restored hub starts quiet. Evidence: the 2026-10-09 restore drill +// (documentation/audits/dooplex-survival-2026-10-09/partD-05-checks.txt): a hub started on a restored database mailed +// households pending kernel notices within a minute, and `notifications.operator_enabled: false` did not stop it. +// +// COMPANION RED-PROOF (REPORT): make deliver() skip the mailHold.Check call (the pre-fix shape: sendEmailFn is reached +// directly) → TestMailHold_DispatcherSendsNothingWhileHeld fails with every path's mail SENT. + +type holdRecorder struct { + mu sync.Mutex + sent []string // subjects +} + +func (r *holdRecorder) fn(to, subject, body string, headers map[string]string) error { + r.mu.Lock() + defer r.mu.Unlock() + r.sent = append(r.sent, subject) + return nil +} + +func (r *holdRecorder) count() int { + r.mu.Lock() + defer r.mu.Unlock() + return len(r.sent) +} + +// holdDispatcher returns a dispatcher whose household c1 has a registered address, notification prefs with +// backup_failed on, the operator channel ON, the MAIL-HOLD marker present, and a log captured in buf. +func holdDispatcher(t *testing.T) (*Dispatcher, *store.Store, *holdRecorder, *mailhold.Hold, *bytes.Buffer) { + t.Helper() + st := newDispStore(t) + if err := st.SaveCustomerConfig(&store.CustomerConfig{CustomerID: "c1", APIKey: "k", RetrievalPassword: "p", Email: "household@example.hu"}); err != nil { + t.Fatal(err) + } + if err := st.SaveNotificationPrefs("c1", "household@example.hu", []string{"backup_failed"}, 6); err != nil { + t.Fatal(err) + } + buf := &bytes.Buffer{} + logger := log.New(buf, "", 0) + d := NewDispatcher(st, "test-key", "from@felhom.eu", "op@felhom.eu", true, logger) + rec := &holdRecorder{} + d.sendEmailFn = rec.fn + d.afterFn = func(time.Duration, func()) {} // a retry must never fire in these tests + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, mailhold.FileName), nil, 0o644); err != nil { + t.Fatal(err) + } + hold := mailhold.New(dir, logger) + d.SetMailHold(hold) + return d, st, rec, hold, buf +} + +// The consequence asserted: with the marker present, NOT ONE mail reaches the sender, on any dispatcher path — +// household event, operator event, recovery, test mail, kernel notice, claim code, self-bind link. +func TestMailHold_DispatcherSendsNothingWhileHeld(t *testing.T) { + d, st, rec, _, buf := holdDispatcher(t) + + d.ProcessEvent("c1", "backup_failed", "error", "Backup failed", "", "box") // operator + household + d.ProcessEvent("c1", "test", "info", "test", "", "operator") // test mail, both channels + if _, err := d.SendKernelNotice("c1", "6.8.12-1"); err == nil { + t.Errorf("kernel notice: a held mail must report an error (the caller then runs no kernel step)") + } + if err := d.SendClaimEmail("claim", "c1", "household@example.hu", "c1.example", "CODE-1234"); err == nil { + t.Errorf("claim mail: a held mail must report an error") + } + if err := d.SendSelfBindEmail("c1", "household@example.hu", "https://hub.example/bind/x"); err == nil { + t.Errorf("self-bind mail: a held mail must report an error") + } + + if n := rec.count(); n != 0 { + t.Fatalf("MAIL-HOLD present but %d mail(s) were SENT: %v", n, rec.sent) + } + out := buf.String() + if got := strings.Count(out, "[WARN] MAIL-HOLD: not sending"); got < 7 { + t.Errorf("each held mail is logged once: want >= 7 MAIL-HOLD lines, got %d\n%s", got, out) + } + if strings.Contains(out, "household@example.hu") || strings.Contains(out, "op@felhom.eu") { + t.Errorf("the hold's log must carry no address:\n%s", out) + } + if strings.Contains(out, "CODE-1234") { + t.Errorf("the hold's log must carry no mail body") + } + // The record says what happened: held, not sent and not failed. + if _, ok, _ := st.LastCustomerSentAt("c1", []string{"backup_failed"}); ok { + t.Errorf("a held household mail was recorded as SENT") + } +} + +// After the release the hub mails again — and the very same event key mails at once: the cooldown a held mail armed is +// given back, so the first real alarm after the release is not silenced by a mail that was never sent. +func TestMailHold_ReleaseResumesSends(t *testing.T) { + d, _, rec, hold, _ := holdDispatcher(t) + + d.ProcessEvent("c1", "backup_failed", "error", "Backup failed", "", "box") + if rec.count() != 0 { + t.Fatalf("held: %d mail(s) sent", rec.count()) + } + if err := hold.Release(); err != nil { + t.Fatalf("release: %v", err) + } + if hold.Held() { + t.Fatalf("the marker is still there after Release") + } + d.ProcessEvent("c1", "backup_failed", "error", "Backup failed", "", "box") + if n := rec.count(); n != 2 { + t.Fatalf("after the release the operator AND the household mail must go out: sent=%d %v", n, rec.sent) + } + // Held mails are DROPPED, not re-sent: exactly the two fresh ones, no backlog. +} + +// Belt for the "one gate" claim: in dispatcher.go the sender seam is called in exactly ONE place, inside deliver(). +// A new send path that calls sendEmailFn directly would bypass the hold; this fails when one appears. +func TestMailHold_EverySendPathIsGated(t *testing.T) { + src, err := os.ReadFile("dispatcher.go") + if err != nil { + t.Fatal(err) + } + calls := regexp.MustCompile(`d\.sendEmailFn\(`).FindAllIndex(src, -1) + if len(calls) != 1 { + t.Fatalf("dispatcher.go calls d.sendEmailFn( %d times; every send must go through deliver() (the MAIL-HOLD gate)", len(calls)) + } + body := string(src) + i := strings.Index(body, "func (d *Dispatcher) deliver(") + if i < 0 || calls[0][0] < i || calls[0][0] > i+600 { + t.Fatalf("the one d.sendEmailFn( call is not inside deliver()") + } +} diff --git a/hub/internal/web/funcmap_test.go b/hub/internal/web/funcmap_test.go index afe71344..0b5bd12e 100644 --- a/hub/internal/web/funcmap_test.go +++ b/hub/internal/web/funcmap_test.go @@ -50,6 +50,7 @@ func TestTemplatesParseWithFuncmap(t *testing.T) { "memoryColor": memoryColor, "accuracyClass": accuracyClass, "gt": func(a, b int) bool { return false }, + "mailHeld": func() bool { return false }, }).ParseFS(templateFS, "templates/*.html"); err != nil { t.Fatalf("templates failed to parse: %v", err) } diff --git a/hub/internal/web/mailhold_test.go b/hub/internal/web/mailhold_test.go new file mode 100644 index 00000000..ab8dbcb1 --- /dev/null +++ b/hub/internal/web/mailhold_test.go @@ -0,0 +1,135 @@ +package web + +import ( + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-hub/internal/mailhold" +) + +// MAIL-HOLD on the operator UI: the banner on every operator page while the marker exists, and the release endpoint +// behind the operator's auth + the R-135 CSRF gate. + +const mailHoldBannerText = "E-mail is on hold" + +func holdIn(t *testing.T, held bool) (*mailhold.Hold, string) { + t.Helper() + dir := t.TempDir() + marker := filepath.Join(dir, mailhold.FileName) + if held { + if err := os.WriteFile(marker, nil, 0o644); err != nil { + t.Fatal(err) + } + } + return mailhold.New(dir, nil), marker +} + +func getPage(t *testing.T, s *Server, path string) string { + t.Helper() + r := httptest.NewRequest(http.MethodGet, path, nil) + w := httptest.NewRecorder() + s.ServeHTTP(w, r) + if w.Code != http.StatusOK { + t.Fatalf("GET %s: %d", path, w.Code) + } + return w.Body.String() +} + +// Both branches of the {{if mailHeld}} gate, on the dashboard and on Configuration (which also carries the button). +func TestMailHoldBanner_RendersOnlyWhileHeld(t *testing.T) { + for _, held := range []bool{true, false} { + s, _ := newTestServer(t) + h, _ := holdIn(t, held) + s.SetMailHold(h) + for _, p := range []string{"/", "/configuration", "/hosts", "/configs"} { + body := getPage(t, s, p) + if got := strings.Contains(body, mailHoldBannerText); got != held { + t.Errorf("held=%v GET %s: banner shown=%v", held, p, got) + } + } + cfg := getPage(t, s, "/configuration") + if got := strings.Contains(cfg, `action="/configuration/mail-hold/release"`); got != held { + t.Errorf("held=%v: release button shown=%v", held, got) + } + } +} + +// No hold wired at all (nil): no banner, pages render. +func TestMailHoldBanner_NilHoldRendersNoBanner(t *testing.T) { + s, _ := newTestServer(t) + if strings.Contains(getPage(t, s, "/"), mailHoldBannerText) { + t.Fatal("a nil hold must render no banner") + } +} + +// The operator's release, through the production wiring (RequireAuth around ServeHTTP) with a session and its CSRF +// token: the marker is gone, and the banner with it. +func TestMailHoldRelease_RemovesMarker(t *testing.T) { + s, h := r135Handler(t) + hold, marker := holdIn(t, true) + s.SetMailHold(hold) + s.sessionsMu.Lock() + s.sessions["sess1"] = &hubSession{expiresAt: time.Now().Add(time.Hour), csrfToken: "tok1"} + s.sessionsMu.Unlock() + + w := r135Post(h, "/configuration/mail-hold/release", func(r *http.Request) { + r.AddCookie(&http.Cookie{Name: SessionCookieName, Value: "sess1"}) + r.Header.Set("X-CSRF-Token", "tok1") + }) + if w.Code != http.StatusSeeOther || !strings.Contains(w.Header().Get("Location"), "flash=mail_hold_released") { + t.Fatalf("release: %d Location=%q", w.Code, w.Header().Get("Location")) + } + if _, err := os.Stat(marker); !os.IsNotExist(err) { + t.Fatalf("the marker is still there after the release: %v", err) + } + if hold.Held() { + t.Fatal("still held after the release") + } +} + +// Refusals: Basic auth without the operator header (the R-135 cross-site shape), a session without its token, and no +// credentials at all. The marker stays in every case. +func TestMailHoldRelease_RefusedWithoutOperatorAuthOrCSRF(t *testing.T) { + s, h := r135Handler(t) + hold, marker := holdIn(t, true) + s.SetMailHold(hold) + s.sessionsMu.Lock() + s.sessions["sess1"] = &hubSession{expiresAt: time.Now().Add(time.Hour), csrfToken: "tok1"} + s.sessionsMu.Unlock() + + cases := []struct { + name string + mut func(*http.Request) + ok func(int) bool + }{ + {"basic without operator header", func(r *http.Request) { r.SetBasicAuth("", "op-pass") }, func(c int) bool { return c == http.StatusForbidden }}, + {"session without token", func(r *http.Request) { r.AddCookie(&http.Cookie{Name: SessionCookieName, Value: "sess1"}) }, func(c int) bool { return c == http.StatusForbidden }}, + {"no credentials", func(r *http.Request) { r.Header.Set(OperatorCLIHeader, "cli") }, func(c int) bool { return c == http.StatusFound || c == http.StatusUnauthorized }}, + } + for _, c := range cases { + w := r135Post(h, "/configuration/mail-hold/release", c.mut) + if !c.ok(w.Code) { + t.Errorf("%s: status %d", c.name, w.Code) + } + if _, err := os.Stat(marker); err != nil { + t.Fatalf("%s: the marker was removed by a refused request: %v", c.name, err) + } + } + + // The CLI shape (Basic + the operator header) passes. + w := r135Post(h, "/configuration/mail-hold/release", func(r *http.Request) { + r.SetBasicAuth("", "op-pass") + r.Header.Set(OperatorCLIHeader, "cli") + }) + if w.Code != http.StatusSeeOther { + t.Fatalf("operator CLI release: %d", w.Code) + } + if hold.Held() { + t.Fatal("operator CLI release left the hold in place") + } +} diff --git a/hub/internal/web/r135_csrf_test.go b/hub/internal/web/r135_csrf_test.go index a2fdb306..e5a97e6f 100644 --- a/hub/internal/web/r135_csrf_test.go +++ b/hub/internal/web/r135_csrf_test.go @@ -42,6 +42,7 @@ var r135PostRoutes = []string{ "/configuration/global-floor", "/configuration/artifacts", "/configuration/password", + "/configuration/mail-hold/release", "/configs/c1/delete", "/configs/c1/edit", "/configs/c1/offsite-reissue", diff --git a/hub/internal/web/server.go b/hub/internal/web/server.go index 4f5ec022..b8fe439c 100644 --- a/hub/internal/web/server.go +++ b/hub/internal/web/server.go @@ -20,6 +20,7 @@ import ( "gitea.dooplex.hu/admin/felhom-hub/internal/claim" "gitea.dooplex.hu/admin/felhom-hub/internal/gitea" "gitea.dooplex.hu/admin/felhom-hub/internal/intent" + "gitea.dooplex.hu/admin/felhom-hub/internal/mailhold" "gitea.dooplex.hu/admin/felhom-hub/internal/monitor" "gitea.dooplex.hu/admin/felhom-hub/internal/offsite" "gitea.dooplex.hu/admin/felhom-hub/internal/poke" @@ -124,13 +125,20 @@ type Server struct { // beforeCreateSave is a TEST seam: called in handleConfigCreate after the duplicate check, before the // save. nil in production. beforeCreateSave func(customerID string) + // mailHold is the hub-wide mail gate (internal/mailhold); nil never holds. + mailHold *mailhold.Hold // cfAPIBase is a TEST seam for the Cloudflare token reach check (R-138 option C): "" = the real API. cfAPIBase string } // New creates a new web server. func New(store *store.Store, passwordHash, apiKey, version string, staleThreshold time.Duration, logger *log.Logger) *Server { + // srv is assigned below; template funcs that read server state close over it (they run at render time only). + var srv *Server funcMap := template.FuncMap{ + // mailHeld drives the MAIL-HOLD banner on every operator page (internal/mailhold). Read at render time, so the + // banner disappears the moment the marker is gone. Pinned by TestMailHoldBanner_*. + "mailHeld": func() bool { return srv != nil && srv.mailHold.Held() }, "timeAgo": timeAgo, "timeAgoPtr": func(t *time.Time) string { if t == nil { @@ -160,7 +168,7 @@ func New(store *store.Store, passwordHash, apiKey, version string, staleThreshol tmpl := template.Must(template.New("").Funcs(funcMap).ParseFS(templateFS, "templates/*.html")) - return &Server{ + srv = &Server{ store: store, configPasswordHash: passwordHash, apiKey: apiKey, @@ -171,6 +179,7 @@ func New(store *store.Store, passwordHash, apiKey, version string, staleThreshol sessions: make(map[string]*hubSession), bindLimiter: newBindRateLimiter(30), // public /bind/ surface: 30 req/min/IP burst (R-27) } + return srv } // effectivePasswordHash returns the operator login password bcrypt hash in force: the UI-set DB @@ -406,6 +415,29 @@ func (s *Server) artifactChoices(ctx context.Context, pkg, file string) []artifa } // SetEventEmitter wires the notification dispatcher (R-604). INIT-ONLY. +// SetMailHold wires the hub-wide mail gate (internal/mailhold): the banner on every operator page and the release +// endpoint. nil = never held (no banner, the release is a no-op). +func (s *Server) SetMailHold(m *mailhold.Hold) { + s.mailHold = m +} + +// handleMailHoldRelease lifts the MAIL-HOLD: it removes the marker, and the hub sends e-mail again from the next mail +// on. Mails dropped during the hold are NOT re-sent (internal/mailhold explains why). Operator-only: it sits behind +// RequireAuth and the R-135 CSRF gate in ServeHTTP like every other state-changing route. +func (s *Server) handleMailHoldRelease(w http.ResponseWriter, r *http.Request) { + if s.mailHold == nil || !s.mailHold.Held() { + http.Redirect(w, r, "/configuration?flash=mail_hold_not_held", http.StatusSeeOther) + return + } + if err := s.mailHold.Release(); err != nil { + s.logger.Printf("[ERROR] MAIL-HOLD: release failed — the marker is still in place: %v", err) + http.Redirect(w, r, "/configuration?flash=mail_hold_release_failed", http.StatusSeeOther) + return + } + s.logger.Printf("[INFO] MAIL-HOLD released by the operator from %s — the hub sends e-mail again; mail held until now was dropped", r.RemoteAddr) + http.Redirect(w, r, "/configuration?flash=mail_hold_released", http.StatusSeeOther) +} + func (s *Server) SetEventEmitter(f func(customerID, eventType, severity, message, detailsJSON, source string)) { s.emit = f } @@ -668,6 +700,12 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { } else { http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) } + case path == "/configuration/mail-hold/release": + if r.Method == http.MethodPost { + s.handleMailHoldRelease(w, r) + } else { + http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) + } case path == "/configuration/password": if r.Method == http.MethodPost { s.handleChangePassword(w, r) diff --git a/hub/internal/web/templates/app_detail.html b/hub/internal/web/templates/app_detail.html index cc52a990..07156416 100644 --- a/hub/internal/web/templates/app_detail.html +++ b/hub/internal/web/templates/app_detail.html @@ -23,6 +23,7 @@ Configuration + {{template "mail_hold_banner"}} ← Apps diff --git a/hub/internal/web/templates/apps.html b/hub/internal/web/templates/apps.html index 28561f64..a5829d4b 100644 --- a/hub/internal/web/templates/apps.html +++ b/hub/internal/web/templates/apps.html @@ -21,6 +21,7 @@ Configuration + {{template "mail_hold_banner"}}

App Telemetry

diff --git a/hub/internal/web/templates/config_form.html b/hub/internal/web/templates/config_form.html index ece1a6a1..c97692d9 100644 --- a/hub/internal/web/templates/config_form.html +++ b/hub/internal/web/templates/config_form.html @@ -22,6 +22,7 @@ Configuration + {{template "mail_hold_banner"}} ← Back

{{if .IsNew}}Add Customer{{else}}Edit: {{.Config.CustomerID}}{{end}}

diff --git a/hub/internal/web/templates/configs.html b/hub/internal/web/templates/configs.html index d689c014..1b791405 100644 --- a/hub/internal/web/templates/configs.html +++ b/hub/internal/web/templates/configs.html @@ -21,6 +21,7 @@ Configuration + {{template "mail_hold_banner"}} {{if .Flash}}
diff --git a/hub/internal/web/templates/configuration.html b/hub/internal/web/templates/configuration.html index c539b38c..ad1d50a0 100644 --- a/hub/internal/web/templates/configuration.html +++ b/hub/internal/web/templates/configuration.html @@ -21,9 +21,26 @@ Configuration + {{template "mail_hold_banner"}}

Configuration

+ {{if mailHeld}} +
+ + + Removes the marker. The hub sends e-mail again from the next mail on; mail held until now is not re-sent. +
+ {{end}} + {{if eq .Flash "mail_hold_released"}} +
Mail hold released — the hub sends e-mail again. Mail held until now was not re-sent.
+ {{end}} + {{if eq .Flash "mail_hold_release_failed"}} +
The mail hold could not be released — the marker is still in place. Check the hub log.
+ {{end}} + {{if eq .Flash "mail_hold_not_held"}} +
The mail hold was not on — nothing to release.
+ {{end}} {{if eq .Flash "assets_refreshed"}}
Assets refreshed successfully from image seed.
{{end}} diff --git a/hub/internal/web/templates/customer_unified.html b/hub/internal/web/templates/customer_unified.html index 91321aba..a3f32cc8 100644 --- a/hub/internal/web/templates/customer_unified.html +++ b/hub/internal/web/templates/customer_unified.html @@ -42,6 +42,7 @@

No reports received yet

{{end}} + {{template "mail_hold_banner"}} {{if .Flash}}
diff --git a/hub/internal/web/templates/dashboard.html b/hub/internal/web/templates/dashboard.html index 24c9c3a2..0524c536 100644 --- a/hub/internal/web/templates/dashboard.html +++ b/hub/internal/web/templates/dashboard.html @@ -22,6 +22,7 @@ Configuration + {{template "mail_hold_banner"}} {{if or .OffsiteTile .PBSTile}}
diff --git a/hub/internal/web/templates/host_detail.html b/hub/internal/web/templates/host_detail.html index ddbaab28..081b2f30 100644 --- a/hub/internal/web/templates/host_detail.html +++ b/hub/internal/web/templates/host_detail.html @@ -21,6 +21,7 @@ Configuration + {{template "mail_hold_banner"}} ← Hosts diff --git a/hub/internal/web/templates/hosts.html b/hub/internal/web/templates/hosts.html index 20e82295..483c7d2e 100644 --- a/hub/internal/web/templates/hosts.html +++ b/hub/internal/web/templates/hosts.html @@ -22,6 +22,7 @@ Configuration + {{template "mail_hold_banner"}}

Hosts

diff --git a/hub/internal/web/templates/log_tail.html b/hub/internal/web/templates/log_tail.html index 9451eaf5..2a7a4342 100644 --- a/hub/internal/web/templates/log_tail.html +++ b/hub/internal/web/templates/log_tail.html @@ -21,6 +21,7 @@ Configuration + {{template "mail_hold_banner"}} ← {{.CustomerID}} diff --git a/hub/internal/web/templates/mail_hold.html b/hub/internal/web/templates/mail_hold.html new file mode 100644 index 00000000..dd168d3b --- /dev/null +++ b/hub/internal/web/templates/mail_hold.html @@ -0,0 +1,8 @@ +{{define "mail_hold_banner"}}{{if mailHeld}} + +{{end}}{{end}} diff --git a/hub/internal/web/templates/offsite.html b/hub/internal/web/templates/offsite.html index 7d67288c..ba921764 100644 --- a/hub/internal/web/templates/offsite.html +++ b/hub/internal/web/templates/offsite.html @@ -21,6 +21,7 @@ Configuration + {{template "mail_hold_banner"}}

Offsite

diff --git a/hub/internal/web/templates/style.css b/hub/internal/web/templates/style.css index a4f5aa61..b7050dc2 100644 --- a/hub/internal/web/templates/style.css +++ b/hub/internal/web/templates/style.css @@ -1020,3 +1020,14 @@ body.js-tabs .tab-panel:not(.tab-panel-active) { display: none; } @media (prefers-reduced-motion: reduce) { *, *::before, *::after { animation: none !important; transition: none !important; } } + +/* MAIL-HOLD release (internal/mailhold): the button and its one-line explanation sit on one row under the banner. */ +.mail-hold-release { + display: flex; + align-items: center; + gap: 0.75rem; + flex-wrap: wrap; + margin-bottom: 1rem; +} +.mail-hold-release .text-muted { font-size: 0.8rem; } +#mail-hold a { color: var(--text-1); } diff --git a/hub/internal/web/templates/system.html b/hub/internal/web/templates/system.html index 74cec460..8cf9e551 100644 --- a/hub/internal/web/templates/system.html +++ b/hub/internal/web/templates/system.html @@ -30,6 +30,7 @@ Configuration + {{template "mail_hold_banner"}}

System — versions and OS updates

diff --git a/scripts/wire_contract_gate.py b/scripts/wire_contract_gate.py index 920863c3..3bac628b 100644 --- a/scripts/wire_contract_gate.py +++ b/scripts/wire_contract_gate.py @@ -94,6 +94,10 @@ ROOTS = [ # ACK's operator_actions list. Named type for the same reason as R-311's root above. ("hub -> controller (report ACK, `operator_actions` entry)", "hub", "internal/store", "OperatorActionDirective", "controller"), + # R-922 (2026-10-09): the notification-prefs push. Declared when the household's deliberate clear + # (`email_cleared`) joined it — a flag the hub cannot decode would leave the cleared address stored. + ("controller -> hub (POST /preferences)", + "controller", "internal/notify", "preferencesRequest", "hub"), ] # R-315: a root whose receiver decodes it into ONE NAMED MIRROR TYPE gets the stronger check —