mixandClaude Opus 5.5
d6ed6bcb30
test / test (push) Canceled after 0s
Cut without asking, as the owner said on 2026-10-15. Signing in costs the same for an unknown name (M8) and ends the session the browser presented (L16); the Account password check has its own limit (L15). The HTTP server bounds the request, the answer and idle connections; the full backup moves its own deadline (L14). Monitoring refreshes are scheduled from htmx:afterRequest with a back-off and a 30 s timeout, and a lost session sends htmx to sign-in (M11). The Outbound log clamps its page (L10) and filters by status and day; the multiplier refuses NaN (L11); a rate-limited refusal is red (L18); Recalculate asks before it drops an unsaved edit (L19); Reload configuration writes what a restore writes and removes the keys of deleted domains (L13, L17). The cards layout orders by number. A notification is owed to each recipient (migration 0011). A DNS outage is unknown, each resolver with its own time (M14, M15). DMARC reports too large are refused for their size. An inbound upstream has one TLS mode, changed for all its domains (L5). Plus the findings of both reviews (eaa7de0). Commits07be0cf,643892a,ca876c1,80e3979,2b8e0c0,b90a2f0,eee089a,006bed7,32dfe4a,eaa7de0. Rolled out to the test server as 2.5.3-post.eaa7de0 (the same code): migration 0011 applied, schema 11; all programs RUNNING, the container healthy. Resync's key cleanup checked against the live data: every key directory is a domain's or the hostname's, nothing to remove. CHANGELOG cut; pin 2.5.4 in deploy/docker-compose.yml, README, guide (and its sample restore message); development.md "from pin" and the stage's row; roadmap row and recommended order; stage 4 marked shipped in the plan. No tag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1482 lines
96 KiB
Markdown
1482 lines
96 KiB
Markdown
# 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. *Owner, 2026-10-14: leave it* - the probe is a trusted
|
||
application's guess, one name at a time, as before L8; refusing every
|
||
foreign address in a display name is not wanted (security.md, accepted
|
||
risks).
|
||
*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` (shipped 2026-10-14)
|
||
|
||
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.
|
||
|
||
- [x] **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**
|
||
- [x] **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**
|
||
- [x] **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**
|
||
- [x] **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**
|
||
- [x] **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**
|
||
- [x] **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**
|
||
- [x] **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**
|
||
- [x] 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 2c — the sender check reads From: as OpenDKIM does → `2.5.2` (shipped 2026-10-15)
|
||
|
||
Decided by the owner on 2026-10-14, after reading OpenDKIM's source together
|
||
(libopendkim `dkim_mail_parse`, called in `mlfi_eoh` on the first `From:`
|
||
with `SenderHeaders` unset - `build/opendkim.conf` sets none): the signing
|
||
address is the one in angle brackets, or a bare address, found while quoted
|
||
strings and comments are skipped; encoded words are not decoded. The check's
|
||
scan of the display name and comments for another sender of the server is a
|
||
guard against two parsers parting ways - and it is what makes L8 half
|
||
closed. This stage replaces the guard with the parser itself. Stage 3 and
|
||
those after it move up one patch number each.
|
||
|
||
- [x] **Port `dkim_mail_parse` to Go, from the version the image runs**
|
||
(Debian bookworm's opendkim 2.11.0~beta2 source package, not upstream's
|
||
develop branch), line for line, with its tokenizer
|
||
(`dkim_mail_first_special`, `dkim_mail_token`). The check then decides by
|
||
the address OpenDKIM will take: the application's - any address of its
|
||
domain, or one of its list - passes, whatever the display name or a
|
||
comment holds; anything else, a parse failure included, is refused. One
|
||
`From:` line and one address stay required. The scan for another sender
|
||
of the server in the name, in a comment and in a decoded name goes, and
|
||
with it the reason that told an application which domains are hosted
|
||
here (L8 closed; security.md's accepted risk struck). Whether the "one @
|
||
outside quotes" rule is still needed is decided against the port, and
|
||
said. - **Opus**
|
||
*Done 2026-10-15:* `internal/milter/mailparse.go`; the C itself compiled
|
||
from the image's source agrees with the port on 200 019 lines
|
||
(`testdata/mailparse_harness.c`). The "one @ outside quotes" rule goes:
|
||
the owner's decision says the address decides "whatever the display name
|
||
or a comment holds", and with OpenDKIM's parser there is no second one to
|
||
part ways with. Found reading the source: OpenDKIM matches its
|
||
SigningTable against "user@domain" cut to 256 bytes (MAXADDRESS), so a
|
||
longer address is refused - cut there, a quoted local part could end in
|
||
another tenant's domain. The check asks for header values with their
|
||
leading space, as OpenDKIM does; the SigningTable reader goes.
|
||
- [x] **The port checked against OpenDKIM itself.** On the stand, several
|
||
hundred generated `From:` lines - quotes, escapes, comments nested and
|
||
open, groups, brackets, encoded words, a second domain of the server in
|
||
every place - are sent; for each, the domain the port names must be the
|
||
`d=` the message arrives signed with, or both say "no signature". The
|
||
shared files of lines (`from-lines-passed.txt`, `from-lines-other.txt`)
|
||
are kept: the second one's test becomes "never signed for {other}"
|
||
rather than "refused". - **Opus**
|
||
*Done 2026-10-15:* not through Postfix, where the check refuses most of
|
||
the lines first, but straight to OpenDKIM: `TestPortAgainstOpenDKIM`
|
||
(internal/milter, test/upstream/opendkim-probe.sh)
|
||
starts OpenDKIM from the image with keys for three names and plays it 385
|
||
lines - the two shared files and lines composed from display names,
|
||
comments, groups, routes, literals and quoted local parts - through
|
||
go-milter's client; for each, the d= OpenDKIM signs with is the domain the
|
||
port predicts (its address, cut at 256 bytes, against the SigningTable),
|
||
385 of 385, 287 signed. The two lines padded to be cut at 256 bytes are
|
||
signed by OpenDKIM with the other domain's key, as read in the source.
|
||
- [x] **The heuristics review: every assumption about another component,
|
||
answered from its source** (owner, 2026-10-14: "нам нужно полное ревью
|
||
кода на предмет всей этой эвристики. Её не должно там, где можно
|
||
посмотреть код и ответить точно и однозначно" and "в коде таких наших
|
||
блоков должно быть обоснование со ссылкой на код компонента"; the rule is
|
||
in development.md § Code that relies on another component's behaviour).
|
||
First an inventory, over the whole tree (Go, build scripts, configs,
|
||
tests' fixtures, docs' claims): each place SelfPost relies on what
|
||
Postfix, OpenDKIM, Cyrus SASL, rspamd, supervisord, logrotate, Docker,
|
||
SQLite/modernc, go-milter or a browser does, with what is assumed and
|
||
what justifies it today (read, measured, nothing). It is written down as
|
||
`docs/assumptions-2026-10-14.md`. Then each is answered from the source
|
||
or documentation of the version the image runs, and the code gets the
|
||
comment that cites it; a measurement stays a measurement and says so;
|
||
where the source contradicts the code, the code is fixed in this stage.
|
||
Known before the inventory: qmgr's expiry against the queue lifetime (the
|
||
margin of queued_over_lifetime), postlogd after a rename (the rotation
|
||
window), the smtpd that outlives a reload (measured, not understood), the
|
||
NOQUEUE line's fields (two fixtures carry a sasl_username= Postfix does
|
||
not write, one presented as seen on the stand). - **Opus**
|
||
*Done 2026-10-14:* the register holds 621 entries and their answers; the
|
||
citations are in the code (`ce363f4`, `b113e72`, `599149b`, `ad3d834`).
|
||
What the sources contradict is fixed by the lines below, each named by
|
||
its register entries; the measurements the sources cannot replace are
|
||
the last of them.
|
||
- [x] **Forged filter verdicts signed** (C-R7, B-R1): rspamd 3.11.1's
|
||
milter_headers skips its verdict routines - nothing removed, nothing
|
||
added - for a client in `local_addrs` or an authenticated one
|
||
(`skip_local`, `skip_authenticated`, both on by default), while the
|
||
custom signature routine runs for every message; a sender on a private
|
||
address, or every sender when Docker's proxy shows the bridge gateway,
|
||
gets his own `X-Spamd-Result` signed. Both set to false; a dev trial in
|
||
a throwaway rspamd 3.11.1 (`rspamc --ip 10.0.0.5` with a forged header)
|
||
shows it removed and rspamd's own added. The image tag pinned to
|
||
`3.11.1` (C-R12); the from-rules take the envelope only,
|
||
`extract_from = "smtp"` (C-R4); the dead `prefilter = true` lines go
|
||
(C-R5). - **Opus**
|
||
*Done 2026-10-14:* `31c2b50`.
|
||
- [x] **The sender map read as Postfix reads it** (B-PS1): Postfix takes the
|
||
owners of the first key `mail_addr_find()` finds - the address, then
|
||
without its extension, then `@domain` - not any key that lists the
|
||
login; today an application holding `@domain` may put in `From:` an
|
||
address a list-mode application of the same domain owns, which Postfix
|
||
refuses it as envelope sender. First key wins; tests with a list and a
|
||
wildcard application on one domain. The map parser reads continuation
|
||
lines and blank-separated owners as Postfix does (B-PS4). - **Opus**
|
||
*Done 2026-10-14:* `55aef6e`; a key given twice makes the check defer,
|
||
as it makes Postfix refuse the table.
|
||
- [x] **Queue lifetimes read as Postfix reads them** (A-8, B-PQ1, B-PQ10,
|
||
B-PQ11): expiry is one qmgr line per message, `<id>: from=<..>,
|
||
status=expired, returned to sender`, without `to=` - the tailer maps it
|
||
to every open row of that id, closed as bounced, and the dead
|
||
per-recipient `expired` case goes; `postqueue -p` marks a forced-expired
|
||
message `#`, which the listing parser keeps; a bare number in a time
|
||
parameter takes the parameter's own default unit (d for the lifetimes, h
|
||
for delay_warning_time) and `conv_time()` reads one number and one unit.
|
||
- **Opus**
|
||
*Done 2026-10-14:* `0ed03b2`, with the next line.
|
||
- [x] **Refusals matched by what Postfix writes** (A-21, A-23, A-15, A-24):
|
||
smtpd's `milter-reject` line carries `sasl_username=` - the refusal is
|
||
matched on the login too, so two applications behind one address sending
|
||
as one sender are told apart; `sasl_username=` ends at a comma
|
||
(`sasl_sender=` may follow); cleanup's `to=` is the last recipient only,
|
||
which the rows built from it say; the `IPv6:` trim goes. - **Opus**
|
||
*Done 2026-10-14:* `0ed03b2`.
|
||
- [x] **Recipients matched by what Postfix writes** (B-G5, D-S20, A-9,
|
||
A-31): go-milter hands the address without brackets and parameters, so
|
||
`cleanAddress` cuts no blank; Postfix keeps a recipient's case, so the
|
||
match is exact; a rewritten recipient is matched by `orig_to=`; a
|
||
recipient given twice is one row (cleanup drops the duplicate). Tests
|
||
through go-milter's own client. - **Opus**
|
||
*Done 2026-10-14:* `c8f6697`; the match stays case-insensitive, now with
|
||
its reason from the source (D-S20).
|
||
- [x] **The log tailer's rotation and stamps** (A-33, A-12): postlogd writes
|
||
to the renamed file until `postfix reload`, so the old file is drained
|
||
for a while after the switch, not once; no Postfix writes the ISO stamp
|
||
`isoStampRe` was made for - it goes, and the fixtures take the syslog
|
||
stamp Postfix 3.7 writes. - **Opus**
|
||
*Done 2026-10-14:* `463d4ff`; the renamed file is read for a minute
|
||
after the switch.
|
||
- [x] **Inbound mail over 10 MB** (C-P31): cleanup runs with main.cf's
|
||
`message_size_limit`, never set, so 10240000 bytes, whatever port 25's
|
||
`-o` says - a 12 MB message to the inbound relay is accepted through
|
||
DATA and then refused 552. cleanup gets the largest listener limit; e2e
|
||
sends 12 MB inbound. And `local_transport = error:...` (C-P8): an
|
||
address literal of the container's own address is local today.
|
||
- **Opus**
|
||
*Done 2026-10-14:* `e174537`; cleanup takes port 25's limit when it is
|
||
larger than main.cf's; Postfix refuses the address literal at RCPT.
|
||
- [x] **Supervision as supervisord does it** (C-P42, C-V5, C-V6, C-V10,
|
||
B-U2, C-P34): master leaves the shell's process group, so stopping the
|
||
program never stops Postfix - the wrapper traps TERM and runs `postfix
|
||
stop`; `startsecs` above the milter wait for Postfix and 1 for logrotate
|
||
so a failure reaches FATAL (logrotate's not taking the container down);
|
||
postfix-reload rests in EXITED after a run; a reload's exit status is
|
||
read, not assumed; the dead set-permissions branch goes. e2e: restart
|
||
of the program, stop of the container. - **Opus**
|
||
*Done 2026-10-14:* `f2ad699`; startsecs is MILTER_WAIT_TIMEOUT + 10, set
|
||
by the entrypoint; the reload's status is read over supervisord's XML-RPC.
|
||
- [x] **sasldb written in place** (C-S3, C-Q3): Cyrus opens sasldb2 without
|
||
a Berkeley DB environment, so without locks, and `saslpasswd2` writes in
|
||
place; an AUTH or a backup during a change can read it torn. The panel
|
||
writes a copy and renames it, and the backup reads it under the same
|
||
lock. - **Opus**
|
||
*Done 2026-10-15:* no lock for the backup after all: it reads each file
|
||
through one descriptor, and a rename leaves that file whole; the panel's
|
||
own changes are serialised by a mutex.
|
||
- [x] **Health says what is served** (A-38, A-65, D-P1, D-P3): a socket file
|
||
is not a listener - unix sockets are dialled as inet ones are; the
|
||
certificate shown is the one Postfix serves (`postconf -h
|
||
smtpd_tls_cert_file`), with a renewal waiting named; the wording of
|
||
"Postfix defers mail" and of Required TLS (mockup first). - **Opus**
|
||
*Done 2026-10-15:* `ea8b6bf` (mockup) and the commit after it; a probe
|
||
connection is silent in all three milters (libmilter read: sendmail
|
||
8.17.1.9 fetched into dev/upstream); the expiry notice waits for a renewal
|
||
that is on time.
|
||
- [x] **The panel in browsers without Sec-Fetch-Site** (D-B3, D-B13): under
|
||
`Referrer-Policy: no-referrer` a form POST carries `Origin: null`, which
|
||
the origin check refuses - every POST, sign-in included, fails in Safari
|
||
before 16.4 and over plain HTTP at a name; `same-origin` instead, with a
|
||
header test and one POST on the stand. A shown-once secret is blanked on
|
||
`pagehide`; `Pragma` goes. - **Opus**
|
||
*Done 2026-10-15.* Measured the same day in Chrome 152 (the desktop
|
||
app's browser): a `no-store` page is not restored from the back/forward
|
||
cache (pageshow persisted=false, a fresh load); the `pagehide` handler is
|
||
for browsers that do restore one. Firefox and Safari not measured.
|
||
- [x] **Two processes opening an old database** (D-S3): `user_version` is
|
||
read outside the transaction, so the panel and dmarc-ingest can both
|
||
run one migration; `_txlock=immediate` and the version re-read inside
|
||
it; a test with two concurrent opens behind a held lock. - **Opus**
|
||
*Done 2026-10-15:* the test fails without the fix exactly as the
|
||
register said ("table settings already exists") and passes with it.
|
||
- [x] **Certificates from certbot** (C-A1, C-Q4, D-guide:258, :291, :722):
|
||
certbot's `live/` files are relative links into `archive/`, so mounting
|
||
`live/<host>` alone, or linking `certs` to it, leaves Postfix without a
|
||
certificate and the backup without it. The deploy fragments mount
|
||
`/etc/letsencrypt` read-only and point the paths into it; the backup
|
||
follows a link under `certs/` to a regular file. - **Opus**
|
||
*Done 2026-10-15:* `certs/` holds links into `/etc/letsencrypt/live/`,
|
||
mounted read-only at the same path; certbot's own `chmod 0755` and
|
||
`chgrp`/`chmod 0640` for a server that is not root (kept on renewal) let the
|
||
panel read them; no copying hook.
|
||
- [x] **The rest of what the sources contradict** (C-M7 `<pct>0</pct>`,
|
||
C-E2 the e2e zone serial, C-D1 a release gate comparing the image's
|
||
package versions with the ones cited, C-K8/K9 and the comments the
|
||
answers name), and **the documents**: every claim of architecture.md,
|
||
security.md and guide.md the answers find false or partly true is
|
||
rewritten as the answers propose (D-arch, D-sec, D-guide). - **Sonnet**
|
||
*Done 2026-10-15:* the release gate is `checkCitedPackageVersions` (e2e)
|
||
against the table in development.md; the documents by three agents, one
|
||
per document, reviewed. Two register proposals were outdated by this
|
||
stage's fixes (a bounce's `From:` line, `Origin: null`) and not used; the
|
||
nginx renewal needed `--webroot` in the loop, which the register missed.
|
||
- [x] **What only the stand can answer** (A-43, C-K5, C-M8, C-X1, D-S7,
|
||
D-B16 and the document claims marked so): postlog's set-gid in the
|
||
image; the client address Postfix sees through Docker's port publishing
|
||
over IPv4 and IPv6; the largest DMARC report received; the public keys
|
||
of an ECDSA pair as the reload hook compares them; the longest write
|
||
against busy_timeout at the retention limit; the favicon each browser
|
||
takes. Each measured, dated and written beside the code it settles.
|
||
- **Opus**
|
||
*Done 2026-10-15,* each written beside what it settles: postlog is not
|
||
set-gid in Debian's package (A-43); Docker's proxy hands every IPv6
|
||
client and the host's loopback over as the gateway, IPv4 keeps the
|
||
client (C-K5; a new guide paragraph, and a Security note: an allow-list
|
||
naming the gateway admits every IPv6 client); one DMARC report of one
|
||
record so far, too little to judge the cap (C-M8); OpenSSL 3.0.22 prints
|
||
one public key alike for RSA and ECDSA (C-X1); the longest write, the
|
||
line prune, 1.9 s at 1 900 messages a day after a 30-day gap, under
|
||
busy_timeout (D-S7, D-arch:516); about 6 lines of 146 bytes a message
|
||
(D-arch:532); Edge takes the SVG favicon, the desktop app's browser the
|
||
PNG (D-B16); a published port nothing listens on is refused over IPv4,
|
||
accepted by the proxy otherwise (D-guide:61); deny beats allow in every
|
||
mix on rspamd 3.11.1-3 (D-arch:121); rspamd runs as uid 11333 (D-sec:424).
|
||
- [x] Docs and wording: help topic sender-check and guide § Who an
|
||
application may send as (mockup first), security.md § Mail path,
|
||
CHANGELOG. - **Sonnet**
|
||
*Done 2026-10-15* with the port (`2cf118c` the help mockup, `3584adc`).
|
||
|
||
### Stage 3 — persistence and restore (M1–M5, L1–L4, L7) → `2.5.3` (shipped 2026-10-15)
|
||
|
||
- [x] **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**
|
||
*Done 2026-10-15:* `cmd/panel` `openData` runs CheckRestore, `store.Open`,
|
||
the Resync and then `backup.ConsumeRestore`. A truncated archive had
|
||
nothing to refuse it - the manifest is the first entry, so half an
|
||
archive was a restore on half a database: every archive now ends with
|
||
`data/contents.json` (size and SHA-256 of each `data/` file; the deploy
|
||
files are the operator's to edit), CheckRestore compares `/data` with it
|
||
before the database is opened and renames it `contents.checked.json`, so
|
||
a start after a failed one (which had the database open) does not compare
|
||
again unless the manifest changed. Tests: the start that fails, a cut in
|
||
the database / before the deploy files / in the last entry (database
|
||
untouched, no `-wal`), a damaged key named; the domain import of a cut
|
||
plain export, a cut and an altered `.spde` (each refused, nothing
|
||
created). The CLI's `-decrypt -o` is step 3.
|
||
- [x] **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**
|
||
*Done 2026-10-15,* migration `0010` (0009 went to stage 2b). Stage 2
|
||
already counted by `(domain_id, created_at)`; `idx_send_log_domain` stays,
|
||
the global log's filter by name reads its page off it in id order. The
|
||
plans of every `send_log` statement showed four more: the sweep now asks
|
||
for queued *and* deferred rows - `idx_send_log_open (created_at) WHERE
|
||
status IN ('queued', 'deferred')`, the statuses literal and `ORDER BY +id`
|
||
(else SQLite walks the table in id order); `NOT EXISTS (... refusal_line
|
||
= ?1)` of stage 2b scanned the table per refusal line - partial
|
||
`idx_send_log_refusal_line`, and its row is found by
|
||
`idx_send_log_rejected` (status literal, `+queue_id`); the application's
|
||
level-2 count went by the domain's index - `+domain_id`; a page of one
|
||
domain or one application - `idx_send_log_domain_id`,
|
||
`idx_send_log_app_login` (id order, no sort; several domains still sort,
|
||
top-N). The statements are constants/builders the test explains
|
||
(`TestSendLogQueriesUseTheirIndexes`, which replaces the hand-written
|
||
plan check of `TestLimitRefusalsSince`).
|
||
- [x] **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**
|
||
*Done 2026-10-15.* The source check is in `backup.snapshotDB`, so the
|
||
panel has it too; `mode=ro` measured not to create a missing file
|
||
(`TestSnapshotRefusesAMissingDatabase`). `-o` of both directions writes
|
||
a temporary file beside the name and renames it whole (a backup that
|
||
fails half way left a partial file too); `-decrypt` peeks the first
|
||
chunk - the password, and the envelope's type, authenticated - before
|
||
it opens the output. Since 2.5.2 links under `certs/` are followed, so
|
||
what is still left out and now warned of: a link under `data/`, a link
|
||
under `certs/` to a directory, a socket or the like; `certs/` with no
|
||
file refused by Check and Create alike. L4, in this stage's list without
|
||
a line: a bare address takes /128 when written with a colon, /32
|
||
otherwise (`::ffff:10.0.0.1/32` was `::/32`).
|
||
- [x] **`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**
|
||
*Done 2026-10-15:* 128 MiB of scrypt's `V` (x/crypto v0.58.0 `Key`:
|
||
`make([]uint32, 32*N*r)`), `p` ≤ 2, `logN` ≤ 30 for the arithmetic; the
|
||
refusal is the existing `ErrCorrupt` (there is no `ErrHeader`), which the
|
||
import shows as "damaged or incomplete". One byte is read after the last
|
||
chunk; any is `ErrCorrupt`. `TestKDFBudget` (the old cap's 4 GiB among
|
||
the refused), `TestBytesAfterTheLastChunkRejected`.
|
||
- [x] **`atomicfile`**: fsync the directory after rename; a test file for the
|
||
package. — **Opus**
|
||
*Done 2026-10-15:* `atomicfile.SyncDir`, best effort as sasl.go's (the
|
||
file is in place either way; Go on Windows cannot sync a directory), used
|
||
by `Write`, `app.replaceSASLDB` (its inline copy replaced) and the CLI's
|
||
`-o`. `atomicfile_test.go`: create, replace, mode, nothing left beside;
|
||
an open reader keeps the old file; a failed write (missing directory,
|
||
a directory in the way) leaves the old state; SyncDir works on Linux.
|
||
- [x] Docs: security.md § Backup; guide § Backup. — **Sonnet**
|
||
*Done 2026-10-15:* security.md § Backup and domain export (what is warned
|
||
of, the missing database, the restore's record - integrity, not
|
||
authenticity -, the KDF budget, trailing bytes, the CLI's floor and
|
||
`-o`); guide § Restore (step 1), § Full backup (certs/, `-o`, the
|
||
database) and § Encrypting (the first block, a cut file, the floor).
|
||
|
||
*Changed by the reviews of stage 3 (2026-10-15; technical and security,
|
||
reports in the session's scratchpad):* **H1** - a restore of a server with
|
||
DMARC on was refused on every start: the archive records no owner, tar as
|
||
root leaves the files root's, and the entrypoint handed back to the panel
|
||
only the maps it named, not the three DMARC maps, which the comparison then
|
||
could not read; the entrypoint now hands every file beside `queue/` in
|
||
`/data/postfix` to `panel:selfpost` 0640 (e2e: an extra map written before
|
||
the backup, unpacked as root, `panel:selfpost 640` after). The comparison
|
||
reads regular files only (`Lstat`, size first, `LimitReader`: a link or a
|
||
pipe in a crafted archive is refused, not followed or waited on); a record
|
||
unpacked again wins over the mark of an earlier check, and ConsumeRestore
|
||
removes both; the mark is synced; a `selfpost.db-wal/-shm/-journal` beside a
|
||
freshly unpacked database is refused. `secretfile`: r ≤ 64 as well (N=2
|
||
with a huge r asked 384 MiB of B and XY). The panel names an empty `certs/`
|
||
(`backup.ErrNoCerts`) instead of "could not read". `AttachRefusalLine`
|
||
refuses an empty line, as the old statement did. A skipped entry's kind in
|
||
words. `-decrypt` of an empty payload is not a failure. Guide: the shell's
|
||
`>` empties the file before a wrong password is found, a killed CLI leaves
|
||
`.NAME.tmp-*`, unpack again into an empty directory, the comparison is made
|
||
once, and step 2 of the in-place restore stepped out of `.` (`mv .` fails;
|
||
older than this stage). Accepted, with the reason: hashing every `data/`
|
||
file delays the first restore start - SHA-256 runs at hundreds of MB/s and
|
||
the database measured on the test server is under a megabyte (a 30-day
|
||
journal at 1 900 messages a day is tens of MB), well inside Postfix's wait
|
||
for the milter sockets; a CLI killed by SIGKILL cannot clean its temporary
|
||
file, and a handler for the gentler signals buys little for a one-off
|
||
command whose leftover is the owner's alone.
|
||
|
||
### Stage 4 — panel behaviour (M8, M11, L10–L19) → `2.5.4` (shipped 2026-10-15)
|
||
|
||
- [x] 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**
|
||
*Done 2026-10-15:* `noSuchUserHash` (BcryptCost, made once) compared
|
||
against for an unknown name or a failed lookup;
|
||
`TestLoginTakesAsLongForAnUnknownName` holds the medians of five tries
|
||
each within a factor of two at the real work factor. The bucket is per
|
||
user (`AllowPasswordChange(userID)`, 10 in 15 min): the guesser is
|
||
whoever holds that user's session. Sign-in destroys every session cookie
|
||
the request carries before it creates its own.
|
||
- [x] `http.Server`: `ReadTimeout`, `WriteTimeout`, `IdleTimeout`. — **Opus**
|
||
*Done 2026-10-15:* 30 s, 60 s, 2 min (`newHTTPServer`); the full backup
|
||
would have been cut at 60 s, so `HandleBackup` moves its write deadline
|
||
to two hours through `http.ResponseController` (no wrapper of the
|
||
ResponseWriter in the chain; e2e checks the log has no failure to move
|
||
it). A test checks the four are set and long enough for a form and a
|
||
DNS page.
|
||
- [x] **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**
|
||
*Done 2026-10-15,* read in htmx 2.0.4's source (the npm build, byte-equal
|
||
to the shipped `htmx.min.js`, in `dev/upstream/htmx-2.0.4`): both -
|
||
`source: el`, and the schedule from `htmx:afterRequest`, which
|
||
`issueAjaxRequest()` fires on load, error, abort and timeout, and after a
|
||
swap that removed the box fires again on the nearest parent still in the
|
||
page (so the box is found by `detail.target`'s id);
|
||
`detail.successful` tells failure. Back-off doubles per failure in a row
|
||
to 60 s. `toLogin`: 401 + `HX-Redirect` for htmx, which
|
||
`handleAjaxResponse()` follows before it looks at the status. No jsdom
|
||
(no node tooling in the repo): `testdata/poll_test.js` runs panel.js in
|
||
node against a stand-in for the DOM and htmx with a moved clock, from
|
||
`TestPanelJS` (skipped where node is absent - the test server's
|
||
Go has none; it fails on the old panel.js with ten findings).
|
||
- [x] `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**
|
||
*Done 2026-10-15:* the Outbound log counts first and clamps to the last
|
||
page; a number too large for an int is the last page too (strconv's
|
||
ErrRange with the largest int), on both logs. `checkAutoMultiplier` is
|
||
written "not within", so NaN and the infinities are out. The mockup drew
|
||
rate-limited grey: `design:` commit first (out-log, and the kit's status
|
||
row, which had also lost `reject`), then `components.go`. The recalc
|
||
button warns: `data-unsaved` on both recalc buttons, and panel.js asks
|
||
on submit (`ev.submitter`) when a field of the page differs from what it
|
||
was drawn with (`testdata/unsaved_test.js` under `TestPanelJS`).
|
||
`HandleReload` covers what the restore's Resync covers, the filter lists
|
||
and the DMARC maps included, and a failed step is an error flash on
|
||
Health naming it.
|
||
- [x] `domain.Service.Delete`: say the domain is deleted when a later step fails,
|
||
and run `apps.Resync()` anyway. — **Opus**
|
||
*Done 2026-10-15:* past `DeleteDomain` both rebuilds run and the errors
|
||
join under `domain.ErrDeletedIncomplete`; the key is removed only when
|
||
the tables were rebuilt (they may still name it otherwise). The domain
|
||
list says "Domain X deleted, but …; run Reload configuration" (500) -
|
||
which since the step above rewrites everything.
|
||
- [x] **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**
|
||
*Done 2026-10-15:* `design:` commit (the list corrected, the two dates),
|
||
then `view.OutLogFilter` (domain, app, status, from, to) carried by the
|
||
poll, the pages, the rows' links and the message's way back;
|
||
`SendLogFilter.Status`, `Since`, `Until` (whole UTC days, as the
|
||
Inbound log's). A status not in the list is no narrowing. Plans: a
|
||
domain user's filtered page goes by `idx_send_log_domain_id` (with a day
|
||
filter by `idx_send_log_domain_id_created_at` and a sort - not pinned by
|
||
the plan test); the
|
||
global role's status-only count walks the table (a page view or a 5 s
|
||
poll while the log is open on a status - tens of thousands of rows at
|
||
the retention limit, milliseconds), accepted.
|
||
- [x] **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**
|
||
*Done in 2.5.2 (stage 2c, `98e09fe`), ticked here 2026-10-15:*
|
||
`health.CheckSocket` dials and closes; a refused connection says nobody
|
||
answers, a refused permission is Unknown; a connection closed without a
|
||
byte ends a milter session quietly on both sides (libmilter's
|
||
`mi_rd_cmd`, go-milter v0.4.1 `HandleMilterCommands` on `io.EOF` - the
|
||
comment on CheckSocket cites them). The socket nobody listens on is
|
||
`milter_test.go`.
|
||
- [x] **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**
|
||
*Done 2026-10-15:* an attribute, not a class: the mockup's check
|
||
(`check.py`) refuses a class `theme.css` defines and no page uses, and a
|
||
class per number up to twelve would be eight such. `data-order="N"` is
|
||
not a style either, so the CSP holds; `theme.css` and `panel.css` have
|
||
`.sp-cards [data-order="1..12"]`. `box_open` writes it for a box with
|
||
`InCards` (Health's eight, the kit's three) and a number.
|
||
`TestCardsBoxesCarryTheirNumberAsTheirOrder` reads the rendered pages:
|
||
each box's data-order is its number, and the stylesheet has its rule.
|
||
- [x] **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**
|
||
*Done 2026-10-15:* migration `0011`, `notify_addressees (key, address,
|
||
told_at)`, gone with the subject's row. `open` sends the first message
|
||
to every recipient and records who was handed it; `followUp`, on every
|
||
check while the subject is open and still wrong (DNS bad, the
|
||
certificate at its level, the queue still stuck, a limit within its
|
||
day), hands the current message to the owed and untold who are still
|
||
recipients; `closed` sends the end to the told (all recipients for a
|
||
subject opened before the table). A cleared copy that fails is not
|
||
retried - the subject is gone with it, as before. Tests: a failed copy
|
||
goes out at the next check, once; the ceiling holds one back; a user
|
||
added mid-subject hears nothing of it, its end included.
|
||
- [x] **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**
|
||
*Done 2026-10-15:* `queryEach` gives each resolver
|
||
`perResolverTimeout` (2 s) within the check's context and asks the one
|
||
that answered last first (a dropping first resolver costs one 2 s wait,
|
||
then is asked last); an address or PTR lookup not answered is Unknown
|
||
(`lookupFailed`), an answered mismatch beside it still the error it is;
|
||
`Checker.Server` keeps the last answer's addresses when the lookup was
|
||
not answered. `notify.dnsReading` already reported faults only - now
|
||
tested with SPF unknown alone and beside a wrong DKIM. architecture.md
|
||
no longer has the sentence the audit cited; the guide's
|
||
`SELFPOST_DNS_RESOLVERS` row says the budget and the order.
|
||
- [x] **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**
|
||
*Done 2026-10-15.* Since the audit the package had gained tests (bounds,
|
||
unknown domain, tag mismatch, not a report, a store failure, the
|
||
allow-list) and the handlers theirs. Added: malformed XML, a gzip bomb
|
||
and a zip bomb (9 MiB of zeroes), a message over the limit - each
|
||
refused with its reason, nothing stored; the service's Resync on and
|
||
off, the hosted suggestion and its validation. The finding: both limits
|
||
cut silently (`io.LimitReader`), so an oversized report failed as
|
||
damaged; `readAtMost` refuses it for its size (`MaxMessage` 10 MiB, the
|
||
message limit moved from cmd/panel into `IngestMessage`). The panel
|
||
is not in the path: ingest is its own process per message, and a
|
||
refusal exits 0 with the reason in `mail.log`.
|
||
- [x] **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**
|
||
*Done 2026-10-15:* `healthReadings`, read by `healthPage` and
|
||
`healthCards` and the tests' fixtures; no `map[string]any` is left in
|
||
internal/web outside tests.
|
||
- [x] **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**
|
||
*Done 2026-10-15:* `inbound.Service.checkOneTLSModePerUpstream` (the
|
||
domain itself may change its own mode; another port is another next
|
||
hop); `renderInboundMaps` writes a next hop's policy line once (rows
|
||
saved before keep the first by domain name, as Postfix did). Tests in
|
||
inbound and postfix (`TestRenderInboundMapsOneTLSLinePerNextHop`).
|
||
|
||
*Changed by the reviews of stage 4 (2026-10-15; technical and security):*
|
||
**two domains on one upstream could never change its TLS mode** - each
|
||
save was refused for the other (medium); `inbound.Service.SetTransportAs`
|
||
now changes the mode for every domain on the upstream when the one saving
|
||
may change them all, and refuses a domain user who may not with the mode,
|
||
not the other tenant's name (the security review's F1: the first version's
|
||
refusal named it). The deletion's leftover key is removed by Resync (Reload
|
||
configuration, a restore's start) for names no domain has. The backup's
|
||
write deadline moves before Check. `hasUnsavedEdit` skips password and file
|
||
fields (a password manager filling the export password made Recalculate
|
||
ask) and compares a multiple select's options. `htmx.config.timeout` 30 s,
|
||
so a refresh that is never answered ends and is tried again; the rejected
|
||
promise of a failed refresh is handled. A filtered empty Outbound log says
|
||
so. The dummy hash is made at start. A DMARC part over 8 MiB is refused for
|
||
its size like the others. The Configuration box on Health says what Reload
|
||
writes (`design:` first). Tests: the shared upstream (service and handler),
|
||
the leftover key, all eight Health boxes ordered, the legacy subject.
|
||
Accepted, with the reason: a notification is owed to an address
|
||
(architecture.md § Notifications); SERVFAIL from every resolver is Unknown
|
||
- it is not an answer about the record; Reload repeats the restore's list
|
||
of steps by hand (`HandleReload` and `mailStack.Resync`, both commented); a
|
||
failed `SetNotifyAddressees` is logged and the subject stays said, the next
|
||
follow-up finds nobody owed; the profile form's "username taken" is
|
||
documented in security.md.
|
||
|
||
### 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.
|