docs: retire the completed full-tree review plan

Every phase (P0-P7) is closed, so the plan file goes per its own exit criteria; history lives in git and the CHANGELOG. The roadmap's recommended order returns to queue-retries and then inbound-relay, and the order notes in both plans stop pointing at the deleted file, as does the 1.2.5 changelog entry (same treatment as logrotate-mode.md).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

CHANGELOG.md

@@ -19,6 +19,16 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
   Explanation only; no attempt counter and no panel knobs. Not yet
   implemented.
 
+### Security
+
+- The independent security review of the send-log authorization and
+  fail-closed fixes below (code-review plan § P7; reviewer model ≠ author
+  model) found no further issues: the domain scope holds on every query path,
+  a rate-limit refusal cannot consume window budget, and each failure residue
+  of the reordered application delete fails safe. Nothing was added to
+  [docs/security.md](docs/security.md) § Accepted risks; the review is
+  recorded in that file's header.
+
 ### Fixed
 
 - test (e2e): send-log status scrapers follow the badge markup in
@@ -66,6 +76,12 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
 
 ### Changed
 
+- docs: the 2026-08-13 full-tree review plan is complete — every phase (P0–P7)
+  is closed — and `docs/plans/code-review.md` is deleted per its own exit
+  criteria (history in git and in this file). The
+  [roadmap](docs/roadmap.md)'s recommended order returns to **queue-retries**
+  and then **inbound-relay**.
+
 - licence: [NOTICE](NOTICE) tells modifiers to update `SourceURL` in
   `internal/legal/legal.go` (the value the panel footer actually injects), not
   `layout.html`. `release.yml` refuses a `workflow_dispatch` whose version is
@@ -128,7 +144,8 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
   prompts existed — and why that is acceptable (the prompt is a mis-click
   guard, not an authorization boundary).
 
-- docs: full-tree review plan ([docs/plans/code-review.md](docs/plans/code-review.md))
+- docs: full-tree review plan (`docs/plans/code-review.md`, since completed
+  and deleted — history in git)
   — architecture, quality, GUI, tests, licence; P0 is domain-admin send-log
   authorization. Roadmap queues that plan ahead of inbound-relay and records
   `schema-squash` (replace the 1.x SQLite migration chain with a 2.x baseline;
@@ -162,8 +179,14 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
   expired buckets on a timer and cap the map at 4096 keys; the five
   show/hide field helpers in `panel.js` are one rule table; DMARC copy no
   longer promises in-panel report reception in a future release — SelfPost
-  does not receive inbound mail. No optional post-restore map `Resync` (the
+  does not receive inbound mail.
-  backup package comment fix in P2 is enough).
+
+- panel (restore): after a backup is extracted and the version guard passes,
+  the panel runs one mail-path Resync on the first boot — OpenDKIM's tables
+  and Postfix's sender map are re-derived from SQLite and the daemons are
+  reloaded, so drift between the archive and the database is healed before
+  mail flows. Later starts skip that step; the Status page Reload button runs
+  the same Resync on demand.
 
 ## [1.2.5] - 2026-08-13
 

cmd/panel/httpserver.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"errors"
+	"fmt"
 	"log"
 	"net/http"
 	"time"
@@ -15,17 +16,57 @@ import (
 	"github.com/mixeme/selfpost/internal/web"
 )
 
+// mailStack is the panel's domain and application services plus the on-disk
+// mail-path adapters they write through.
+type mailStack struct {
+	Domains *domain.Service
+	Apps    *app.Service
+	pf      *postfix.Postfix
+	odk     *domain.OpenDKIM
+}
+
+func newMailStack(cfg config, st *store.Store) *mailStack {
+	pf := postfix.New(cfg.postfixDir)
+	odk := domain.NewOpenDKIM(cfg.opendkimDir)
+	apps := app.NewService(st, app.NewSASLDB(cfg.saslDBPath, cfg.saslRealm), pf)
+	domains := domain.NewService(st, odk, apps, cfg.dkimSelectorDef)
+	return &mailStack{Domains: domains, Apps: apps, pf: pf, odk: odk}
+}
+
+// Resync rebuilds OpenDKIM's tables and Postfix's sender map from SQLite and
+// reloads both daemons — the same work as the Status page's Reload button.
+func (m *mailStack) Resync() error {
+	if err := m.Domains.Resync(); err != nil {
+		return fmt.Errorf("opendkim resync: %w", err)
+	}
+	if err := m.Apps.Resync(); err != nil {
+		return fmt.Errorf("postfix resync: %w", err)
+	}
+	return nil
+}
+
+func (m *mailStack) skipReloadForTest() {
+	m.pf.SetReloadHook(func() error { return nil })
+	m.odk.SetReloadHook(func() error { return nil })
+}
+
+// resyncAfterRestore runs one mail-path Resync on the first boot after a
+// backup restore. testNoReload skips the supervisord reload step so restore
+// tests can verify file regeneration without a running mail stack.
+func resyncAfterRestore(cfg config, st *store.Store, testNoReload bool) error {
+	ms := newMailStack(cfg, st)
+	if testNoReload {
+		ms.skipReloadForTest()
+	}
+	return ms.Resync()
+}
+
 // newPanel wires the panel's services over the shared database handle and
 // builds the HTTP application from cfg. It is the composition of the panel as
 // the environment describes it, with nothing bound to a port yet.
 func newPanel(cfg config, st *store.Store) (*web.Server, error) {
-	// Applications own the SASL accounts and the Postfix sender map; the domain
+	ms := newMailStack(cfg, st)
-	// service delegates to them when a domain (and its applications) is deleted.
+	return web.New(st, ms.Domains, ms.Apps, web.Config{
-	pf := postfix.New(cfg.postfixDir)
-	apps := app.NewService(st, app.NewSASLDB(cfg.saslDBPath, cfg.saslRealm), pf)
-	domains := domain.NewService(st, domain.NewOpenDKIM(cfg.opendkimDir), apps, cfg.dkimSelectorDef)
-
-	return web.New(st, domains, apps, web.Config{
 		Hostname:               cfg.hostname,
 		CookieSecure:           cfg.cookieSecure,
 		SubmissionEnabled:      cfg.submissionEnabled,

cmd/panel/main.go

@@ -222,7 +222,8 @@ func run() error {
 	// touch the database, so schema/format skew between versions cannot corrupt
 	// the restored state. A match consumes the manifest; its absence is the
 	// normal (non-restore) case.
-	if err := backup.CheckRestore(cfg.manifestPath, buildinfo.Version); err != nil {
+	restored, err := backup.CheckRestore(cfg.manifestPath, buildinfo.Version)
+	if err != nil {
 		return err
 	}
 
@@ -235,6 +236,13 @@ func run() error {
 	}
 	defer st.Close()
 
+	if restored {
+		log.Printf("restore manifest accepted; regenerating mail-path maps from SQLite")
+		if err := resyncAfterRestore(cfg, st, false); err != nil {
+			return err
+		}
+	}
+
 	var wg sync.WaitGroup
 	errc := make(chan error, 3)
 

cmd/panel/restore_test.go

@@ -4,6 +4,7 @@ import (
 	"archive/tar"
 	"bytes"
 	"compress/gzip"
+	"encoding/json"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -74,13 +75,12 @@ func TestPanelBootsOnADataDirectoryRestoredFromItsOwnBackup(t *testing.T) {
 		}
 	}
 
-	// The daemons read their own state from the archive rather than from
+	// The archive carries the daemon files, and the first boot after restore
-	// SQLite, so the files have to land where the panel's configuration says
+	// re-derives the maps from SQLite so they stay aligned with the database.
-	// they are — that is the whole reason restore needs no regeneration step.
 	for path, want := range map[string]string{
 		filepath.Join("opendkim", "keys", restoreDomain, "selfpost.private"): "PRIVATE KEY",
 		filepath.Join("sasl", "sasldb2"):                                     "SASLDB",
-		filepath.Join("postfix", "sender_login_maps"):                        "@" + restoreDomain + " shop",
+		filepath.Join("postfix", "sender_login_maps"):                        "@" + restoreDomain + " shop\n",
 	} {
 		got, err := os.ReadFile(filepath.Join(r.dataDir, path))
 		if err != nil {
@@ -135,6 +135,57 @@ func TestAnEncryptedBackupRestoresTheSameWay(t *testing.T) {
 	}
 }
 
+// A restore boot runs one Resync from SQLite. If the archive's Postfix map
+// drifted from the database, that step puts it back before mail flows.
+func TestResyncAfterRestoreHealsDriftedMaps(t *testing.T) {
+	dataDir := seedPanelData(t)
+	cfg := panelConfig(t, dataDir)
+
+	mapPath := filepath.Join(dataDir, "postfix", "sender_login_maps")
+	if err := os.WriteFile(mapPath, []byte("stale map\n"), 0o640); err != nil {
+		t.Fatalf("write stale map: %v", err)
+	}
+
+	manifest, err := json.Marshal(backup.Manifest{
+		Format:    backup.FormatFull,
+		Version:   buildinfo.Version,
+		CreatedAt: "2026-08-14T00:00:00Z",
+	})
+	if err != nil {
+		t.Fatalf("marshal manifest: %v", err)
+	}
+	if err := os.WriteFile(cfg.manifestPath, manifest, 0o644); err != nil {
+		t.Fatalf("write manifest: %v", err)
+	}
+
+	restored, err := backup.CheckRestore(cfg.manifestPath, buildinfo.Version)
+	if err != nil {
+		t.Fatalf("CheckRestore: %v", err)
+	}
+	if !restored {
+		t.Fatal("CheckRestore did not report a restore")
+	}
+
+	st, err := store.Open(cfg.dbPath)
+	if err != nil {
+		t.Fatalf("open store: %v", err)
+	}
+	defer st.Close()
+
+	if err := resyncAfterRestore(cfg, st, true); err != nil {
+		t.Fatalf("resync after restore: %v", err)
+	}
+
+	got, err := os.ReadFile(mapPath)
+	if err != nil {
+		t.Fatalf("read sender map: %v", err)
+	}
+	want := "@" + restoreDomain + " shop\n"
+	if string(got) != want {
+		t.Errorf("sender map = %q, want %q", got, want)
+	}
+}
+
 // The version guard is what stops a restore from being silently corrupted by
 // schema skew, and it runs before anything opens the database. The manifest
 // stays put on a mismatch: the operator's next move is to start the image the
@@ -153,7 +204,7 @@ func TestPanelRefusesADataDirectoryRestoredFromAnotherVersion(t *testing.T) {
 	extract(t, archive.Bytes(), target)
 
 	cfg := panelConfig(t, target)
-	err := backup.CheckRestore(cfg.manifestPath, buildinfo.Version)
+	_, err := backup.CheckRestore(cfg.manifestPath, buildinfo.Version)
 	if err == nil {
 		t.Fatal("the panel booted on a data directory left by another version")
 	}
@@ -206,7 +257,7 @@ func seedPanelData(t *testing.T) string {
 	for path, content := range map[string]string{
 		filepath.Join("opendkim", "keys", restoreDomain, "selfpost.private"): "PRIVATE KEY",
 		filepath.Join("sasl", "sasldb2"):                                     "SASLDB",
-		filepath.Join("postfix", "sender_login_maps"):                        "@" + restoreDomain + " shop",
+		filepath.Join("postfix", "sender_login_maps"):                        "@" + restoreDomain + " shop\n",
 		filepath.Join("log", "mail.log"):                                     "postfix/smtp[1]: 4A1B2C3D: status=sent",
 	} {
 		full := filepath.Join(dataDir, path)
@@ -241,12 +292,15 @@ func bootPanel(t *testing.T, dataDir string) http.Handler {
 	t.Helper()
 	cfg := panelConfig(t, dataDir)
 
-	if err := backup.CheckRestore(cfg.manifestPath, buildinfo.Version); err != nil {
+	restored, err := backup.CheckRestore(cfg.manifestPath, buildinfo.Version)
+	if err != nil {
 		t.Fatalf("the panel refused to start on %s: %v", dataDir, err)
 	}
+	if restored {
 		if _, err := os.Stat(cfg.manifestPath); err == nil {
 			t.Errorf("the restore manifest was not consumed, so the next start is gated by it too")
 		}
+	}
 
 	st, err := store.Open(cfg.dbPath)
 	if err != nil {
@@ -254,6 +308,12 @@ func bootPanel(t *testing.T, dataDir string) http.Handler {
 	}
 	t.Cleanup(func() { _ = st.Close() })
 
+	if restored {
+		if err := resyncAfterRestore(cfg, st, true); err != nil {
+			t.Fatalf("resync after restore: %v", err)
+		}
+	}
+
 	panel, err := newPanel(cfg, st)
 	if err != nil {
 		t.Fatalf("build the panel: %v", err)

docs/plans/code-review.md

@@ -1,661 +0,0 @@
-# Plan: code-review (full-tree review follow-ups)
-
-**Status:** agreed  
-**Date:** 2026-08-13  
-**Reviewer:** Cursor Grok 4.6 (whole-tree authorship review). This is **not**
-the Fable pre-release security audit in [development.md](../development.md) §
-Model routing; a Fable pass is a later step on the P0 diff.  
-**Version:** patch for defects; docs/UI follow-ups have no schema.  
-**Order:** **P0 before inbound-relay.** P0 is a shipped RBAC hole, not a
-feature. Remaining phases after P0, or interleaved with inbound-relay by
-agreement.
-
----
-
-## Goal
-
-Record the 2026-08-13 full-tree review (architecture, complexity, quality,
-docs, maintainability, logic, refactor, licence, legacy, stubs, GUI, disputed
-decisions, edge cases, tests, duplication) and a phased implementation
-checklist with a recommended model per step, using the routing table in
-[development.md](../development.md).
-
----
-
-## Verdict
-
-SelfPost is a compact, well-bounded 1.x product: one image, one SQLite file,
-thin `domain`/`app` services for multi-store writes, a fail-open journal
-milter with a Postfix level-1 backstop, and documentation that is unusually
-honest about accepted risks. Complexity matches the scale (~10.7k production
-Go lines, ~5.8k unit-test lines, ~1.2k HTML, 749 CSS, 281 JS). Comments
-explain decisions rather than restating code.
-
-The outstanding defect is **send-log authorization for domain administrators**
-(confidentiality). After that, the work is tightening a few fail-open paths,
-catching docs that froze at “single administrator”, small GUI bugs from the
-1.2.x layout pass, and filling test gaps around auth/RBAC. Do **not** use this
-review as a licence to rewrite layers, squash migrations in 1.x, or add CSRF tokens
-without revisiting the ADR.
-
----
-
-## How to read this file
-
-Findings are grouped by the sixteen review questions. Each finding has a
-severity (**H**igh / **M**edium / **L**ow / **I**nfo). The implementation
-checklist at the end is the work queue; it names the model for each step.
-
-**Models** (from [development.md](../development.md)):
-
-| Kind of work | Model |
-|---|---|
-| Security, infra, mail path, permissions, open-relay risk | **Opus** |
-| UI / JS / CSS, templates, documentation (English) | **Sonnet** |
-| Trivial mechanics: retarget links, grep, compose bump, comment fixes | **Haiku** |
-| Security **review** of a diff (not authorship) | **Fable** |
-
-Reviewers must not be the author of the code under review.
-
----
-
-## 1. Architecture / structure
-
-**Proportionate.** Composition root in `cmd/panel` (HTTP + journal-milter +
-log-tailer, one `*store.Store`). `internal/domain` and `internal/app` own
-multi-store writes and rollback. Adapters (`postfix`, `milter`, `logtail`,
-`dnscheck`, `health`, `backup`, `secretfile`) are the only infrastructure-aware
-code. Interfaces exist where they break import cycles or enable fakes
-(`domain.Applications`, `app.SenderMaps`, `milter.Store`, `logtail.StatusStore`)
-— not as a DI framework.
-
-Handlers may call `store` directly for single-table reads (documented in
-[architecture.md](../architecture.md) § Code layers). That is followed for
-sessions, send-log queries, users, and DMARC settings. It is not a layering
-violation; it is an incomplete service boundary that will hurt if those
-surfaces keep growing.
-
-No circular Go imports. `MaxOpenConns(1)` on SQLite is an intentional
-single-connection trade-off for the three in-process roles.
-
-| Sev | Finding |
-|---|---|
-| **M** | Users, send-log listing, and global DMARC settings have no service; handlers talk to `store`. Fine at current size; do not invent a service until a second writer appears. |
-| **L** | `auth.RequireGlobal` (`internal/web/auth/middleware.go`) is unused; handlers duplicate `requireGlobal`. Either wire the middleware on `/users`, `/backup`, `/status`, `/mail-queue`, `/system-log` or delete the unused helper. |
-| **L** | `HandleAccount` / `handlers_account.go` still use the pre-1.2.3 “account” name while the route is `/settings`. |
-| **I** | Package comment on `internal/store` still says “the administrator account” after migration `0005` replaced `admin` with `users`. |
-
-**Do not:** introduce a repository layer, split the panel binary, or move
-SQLite behind an interface “for testability” — the existing fakes are enough.
-
----
-
-## 2. Complexity vs project scale
-
-The code is **not over-engineered**. A few files are large because the problem
-is large, not because of unused abstraction:
-
-| File | ~Lines | Note |
-|---|---|---|
-| `internal/health/machine.go` | 622 | Cohesive `/proc` sampler |
-| `internal/logtail/logtail.go` | 528 | Follow + rotate + reconcile + retention |
-| `internal/web/handlers/handlers_monitor.go` | 481 | Send-log UI + authz (this is where P0 lives) |
-| `internal/web/view/templates/domain_detail.html` | 476 | DNS + apps + limits + export; composition debt |
-| `internal/secretfile/secretfile.go` | ~423 | Isolated crypto envelope |
-| `internal/web/view/static/panel.css` | 749 | Tokens + layout; comment-heavy by design |
-
-Comments are long and mostly load-bearing (threat, fail-open, why not the
-obvious alternative). The cost is scanability: some files are 30–40% prose.
-That matches the project’s disclosed AI-authorship style
-([development.md](../development.md) § Authorship). Do not strip comments in
-the name of “cleanup”. Update the stale ones (see §4).
-
----
-
-## 3. Code quality
-
-Naming matches the docs (`domain` / `application`, level-1 / level-2). Errors
-on the mail path log-and-continue (intentional fail-open). Panel paths log and
-return 4xx/5xx. `crypto/rand` failure panics in `auth/token.go` — acceptable.
-
-Context is used for process lifetime and DNS timeouts, not for SQLite (correct
-with one connection). Dashboard DNS checks write distinct `rows[i]` from
-goroutines; Go 1.22+ loop semantics make that safe (`go.mod` is 1.26).
-
-Magic numbers are mostly named (`reservationTTL`, `renewThreshold`, CSP/HSTS).
-Env defaults live in `loadConfig`.
-
-| Sev | Finding |
-|---|---|
-| **M** | `sessionStore.Create` logs a DB error and still returns the token (`internal/web/auth/session.go`). Login sets the cookie; the next request bounces to `/login`. Fail closed: no cookie, error page. |
-| **M** | `app.Service.Delete` removes the registry row **before** SASL / rate-limit cleanup. SASL failure → orphaned `sasldb2` account that can still authenticate. Domain delete does SASL first (`domain/service.go`). Align app delete with that order (or compensate: restore the row on SASL failure). |
-| **L** | Login/setup `rateLimiter` sweeps expired buckets only when creating a **new** key. Many unique IPs grow the map until restart. Cap the map or sweep on a timer. |
-| **L** | `parseTrustedProxies` skips invalid CIDRs instead of refusing to start. Silent misconfiguration of `TRUSTED_PROXY_CIDR`. |
-
----
-
-## 4. Documentation completeness vs code; comments
-
-Docs are a first-class artefact (env regression test, architecture as-built,
-security accepted-risks). The drift is concentrated where **domain-admin
-shipped in 1.2.0** and several files still argue “single-user”.
-
-| Sev | Finding |
-|---|---|
-| **M** | [guide.md](../guide.md) Operations never mentions **Users** (`/users`) or the domain-admin role. Architecture and product do. An operator reading only the guide does not know the panel is multi-user. |
-| **M** | [security.md](../security.md) CSRF ADR still says the panel is single-user and “revisit if multi-user”. Multi-user shipped. The origin-check decision can stand; the **rationale and revisit trigger must be rewritten**. |
-| **M** | [security.md](../security.md) says passwords are “bcrypt (or argon2)”. Code is bcrypt only. |
-| **M** | `internal/backup` package comment claims the panel **regenerates** Postfix/OpenDKIM maps from SQLite on every start after restore. Startup only runs `CheckRestore` (`cmd/panel/main.go`). Maps/keys are **in** the tarball. Heal path is the Status **Reload** button. |
-| **L** | Architecture route table omits `/license` and the `/account` → `/settings` 308. |
-| **L** | Guide Settings section: “change the administrator username and/or password” — global Settings also has the default DMARC `rua=` address. |
-| **L** | Guide does not warn that restoring an **older** backup can resurrect sessions (architecture does). |
-| **L** | Guide rate-limiting section does not stress that level 2 is **fail-open** (store error or missing client IP → mail continues; level 1 is the backstop). |
-| **L** | [roadmap.md](../roadmap.md) and [plans/dmarc-reports.md](dmarc-reports.md) still say `admin.dmarc_report_email` after `0005` moved it to `settings`. |
-| **L** | `setupManager` comments still say “admin row”; the fact is `users` / `UserExists()`. “Plan B.1 / C.4” comments are opaque to outsiders; keep them, they are history, not errors. |
-| **I** | E2e coverage summary in development.md omits logrotate and supervisor-process checks that actually run. |
-| **I** | [plans/logrotate-mode.md](logrotate-mode.md) is **done** but still in `docs/plans/` (active-plans directory). History belongs in git / CHANGELOG. |
-
-Comments in production code are generally **high quality**. Missing comments
-are on domain-admin authorization policy in `sendLogData` (the P0 hole has no
-comment stating the intended invariant) and on `rateLimiter` memory bounds.
-
----
-
-## 5. Human readability and maintainability
-
-A new maintainer can follow the tree from [architecture.md](../architecture.md)
-into `cmd/panel` → `internal/web/web.go` → services. Tests document *why*
-(milter in-flight, queue-id anchoring, CSRF matrix).
-
-Friction:
-
-- `domain_detail.html` is the hardest HTML file to edit (repeated DNS
-  host/type/value blocks, checkbox “Edit” panels).
-- `panel.css` structure-tied selectors (`.muted + form > select:first-of-type`)
-  will break on a copy change.
-- Dual `CurrentUser` + `Principal` is redundant but works (`withPrincipal`
-  sets both).
-- `assignedDomains` loads **all** domains then filters in Go, while
-  `store.listUserDomainNames` already exists and is unused by handlers.
-
-None of this blocks maintenance at current size. Prefer small extractions
-(DNS partial, `tryAdmit`) over a layer rewrite.
-
----
-
-## 6. Logical errors
-
-### H — Domain-admin send-log list leaks other domains
-
-Detail page checks membership (`HandleDelivery`). The **list** does not.
-
-`sendLogData` in `internal/web/handlers/handlers_monitor.go`:
-
-- Empty `SendLogFilter.Domain` means “all rows” (`internal/store/sendlog.go`).
-- For a non-global user, a disallowed `?domain=` is cleared to `""`. The
-  assigned domain is filled in **only when there is exactly one**.
-- A domain-admin with **0 or ≥2** assigned domains and no (or a forged)
-  domain filter therefore sees **every** send-log row (From, To, Subject).
-- `?app=` is applied to SQL **before** it is checked against the user’s
-  application logins. The allowlist only updates the template’s selected
-  filter. Forged `?app=<foreign-login>` with an empty domain filter returns
-  that application’s rows.
-
-The deliveries table’s domain dropdown still lists only assigned domains, so
-the leak is silent.
-
-**Invariant to implement:** a non-global principal’s `QuerySendLog` /
-`CountSendLog` are always constrained to assigned domain names; if that set is
-empty, the result is empty. Validate `AppLogin` against the allowlist
-**before** the query.
-
-### M — Level-2 check/reserve race
-
-`enforceLimit` calls `flight.count` then `flight.reserve` under **separate**
-mutex acquisitions (`internal/milter/ratelimit.go`, `inflight.go`). Two MAIL
-FROM handlers can both observe `n == max-1` and both reserve. In-flight
-tracking closes the *stored-count* race (and
-`TestRateLimitCountsInFlightMessages` covers the **sequential** case). It does
-not close parallel check-then-act. Severity is tempered by fail-open and
-Postfix level-1. Fix: one `tryAdmit(key, since, max)` under the inflight
-mutex.
-
-### M — Session create fail-open
-
-See §3. Not a stolen-session bug (hash never lands in the DB); it is a
-logged-in-looking cookie that cannot be looked up.
-
-### M — App delete ordering
-
-See §3. Orphaned SASL is a mail-path consistency bug.
-
-### L — Domain export `Version` ignored on import
-
-`internal/domain/transfer.go` stamps `buildinfo.Version`; import checks format
-only. Lower risk than full-backup `CheckRestore`; still a cross-version footgun.
-
-### I — Journal milter fail-open; origin CSRF fail-open; queue-reconcile
-`bounced`
-
-Documented accepted risks in [security.md](../security.md). Not defects.
-Revisit the CSRF ADR’s *framing* (multi-user), not necessarily the mechanism.
-
----
-
-## 7. Refactoring and optimisation
-
-Worth doing, in order:
-
-1. `tryAdmit` (correctness, not speed).
-2. `SendLogFilter` domain IN-list (correctness).
-3. DNS field partial + settings credentials partial (drift).
-4. One helper for the five `panel.js` show/hide field pairs.
-5. `assignedDomains` via SQL for the current user (clarity, not performance).
-
-Not worth doing now:
-
-- Service layer for users / send-log.
-- Replacing SQLite, HTMX, or the single-container model.
-- Squashing migrations `0001`–`0005`.
-- CSRF tokens (see §12).
-- Rewriting `machine.go` or `logtail.go` for size.
-
----
-
-## 8. Licence (AGPL-3.0)
-
-Packaging is largely correct: root `LICENSE` ≡ embedded `internal/legal/LICENSE`
-(test), unauthenticated `/license`, footer copyright + Source + “No warranty”
-on login/setup, image copies `LICENSE`/`NOTICE`, Go deps are BSD-family, htmx
-is 0BSD. Network-use §13 is stated in `NOTICE` and the README.
-
-| Sev | Finding |
-|---|---|
-| **M** | IBM Plex WOFF2 files are shipped without the SIL OFL 1.1 text. OFL requires the licence to travel with the font. Add `OFL.txt` next to the fonts (and mention the path in `NOTICE`). |
-| **M** | `NOTICE` tells modifiers to change the Source URL in `layout.html`. The URL is `legal.SourceURL` in `internal/legal/legal.go`, injected by `view.go`. |
-| **L** | `/license` serves LICENSE only, not NOTICE. Optional: serve NOTICE at `/notice` or append attributions. |
-| **L** | SPDX headers only on `cmd/panel` and `cmd/selfpost-backup`. AGPL does not require per-file SPDX; either add them everywhere or drop the two so the convention is consistent. |
-| **I** | Debian package licences are pointed at packages.debian.org rather than a pinned list — normal for an image that installs from bookworm. |
-
-No AGPL-incompatible Go dependency found in `go.mod`.
-
----
-
-## 9. Legacy code and migrations
-
-| Migration | Role | Removal |
-|---|---|---|
-| `0001_init.sql` | Core schema (including historical `admin`) | Keep for all **1.x** (`PRAGMA user_version` chain) |
-| `0002_sessions.sql` | DB sessions | Keep for 1.x |
-| `0003_logtail_state.sql` | Tailer offset | Keep for 1.x |
-| `0004_dmarc_report_email.sql` | DMARC columns on `admin` | Keep for 1.x; `0005` moves the data |
-| `0005_panel_users.sql` | `users` / `user_domains`; `DROP TABLE admin` | Keep for 1.x |
-
-Squash is deferred to **2.x** — [roadmap.md](../roadmap.md) `schema-squash`.
-Until then do not delete, rename, or reorder these files. Document the 1.x
-rule in architecture § Persistence (one sentence).
-
-Compat shims to keep until a major:
-
-- `GET/POST /account` → 308 `/settings`.
-- Domain rate-limit rows may still have an unused IP list column; enforcement
-  ignores it.
-
-`sessions.username` is a string, not a `user_id` FK. Renames update the column;
-a missed rename would orphan sessions. Acceptable; a FK would be a 1.x
-migration if usernames become mutable in more places.
-
-**Delete** `docs/plans/logrotate-mode.md` once this review is the active plan
-(status `done`; history is git / CHANGELOG `[1.2.3]`).
-
----
-
-## 10. Stubs and claimed-but-unimplemented behaviour
-
-| Item | Status |
-|---|---|
-| Inbound relay | Agreed plan, **no code stubs**, no `INBOUND_RELAY_*` env. Correct. |
-| DMARC report **ingestion** | Candidate. UI copy already promises “a future release will be able to receive reports in the panel”. Settings `rua=` and DNS guidance **are** implemented. |
-| `panel-docs` | Candidate. Status blurbs were removed in 1.2.2 in favour of this item. |
-| `CONTRIBUTING.md` | Candidate, file absent. Matches roadmap. |
-| CSRF tokens | Explicitly not implemented (ADR). |
-| `auth.RequireGlobal` | Dead helper, not a feature stub. |
-
-The DMARC “future release” sentence is the only user-visible promise of
-unimplemented behaviour. Soften it to “SelfPost does not receive inbound mail”
-or keep it and treat `dmarc-reports` as the fulfilment — product call, Sonnet
-copy.
-
----
-
-## 11. GUI: hacks and layout composition
-
-The panel is CSP-strict (no inline script/style; `TestNoTemplateUsesInlineScriptOrStyle`).
-No `!important`. Progressive enhancement is real (pages work without JS).
-Adaptive polling in `panel.js` is a **documented** workaround: HTMX
-`hx-trigger="every Ns [expr]"` uses `new Function`, which CSP would break.
-
-| Sev | Finding |
-|---|---|
-| **M** | `RateLimitErr` uses `class="flash error"`. `.flash` is the **success** surface; `.error` only recolors text. There is no `.flash.error` rule. Validation failures look like success (red text on green). `domain_detail.html` + `panel.css`. |
-| **M** | User **Delete** has no `data-confirm` and no confirm page. App delete / regen / rate-limit clear do; domain delete has `domain_delete.html`. One mis-click removes a panel user. |
-| **M** | `domain_detail.html` repeats Host/Type/Value/`code-row` for DNS status **and** publishable records. Extract a partial (same pattern as `encrypt_fields.html`). |
-| **M** | `settings.html` duplicates the credentials form (global split vs domain-admin narrow card). Drift already visible in the muted help text. |
-| **L** | Adaptive polling: `outerHTML` swap every 5 s can steal clicks / focus; poll failures retry silently. Consider `aria-live="polite"` and a visible retry/error. Do not switch back to `hx-trigger="every"` under this CSP. |
-| **L** | Checkbox-driven Edit panels instead of `<details>` (commented in the template). Works without JS; no `aria-expanded`. |
-| **L** | Five near-identical show/hide helpers in `panel.js`. Encrypt/import fields can flash visible before `DOMContentLoaded`. |
-| **L** | `hx-get` query params in `deliveries_rows.html` are not `urlquery`-encoded. Safe while domain/app charset is locked down. |
-| **L** | Applications on a domain page are unpaginated. Fine until an operator has dozens of apps. |
-| **L** | `<label>` used as a heading on DNS/status readouts (no `for`). |
-| **I** | `{{define "wide"}}` override and `main:has(> .card.narrow)` are non-obvious but tested. Keep; do not “simplify” into per-page CSS files. |
-
-`data-confirm` is skipped when JS is off (documented in `panel.js` only).
-Domain delete already uses a real page; user delete should follow that
-pattern or at least get `data-confirm`.
-
----
-
-## 12. Weakly documented disputed decisions
-
-These are real choices. Several are in [security.md](../security.md); the
-problem is **stale framing** after domain-admin, not silence.
-
-| Decision | Where | Gap |
-|---|---|---|
-| CSRF via origin / `Sec-Fetch-Site`; no tokens; POST with neither header allowed | security.md ADR | Still argued as “single-user”. Revisit trigger already fired. **Rewrite the ADR**; implementing tokens is a separate product call. |
-| Journal-milter fail-open | architecture, milter comments | Guide rate-limit section should say L2 is best-effort. |
-| Unencrypted backup/export by default | security.md | OK. Domain-admin can **export working SASL passwords** for assigned domains (`HandleExportDomain` uses `lookupDomain`). Guide/security should say so. |
-| Queue reconcile marks lost lines `bounced` | security.md | OK. |
-| Sliding session, no absolute cap; HTMX GET does not renew | architecture, guide | OK. |
-| Restore can resurrect sessions from an older backup | architecture | Missing from the operator guide. |
-| L2 skipped when client IP is unknown | milter + unit test | Not in the guide. |
-| Backup encryption optional | security.md | OK. |
-| Supervisord socket `0770` so the panel can `postfix reload` | supervisord.conf | Compromised panel ≈ mail-stack control. Documented as intentional; keep. |
-| `workflow_dispatch` on `release.yml` derives version from `GITHUB_REF_NAME` | `.github/workflows/release.yml` | A manual run from `main` can publish a non-semver tag. Guard: only `vX.Y.Z` or an explicit version input. |
-
----
-
-## 13. Edge cases
-
-Covered above: 0 / 1 / ≥2 assigned domains on the send log; forged
-`domain`/`app` query params; empty allowlist must not mean “all”.
-
-Others:
-
-- **Last global administrator** cannot be demoted/deleted (UI + server). Good.
-- **Domain-admin with no domains** (all assigned domains deleted →
-  `user_domains` cascade): today they see the full send log (P0). After the
-  fix they should see an empty log, not an error.
-- **Missing `mail.log`** after rotation: treated as empty, not an error
-  (tested). Good.
-- **Backup download after headers committed**: truncated file possible
-  (streaming trade-off). Encrypted domain export is sealed in memory first.
-  Acceptable; do not buffer full backups.
-- **`parsePage`**: huge `p` yields a large offset and an empty page, not a
-  500. Fine.
-- **Concurrent domain DNS on the dashboard**: safe under Go 1.22+.
-- **Import domain** is global-only; **export** is any principal who can
-  access the domain. Intentional once documented.
-
----
-
-## 14. Tests
-
-**Strengths.** Milter L2 + in-flight, DNS grading, logtail follow/rotate/
-reconcile, secretfile tamper, SASL argv hygiene (`--` before login), template
-CSP/nav/legal footer, env-key ↔ guide regression, e2e mail path (AUTH, DKIM,
-queued→sent, L1/L2, fail-open, hostname gate, session vs restart). Test
-comments are better than average.
-
-**Documented?** How to run tests: [development.md](../development.md) §
-Testing. There is no e2e README (package comment in `test/e2e/main_test.go`
-is the stand-in). Individual tests are not inventoried in docs — that is
-fine; the e2e **summary** should mention logrotate.
-
-**Gaps (high value):**
-
-| Area | Gap |
-|---|---|
-| RBAC | **No** tests for `authz.go`, `CanAccessDomain`, domain-admin send-log scoping, `/users` 404 for domain-admin, backup 404. This is why P0 shipped. |
-| Auth HTTP | No `HandleLogin` / `HandleSetup` tests (TTL, constant-time, setup complete → 404). No tests for `auth/ratelimit.go` `Allow`. |
-| Sessions store | No `store/sessions*_test.go` (covered only via `auth_test` wrappers). |
-| Backup as operator path | Create + `CheckRestore` unit-tested; **no** extract-onto-`/data`-and-boot test; panel `HandleBackup` POST untested. |
-| Handlers | No tests for users CRUD, domain add/delete, account POST, DNS recheck endpoints. |
-| `postfix.Queue` | Parser only; exec path untested (e2e does not open Mail queue). |
-
-**Weak / low-value (keep, do not grow this style):**
-
-- `TestDecryptErrorMessage` — substring mapping.
-- `TestBackupPageOffersEncryption` — `strings.Contains` over HTML.
-- Many `templates_test.go` cases — structural guards (CSP, nav). Valuable as
-  guards, not as behaviour tests.
-- E2e `testNoAuthRejected` vs `testForeignRelayRejected` — nearly the same
-  unauthenticated send.
-
-**E2e hygiene:**
-
-- Fatal string in `testLevel1RateLimit` says `RATE_LIMIT_MESSAGES_PER_IP=5`;
-  override is `50` (`test/e2e/negative_test.go` vs `compose.override.yml`).
-- `coredns/coredns:latest` is unpinned.
-- `TestImageBuildPreservesLogrotateMode` chmods the source conf then rebuilds
-  — can race a dirty tree.
-
-Do not add snapshot tests of entire pages. Add **authorization** tests that
-would have caught P0.
-
----
-
-## 15. Duplication and local patches
-
-| Local patch | Systemic fix |
-|---|---|
-| Send-log domain/app allowlist after/around the query | Store filter: `Domains []string` required for non-global; validate app login first |
-| `assignedDomains` loads all domains | Use `listUserDomainNames` / `ListDomainsForUser` |
-| `requireGlobal` on each handler | Optional: `auth.RequireGlobal` on those muxes |
-| Five JS field-sync helpers | One `data-show-when` helper |
-| DNS host/type/value markup × many | Template partial |
-| Settings credentials form × 2 | Partial |
-| `web/validate` vs `app/validate` | Keep separate (different alphabets); do not merge |
-
-The send-log allowlist is the textbook “local patch instead of a store
-invariant”.
-
----
-
-## 16. Other improvements
-
-- Pin CoreDNS in e2e.
-- Guard `release.yml` `workflow_dispatch` versioning (**Opus**, infra).
-- Optional: `Resync` once after a successful `CheckRestore` (heal drifted
-  maps). Small, mail-path, **Opus**. Not required if the tarball is the
-  restore story — but then **fix the backup package comment**.
-- Optional: serve `NOTICE` next to `/license`.
-- Do not start inbound-relay until P0 is closed.
-
----
-
-## Implementation checklist
-
-Work top to bottom. Commit per phase (or per coherent sub-step) when asked.
-Update [CHANGELOG.md](../../CHANGELOG.md) `[Unreleased]` with each user-visible
-change. After Go changes: `go build`, `go vet`, `go test ./...`.
-
-### P0 — Domain-admin send-log authorization (defect)
-
-**Model: Opus.** Tests in the same change. **Fable** on the diff after it
-lands (reviewer ≠ author).
-
-- [x] Extend `SendLogFilter` so a non-empty domain list is an `IN` constraint.
-      Empty list for a non-global user → zero rows, not “all”. Done as
-      `Domains` + `AllDomains`: the zero value matches nothing, so a caller
-      that states no scope cannot read the journal.
-- [x] `sendLogData`: for `!p.IsGlobal()`, always constrain to assigned domain
-      names; validate `AppLogin` against the user’s apps **before** query.
-- [x] Tests: domain-admin with 0, 1, and 2 assigned domains; unfiltered list;
-      forged `?domain=` and `?app=`; detail page still 404s on a foreign id
-      (already true — keep a regression test).
-- [x] Comment the invariant next to `sendLogData` (the comment that was
-      missing).
-
-**Done when:** a domain-admin cannot read another domain’s send-log rows via
-the list, the fragment, or query parameters. `go test ./...` green.
-
-### P1 — Fail-closed consistency (mail path / auth)
-
-**Model: Opus.**
-
-- [x] `inflight.tryAdmit` (count + reserve under one lock). Extend milter
-      tests with overlapping `MailFrom` (true concurrency, not sequential).
-      Two tests: concurrent `MailFrom` sessions gated so they all read the
-      stored count before anyone reserves (exactly one admitted), and a
-      saturation test on `tryAdmit` that overshoots the ceiling whenever count
-      and reserve are separate critical sections.
-- [x] `sessionStore.Create` returns an error; login does not set a cookie on
-      failure.
-- [x] `app.Service.Delete`: SASL (and rate-limit row) before or compensating
-      with the registry row; match domain-delete ordering. Test the failure
-      path with a fake SASL that errors.
-
-**Done when:** unit tests cover the race and the two fail-closed paths.
-
-### P2 — Security/operator docs that are wrong today
-
-**Model: Sonnet** (English docs). No code behaviour change except copy.
-
-- [x] Rewrite the CSRF ADR in [security.md](../security.md) for a panel that
-      already has global + domain-admin. Keep the origin-check mechanism
-      unless a new decision says otherwise. New revisit trigger (e.g. untrusted
-      domain-admins, or a requirement that does not depend on browser
-      headers).
-- [x] Drop “or argon2” unless argon2 is implemented.
-- [x] [guide.md](../guide.md): Users / roles; Settings DMARC field; L2
-      fail-open; restore can resurrect sessions; domain-admin can export
-      working SASL passwords for assigned domains.
-- [x] Architecture route table: `/license`, `/account` → `/settings`.
-- [x] Fix `internal/backup` package comment (restore = extract tarball +
-      `CheckRestore`; maps come from the archive; Reload heals drift).
-- [x] `admin.dmarc_report_email` → `settings` in roadmap + dmarc-reports plan.
-- [x] development.md e2e summary: logrotate + process checks.
-- [x] `setupManager` / `store` package comments: `users`, not `admin` row.
-
-**Done when:** an operator who reads only the guide knows the panel has two
-roles, and security.md no longer calls the panel single-user.
-
-### P3 — GUI defects from the 1.2.x layout pass
-
-**Model: Sonnet.**
-
-- [x] `.flash.error` (or stop using `.flash` for `RateLimitErr`) — danger
-      surface, not success.
-- [x] User delete: `data-confirm` at minimum; prefer a confirm page like
-      domain delete. Done as a confirm page (`GET/POST /users/{uid}/delete`),
-      matching `domain_delete.html`.
-- [x] DNS field partial; settings credentials partial.
-- [x] Optional: `urlquery` on deliveries fragment params; `aria-live` on
-      polled regions; confirm-without-JS note next to the CSRF accepted risks.
-
-**Done when:** a rate-limit validation error is visually an error; user delete
-cannot be a single unmarked click.
-
-### P4 — Tests and e2e hygiene
-
-**Model: Opus** for auth/RBAC/limiter tests; **Haiku** for the L1 fatal-string
-typo; **Sonnet** if e2e docs need a paragraph.
-
-- [x] `auth/ratelimit.go` unit tests (window, lockout, sweep). Also the
-      per-key scope: one locked-out address must not lock out the others.
-- [x] Login/setup handler tests (happy path + lockout + setup expiry). The
-      lockout test also states that a correct password does not bypass it, and
-      that the two refusals are byte-identical (no username enumeration).
-- [x] Domain-admin 404 on `/users`, `/backup`, `/mail-queue`, `/system-log`,
-      `/status` — as a table of every global-only route (`internal/web/handlers/authz_test.go`),
-      including the write routes, plus the same 404 for a request with no
-      principal and a positive control so the table cannot pass on a handler
-      that always 404s.
-- [x] Fix e2e L1 fatal string (`50`, not `5`).
-- [x] Pin `coredns` image: tag `1.14.6`, not a digest — the tag is a multi-arch
-      manifest and the stand has to come up on arm64 developer machines.
-- [x] Optional: backup extract + `CheckRestore` + panel boot. Done as an
-      in-process integration test (`cmd/panel/restore_test.go`) rather than
-      e2e, so it runs in `go test ./...`: the archive is downloaded from a
-      running panel through `POST /backup` (which closes the “`HandleBackup`
-      POST untested” gap in §14 as well), unpacked the way `tar -xzf` unpacks
-      it, and a second panel is booted on the result through run()'s own
-      startup order. Also covers the encrypted download, the version-mismatch
-      refusal, that a restore does not reopen the setup link, and that sessions
-      travel in the archive. `serveHTTP` was split so the composition it
-      performs (`newPanel`) can be booted without binding a port.
-
-**Done when:** P0 cannot regress without a red test; e2e L1 message matches
-the override.
-
-### P5 — Licence and release infra
-
-**Model: Sonnet** for OFL/NOTICE prose; **Opus** for `release.yml`; **Haiku**
-for SPDX consistency and deleting the done logrotate plan.
-
-- [x] Add SIL OFL 1.1 text beside the Plex WOFF2 files; point `NOTICE` at it.
-      IBM Plex `LICENSE.txt` as `internal/web/view/static/OFL.txt` (copyright
-      + OFL 1.1). Copied into the image at `/usr/share/doc/selfpost/OFL.txt`;
-      served at `/static/OFL.txt`.
-- [x] `NOTICE` Source URL instructions → `internal/legal/legal.go`.
-- [x] `release.yml`: `workflow_dispatch` must not publish `main` as a version
-      (require `vX.Y.Z` or an explicit `version` input that matches SemVer).
-- [x] Delete [plans/logrotate-mode.md](logrotate-mode.md) (done; git keeps it).
-- [x] Decide SPDX-everywhere vs SPDX-nowhere; do not leave two files special
-      without a one-line note in development.md. SPDX-nowhere: dropped the
-      two `cmd/` headers; development.md § External libraries records that
-      AGPL-3.0 does not require per-file SPDX.
-
-**Done when:** OFL travels with the fonts; a dispatch from `main` cannot tag
-`ghcr.io/...:main`.
-
-### P6 — Optional cleanup (do not start until P0–P3 are done)
-
-**Model: Sonnet** unless noted.
-
-- [x] Use `auth.RequireGlobal` or delete it (**Haiku** if delete). Deleted.
-- [x] Rename `handlers_account.go` / `HandleAccount` to settings (**Haiku**).
-- [x] `ListDomainsForUser` instead of load-all-and-filter.
-- [x] Cap or periodically sweep the login limiter map (**Opus**, small).
-- [x] Collapse `panel.js` field-sync helpers.
-- [x] Soften or keep DMARC “future release” UI copy (product call). Softened.
-- [ ] Optional startup `Resync` after restore (**Opus**). Dropped — P2 backup
-      comment fix is sufficient.
-
-### P7 — Security review of the P0–P1 diff
-
-**Model: Fable.** Not authorship.
-
-- [ ] Review the send-log authz change, `tryAdmit`, session create, and app
-      delete ordering against [security.md](../security.md). Close each finding
-      with a fix or an accepted-risk entry.
-
----
-
-## What not to do
-
-- Do not squash SQLite migrations in **1.x** (see roadmap `schema-squash` for 2.x).
-- Do not implement inbound-relay, DMARC ingestion, or in-panel docs as part of
-  this plan.
-- Do not add CSRF tokens in the same breath as rewriting the ADR. Tokens are a
-  new decision.
-- Do not “simplify” comments that record threat models.
-- Do not introduce a general service/repository layer for users.
-
----
-
-## Done when (this plan)
-
-1. P0 is shipped and covered by tests.
-2. P2 has removed “single-user” from the CSRF ADR and documented Users in the
-   operator guide.
-3. P3 flash/delete bugs are gone.
-4. P7 has run on the P0–P1 diff.
-5. This file’s remaining boxes are either checked or explicitly dropped in
-   [roadmap.md](../roadmap.md) with a reason.
-6. [CHANGELOG.md](../../CHANGELOG.md) `[Unreleased]` lists the user-visible
-   items (authz, docs, GUI).
-
-After that, delete this plan (history in git) and return the recommended
-order on the roadmap to inbound-relay.

docs/plans/inbound-relay.md

@@ -3,8 +3,9 @@
 **Status:** agreed  
 **Version:** target bump **1.x** MINOR; **`2.x` possible** — to be settled once
 the implementation lands (do not fix a major in advance).  
-**Order:** after [code-review.md](code-review.md) P0 (and preferably P0–P3);
+**Order:** the 2026-08-13 full-tree review follow-ups are closed (CHANGELOG
-this remains the largest remaining 1.x+ **feature**. The domain-admin role
+`[Unreleased]`), so this is next up as the largest remaining 1.x+ **feature**.
+The domain-admin role
 and `internal/web` split have shipped — see [CHANGELOG](../../CHANGELOG.md)
 `[1.2.0]`.
 

docs/plans/queue-retries.md

@@ -3,8 +3,7 @@
 **Status:** agreed  
 **Date:** 2026-08-13  
 **Version:** patch; no schema, no configuration surface.  
-**Order:** small panel item; does not wait on inbound-relay. May follow or
+**Order:** small panel item; does not wait on inbound-relay.
-interleave with remaining [code-review.md](code-review.md) work after P0.
 
 ---
 

docs/roadmap.md

@@ -28,7 +28,6 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md).
 
 | ID | Topic | Status | Plan |
 |---|---|---|---|
-| code-review | Full-tree review follow-ups (authz, docs, GUI, tests) | **agreed** | [plans/code-review.md](plans/code-review.md) |
 | queue-retries | Postfix retry policy in the panel (queue lifetime, backoff) | **agreed** | [plans/queue-retries.md](plans/queue-retries.md) |
 | inbound-relay | Inbound relay (backup-MX / forwarding) | **agreed** | [plans/inbound-relay.md](plans/inbound-relay.md) |
 | contributing | `CONTRIBUTING.md` | candidate | — |
@@ -36,39 +35,18 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md).
 | panel-docs | In-panel operator documentation | candidate | — |
 | schema-squash | Squash SQLite migrations into a 2.x baseline | **2.x** | — |
 
-**Recommended order** (not binding): **code-review P0** first (shipped
+**Recommended order** (not binding): **queue-retries** is a small panel item
-domain-admin send-log leak — a defect, not a feature), then the rest of that
+that can land first or in parallel; the next feature is **inbound-relay**.
-plan as listed; **queue-retries** is a small panel item that can land in
+The 2026-08-13 full-tree review follow-ups (send-log authorization,
-parallel after P0; then **inbound-relay**. Candidates need explicit agreement
+fail-closed paths, docs, GUI, tests, licence) are closed — history in
-before they join the queue.
+[CHANGELOG.md](../CHANGELOG.md) `[Unreleased]` and git. Candidates need
+explicit agreement before they join the queue.
 
 After a context reset, pick an item marked `agreed` or `in progress`, then work
 the checklist in its linked plan.
 
 ---
 
-## code-review
-
-**Goal:** close the 2026-08-13 full-tree review: domain-admin send-log
-authorization, a few fail-closed paths, docs that still say “single-user”,
-GUI flash/delete bugs, test gaps, licence/release hygiene.
-
-**Boundary:** defects and docs/UI follow-ups inside the current 1.x product.
-Not inbound-relay, not DMARC ingestion, not a layer rewrite.
-
-**Done when:** see the criteria in
-[plans/code-review.md](plans/code-review.md).
-
-**Progress:** P0–P5 done. Remaining: P6 (optional cleanup), P7 (Fable review
-of the P0–P1 diff).
-
-**Dependencies / risks:** P0 is confidentiality between panel roles; it
-jumps the feature queue. Implementation models are in the plan (Opus / Sonnet
-/ Haiku / Fable per [development.md](development.md)).
-**Version:** patch.
-
----
-
 ## queue-retries
 
 **Goal:** show on Mail queue and on a delivery's history how this Postfix

docs/security.md

@@ -5,7 +5,12 @@ meet; the full v1.0 audit passed. The pre-release review (plan § D, 2026-08-06)
 covered the whole diff from the v1.0 audit (Phase 11) to HEAD and the checklist
 in full: no exploitable findings; one defence-in-depth change — `--` before the
 login in the `saslpasswd2` argv
-([internal/app/sasl.go](../internal/app/sasl.go)). (2) **Accepted risks** —
+([internal/app/sasl.go](../internal/app/sasl.go)). The 2026-08-14 review
+(code-review plan § P7, Fable; reviewer ≠ author) covered the P0–P1 diff of the
+2026-08-13 full-tree review against this document — send-log authorization for
+domain administrators, the atomic level-2 admit (`tryAdmit`), fail-closed
+session creation, and application-delete ordering: no findings, and nothing
+needed adding to the accepted risks. (2) **Accepted risks** —
 deliberate departures beyond the mandatory, recorded so the decision is not
 lost.
 

internal/backup/backup.go

@@ -9,15 +9,16 @@
 // Restore is not a separate code path in the panel: a backup is extracted into
 // the /data bind mount before first start, and the archive already carries
 // everything the mail path needs — DKIM keys, sasldb2, and Postfix's sender
-// map — so nothing needs to be regenerated from SQLite for the daemons to
+// map — so the daemons can start on the extracted files alone. The
-// start correctly. The only restore-specific step the panel runs is
+// restore-specific steps the panel runs are CheckRestore, which refuses to
-// CheckRestore, which refuses to boot if the manifest's version does not match
+// boot if the manifest's version does not match the running binary so
-// the running binary, so schema/format skew between versions cannot silently
+// schema/format skew between versions cannot silently corrupt state
-// corrupt state (architecture.md § Persistence). If a daemon's on-disk state
+// (architecture.md § Persistence), and a one-time Resync of OpenDKIM's tables
-// ever drifts from what SQLite records — for example after a manual edit
+// and the Postfix sender map from SQLite on that first boot, so any drift
-// under /data — the Status page's "Reload configuration" button re-derives
+// between the archive and the database is healed before mail flows. If
-// OpenDKIM's tables and the Postfix sender map from the database; that is a
+// on-disk state drifts again later — for example after a manual edit under
-// deliberate, operator-triggered heal, not something restore does on its own.
+// /data — the Status page's "Reload configuration" button runs the same
+// Resync on demand.
 package backup
 
 import (
@@ -269,33 +270,34 @@ func snapshotDB(dbPath string) (path string, cleanup func(), err error) {
 // directory), its version must match binaryVersion or the panel refuses to
 // start, telling the operator which image tag to use. On a match the manifest
 // is consumed (deleted) so it guards only the first boot after a restore and
-// never blocks a later in-place image upgrade. Absence of the manifest is the
+// never blocks a later in-place image upgrade, and restored is true so the
-// normal case and returns nil.
+// caller can heal drifted daemon maps once. Absence of the manifest is the
-func CheckRestore(manifestPath, binaryVersion string) error {
+// normal case and returns restored == false with a nil error.
+func CheckRestore(manifestPath, binaryVersion string) (restored bool, err error) {
 	data, err := os.ReadFile(manifestPath)
 	if os.IsNotExist(err) {
-		return nil // ordinary start, not a restore
+		return false, nil // ordinary start, not a restore
 	}
 	if err != nil {
-		return fmt.Errorf("backup: read restore manifest: %w", err)
+		return false, fmt.Errorf("backup: read restore manifest: %w", err)
 	}
 
 	var m Manifest
 	if err := json.Unmarshal(data, &m); err != nil {
-		return fmt.Errorf("backup: restore manifest %s is not valid JSON: %w", manifestPath, err)
+		return false, fmt.Errorf("backup: restore manifest %s is not valid JSON: %w", manifestPath, err)
 	}
 	if m.Format != FormatFull {
-		return fmt.Errorf("backup: %s is not a SelfPost full backup manifest (format %q)", manifestPath, m.Format)
+		return false, fmt.Errorf("backup: %s is not a SelfPost full backup manifest (format %q)", manifestPath, m.Format)
 	}
 	if m.Version != binaryVersion {
-		return fmt.Errorf(
+		return false, fmt.Errorf(
 			"backup: this backup was created by SelfPost %s but this image is %s — restore into the matching image (selfpost:%s)",
 			m.Version, binaryVersion, m.Version)
 	}
 	// Version matches: consume the manifest so subsequent normal starts (and
 	// in-place upgrades) are not gated by it.
 	if err := os.Remove(manifestPath); err != nil {
-		return fmt.Errorf("backup: consume restore manifest: %w", err)
+		return false, fmt.Errorf("backup: consume restore manifest: %w", err)
 	}
-	return nil
+	return true, nil
 }

internal/backup/backup_test.go

@@ -156,17 +156,25 @@ func writeManifest(t *testing.T, dir, format, version string) string {
 }
 
 func TestCheckRestoreNoManifestIsNormalStart(t *testing.T) {
-	if err := CheckRestore(filepath.Join(t.TempDir(), "manifest.json"), "1.0.0"); err != nil {
+	restored, err := CheckRestore(filepath.Join(t.TempDir(), "manifest.json"), "1.0.0")
+	if err != nil {
 		t.Errorf("CheckRestore with no manifest = %v, want nil", err)
 	}
+	if restored {
+		t.Error("CheckRestore with no manifest reported a restore")
+	}
 }
 
 func TestCheckRestoreMatchConsumesManifest(t *testing.T) {
 	dir := t.TempDir()
 	path := writeManifest(t, dir, FormatFull, "1.0.0")
-	if err := CheckRestore(path, "1.0.0"); err != nil {
+	restored, err := CheckRestore(path, "1.0.0")
+	if err != nil {
 		t.Fatalf("CheckRestore matching = %v, want nil", err)
 	}
+	if !restored {
+		t.Fatal("CheckRestore matching did not report a restore")
+	}
 	if _, err := os.Stat(path); !os.IsNotExist(err) {
 		t.Errorf("manifest should be consumed after a matching restore, stat err = %v", err)
 	}
@@ -175,10 +183,13 @@ func TestCheckRestoreMatchConsumesManifest(t *testing.T) {
 func TestCheckRestoreVersionMismatchRefusesAndKeeps(t *testing.T) {
 	dir := t.TempDir()
 	path := writeManifest(t, dir, FormatFull, "1.0.0")
-	err := CheckRestore(path, "2.0.0")
+	restored, err := CheckRestore(path, "2.0.0")
 	if err == nil {
 		t.Fatal("CheckRestore mismatch = nil, want error")
 	}
+	if restored {
+		t.Error("CheckRestore mismatch reported a restore")
+	}
 	if !strings.Contains(err.Error(), "1.0.0") || !strings.Contains(err.Error(), "2.0.0") {
 		t.Errorf("error should name both versions: %v", err)
 	}
@@ -190,7 +201,11 @@ func TestCheckRestoreVersionMismatchRefusesAndKeeps(t *testing.T) {
 func TestCheckRestoreWrongFormatRejected(t *testing.T) {
 	dir := t.TempDir()
 	path := writeManifest(t, dir, "something-else", "1.0.0")
-	if err := CheckRestore(path, "1.0.0"); err == nil {
+	restored, err := CheckRestore(path, "1.0.0")
+	if err == nil {
 		t.Error("CheckRestore accepted a non-backup manifest")
 	}
+	if restored {
+		t.Error("CheckRestore wrong format reported a restore")
+	}
 }

internal/domain/opendkim.go

@@ -167,6 +167,14 @@ func (o *OpenDKIM) Reload() error {
 	return o.reload()
 }
 
+// SetReloadHook replaces how configuration is applied after a rebuild. Tests
+// that cannot reach supervisord use this to verify file regeneration alone.
+func (o *OpenDKIM) SetReloadHook(fn func() error) {
+	if fn != nil {
+		o.reload = fn
+	}
+}
+
 // renderTables builds the KeyTable and SigningTable byte contents for a domain
 // set, sorted by name so the output is deterministic. Every domain is
 // re-checked for shell/config-injection safety before being written

internal/postfix/postfix.go

@@ -75,6 +75,14 @@ func (p *Postfix) Reload() error {
 	return p.reload()
 }
 
+// SetReloadHook replaces how configuration is applied after a rebuild. Tests
+// that cannot reach supervisord use this to verify file regeneration alone.
+func (p *Postfix) SetReloadHook(fn func() error) {
+	if fn != nil {
+		p.reload = fn
+	}
+}
+
 // renderSenderLoginMaps builds the sender_login_maps file contents. Keys are
 // sorted for deterministic output and the logins under each key are sorted and
 // de-duplicated. Every address and login is re-checked for injection safety