From e3bcafb6d970173fc163d0d7237247f36aa5b659 Mon Sep 17 00:00:00 2001 From: Mix Date: Mon, 12 Oct 2026 23:15:34 +0300 Subject: [PATCH] audit: decisions of 2026-10-12 written into the plan Stage 0 then 1+2 agreed; H4 folds logins; H3 measured without a screen (delivery_log_days, decision rule, no card); L5 validated at save; L12 the send log gets domain_id; L8 the reply names no other domain; L9 local comment and test only; stage 5 taken whole. Migrations renumbered 0007-0009. Co-Authored-By: Claude Fable 5.1 --- docs/audit-2026-10-12.md | 112 +++++++++++++++++++++++++++++---------- 1 file changed, 83 insertions(+), 29 deletions(-) diff --git a/docs/audit-2026-10-12.md b/docs/audit-2026-10-12.md index 8f0576b..d0b47a5 100644 --- a/docs/audit-2026-10-12.md +++ b/docs/audit-2026-10-12.md @@ -552,30 +552,54 @@ says so; the order is my recommendation by harm per hour. does; `reconcile` already treats an error as no information. Test: a 5 MiB fake `postqueue -p`. — **Opus** 2. **Reconcile only closes `queued` rows** (`AND status = 'queued'` in the - reconcile path's update) and **only while the tailer is reading**: `follow` - publishes "healthy" (last successful drain within N ticks) and - `reconcileLoop` skips a sweep when it is not. Test: reopen fails → no rows - closed. — **Opus** -3. **Rotation window**: after switching inodes keep draining the old one until - it has not grown for one full tick *and* the new file has received a line, - or for a bounded number of ticks. Test on the stand with `postfix reload` - timing. — **Opus** -4. **Anchor `deliveryRe`** at the head of the line like its two siblings; + reconcile path's update) and marks what it closed: `reason = 'reconcile'` + on the row (the 0006 column; a new value, no schema change). — **Opus** +3. **Measure before deciding the sweep's fate** (owner, 2026-10-12: the + keep / remove / `lost` question needs real data, and today the only + record of a sweep is a stdout line that dies with the container). A table + `delivery_log_days` (migration `0007`, DDL only; a row per UTC day, kept a + year, the shape of `sender_check_days`) with four counters: + `reconcile_closed` (rows the sweep closed); `reconcile_contradicted` (a + `reason = 'reconcile'` row the tailer later rewrote from a `status=` line — + the log wins, and the rewrite is counted); `tailer_blind_seconds` + (`follow()` unable to read: reopen failing, file gone — the sender check's + downtime mechanics); `queued_over_lifetime` (rows still `queued` past + `maximal_queue_lifetime` at the day's end — what removing the sweep would + leave visible). **No screen for this** (owner, 2026-10-12: nothing is + drawn that ordinary operation will not need): the table is read off a + copy of the database when the month is up. Whether a durable *Delivery + log* card (tailer blindness, stuck rows — the two counters that outlive + the decision) earns a place on Health is a separate question, asked only + if the data shows those numbers are ever non-zero. + Decision rule, written now: `closed = 0` for a month → remove the sweep; + `closed > 0, contradicted = 0` → keep; `contradicted > 0` → each case + explained by `tailer_blind` means fix the tailer and keep the sweep, + otherwise `lost` or remove. Decide after 30 days of data, or sooner if + `contradicted` is ever non-zero. — **Opus** +4. **Deferred until the data is in:** pausing the sweep while the tailer is + not reading (publish "healthy" from `follow`, skip the sweep otherwise), + and the rotation window (keep draining the old inode until it has not + grown for a tick and the new file has a line). Both are fixes to the + tailer's blindness and stay on the list whatever the sweep's fate; their + priority follows `tailer_blind_seconds`. +5. **Anchor `deliveryRe`** at the head of the line like its two siblings; test with the probed HELO and quoted-sender lines. — **Opus** -5. **TLS re-copy** (H2): `postfix-cert-reload.sh` compares the mounted files +6. **TLS re-copy** (H2): `postfix-cert-reload.sh` compares the mounted files to the copy and re-copies before `postfix reload` (atomically, same modes); e2e replaces the cert in the stand and reads the new serial off 465. — **Opus** -6. **Deferred counter** (M13): on the stand, `kill -STOP` the panel mid-message +7. **Deferred counter** (M13): on the stand, `kill -STOP` the panel mid-message and record which program logs the warning; widen the matcher accordingly and replace the composed fixture with the seen lines. — **Opus** -7. Docs: architecture § Log tailer, § Queue reconcile, § Sender check "what it - costs"; guide § Delivery log. — **Sonnet** +8. Docs: architecture § Log tailer, § Queue reconcile (the measurement and + the rule), § Sender check "what it costs"; guide § Delivery log, § Health. + — **Sonnet** -### Stage 2 — the tenant boundary and the registry (H4, M7, M9, M10) → in `2.5.0` (migration `0007`) +### Stage 2 — the tenant boundary and the registry (H4, M7, M9, M10) → in `2.5.0` (migration `0008`) -1. **Logins lower-cased** at validation and `applications.login COLLATE - NOCASE` (migration rebuilds the unique index). Before the migration a check +1. **Logins lower-cased** at validation (owner, 2026-10-12: fold, do not + refuse) and `applications.login COLLATE NOCASE` (migration `0008` rebuilds + the unique index). Before the migration a check that no two existing logins differ only in case (there is one installation; look). Tests: `Ops`/`ops` refused; the map written lower- case. Then verify Postfix's compare on the stand once (`MAIL FROM` as the @@ -588,16 +612,37 @@ says so; the order is my recommendation by harm per hour. 4. **Import validates like the form**: route `DMARCRua`, IPs, limit mode, window and count through the same parsers; refuse the file with the validator's text. Tests per field. — **Opus** -5. Docs: security.md § Roles (the guard), guide § Domain export. — **Sonnet** +5. **Send-log rows belong to a domain by id** (L12; owner, 2026-10-12). + `send_log.domain_id INTEGER REFERENCES domains(id) ON DELETE SET NULL` in + migration `0008`, backfilled once by name against the registry as it is at + migration time (rows of a domain already gone stay NULL); written by the + journal milter and by the sender check's refusal rows at insert, looked up + from the login's domain; the `domain` name column stays for display and + for the global view. A domain user's log, stats, auto rate limit and the + notifier's refusal count select by id; a NULL row is nobody's but the + global role's. A domain deleted and re-added under its old name therefore + starts with an empty log. The inbound journal (`inbound_spam_log`, the + same accepted risk in security.md:419) gets the same treatment in the same + migration — my proposal, so that the two logs do not follow two rules. + Tests: delete-and-re-add shows nothing to the new assignee; the old rows + are still in the global log. — **Opus** +6. Docs: security.md § Roles (the guard) and the struck accepted risk, guide + § Domain export, schema-migrations.md. — **Sonnet** +7. **The 550 reply names no other domain** (L8): `sender.go:334` stops at + "it names another sender of this server"; the refused message's page + still shows the `From:` line as written (`header_from`), so the operator + loses nothing. Test pins the reply text; e2e's sender-check negatives + assert the reply does not contain the other tenant's name. — **Opus** ### Stage 3 — persistence and restore (M1–M5, L1–L4, L7) → `2.5.1` 1. **Consume the manifest after the resync succeeds** (`CheckRestore` reports; `run` deletes after `resyncAfterRestore` returns nil). Test: Resync fails → manifest kept → second boot resyncs. — **Opus** -2. **Indexes**: `idx_send_log_domain_created_at (domain, created_at)`; +2. **Indexes**: `idx_send_log_domain_created_at (domain_id, created_at)` — + on the id column stage 2 adds, replacing `idx_send_log_domain`; partial `idx_send_log_queued (created_at) WHERE status = 'queued'`; - consider `(domain, id)` for the scoped page. Migration `0008` DDL only; + consider `(domain, id)` for the scoped page. Migration `0009` DDL only; `TestLimitRefusalsSince`-style EXPLAIN tests against the **real** query strings. — **Opus** 3. **CLI backup**: open the source `mode=ro`, refuse a missing file; password @@ -629,6 +674,12 @@ says so; the order is my recommendation by harm per hour. the flash and the two comments say what it covers. — **Sonnet** 5. `domain.Service.Delete`: say the domain is deleted when a later step fails, and run `apps.Resync()` anyway. — **Opus** +6. **Upstream TLS mode validated at save** (L5): saving an inbound domain + whose `[host]:port` is already used by another domain with a different + TLS mode is refused with the validator's text naming that domain and its + mode; `TestRenderInboundMaps` gets the two-routes-one-next-hop case and the + map is asserted to hold one key. Guide § Inbound domains says a next hop + has one TLS mode. — **Opus** ### Stage 5 — image and Postfix hardening (M17–M19, M21, L30–L32) → `2.6.0` @@ -676,14 +727,17 @@ says so; the order is my recommendation by harm per hour. ### Not planned, said out loud -- L5 (`tls_policy` per next-hop): the right fix is a model change (TLS mode - per upstream, not per domain) and a form change — a design item, not a - patch. Until then the guide should say two domains to one upstream share - the first one's TLS mode. -- L12 (rows by domain name): accept it for outbound as security.md already - does for inbound, and write it there. -- L8 (the signing-name oracle): the names are in DNS; shortening the reply - costs a less useful error. Leave, note in security.md § Accepted risks. -- L9 (go-milter header accumulation): a one-line comment on `Headers` and a - test with a polluted `h`; the library behaviour itself is upstream's. +- L5 (`tls_policy` per next-hop): the full fix is a model change (TLS mode per + upstream, not per domain) — a design item. Decided (owner, 2026-10-12): + validate instead — stage 4, item 6. +- L12 (rows by domain name): decided (owner, 2026-10-12): the send log gets + the domain's id — stage 2, item 5. +- L8 (the signing-name oracle): decided (owner, 2026-10-12): do it — stage 2, + item 7. (The names are not public: rDNS gives the hostname only, so the + reply told one tenant which other domains are hosted here.) +- L9 (go-milter header accumulation): decided (owner, 2026-10-12): locally + only — a comment on `senderSession.Headers` saying why `h` is ignored, and + `TestSenderCheckResetsBetweenMessages` passing an `h` that still holds the + first message's `From:` (stage 2, with item 7). The library itself stays + as vendored; an upstream patch is a separate, unhurried matter. - The Windows-bound tests: not worth platform guards; CI is Linux.