Files
selfpost/docs/audit-2026-10-12.md
T
mixandClaude Opus 5.5 5be109b871
test / test (push) Canceled after 0s
release: 2.5.0: audit stages 1 and 2 - the delivery log and the tenant boundary
Stage 1: a cut queue listing no longer bounces held mail; the reconcile
sweep closes only queued rows, by id, marked "reconcile", and its fate is
measured (delivery_log_days, migration 0007); delivery lines are read at
the head of the line; a renewed TLS certificate is served without a
restart; Health counts the message a hung panel deferred.
Stage 2: logins folded to lower case and unique without regard to case;
the last global administrator kept inside the transaction, users saved
whole; a domain import held to the forms' rules; log rows by domain id
(migration 0008); the 550 reply names no other domain; which key OpenDKIM
signs with, tested. Plus the findings of both reviews of each stage.

Rolled out to the test server as 2.4.2-post.ef18ef2 (the same code):
migrations 0007 and 0008 ran on the live database - schema 8, 68/68 send-log
and 10/10 inbound rows tied to their domain, integrity ok.

CHANGELOG cut; pin 2.5.0 in deploy/docker-compose.yml, README (with the
local-build note), guide; development.md "from pin"; roadmap row and the
recommended order; stages 1 and 2 marked shipped in the plan. No tag (the
owner's command only).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-14 16:22:04 +03:00

1028 lines
66 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Code audit — 2026-10-12
**What this is.** A full-tree audit of the code and infrastructure at `8f226bd`
(release `2.4.0`): leftovers, unfinished work, defects, performance, doc drift
and test gaps — explicit ones (there are no `TODO`/`FIXME` markers anywhere in
the product code) and the implicit ones that only reading finds. The panel's
visual design (mockups, `panel.css`, layout) is a separate audit and is not
touched here. Nothing was changed in the tree; this file is the only output.
**Method.** Six parallel readers, one per zone (mail path; background roles;
store/backup/composition root; HTTP surface; view and services; build/deploy/
CI/e2e), each reading every file of its zone and its tests in full, each
told to find where the code does not do what `architecture.md` says, and to
prove it (a throwaway test, `EXPLAIN QUERY PLAN`, a probe) rather than guess.
Machine checks: `go build`, `go vet`, `gofmt -l`, `go mod tidy`/`verify`,
`staticcheck`, `govulncheck`, `go test ./...`. Five claims the readers could
not settle from source were settled on the test server
(`selfpost.mixfed.ru`, running a locally built `2.3.0-post.c817f08`) and
against GHCR. Two hypotheses were refuted there and are **not** findings:
`saslpasswd2 -d` of a missing login exits 0 (deletion is idempotent, as the
code says), and a duplicate key in a `texthash:` map is a warning with the
first entry winning, not a fatal error.
**Machine results.** Build, vet, gofmt, tidy, verify: clean. `staticcheck`:
four items (an unused `domainIDSet` in `handlers/authz.go:63`, two unused
assignments in `handlers_outbound_pages_test.go:174,194`, one `S1003` in
`view/pages_test.go:170`). `govulncheck`: nine standard-library findings
(`net/http` HTTP/2, `net/textproto`, `html/template`), all fixed in the
current Go patch release; `golang.org/x/crypto` `0.54.0` has three published
fixes in `0.56.0` that this code does not call. `modernc.org/sqlite` is seven
minor versions behind (`1.53.0` → `1.60.1`). `go test ./...`: green on Linux
semantics; on Windows two tests fail for OS reasons (`TestWriteLoadPrivateKeyRoundtrip`
expects a POSIX mode; `TestFollowTailsAndRotates` renames an open file) — the
tests, not the product, are platform-bound.
Severity: **critical** — mail or data lost or the security boundary crossed
today; **high** — a documented guarantee is false in a way an operator will
hit; **medium** — wrong under a realistic condition, or a stated guard that is
not there; **low** — wrong under an unlikely condition, or cost without
failure; **nit** — hygiene. Confidence is stated where a finding was not
proven.
---
## Findings
### High
**H1. Every published release image is stamped `VERSION=e2e`, so the
backup/restore version check never checks anything.**
[release.yml:75-84](../.github/workflows/release.yml) builds the image with
`VERSION=X.Y.Z` and tags it `selfpost:e2e`; the e2e step then runs
`docker compose build` through
[test/e2e/compose.override.yml:23-32](../test/e2e/compose.override.yml),
which rebuilds **the same tag** with `args: VERSION: e2e`; `docker tag
selfpost:e2e …X.Y.Z-amd64` pushes the rebuilt one. Confirmed: `docker run
ghcr.io/mixeme/selfpost:1.3.0 panel -version` prints `e2e`. Consequences:
the footer and `-version` lie; `backup.CheckRestore` compares a manifest's
version to `e2e`, so a backup of any release restores into any other and the
promise in development.md:270 and guide.md:191 ("the version is the single
source … so they cannot drift apart") is false. Every image this workflow has
ever published is affected; they stay as they are (owner, 2026-10-12: old tags
and images are history and are not touched) — the fix is for the next
publication.
**H2. Renewed TLS certificates are never picked up while the container
runs.** [postfix-config.sh:75-89](../build/postfix-config.sh) copies the
mounted cert and key into `/etc/postfix/tls-internal/` at start and points
Postfix at the copy; [postfix-cert-reload.sh](../build/postfix-cert-reload.sh)
runs `postfix reload` daily but nothing re-copies. The copy is the only
`tls-internal` reference in the tree; on the test server the copy carries the
container's start time. Scenario: Let's Encrypt renews at day 60, nobody
restarts, from day 90 ports 465/587 serve an expired certificate.
architecture.md:33,154-155 and supervisord.conf:92-95 say the opposite.
**H3. The queue-reconcile sweep marks delivered mail `bounced` whenever the
tailer is blind.** Four mechanisms, one consequence (a row the log would have
resolved as `sent` is closed as a failure, which the Outbound log shows and
the notifier believes):
- *Tailer stuck, sweep running.* [logtail.go:559-581](../internal/logtail/logtail.go):
`caughtUp` closes after the first drain and never re-opens; `reconcileLoop`
(:174-191) then runs every 5 min regardless. If `openAt` fails after a
rotation (:573-576, e.g. postlogd recreated the file `0600` — entrypoint.sh:132
says this happens) the loop logs and retries forever on the old inode while
every new delivery line goes unread; two minutes later `reconcile()` finds the
rows absent from `postqueue -p` (they were delivered) and bounces them.
- *Truncated queue listing read as complete.*
[postfix/queue.go:64-68,81-87](../internal/postfix/queue.go): `Queue()` caps
`postqueue -p` at 4 MiB and appends a text note; `QueueIDs()` parses that
text and returns the first 4 MiB of ids with no error. A deferred backlog of
~30k messages — the outage the sweep exists for — makes every row past the
cut `bounced` while Postfix still holds the mail. `queuejson.go:48-52` handles
the same cap correctly for the notifier; the two drifted.
- *Rotation drain window.* :567-578 drains the renamed inode once more; postlogd
keeps writing to it until master's reload reaches it (seconds), so lines
written in that window are never read. architecture.md:372-373 claims the
drain covers it.
- *No status guard on close.* `UpdateStatus` ([store/sendlog.go:86-98](../internal/store/sendlog.go))
is `WHERE queue_id = ? AND to_addr = ?`; the reconcile path can overwrite a
`sent` written an instant earlier, and a reused short queue id to the same
recipient rewrites a months-old row.
**H4. Application logins are case-sensitive in the panel and case-insensitive
in Postfix; a tenant can take another tenant's envelope sender.**
[app/validate.go:35-48](../internal/app/validate.go) accepts `A-Z` and nothing
lowercases a login; `applications.login` is `UNIQUE` under BINARY collation
([0001_init.sql:91](../internal/store/migrations/0001_init.sql)), so `Ops` and
`ops` are two applications. The sender map is written as stored
([postfix/postfix.go:95-115](../internal/postfix/postfix.go)); Postfix's
`reject_sender_login_mismatch` compares logins without regard to case (the
code's own comment, [milter/sender.go:35-36](../internal/milter/sender.go),
and architecture.md:239-240 say so). Scenario: domain A has `Ops` bound to
`@a.com`; a *domain*-role administrator of B creates `ops` in B, authenticates
as `ops`, sends `MAIL FROM:<x@a.com>` with `From: y@b.com`. Postfix accepts
the envelope (`@a.com Ops` matches), the sender check judges only the header,
which is B's — the message leaves with A as its SPF and bounce identity.
Confidence: the panel side is confirmed; Postfix's compare is taken from the
code's own statement. Fix shape: lower-case at validation, `COLLATE NOCASE`
on the column (a migration — check the live registry for a pair first).
### Medium
Persistence, restore, backup
**M1. The restore manifest is consumed before anything that can fail.**
[backup.go:436-446](../internal/backup/backup.go) deletes `manifest.json`
inside `CheckRestore`; [main.go:282-300](../cmd/panel/main.go) runs
`store.Open` and `resyncAfterRestore` afterwards. If either fails, supervisord
restarts the panel with `restored == false` and the one-time drift-healing
Resync never runs. Docs say "on the first *successful* boot"; the code says
"on the first attempt". `restore_test.go` passes `testNoReload=true` and never
fails Open/Resync, so nothing covers it.
**M2. Three queries on the single connection run without the index they
need.** Measured with `EXPLAIN QUERY PLAN` against the real migrations:
- `CountMessages` by domain ([ratelimits.go:186-190](../internal/store/ratelimits.go))
and `sendStats` ([stats.go:77-94](../internal/store/stats.go)) use
`idx_send_log_domain (domain)` and apply `created_at >=` by scanning every
row of the domain over the retention window — on every `MAIL FROM` of an
application under a domain-level limit, with the milter, tailer and HTTP
queued behind it. The baseline's comment promises a composite index for
"the journal milter … on every MAIL FROM"; only the `app_login` half exists.
- `ListQueuedOlderThan` ([sendlog.go:117-122](../internal/store/sendlog.go))
is a full `SCAN send_log` every 5 min (no index on `status`; the cutoff
matches nearly every row). Mirror of what `0005` fixed for `rejected`: a
partial index `(created_at) WHERE status = 'queued'` makes it a handful of
rows.
- Scoped `QuerySendLog` ([sendlog.go:214-219](../internal/store/sendlog.go))
sorts a domain's whole scope with a temp b-tree for every page a domain user
opens.
**M3. `selfpost-backup` with a wrong `SELFPOST_DB_PATH` creates an empty
database and ships a "valid" backup of nothing.** [backup.go:394-405](../internal/backup/backup.go)
opens the source without `mode=ro` or an existence check; `VACUUM INTO` on a
non-existent path succeeds (measured), creates the file on disk and snapshots a
4 KiB empty database; `Create` and `Check` pass. A restore of it opens the
`/setup` link on a host that still carries live DKIM keys and `sasldb2`.
**M4. `secretfile`'s KDF cap does not do what its comment says.**
[secretfile.go:108-113,184](../internal/secretfile/secretfile.go):
`maxLogN=20, maxR=32, maxP=16` lets a 44-byte header demand
`128·2²⁰·32` = 4 GiB and sixteen times the CPU, under a comment that says the
cap exists "so a hostile header cannot make the reader allocate its way to an
out-of-memory kill". The domain-import form calls `NewReader` on the upload.
Operator foot-gun rather than attack surface (global role only), but the
stated guarantee is false; the real writer never exceeds N=2¹⁵, r=8, p=1.
**M5. The CLI enforces no password floor for an encrypted backup.**
[cmd/selfpost-backup/main.go:170-187](../cmd/selfpost-backup/main.go) takes
any non-empty string; the panel enforces `validate.MinSecretFilePasswordLen`
(12); security.md:291 says the minimum applies to these files. (The roadmap's
"empty password accepted" item: an empty password is *not* accepted as an
encryption password — empty means plain output by design; the gap is the
floor.)
**M6. The compose pin, README and guide point at an image that does not
exist.** `deploy/docker-compose.yml:24`, `README.md:115` and guide.md:187 pin
`ghcr.io/mixeme/selfpost:2.4.0` (`2.4.1` since this cut); GHCR holds `1.0.0`
and `1.3.0` only (checked
through the Packages API and by pulling); no 2.x was ever published, and tag
`v2.0.0` exists in git without a release. The quick start cannot work for
anyone but the owner, who runs locally built `-post.<sha>` images. Decided
(owner, 2026-10-12): the old tags and images are history and stay untouched;
the pin points forward — it is made true by the next publication, after H1 is
fixed, and until then the documents should not claim an image that is not
there.
HTTP surface
**M7. The last-global-user guard is check-then-act without a transaction.**
[store/users.go:315-341](../internal/store/users.go) (`DeleteUser`:
`CountGlobalUsers` then `DELETE`), [handlers_users.go:277-287,317-322](../internal/web/handlers/handlers_users.go)
(demotion), [users.go:80-97](../internal/store/users.go) (`CreateGlobalUser`:
`UserExists` then `INSERT`). `MaxOpenConns(1)` serialises statements, not
pairs. Two globals deleting or demoting each other at once leave no global;
`/server/*` answers 404 to everyone and recovery is database surgery.
security.md:155 promises the guard.
**M8. Login discloses whether a username exists by timing.**
[auth/handlers.go:109-121](../internal/web/auth/handlers.go): unknown user
returns at :114 without bcrypt; a known one costs ~250 ms (cost 12). The test
compares bodies only. Rate limiting slows enumeration; a dummy compare stops
it.
**M9. `submitUserUpdate` is five writes with no transaction.**
[handlers_users.go:304-336](../internal/web/handlers/handlers_users.go):
`UpdateUser` → `endSessionsAfterPasswordSet` → `SetUserRole` →
`ClearUserReach` (error only logged) → `SetUserReach`. A failure after the
first leaves a user renamed and signed out everywhere with the old role, or a
`domain` user with empty reach — the state the "Assign at least one domain"
check exists to prevent.
**M10. Domain import trusts the export file for everything but the domain
name, selector and password.** [domain/transfer.go:145-190](../internal/domain/transfer.go):
`DMARCRua` is not run through `validate.Email` (the form path is), client IPs
not through `parseIPList`, rate-limit `Mode`/`WindowSeconds`/`MaxMessages`
not through the form's parsers. `WindowSeconds: 0` yields a limit that counts
nothing ([milter/ratelimit.go:61](../internal/milter/ratelimit.go)); garbage
IPs make the imported application refuse every client with no word at import.
[app/validate.go:50-55](../internal/app/validate.go) calls the file "untrusted
input" — for the password only.
**M11. Polling stops for good after one failed refresh.**
[panel.js:202-257](../internal/web/view/static/panel.js): the scheduled poll
is `htmx.ajax(...)` without `source`, so htmx attributes it to `document.body`;
`htmx:responseError` therefore reaches an element with no `data-poll` and
nothing reschedules; `htmx:sendError` and `htmx:timeout` are not handled at
all. A panel restart while Overview or Health is open freezes the page until
the tab is hidden and shown. Related: an expired session answers an
`HX-Request` with `303 /login` ([auth/middleware.go:14](../internal/web/auth/middleware.go));
the XHR follows it and the sign-in document is swapped into the box — no
`HX-Redirect` on either side.
Log tailer and checks
**M12. `deliveryRe` is unanchored; a stranger's HELO parses as a delivery
result.** [logtail.go:85](../internal/logtail/logtail.go): a port-25 reject
line ending `helo=<41E862C00D9E: to=<victim@…>, status=sent>` yields
`(id, recipient, sent)` (probed), and `UpdateStatus` rewrites the matching row.
The attacker must guess an id+recipient pair, so impact is bounded — but
`lines.go:21-24` and `inbound.go:48-51` anchor at the head for exactly this
reason, and this pattern, the one that writes status, does not.
**M13. The sender-check deferred counter misses the case it is documented to
see.** [sendercheck.go:50](../internal/logtail/sendercheck.go) matches only
`postfix/…smtpd[pid]`; header/EOH events run in cleanup(8), so the warning for
a panel that accepts the socket but hangs at end-of-headers is logged by
`postfix/cleanup`. The fixture at `sendercheck_test.go:64-68` puts those lines
under `smtpd` and says it was "composed from Postfix's source rather than
seen". architecture.md:286-287 claims the figure sees "a panel that is running
and not answering". Confidence: plausible; one `kill -STOP` on the stand
settles it.
**M14. Resolver fall-back is hollow when the first resolver drops packets.**
[dnscheck/resolver.go:72-90](../internal/dnscheck/resolver.go) tries resolvers
in turn under one 5 s context; a blackhole eats the budget and the second is
asked with an expired context. The `DefaultResolvers` comment promises
otherwise. **M15.** [dnscheck/server.go:26-33,42-46](../internal/dnscheck/server.go)
turns a resolver timeout into "does not resolve" / "no PTR record" (every
other check routes transient errors to Unknown), and empties `ServerIPs` so
the SPF check goes Unknown in the same minute. architecture.md:339-340.
Infrastructure
**M16. `envdoc_test.go` compares three hand-written lists to each other and
reads neither `main.go`, the scripts nor `guide.md`.**
[envdoc_test.go:70-136](../cmd/panel/envdoc_test.go). Already drifted:
`ANTISPAM_DIR`, `INBOUND_JOURNAL_MILTER_SOCKET`, `INBOUND_SPAM_LOG_RETENTION_DAYS`,
`RATE_LIMIT_MESSAGES_PER_IP`, `RATE_LIMIT_WINDOW_SECONDS` are read by
`loadConfig` and absent from `loadConfigKeys`; `postfix-config.sh` and
`postfix-wrapper.sh` read `INBOUND_JOURNAL_MILTER_SOCKET`, `SELFPOST_DATA_DIR`,
`SELFPOST_DB_PATH`, `INBOUND_RELAY_ENABLE`, absent from `buildScriptKeys`. The
tests pass; development.md:353-357 and :503-504 describe a guard that does
not exist.
**M17. The supervisord control socket is `0770 root:selfpost` and the
`postfix` and `opendkim` users are in `selfpost`.**
[supervisord.conf:26-29](../build/supervisord.conf) ("only root and the two
SelfPost services"), [Dockerfile:68-71](../build/Dockerfile) (three users);
confirmed on the server. A compromised smtpd can `supervisorctl stop panel`
(all submission deferred), `stop opendkim` or `shutdown`. No escalation; a
whole-stack denial from the least trusted process.
**M18. DMARC maps are never ownership-normalised.** entrypoint.sh:108-125
normalises five maps; `dmarc_recipients`/`dmarc_transport`/`dmarc_relay_domains`
are created by postfix-config.sh:315-317 as root only when missing. A tar
restore as root (guide, methods 3/4) yields `root:root 0640`, which smtpd
cannot open → `451 table lookup problem` on every report until the panel next
rewrites them. Confidence: plausible, needs a run.
**M19. logrotate state lives in the container layer.** [logrotate-loop.sh:42](../build/logrotate-loop.sh)
uses the default `/var/lib/logrotate/status`, not under `/data`. On the server
the state file is dated the container's start; with no entry logrotate records
today and does not rotate, so a host that recreates the container daily
(`.env` change, image bump) never rotates `mail.log`. The e2e uses `-f` and
cannot see it.
**M20. `test.yml` never compiles `test/e2e`.** A type error there surfaces in
`release.yml` after the GitHub Release is published and the tag checked out.
**M21. `deploy/traefik/extract-cert.sh` writes empty PEM files and exits 0 on
a `jq` miss** (:17-25, `sh` without pipefail; `jq` exits 0 on no match); the
files are `0644` until :27. Postfix TLS breaks at the next restart.
### Low
- **L1** `-decrypt -o FILE` truncates the output before the password is
verified ([main.go:132-148](../cmd/selfpost-backup/main.go)); a wrong
password leaves an empty file where a good one may have been.
- **L2** Backups skip symlinks silently and `Check` does not notice
([backup.go:323-327](../internal/backup/backup.go)); a `certs/` of symlinks
(the Let's Encrypt layout) yields an archive with no TLS material while the
guide promises refusal.
- **L3** Trailing bytes after the last `secretfile` chunk are ignored
(:400-416): two envelopes concatenated open as the first.
- **L4** `parseTrustedProxies` turns a bare `::ffff:10.0.0.1` into `::/32`
silently ([main.go:247-253](../cmd/panel/main.go)).
- **L5** `tls_policy` is keyed by next-hop; two inbound domains to one
`[host]:port` with different TLS modes write duplicate keys and the first
(by domain order) wins with a warning — confirmed on the server. The per-
domain TLS mode in the form is not what Postfix does
([postfix/inbound.go:69-72](../internal/postfix/inbound.go)).
- **L6** Multi-file map rebuilds are not all-or-nothing
([inbound.go:42-55](../internal/postfix/inbound.go), `dmarc.go:31-41`); a
write failure mid-sequence leaves `relay_domains` ahead of `transport`.
- **L7** `atomicfile.Write` never fsyncs the directory and the package has no
tests ([atomicfile.go:40-44](../internal/atomicfile/atomicfile.go)); every map
writer goes through it.
- **L8** The refusal reply names the other sender's domain
([sender.go:334](../internal/milter/sender.go)) — an oracle for the server's
signing names, available to any authenticated application.
- **L9** go-milter accumulates every message's headers for a connection's
life (library `session.go:173-187`, cleared on ABORT only); the sender check
is safe because it ignores `h`, and the test passes `nil` so would not catch
a "simplification" to `h.Values("From")`.
- **L10** `parsePage` is not clamped to `lastPage` for the Outbound log
([handlers_monitor.go:357-371](../internal/web/handlers/handlers_monitor.go));
the inbound log clamps. Known roadmap row, confirmed.
- **L11** `parseAutoMultiplier` accepts `NaN` ([handlers_ratelimit.go:44-49](../internal/web/handlers/handlers_ratelimit.go));
SQLite stores NULL, read-back is 0, recalc silently uses 2.5 (probed).
- **L12** Outbound rows belong to a domain by **name** ([handlers_monitor.go:97,345](../internal/web/handlers/handlers_monitor.go)):
a domain deleted and re-added shows its earlier rows to the new assignee.
security.md:419 accepts this for the inbound log only.
- **L13** `HandleReload` answers failures as plain-text `http.Error`
(handlers_domains.go:173-184), and the flash "Configuration regenerated from
the database" oversells: DMARC maps and filter-list files are skipped
(architecture.md:856-859 says so for DMARC; `Lists.Sync` is documented
nowhere).
- **L14** `http.Server` has only `ReadHeaderTimeout` ([httpserver.go:145-149](../cmd/panel/httpserver.go)).
- **L15** The Account password change shares the per-IP login bucket
([handlers_account.go:142](../internal/web/handlers/handlers_account.go)):
ten wrong current passwords lock sign-in for that IP 15 min.
- **L16** Login issues a fresh token but does not destroy the session a
presented cookie already holds ([auth/handlers.go:123-130](../internal/web/auth/handlers.go)).
- **L17** `domain.Service.Delete` returns a bare error after the row is gone
when `resync()` fails ([service.go:202-210](../internal/domain/service.go));
`apps.Resync()` is not reached, so the sender map keeps the purged logins.
- **L18** A `rate-limited` refusal is a grey tag in the Outbound log
([outbound_log.go:213](../internal/web/view/outbound_log.go), no such status
in `components.go:132-146`); its two siblings are red, and the test pins the
grey.
- **L19** *Recalculate rate limit* on the application form submits the whole
form and discards every unsaved edit ([out_app.html:67](../internal/web/view/templates/out_app.html),
handlers_ratelimit.go:178-189).
- **L20** DMARC parse rewrites `<pct>0</pct>` to 100 and stores a missing
`date_range` as 1970 ([parse.go:125-136](../internal/dmarc/parse.go)).
- **L21** Several `mailStack` instances per process with no shared lock on
table rebuilds (`run`, `newPanel`, `runNotifier`); the notifier's
`EnsureServerSigned` at start can write a stale table over a domain added
in the same second. **L22** `IncrDMARCParseFailures` is read-modify-write
across concurrent `dmarc-ingest` processes ([dmarc.go:163-173](../internal/store/dmarc.go)).
- **L23** The restore-boot Resync reloads Postfix before Postfix exists
(panel priority 200, postfix 300) — an error line per restore boot; the
DMARC bootstrap works around the same thing with a no-op hook and does not
say why ([main.go:301-307](../cmd/panel/main.go)).
- **L24** `health/processes.go:43`, `supervisor.go:17,30`: `exec.Command`
without context; `/healthz` and the 5 s status poll hang on a wedged
supervisord. **L25** `health/cert.go:70-83` assumes the first PEM block is
the leaf and grades a not-yet-valid certificate OK; `machine.go:193-204`
advances `prevAt` when only one of cpu/net succeeded.
- **L26** Only successful sends count toward the notifier's 60/h ceiling
([notify.go:196-207](../internal/notify/notify.go)); with sendmail broken,
`CheckLimits` spawns a 20 s process per recipient per minute forever.
- **L27** `v=dmarc1` matched case-insensitively ([records.go:74,121](../internal/dnscheck/records.go));
RFC 7489 §6.4 says the version must match exactly, so a lower-case record is
graded OK and ignored by receivers. **L28** The domain cache is keyed by
name only ([dnscheck.go:155-173](../internal/dnscheck/dnscheck.go)) though
the verdict depends on `ExpectedDKIM`/`ServerIPs`.
- **L29** `lineTime` normalises a Feb 29 line read the following January to
Mar 1 ([inbound.go:254-261](../internal/logtail/inbound.go)); `isoStampRe`
accepts a zone-less stamp that `time.Parse` then rejects (dated `now`).
- **L30** OpenDKIM: `Syslog no` with no `LogWhy`/`Logfile` — it logs nothing
anywhere; no `Canonicalization relaxed/simple`; no `OversignHeaders From`
(the header-append replay is the class 2.4.0 is about).
- **L31** Postfix: `smtpd_tls_protocols` at the 3.7 default `>=TLSv1`,
`disable_vrfy_command = no` (VRFY on 25 enumerates `relay_recipient_maps`),
the stock `(Debian/GNU)` banner — all confirmed on the server.
`RATE_LIMIT_*`, `*_SIZE_LIMIT`, `MILTER_*_TIMEOUT` reach `postconf -e`
unvalidated; a non-integer passes `postfix check` and kills every smtpd at
connect.
- **L32** `crashexit` fires on FATAL only, which arises only from
`startretries` exhausted within `startsecs` (1 s): a program that crashes
after one second cycles forever and never exits the container.
architecture.md:40-41 reads wider than that.
- **L33** `.dockerignore` excludes `docs`, `dev`, `bin` but not `test/`; a
developer build after an e2e run ships `test/e2e/.stage/data` (DKIM keys,
`sasldb2`) into the build context. Base images pinned by tag, not digest.
- **L34** e2e: `stage.go:77` pulls `alpine:3.20` from the internet;
`hostname_gate_test.go:97-101` runs with the host's real network;
`sender_check_test.go:141` has a fixed `time.Sleep(2s)` against
development.md:409 "no fixed sleep".
### Nits
Dead code: `handlers/authz.go:63 domainIDSet`; `view.Facts`
(`components.go:414`); `dmarc.Service.HostedSuggestion`, `ValidateHostedAddress`
(`dmarc/service.go:98-109`); `app.Service.UpdateMode` (`app/service.go:132-145`,
superseded by `SaveSettings`); `Settings.WithResult` (`view/account.go:137-145`)
re-implements `resultFlash`; `KitPage()`'s hard-coded `User: "admin"`
(overwritten by the handler); dead method checks in `handlers_dmarc.go:20-24`
and the `default:` branches of `HandleUserNew`/`HandleUserEdit`/`HandleServerSettings`
(mux patterns are method-bound); the `postfix set-permissions` branch in
entrypoint.sh:170-172 (`postfix check` has already created the tree).
Duplication: `canViewDMARCDomain` repeats `lookupDomain`'s check; `ParseForm`
twice on the user-edit POST; `Lookup` then `Touch` per request in
`auth/middleware.go:17,28`; `retry.go:58-67` repeats the six `postconf` keys as
literals instead of `retryConfKeys`; three header-name canonicalisers with two
behaviours (`milter.go:124`, `inbound.go:89`, `sender.go:248`). Behaviour:
`submitUserCreate` does `CreateUser` then `UpdateUser` for the e-mail;
self-demotion redirects to a page that is now 404; `redirectHome` gives a
domain user with no outbound reach an empty 200 where the inbound list gives
404; `sort.Slice` in `postfix/inbound.go:59` reorders the caller's slice;
`strings.Split(raw, "\n")` in `sender.go:65,104` keeps `\r`; `lists.go:428`
trims the filter key while the Lua side reads it verbatim; `InDomain.WithMX`
labels a `warn` status "NOT CHECKED"; `senderSession` does not override `Body`
(the library discards the session after every accepted message — harmless,
inconsistent with `milter.go:130-134`). The handlers test package takes ~105 s
on this machine (likely bcrypt cost 12 on the user-creation paths).
### Doc drift
architecture.md: :167 (§ OpenDKIM) still says the `From:` header is not held
against the login — contradicted by 2.4.0 and by § Sender check in the same
file; :272-274 credits `/run` being tmpfs for a fresh `.alive` mark (it is a
plain image directory; a restarted container keeps the mark); :328-329 "a
deleted domain's row goes with it" — only `dns:` rows, `limit:` lingers up to
48 h; :493 and § Who may open what omit `/inbound/log` and
`/inbound/filter-lists`; :800 names only migration `0002`; :832-836 omits
`inbound_spam_log`, `inbound_spam_list`, `notify_state`, `sender_check_days`;
:838 "the panel is the only writer" — `dmarc-ingest` is a separate process
that opens the store, migrates and writes; :856-859 vs `backup.go:19-23` and
`httpserver.go:52-53` ("the same Resync") — the button skips DMARC maps and
filter lists; the Internal env list omits `ANTISPAM_DIR` and
`INBOUND_JOURNAL_MILTER_SOCKET`; § View engine calls the kit page "fixtures"
(it is production data). schema-migrations.md: :20 "chain head `user_version
= 5`" (six files; `TestBaselineSchema` asserts 6); the "Schema at head (v2)"
table omits `sender_check_days` and `send_log.reason`/`header_from`.
guide.md: :214 `INBOUND_JOURNAL_MILTER_SOCKET` "Set in: compose" (it is not);
:1260 lines "deleted together with the row" (at the next six-hourly prune, and
not while a reused id has a row); :720 "first successful boot" (M1).
development.md: :270 version/tag coupling (H1); :353-357, :503 the env guard
(M16); :404 "milter fail-open" as a covered negative (the suite now proves
deferral); :409 "no fixed sleep". roadmap.md: `template-data-typing` describes
`renderDomainDetail` with forty map keys — gone since 2.0 (one typed-before-
render map remains, the health sampler's); `structured-logging` says `logf`
"in five packages" — it is in 21 files of `internal/web`;
`review-2026-08-followups` row "empty password" is misdescribed (M5). Code
comments: `milter.go:7-11` "it never rejects" (it tempfails at `MAIL FROM`);
`store/sendlog.go:19-22,61-65` `StatusRejected`/`InsertRejected` describe the
level-2 case only; `store.go:2-3` "domain-admins"; `schema_test.go:15`
`TestBaselineIsTheOnlyMigration`; `health/machine.go:225` "see roadmap
panel-docs … later" (shipped 1.8.0); `inboundspam.go:349-350` "does not read
the whole journal" (it scans it); `logtail.go:466` mentions `copytruncate`;
supervisord.conf:25 "two services", :107 "7-14 files"; entrypoint.sh:119-120
"no secrets" in `/data/antispam` (`filter.key`); docker-compose.yml:85,88 cite
a guide section by a title it does not have.
### Test gaps
No test for: the H1 stamp (a `panel -version` assertion in the e2e against the
version the stand was told to build); cert re-copy (H2); `follow()` with an
unreadable or recreated log, a truncated `postqueue -p`, lines written to the
old inode after the switch, `UpdateStatus` from `sent` (H3); two logins
differing in case (H4); M1's ordering (restore tests never fail Open/Resync or
exercise the real reload hook); `CheckRestore` with malformed JSON; M3, M5,
L1, L2; `secretfile` concatenation (L3); the sender-check counter with the
real program name (M13); a `LookupAddr` timeout (M15); `TestLimitRefusalsSince`
EXPLAINs a stand-in query, not the real one; `TestOpenMigratesA20Database`
stops at 0004; `Import` with a bad rua / IPs / window 0 / mode (M10); panel.js
has no test at all (M11); login timing (M8); concurrent last-global (M7);
`TestOriginCheck` lacks the Host-with-port vs Origin case;
`TestServerSubtreeIsGlobalOnly` skips any `/server/` route not in
`routes.always`; `internal/atomicfile` and `internal/supervisor` have no
tests; `TestRenderInboundMaps` has no two-routes-one-next-hop case (L5);
`TestSenderCheckResetsBetweenMessages` passes `h = nil` (L9); the handler test
rows of `review-2026-08-followups` (apps/domains/users/dmarc handlers
directly, `internal/dmarc` service) are still open.
### Checked and sound
Worth writing down so the next review does not re-read it: the sender check's
layered rule holds against every `From:` shape tried (quoted local parts with
`@`, CFWS, encoded words decoding to addresses, groups, `<>`, address
literals, obs-route) — the "one `@` outside quotes" rule is the real backstop;
extension stripping and case folding match Postfix; every error path of the
check tempfails; level-2 accounting reserves under one lock and cannot be
inflated by multi-RCPT or bypassed by ABORT/reconnect; the inbound verdict is
trusted only under the queue-id-bound HMAC; the port-25 `-o` overrides are
complete and read back by the e2e through `postconf -Ph`; the generated
milter chain is exactly sender(tempfail) → OpenDKIM(tempfail) →
journal(accept); `/server/` is guarded at registration and per handler;
scopes fail closed (`1 = 0`); sessions store the hash only, `__Host-` cookie,
constant-time setup token; `SaveSettings` restores prior state on mid-way
failure; `secretfile`'s AAD binds header, counter and last-flag, and
truncation, reorder and tamper are tested; shutdown order (tailer offset
saved, HTTP drained, DB closed last) is right; rspamd map names, deny message,
symbol prefixes and key-file format match the Go side byte for byte.
---
## Plan
Seven stages (six, and stage 2b split off stage 1 on 2026-10-14), each a
self-contained cut, each small enough for one sitting,
with model routing per development.md (security/infra → Opus, UI/docs →
Sonnet). "Done when" is the test that proves it, not the edit.
**Agreed with the owner on 2026-10-12** (roadmap row `audit-2026-10-12`):
stage 0 first, then stages 1 and 2 as one cut, the rest after in any order
(stage 2b after `2.5.0`, before the others).
This is the **active plan** in the sense of development.md § Plan checklists:
the `- [ ]` lines below are the implementation checklist, a line is ticked in
the commit that finishes it, and the roadmap's Progress column counts them.
After a context reset, start at the first unchecked line of stage 0. The
owner's decisions on the contested items are written next to the steps they
changed and under *Decided with the owner* at the end; they are not open.
Working rules that apply, from [development.md](development.md) and the
project's memory: every step updates CHANGELOG `[Unreleased]` in its own
commit; a version is cut only at the owner's word; `main` is pushed, tags
are not; design changes (the only one in this plan would be a *Delivery log*
card, and it is not planned) are mockup-first; a migration the test server
has already run is never edited — the next file is added; the published
`1.x` tags and images are history and are not touched.
### Stage 0 — release hygiene (no product code) → `2.4.2` (shipped 2026-10-14)
- [x] **Stamp the right version** (H1). Make the e2e build take the version from
the environment (`VERSION: ${SELFPOST_E2E_VERSION:-e2e}` in
`compose.override.yml`, set by `release.yml`), or have the e2e reuse the
image already built instead of rebuilding. Done when the e2e asserts
`panel -version` equals what it was told, and `release.yml` runs that
assertion before the push. — **Opus**
- [x] **Compile the e2e in `test.yml`** (M20): `cd test/e2e && go vet ./... && go
test -c -o /dev/null .` (not `go test -run xxx`: `TestMain` would still
run and bring up the stand). — **Haiku**
- [x] **`.dockerignore` + `test/`** (L33). — **Haiku**
- [x] **The pin** (M6): old tags and images are not touched (owner,
2026-10-12). The next publication through the fixed `release.yml` is the
one the pin names; until then README/guide say that the current line is
not yet published rather than name an image that is not there. — **Sonnet**
- [x] **Toolchain and deps**: `modernc.org/sqlite`, `x/crypto`, `x/sys`; note in
development.md that the image is rebuilt after each Go patch release
(`golang:1.26-bookworm` floats, so a rebuild suffices). — **Haiku**
- [x] The four `staticcheck` items. — **Haiku**
- [x] **A negative e2e case passes only on the refusal it is about** (owner,
2026-10-14; seen, not found by the audit). `negative_level1_ratelimit`
sends up to sixty messages and waits for one to be refused; it once
passed on "limit tripped on attempt 1/60" when the connection was reset
because Postfix was still starting. The case counts as the limit only the
reply code of the limit, and only after at least one message was
accepted; a reset, a timeout or any other reply fails it and says what
came. Then every other negative case of the suite is read for the same
mistake - any failure taken for the expected refusal - and held to its
own reply code. - **Opus**
*Done 2026-10-14; what the stand showed:* `waitForSMTPSReady` took a TCP
connect for readiness, and Docker's port proxy accepts before Postfix
listens - that was the reset; it now waits for the TLS greeting. Held to
anvil's `450`, the case then failed on its first message: `553 … not owned
by user l1app`, an smtpd process from before the panel's `postfix reload`
still holding the old `sender_login_maps` - so the level-1 limit had never
been the refusal the case saw. The first message now waits (at most 15 s)
for the map; the limit trips at 51/60 with `450 4.7.1 Error: too much mail
from`. In the product this is a second or so after an application is
saved in which its first message can get a permanent 553; not worth a
change (an application is configured after it is created, not within the
second). *Measured 2026-10-14:* applications created and used after 0,
10, 40 and 110 s of waiting (the first send two seconds after the save at
the soonest) were all served by an smtpd started after the reload; the
stale one only ever answered within the reload's own second. The e2e
waits for the map after each application it creates (`waitForSenderMap`),
after the main positive path failed on it once. The other cases are held to `451 4.7.1` (level 2, both MAIL FROM
of one connection), `553` at RCPT (sender mismatch), `554 … Client host
rejected` at RCPT (no AUTH, and the relay case, whose comment claimed
`reject_unauth_destination` - on 465 the client restriction comes first;
port 25 holds `554 … Relay access denied`), `550 5.1.1` (port 25 unknown
recipients), any `4xx` reply (panel down).
### Stage 1 — mail that is shown wrong (H3, H2, M12, M13) → `2.5.0` (shipped 2026-10-14)
- [x] **`QueueIDs` on a truncated listing returns an error**, as `postqueueJSON`
does; `reconcile` already treats an error as no information. Test: a 5 MiB
fake `postqueue -p`. — **Opus**
- [x] **Reconcile only closes `queued` rows** (`AND status = 'queued'` in the
reconcile path's update) and marks what it closed: `reason = 'reconcile'`
on the row (the 0006 column; a new value, no schema change). — **Opus**
- [x] **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**
*Changed by the reviews of stage 1 (2026-10-14):* a fifth counter,
`reconcile_missed` - a closed row whose delivery line was in `mail.log` or
`mail.log.1` when the sweep closed it; `contradicted` alone cannot see a
lost line, which is never read late. `queued_over_lifetime` cannot show
"what removing the sweep would leave visible" while the sweep runs (those
rows are closed by then); it counts rows stuck despite the sweep, and what
removing it would leave is `closed - missed - contradicted`. A
contradicted row keeps the mark `reconcile-contradicted`. The rule reads
accordingly in architecture.md § Log tailer.
- *Deferred until the data is in* (not a step yet): 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`.
- [x] **Anchor `deliveryRe`** at the head of the line like its two siblings;
test with the probed HELO and quoted-sender lines. — **Opus**
- [x] **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**
- [x] **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**
- [x] 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 `0008`; shipped 2026-10-14)
- [x] **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
other case) so the doc's claim is seen, not assumed. — **Opus**
- [x] **Last global in one transaction**: `DeleteUser`/`SetUserRole` take a
`tx`, count and act inside it; `CreateGlobalUser` likewise. A concurrency
test with two goroutines. — **Opus**
- [x] **`submitUserUpdate` in one transaction** (store method that takes the
whole change). The users pages get direct handler tests while they are
rewired - a row of review-2026-08-followups, placed here 2026-10-14. — **Opus**
- [x] **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**
- [x] **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**
*Done 2026-10-14.* DMARC reports stay by name on purpose: a report is
about the name in DNS, and whoever holds the name now is its reader.
*Changed by the reviews of stage 2:* the backfill follows the rule new
rows follow (the application's domain, then the name) and gives a row
only to a domain - and an application - that existed when it was
written; an application's numbers are its login's rows of its domain,
from its creation on. Deleting a domain now rewrites its rows' domain_id
(14 s for 333k rows, measured by the review; milliseconds at this
installation's size) - accepted, to revisit if a log grows that large.
- [x] Docs: security.md § Roles (the guard) and the struck accepted risk, guide
§ Domain export, schema-migrations.md. — **Sonnet**
- [x] **Which key OpenDKIM signs with, seen and not reasoned** (owner,
2026-10-13, from the Not verified list of from-header-binding). The check
and OpenDKIM read `From:` with two parsers; that they agree was argued
from the check's two rules, never run. An e2e case sends, as one
application, the `From:` lines the check passes - quoted strings,
comments, folded lines, groups, escapes, encoded words, an address
extension - and reads the `d=` of the signature on what arrives: any
domain but the application's fails the test. The lines live in one table
shared with the check's unit test, so a line added to one is run by the
other. A disagreement found is fixed in the check before the cut, and
security.md § Mail path says the claim is tested. - **Opus**
*Changed by the reviews of stage 2:* a second list, lines naming another
sender in a quoted string, a comment, a quoted pair, capitals, an encoded
word (`from-lines-other.txt`), each refused and never signed for it; the
passed list gained capitals, a quoted local part and a name with a space
after its `@` (passed by design, signed for the own domain). L8 is half
closed: refused-or-passed still tells an application which domains are
hosted here - the owner decides whether to refuse every foreign address
in a display name (asked 2026-10-14).
*Done 2026-10-14:* fifteen lines (`internal/milter/testdata/from-lines-passed.txt`),
each signed for the application's domain alone, the signature valid. No
disagreement. Groups are refused by the check, so none is in the file;
a comment before the address - `(the shop) a@domain`, valid RFC 5322 - is
refused too ("it must be one address"): strict, not a hole, left as it is.
- [x] **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 2b — the log tailer, and what the panel shows of a refusal → `2.5.1`
Split off stage 1 on 2026-10-14 at the owner's word: the six steps below were
agreed on 2026-10-13/14, after the plan, and had made stages 1 and 2 a cut of
twenty-one steps. Stages 1 and 2 ship as `2.5.0` in their first size; this
stage follows as a cut of its own, with its own reviews. Five of the six
change one thing, the tailer and the state it saves, and are done in this
order - each stands on the one before.
- [ ] **A log rotated while the panel was down is read to its end** (owner,
2026-10-13; the Not done list of from-header-binding had it as "deferrals
are lost", and it is wider). At start, a saved offset whose fingerprint
is not the current file's means "start on the new file"
([offset.go](../internal/logtail/offset.go)); what Postfix wrote between
the panel's stop and the rotation is never read - delivery statuses,
stored delivery lines, refusals, inbound journal rows, the deferred
counter, and the recovery of unwritten refusals above. The tailer first
opens the rotated file (`mail.log.1`), checks by fingerprint that it is
the one the offset belongs to, reads it from the offset to its end, then
goes on to the new file. The fingerprint gains the inode, which closes
the roadmap's review-2026-08-followups row on collisions. Not found, or
rotated twice: logged, with from when to when nothing was read. A test
that stops the tailer, appends, rotates, appends, starts it, and finds
both halves once. - **Opus**
- [ ] **The deferred counter remembers a place in the log, not a second**
(owner, 2026-10-14, from the Not done list of from-header-binding; he
chose it over leaving the case written down). After an unclean stop the
tailer re-reads lines it had read, and the counter passes over every
warning no later than the last one it counted
([logtail/sendercheck.go](../internal/logtail/sendercheck.go), `last`);
`mail.log` stamps whole seconds, so a warning of that very second, later
in the file, is lost. The count and the place in the log it has reached
are saved in one transaction, and what to pass over is decided by the
place. Done after the rotation step above, on the same saved state - the
place must stay true across a rotated file. A test that stops the tailer
between two warnings of one second and finds both counted, once each.
- **Opus**
- [ ] **A refused message's page shows what Postfix wrote** (owner,
2026-10-13: "the log box is required" - the box is the reason
delivery-log-storage was built, and its absence from the picture he
approved for 2.4.0 was an oversight, not a decision). Today every
`rejected` row - `wrong From`, `wrong client`, `rate-limited` - has no
queue id, and its page shows the box with one sentence,
"This message never reached the queue, so Postfix wrote no delivery lines
for it" ([handlers_monitor.go](../internal/web/handlers/handlers_monitor.go)).
The sentence is untrue: Postfix wrote a line for each (`milter-reject:
END-OF-MESSAGE ... 5.7.1 From: ...` from cleanup, `NOQUEUE: reject: ...`
from smtpd). The tailer attaches that line to the row it belongs to, by
client address, time and the addresses in the line, and stores it in
`send_log_lines` like any other; the page shows it. First on the stand:
read `mail.log` for the three kinds of refusal and write down what
identifies each line. Then the mockup: the refused variant of `out-message`
gets the box with the line in it (`design:` commit), then the code. The
sentence stays only for a row whose line was truly never written or is
older than this change, and says that. Done when the e2e shows the stored
line on the page of each of the three refusals. - **Opus**
- [ ] **A message the sweep closed does not say the receiving server refused
it** (review of stage 1, 2026-10-14). The history step of a `bounced` row
reads "the receiving server refused the message permanently, or Postfix
gave up ... The reason is in the delivery log below"
([handlers_monitor.go](../internal/web/handlers/handlers_monitor.go)); for
a row with reason `reconcile` neither is known and the log has nothing.
Such a row gets its own step: Postfix no longer held the message and
`mail.log` never said what became of it, so the panel closed it as a
failure. Wording in the mockup `out-message` first (`design:` commit),
with the box of the step above. - **Sonnet**
- [ ] **A refusal the panel did not get to write is written from
`mail.log`** (owner, 2026-10-13, from the Not done list of
from-header-binding; built on the step above). The check answers 550 and
writes the row a moment later, through a buffer; a panel that stops in
between - a rollout, a crash - leaves an application that was refused
and a log that does not know it. When the tailer meets a refusal line
that has no row, it makes the row from the line (client, sender,
recipient, reason), marked as recovered. The tailer resumes from its
saved offset, so this covers a kill as well as a clean stop - which a
flush of the buffer at shutdown would not. Done when the e2e refuses a
message, kills the panel before the row is written, and finds the row
after the restart, once. - **Opus**
- [ ] **Downtime is counted when supervisord does not answer at start**
(owner, 2026-10-13, from the Not done list of from-header-binding). The
panel asks supervisord once since when Postfix runs
([sendercheck.go](../cmd/panel/sendercheck.go), `postfixStarted`); an
error is logged and that start's downtime is not counted at all - the
number the owner will decide by reads low. The question is repeated for
about thirty seconds. Still no answer: the downtime is taken from the
alive mark alone - an upper bound, Postfix may not have been running -
and the day is marked as estimated (a column in `sender_check_days`, the
next migration), which the card shows. The mark on the card is drawn in
the mockup with the renamed column above (one `design:` commit). - **Opus**
- [ ] **The Sender check card names what it counts** (owner, 2026-10-13, from
the Not done list of from-header-binding): the column *Messages deferred*
becomes *Connections turned away*. Postfix refuses from the connection on
while the check does not answer, before any message is offered, so a
client that connects three times with one message is three, and a monitor
that only connects is one; messages cannot be counted. *Decided by the
owner, 2026-10-14:* since the M13 fix the figure also counts whole
messages - those a panel that hangs mid-message makes cleanup defer, one
per message, with a queue id - and the two are shown apart, not mixed:
*Connections turned away* (smtpd's warning, the existing `deferred`
column) and *Messages deferred* (cleanup's warning, a new column of
`sender_check_days` in this stage's migration, beside the estimated
mark). The counter writes each warning to its own column; the card has
both columns, and the help topic says what each counts. The figures
collected before do not mix: cleanup's case was counted only from the
M13 fix on, not yet deployed when this was decided. The mockup `health`
and help topic `sender-check` first (`design:` commit), then the page and
`view/help.go`, word for word. - **Sonnet**
- [ ] Docs: architecture § Log tailer (the rotated file, the saved place, a
refusal's line and a recovered row), guide § Outbound log and § Health,
schema-migrations.md for the mark on `sender_check_days`. — **Sonnet**
### Stage 3 — persistence and restore (M1–M5, L1–L4, L7) → `2.5.2`
- [ ] **Consume the manifest after the resync succeeds** (`CheckRestore` reports;
`run` deletes after `resyncAfterRestore` returns nil). Test: Resync fails →
manifest kept → second boot resyncs. And a corrupted or truncated archive
is refused on restore and on import with its message, nothing half
written (a row of review-2026-08-followups, placed here 2026-10-14). — **Opus**
- [ ] **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 `0009` DDL only;
`TestLimitRefusalsSince`-style EXPLAIN tests against the **real** query
strings. — **Opus**
- [ ] **CLI backup**: open the source `mode=ro`, refuse a missing file; password
floor of 12 shared with the panel (`validate.MinSecretFilePasswordLen`);
`-decrypt -o` verifies the password (first chunk) before opening the
output; `OnWarn` for skipped symlinks and `Check` fails when `certs/` is
named but empty. — **Opus**
- [ ] **`secretfile` caps**: bound `128·N·r` to a stated budget (e.g. 128 MiB)
and `p` to 1-2; refuse the rest with `ErrHeader`; reject trailing bytes
after the last chunk. Tests. — **Opus**
- [ ] **`atomicfile`**: fsync the directory after rename; a test file for the
package. — **Opus**
- [ ] Docs: security.md § Backup; guide § Backup. — **Sonnet**
### Stage 4 — panel behaviour (M8, M11, L10–L19) → `2.5.3`
- [ ] Login: dummy bcrypt on unknown user; a test that measures both paths within
a tolerance. Account password change gets its own limiter bucket. Login
destroys the presented session. — **Opus**
- [ ] `http.Server`: `ReadTimeout`, `WriteTimeout`, `IdleTimeout`. — **Opus**
- [ ] **Polling recovery**: schedule from `htmx:afterRequest` on the box (or pass
`source`), handle `sendError`/`timeout` with back-off; `RequireAuth`
answers an `HX-Request` with `HX-Redirect: /login` instead of a 303. A
minimal JS test (node + jsdom) or at least an e2e that restarts the panel
with Overview open. — **Sonnet**
- [ ] `parsePage` clamp; `NaN` refused; `rate-limited` tag red (add the status to
`components.go`, flip the test); the recalc button posts only what it
needs or warns; `HandleReload` either covers DMARC maps and filter lists or
the flash and the two comments say what it covers. The log's paging is
tested at its edges: first page, last, past the last, an empty log (a row
of review-2026-08-followups, placed here 2026-10-14). — **Sonnet**
- [ ] `domain.Service.Delete`: say the domain is deleted when a later step fails,
and run `apps.Resync()` anyway. — **Opus**
- [ ] **A status filter and a day filter in the Outbound log** (owner, 2026-10-13, from the
Not done list of from-header-binding). The mockup `out-log` has drawn a
third list since the redesign and the built log never had it; the Inbound
log has its counterpart built (domain, decision, day). One list: *Any
status*, `queued`, `sent`, `deferred`, `bounced`, `rejected` - the statuses
of a row, and `rejected` shows all three kinds of refusal together. He was
shown the list with the refusals split by reason and chose it without the
split. The mockup's own list is wrong (it offers `rate-limited`, which is
a reason, and lacks `queued` and `rejected`): correct it first (`design:`
commit). And a filter by day, as the Inbound log has it (owner, the same
day): the same control, drawn in the mockup first. Both filters survive
paging and the rows' polling. - **Sonnet**
- [ ] **Milter sockets: "Listening" is asked of the socket** (owner,
2026-10-13, from the Not done list of from-header-binding).
`health.CheckSocket` ([socket.go](../internal/health/socket.go)) stats the
file and writes "Listening"; a process that died and left its socket
reads ok while mail is deferred or goes out unsigned. It connects and
closes instead, and a refused connection is an error that says nobody
answers. Find out first, on the stand: whether the panel's user may
connect to OpenDKIM's socket, and whether a connection closed without a
byte ends the sender check (security.md § Accepted risks: the milter
library panics on an empty packet) - if it does, that is fixed first, in
this step. A test with a socket file nobody listens on. - **Opus**
- [ ] **The `cards` layout takes its phone order from the box's number**
(owner, 2026-10-13, from the Not done list of from-header-binding). The
order on a phone is written in `panel.css` for four boxes to a stack
(`.sp-cards > .column:first-child > .box:nth-child(1..4)`); a fifth box
gets no rule and lands out of place with nothing failing. The box partial
sets an order class from its own number (a class, not a style: the CSP
allows no inline styles), the stylesheet orders by that class, and a test
checks that the numbers of a rendered `cards` page run in order. A change
to a component: the mockup's components page and `theme.css` first
(`design:` commit). - **Sonnet**
- [ ] **A notification is owed to each recipient, not to the subject**
(owner, 2026-10-14; from the reviews of panel-notifications, not found by
the audit). `Notifier.send` ([notify.go](../internal/notify/notify.go))
counts a subject as said when one recipient's copy was handed to Postfix;
a recipient whose copy failed, or was "held back" by the hourly ceiling,
is never told. Unseen while the owner is the only recipient; the first
domain administrator makes it "the global administrator knows the
domain's DNS broke, its administrator does not". The state of a subject
is kept per recipient (the next migration), and a later check sends to
those not yet told - so the ceiling holds a copy back instead of dropping
it. A user who gains the right to hear of a subject while it is already
open is not mailed about it. "Cleared" goes to those who were told of the
problem. Tests: one copy fails and goes out at the next check, once; the
ceiling; a user added mid-subject. - **Opus**
- [ ] **A DNS outage is "unknown", not "wrong"** (M14, M15; owner,
2026-10-14 - the two findings had no step). Each resolver gets a time
budget of its own, so a silent first resolver no longer leaves the second
an expired context ([dnscheck/resolver.go](../internal/dnscheck/resolver.go));
a timeout is Unknown, not "does not resolve" or "no PTR record"
([dnscheck/server.go](../internal/dnscheck/server.go)); a failed lookup
keeps the server's addresses it had instead of emptying them, so SPF does
not go Unknown with it. Notifications: a domain's DNS is reported broken
only on a result that is wrong, never on Unknown - checked in `notify`,
and tested. Tests: a first resolver that drops packets, a PTR timeout.
architecture.md:339-340 and the `DefaultResolvers` comment say what is
true. — **Opus**
- [ ] **DMARC reports tested where they come in** (owner, 2026-10-14; rows of
review-2026-08-followups). The panel parses reports that arrive by mail
from anyone, and the `internal/dmarc` service (`service.go`,
`addresses.go`) and the DMARC handlers have no tests; no other step
touches them. Tests of the service, and of a report that is malformed,
oversized, compressed past its limit, or names a domain the instance does
not have: refused or skipped with a reason, nothing stored, the panel
alive. A finding is fixed in this step. — **Opus**
- [ ] **The last untyped page data** (owner, 2026-10-13): `healthChecks`
([handlers_status.go](../internal/web/handlers/handlers_status.go)) returns
a struct instead of `map[string]any`, so a mistyped key in `healthPage` /
`healthCards` is a compile error and not an empty card on Server › Health.
No template change. This closes what is left of the roadmap's former
`template-data-typing`. — **Sonnet**
- [ ] **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`
- [ ] **Supervisor socket**: a group of its own (`supervisor`), panel only;
`postfix`/`opendkim` keep `selfpost` for the file paths. — **Opus**
- [ ] **OpenDKIM**: `Syslog yes` (supervisord captures it) or `Logfile`;
`Canonicalization relaxed/simple`; `OversignHeaders From`. e2e: DKIM still
verifies; a message with two `From:` is refused before signing anyway.
— **Opus**
- [ ] **Postfix**: `smtpd_tls_protocols = >=TLSv1.2`, `smtpd_tls_mandatory_protocols`
likewise, `disable_vrfy_command = yes`, `smtpd_banner = $myhostname ESMTP`;
integer validation of `RATE_LIMIT_*`, `*_SIZE_LIMIT`, `MILTER_*_TIMEOUT` at
the top of `postfix-config.sh` with a clear exit. — **Opus**
- [ ] **logrotate state under `/data/log/.logrotate.status`** (`-s`), group-
readable as the directory is. — **Opus**
- [ ] **entrypoint**: normalise the three DMARC maps with the other five;
remove the dead `set-permissions` branch. — **Opus**
- [ ] **`extract-cert.sh`**: `set -o pipefail` under `bash`, check both outputs
are non-empty PEM before moving them into place, create `0600` first.
— **Opus**
- [ ] **`crashexit`**: subscribe to `PROCESS_STATE_EXITED` with a restart-count
threshold, or raise `startsecs` so a crash after 1 s still reaches FATAL;
say in architecture.md what actually exits the container. — **Opus**
- [ ] **`envdoc_test` made real**: parse `envDefault|envInt|os.Getenv("…")` out of
`cmd/panel/*.go` and `${VAR` out of `build/*.sh`, parse the key tables out
of guide.md and architecture.md, compare. — **Opus**
- [ ] Docs: architecture § Image and processes, § Configuration; security.md
§ Process. — **Sonnet**
### Stage 6 — documentation and roadmap sweep (no product code, no cut)
- [ ] Every item under *Doc drift* above, file by file. — **Sonnet**
- [ ] roadmap.md: done ahead on 2026-10-13/14 with the owner -
`template-data-typing` struck, `structured-logging` dropped,
`review-2026-08-followups` closed with each row placed in a stage or
dropped with its reason. What is left: check that every *Test gaps* entry
above is named by the step that touches its code, and add it to that step
where it is not. — **Sonnet**
- [ ] Comments named above (`milter.go`, `sendlog.go`, `store.go`, `schema_test.go`,
`machine.go`, `inboundspam.go`, `logtail.go`, supervisord.conf,
entrypoint.sh, docker-compose.yml). — **Haiku**
- [ ] Dead code removal from § Nits, with `staticcheck` added to `test.yml` so it
stays gone. — **Haiku**
### Decided with the owner on 2026-10-12
Four items I had first put below the line; three of them are in the plan
above after discussion, which is why they are recorded here with the
reasoning rather than struck.
- 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.