diff --git a/CHANGELOG.md b/CHANGELOG.md index 26eade3..4082c82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -162,8 +162,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 - backup package comment fix in P2 is enough). + does not receive inbound mail. + +- 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 diff --git a/cmd/panel/httpserver.go b/cmd/panel/httpserver.go index 4b14df4..0449df9 100644 --- a/cmd/panel/httpserver.go +++ b/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 - // service delegates to them when a domain (and its applications) is deleted. - 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{ + ms := newMailStack(cfg, st) + return web.New(st, ms.Domains, ms.Apps, web.Config{ Hostname: cfg.hostname, CookieSecure: cfg.cookieSecure, SubmissionEnabled: cfg.submissionEnabled, diff --git a/cmd/panel/main.go b/cmd/panel/main.go index f07b959..1e0e305 100644 --- a/cmd/panel/main.go +++ b/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) diff --git a/cmd/panel/restore_test.go b/cmd/panel/restore_test.go index 47e3587..3097518 100644 --- a/cmd/panel/restore_test.go +++ b/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 - // SQLite, so the files have to land where the panel's configuration says - // they are — that is the whole reason restore needs no regeneration step. + // The archive carries the daemon files, and the first boot after restore + // re-derives the maps from SQLite so they stay aligned with the database. 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,11 +292,14 @@ 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 _, err := os.Stat(cfg.manifestPath); err == nil { - t.Errorf("the restore manifest was not consumed, so the next start is gated by it too") + 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) @@ -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) diff --git a/docs/plans/code-review.md b/docs/plans/code-review.md index cbabfbd..64ecbc9 100644 --- a/docs/plans/code-review.md +++ b/docs/plans/code-review.md @@ -620,8 +620,7 @@ for SPDX consistency and deleting the done logrotate plan. - [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. +- [x] Optional startup `Resync` after restore (**Opus**). ### P7 — Security review of the P0–P1 diff diff --git a/internal/backup/backup.go b/internal/backup/backup.go index 80f1513..10e44af 100644 --- a/internal/backup/backup.go +++ b/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 -// start correctly. The only restore-specific step the panel runs is -// CheckRestore, which refuses to boot if the manifest's version does not match -// the running binary, so schema/format skew between versions cannot silently -// corrupt state (architecture.md § Persistence). If a daemon's on-disk state -// ever drifts from what SQLite records — for example after a manual edit -// under /data — the Status page's "Reload configuration" button re-derives -// OpenDKIM's tables and the Postfix sender map from the database; that is a -// deliberate, operator-triggered heal, not something restore does on its own. +// map — so the daemons can start on the extracted files alone. The +// restore-specific steps the panel runs are CheckRestore, which refuses to +// boot if the manifest's version does not match the running binary so +// schema/format skew between versions cannot silently corrupt state +// (architecture.md § Persistence), and a one-time Resync of OpenDKIM's tables +// and the Postfix sender map from SQLite on that first boot, so any drift +// between the archive and the database is healed before mail flows. If +// on-disk state drifts again later — for example after a manual edit under +// /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 -// normal case and returns nil. -func CheckRestore(manifestPath, binaryVersion string) error { +// never blocks a later in-place image upgrade, and restored is true so the +// caller can heal drifted daemon maps once. Absence of the manifest is the +// 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 } diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index f7c99bc..f340d0e 100644 --- a/internal/backup/backup_test.go +++ b/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") + } } diff --git a/internal/domain/opendkim.go b/internal/domain/opendkim.go index 434e30b..295bdb9 100644 --- a/internal/domain/opendkim.go +++ b/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 diff --git a/internal/postfix/postfix.go b/internal/postfix/postfix.go index 996952b..1643ea3 100644 --- a/internal/postfix/postfix.go +++ b/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