diff --git a/CHANGELOG.md b/CHANGELOG.md index ec499c9..c8c5482 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,24 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ## [Unreleased] +### Fixed + +- **A restore whose first start failed never ran its Resync.** The panel + removed the restore manifest as soon as it had checked the version, before + it opened the database and regenerated the mail maps from it; a start that + failed in between left a data directory the next start took for an + ordinary one. The manifest is now removed after the Resync is done, so the + next start is a restore again (audit 2026-10-12, M1). +- **An archive unpacked in part was started on.** A download cut short, or a + `tar` stopped by a damaged block, left the manifest (the archive's first + entry) and half of the rest, and the panel opened whatever database was + there. Every archive now ends with `data/contents.json`, the size and + SHA-256 of each file under `data/`, and the first start after a restore + compares `/data` with it before it opens anything: an archive unpacked in + part, or a file that differs, is refused with a message that names it, and + the directory is left as it was (guide § Restore). A damaged or cut domain + export was already refused before anything is created; a test now says so. + ## [2.5.2] - 2026-10-15 Stage 2c of the 2026-10-12 audit: every place SelfPost relied on what Postfix, diff --git a/cmd/panel/main.go b/cmd/panel/main.go index a17da64..e6fae7d 100644 --- a/cmd/panel/main.go +++ b/cmd/panel/main.go @@ -281,6 +281,37 @@ func parseTrustedProxies(raw string) []*net.IPNet { return nets } +// openData opens the database the panel runs on. When a backup was unpacked +// into /data, the restore guard comes first (architecture.md § Persistence): +// the manifest's version must match this binary and the archive must have +// been unpacked whole before the database is touched, so neither schema skew +// nor half an archive can corrupt the restored state. That first start then +// runs resync, and only after it the manifest is consumed: a start that fails +// on the way leaves it, and the next start is a restore again (audit +// 2026-10-12, M1). Without a manifest it is an ordinary start. +func openData(cfg config, resync func(*store.Store) error) (st *store.Store, restored bool, err error) { + restored, err = backup.CheckRestore(cfg.manifestPath, buildinfo.Version) + if err != nil { + return nil, false, err + } + st, err = store.Open(cfg.dbPath) + if err != nil { + return nil, false, err + } + if restored { + log.Printf("restore manifest accepted; regenerating mail-path maps from SQLite") + if err := resync(st); err != nil { + _ = st.Close() + return nil, false, err + } + if err := backup.ConsumeRestore(cfg.manifestPath); err != nil { + _ = st.Close() + return nil, false, err + } + } + return st, restored, nil +} + // run starts the panel's three roles and blocks until a shutdown signal or the // first fatal error from any role. A signal triggers a clean stop of all // roles; a role error cancels the others and is returned so the process exits @@ -297,31 +328,18 @@ func run() error { log.Printf("starting selfpost panel %s", buildinfo.Version) - // Restore version guard (architecture.md § Persistence): if a backup was - // extracted into /data, its manifest version must match this binary before we - // 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. - restored, err := backup.CheckRestore(cfg.manifestPath, buildinfo.Version) - if err != nil { - return err - } - // One database handle shared by every role. The store serialises writes // (MaxOpenConns(1)), so the HTTP panel, the journal-milter and the tailer // can all use it without stepping on each other under WAL. - st, err := store.Open(cfg.dbPath) + st, restored, err := openData(cfg, func(st *store.Store) error { + return resyncAfterRestore(cfg, st, false) + }) if err != nil { return err } 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 - } - } else if cfg.dmarcEnabled { + if !restored && cfg.dmarcEnabled { ms := newMailStack(cfg, st) ms.pf.SetReloadHook(func() error { return nil }) if err := ms.DMARC.Resync(); err != nil { diff --git a/cmd/panel/restore_cut_test.go b/cmd/panel/restore_cut_test.go new file mode 100644 index 0000000..9d8b4d9 --- /dev/null +++ b/cmd/panel/restore_cut_test.go @@ -0,0 +1,184 @@ +package main + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "errors" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/mixeme/selfpost/internal/backup" + "github.com/mixeme/selfpost/internal/buildinfo" + "github.com/mixeme/selfpost/internal/store" +) + +// A restore that goes wrong half way must leave the operator where they +// started: the archive is refused before the database is opened, and a start +// that fails after the guard is a restore again the next time (audit +// 2026-10-12, M1 and stage 3). + +// A start that fails after the guard - here its Resync - leaves the manifest, +// so the next start is a restore again and runs the Resync the failed one did +// not finish. The failed start had the database open, so its files are no +// longer the archive's; the next start does not compare them again. +func TestARestoreStartThatFailsIsARestoreAgain(t *testing.T) { + target := t.TempDir() + extract(t, archiveOf(t, seedPanelProject(t)), target) + cfg := panelConfig(t, target) + + broken := errors.New("postfix reload failed") + if _, _, err := openData(cfg, func(*store.Store) error { return broken }); !errors.Is(err, broken) { + t.Fatalf("a start whose Resync failed = %v, want that failure", err) + } + if _, err := os.Stat(cfg.manifestPath); err != nil { + t.Fatalf("the failed start consumed the manifest: %v", err) + } + + mapPath := filepath.Join(cfg.dataDir, "postfix", "sender_login_maps") + if err := os.WriteFile(mapPath, []byte("stale map\n"), 0o640); err != nil { + t.Fatalf("write stale map: %v", err) + } + st, restored, err := openData(cfg, testResync(cfg)) + if err != nil { + t.Fatalf("the start after a failed one: %v", err) + } + st.Close() + if !restored { + t.Fatal("the start after a failed one did not take the data directory for a restore") + } + if got, _ := os.ReadFile(mapPath); string(got) != "@"+restoreDomain+" shop\n" { + t.Errorf("sender map = %q: the second start did not run the Resync", got) + } + for _, name := range []string{backup.ManifestName, backup.ContentsName, "contents.checked.json"} { + if _, err := os.Stat(filepath.Join(cfg.dataDir, name)); !os.IsNotExist(err) { + t.Errorf("%s is still there after the restore start succeeded (stat err = %v)", name, err) + } + } + + st, restored, err = openData(cfg, func(*store.Store) error { + t.Error("an ordinary start ran the restore Resync") + return nil + }) + if err != nil { + t.Fatalf("the ordinary start after the restore: %v", err) + } + st.Close() + if restored { + t.Error("the start after a finished restore took it for a restore again") + } +} + +// An archive unpacked in part - a download cut short, a tar stopped by a +// damaged block - is refused before the database is opened, with a message +// that says so, and nothing in the directory is changed: the database the +// archive left is as it left it, and the manifest is there for the next try. +func TestAnArchiveUnpackedInPartIsRefused(t *testing.T) { + archive := archiveOf(t, seedPanelProject(t)) + dbSize := entrySize(t, archive, backup.DataArchivePrefix+"selfpost.db") + for _, c := range []struct { + name string + cut string + at int64 + }{ + {"in the database", backup.DataArchivePrefix + "selfpost.db", dbSize / 2}, + {"before the deploy files", backup.ComposeFileName, 0}, + {"in its last entry", backup.DataArchivePrefix + backup.ContentsName, 10}, + } { + t.Run(c.name, func(t *testing.T) { + target := t.TempDir() + extractCut(t, archive, target, c.cut, c.at) + cfg := panelConfig(t, target) + before, err := os.ReadFile(cfg.dbPath) + if err != nil { + t.Fatalf("the cut archive left no database: %v", err) + } + + _, _, err = openData(cfg, func(*store.Store) error { + t.Error("a refused restore ran its Resync") + return nil + }) + if err == nil || !strings.Contains(err.Error(), "not unpacked whole") { + t.Fatalf("the start on an archive cut %s = %v, want it refused as unpacked in part", c.name, err) + } + + if after, _ := os.ReadFile(cfg.dbPath); !bytes.Equal(before, after) { + t.Error("the refused start changed the database") + } + for _, side := range []string{"-wal", "-shm", "-journal"} { + if _, err := os.Stat(cfg.dbPath + side); !os.IsNotExist(err) { + t.Errorf("selfpost.db%s exists: the refused start opened the database", side) + } + } + if _, err := os.Stat(cfg.manifestPath); err != nil { + t.Errorf("the refused start removed the manifest: %v", err) + } + }) + } +} + +// A file the archive holds and the data directory holds otherwise - damaged +// on the way, or unpacked from another archive - is refused by its name. +func TestADamagedRestoredFileIsRefusedByName(t *testing.T) { + target := t.TempDir() + extract(t, archiveOf(t, seedPanelProject(t)), target) + cfg := panelConfig(t, target) + + key := filepath.Join(cfg.dataDir, "opendkim", "keys", restoreDomain, "selfpost.private") + f, err := os.OpenFile(key, os.O_WRONLY|os.O_APPEND, 0) + if err != nil { + t.Fatalf("open key: %v", err) + } + f.WriteString("x") + f.Close() + + _, _, err = openData(cfg, func(*store.Store) error { + t.Error("a refused restore ran its Resync") + return nil + }) + want := "data/opendkim/keys/" + restoreDomain + "/selfpost.private" + if err == nil || !strings.Contains(err.Error(), want) { + t.Fatalf("the start on a damaged key = %v, want a refusal naming %s", err, want) + } +} + +// archiveOf takes a full backup of a seeded project for this binary's version. +func archiveOf(t *testing.T, deployRoot string) []byte { + t.Helper() + dataDir := filepath.Join(deployRoot, "data") + var archive bytes.Buffer + if err := backup.Create(&archive, backup.Params{ + DataDir: dataDir, + DBPath: filepath.Join(dataDir, "selfpost.db"), + Version: buildinfo.Version, + DeployRoot: deployRoot, + }); err != nil { + t.Fatalf("create backup: %v", err) + } + return archive.Bytes() +} + +// entrySize is the size of the archive's entry name. +func entrySize(t *testing.T, archive []byte, name string) int64 { + t.Helper() + gz, err := gzip.NewReader(bytes.NewReader(archive)) + if err != nil { + t.Fatalf("gzip: %v", err) + } + tr := tar.NewReader(gz) + for { + hdr, err := tr.Next() + if err == io.EOF { + t.Fatalf("the archive has no %s", name) + } + if err != nil { + t.Fatalf("read the archive: %v", err) + } + if hdr.Name == name { + return hdr.Size + } + } +} diff --git a/cmd/panel/restore_test.go b/cmd/panel/restore_test.go index ef562eb..1b2c4e5 100644 --- a/cmd/panel/restore_test.go +++ b/cmd/panel/restore_test.go @@ -27,7 +27,8 @@ import ( // below stubs that story out — the archive is downloaded from a running panel // through /backup, unpacked the way `tar -xzf` unpacks it, and a second panel // is started on the result through the same startup sequence run() uses: -// CheckRestore, store.Open, newPanel, Start. +// openData (CheckRestore, store.Open, Resync, ConsumeRestore), newPanel, +// Start. const ( restorePassword = "correct-horse-battery" @@ -171,22 +172,17 @@ func TestResyncAfterRestoreHealsDriftedMaps(t *testing.T) { 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") - } + // A record of no files: this test is about the maps, and the files it + // seeded were never in an archive. + writeContents(t, dataDir, backup.Contents{Format: backup.FormatContents}) - st, err := store.Open(cfg.dbPath) + st, restored, err := openData(cfg, testResync(cfg)) if err != nil { - t.Fatalf("open store: %v", err) + t.Fatalf("open the restored data: %v", err) } defer st.Close() - - if err := resyncAfterRestore(cfg, st, true); err != nil { - t.Fatalf("resync after restore: %v", err) + if !restored { + t.Fatal("the start did not take the data directory for a restore") } got, err := os.ReadFile(mapPath) @@ -326,28 +322,17 @@ func bootPanel(t *testing.T, deployRoot string) http.Handler { t.Helper() cfg := panelConfig(t, deployRoot) - restored, err := backup.CheckRestore(cfg.manifestPath, buildinfo.Version) + st, restored, err := openData(cfg, testResync(cfg)) if err != nil { t.Fatalf("the panel refused to start on %s: %v", deployRoot, err) } + t.Cleanup(func() { _ = st.Close() }) 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 { - t.Fatalf("open the restored database: %v", err) - } - 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) @@ -417,6 +402,15 @@ func downloadBackup(t *testing.T, h http.Handler, session *http.Cookie, password // extract unpacks a backup archive into dir, as `tar -xzf` does onto the project // directory before the image is started. func extract(t *testing.T, archive []byte, dir string) { + t.Helper() + extractCut(t, archive, dir, "", 0) +} + +// extractCut unpacks the archive as extract does, except that the stream ends +// cutAt bytes into the entry named cut: that entry is written as far as the +// stream went and nothing after it is, which is what tar leaves of a download +// cut short or a damaged block. An empty cut unpacks the whole archive. +func extractCut(t *testing.T, archive []byte, dir, cut string, cutAt int64) { t.Helper() gz, err := gzip.NewReader(bytes.NewReader(archive)) if err != nil { @@ -426,6 +420,9 @@ func extract(t *testing.T, archive []byte, dir string) { for { hdr, err := tr.Next() if err == io.EOF { + if cut != "" { + t.Fatalf("the archive has no entry %s to cut", cut) + } break } if err != nil { @@ -436,6 +433,10 @@ func extract(t *testing.T, archive []byte, dir string) { t.Fatalf("the archive escapes the directory it is extracted into: %q", hdr.Name) } path := filepath.Join(dir, name) + var body io.Reader = tr + if hdr.Name == cut { + body = io.LimitReader(tr, cutAt) + } switch hdr.Typeflag { case tar.TypeDir: @@ -450,7 +451,7 @@ func extract(t *testing.T, archive []byte, dir string) { if err != nil { t.Fatalf("create %s: %v", path, err) } - if _, err := io.Copy(f, tr); err != nil { + if _, err := io.Copy(f, body); err != nil { f.Close() t.Fatalf("write %s: %v", path, err) } @@ -458,6 +459,28 @@ func extract(t *testing.T, archive []byte, dir string) { t.Fatalf("close %s: %v", path, err) } } + if hdr.Name == cut { + return + } + } +} + +// testResync is the Resync a restore start runs, without reloading daemons +// the test does not have. +func testResync(cfg config) func(*store.Store) error { + return func(st *store.Store) error { return resyncAfterRestore(cfg, st, true) } +} + +// writeContents writes the record of an archive's files into dataDir, as the +// archive's last entry would have. +func writeContents(t *testing.T, dataDir string, c backup.Contents) { + t.Helper() + body, err := json.Marshal(c) + if err != nil { + t.Fatalf("marshal contents: %v", err) + } + if err := os.WriteFile(filepath.Join(dataDir, backup.ContentsName), body, 0o600); err != nil { + t.Fatalf("write contents: %v", err) } } diff --git a/docs/architecture.md b/docs/architecture.md index 5affeab..3735568 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -960,7 +960,8 @@ single-connection trade-off that follows from it. | `postfix/dmarc_relay_domains` | Domains accepted for DMARC report delivery | | `postfix/queue/` | Postfix transit mail (deferred/active); survives container recreate | | `log/mail.log` | Postfix delivery log + rotated copies (excluded from backups) | -| `manifest.json` | Backup version stamp (consumed on restore) | +| `manifest.json` | Backup version stamp; removed by the first start after a restore once its Resync is done | +| `contents.json` | A backup's last entry: the size and SHA-256 of each file under `data/`, compared by the first start after a restore before it opens the database (renamed `contents.checked.json` once compared, removed with the manifest) | Not in `/data`: TLS certificates for the panel (reverse-proxy mount) — though full backups also archive the operator's `./certs` PEM files when present. diff --git a/docs/audit-2026-10-12.md b/docs/audit-2026-10-12.md index c7427b0..f4d3823 100644 --- a/docs/audit-2026-10-12.md +++ b/docs/audit-2026-10-12.md @@ -1074,11 +1074,24 @@ those after it move up one patch number each. ### Stage 3 — persistence and restore (M1–M5, L1–L4, L7) → `2.5.3` -- [ ] **Consume the manifest after the resync succeeds** (`CheckRestore` reports; +- [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. - [ ] **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'`; diff --git a/docs/guide.md b/docs/guide.md index 224afae..f9839f0 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -809,7 +809,9 @@ blocked. On that same first boot the panel also runs one **Resync** — OpenDKIM tables and Postfix's sender map are re-derived from SQLite (inbound relay maps when `INBOUND_RELAY_ENABLE=true`; DMARC maps when `DMARC_REPORTS_ENABLE=true`) and both daemons are reloaded, healing any drift between the extracted files and -the database. The *Reload configuration* button on Server › Health resyncs +the database. The manifest is deleted only after that Resync is done: a first +start that fails on the way leaves it, and the next start is a restore again +and runs the Resync. The *Reload configuration* button on Server › Health resyncs OpenDKIM, the sender map, and inbound maps only — not DMARC maps. This is why the compose file pins a fixed tag rather than `:latest`: without a known version, there'd be no way to tell which image restoring a given backup actually requires @@ -857,6 +859,20 @@ tag](#fixed-image-tag) if it is not published), then `docker compose up -d` again — the manifest is still there because the failed boot never got to delete it. +An archive unpacked **in part** — a download cut short, or a `tar` that +stopped at a damaged block (it says so and exits non-zero) — is refused at +step 5 as well, before the database is opened: + +``` +backup: the archive was not unpacked whole (data/contents.json, its last entry, is not there) - it was cut short or is damaged; nothing was opened. Unpack it again, from an intact copy, into an empty directory +``` + +The last entry of every panel and CLI archive, `data/contents.json`, records +each file under `data/` with its size and SHA-256, and the first start +compares `/data` with it; a file that differs is named in the message. +`docker-compose.yml`, `.env` and `certs/` are not compared — they are yours to +edit before that start. + **Moving to a different host** is the same flow: create an empty project directory, unpack the backup there, edit `.env` (and `docker-compose.yml` if needed) for the new hostname or proxy, then `docker compose up -d`. The archive diff --git a/docs/roadmap.md b/docs/roadmap.md index 23be959..795823e 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -42,7 +42,7 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md). | from-header-binding | Hold a message's `From:` header against the application that sends it | shipped `2.4.0` | 9/9 | [plans/from-header-binding.md](plans/from-header-binding.md) | | panel-redesign | Panel redesign (accepted mockups, design contract) | shipped `2.0.0` | 27/27 | [plans/panel-redesign.md](plans/panel-redesign.md) | | schema-squash | Squash SQLite migrations into a 2.x baseline | built in panel-redesign (stage 1) | — | [plans/panel-redesign.md](plans/panel-redesign.md) | -| audit-2026-10-12 | Follow-ups of the 2026-10-12 full-tree code audit, seven stages | in progress (stage 0 shipped `2.4.2`, stages 1+2 `2.5.0`, 2b `2.5.1`, 2c `2.5.2`; next stage 3 → `2.5.3`) | 49/81 | [audit-2026-10-12.md](audit-2026-10-12.md) | +| audit-2026-10-12 | Follow-ups of the 2026-10-12 full-tree code audit, seven stages | in progress (stage 0 shipped `2.4.2`, stages 1+2 `2.5.0`, 2b `2.5.1`, 2c `2.5.2`; next stage 3 → `2.5.3`) | 50/81 | [audit-2026-10-12.md](audit-2026-10-12.md) | **Recommended order** (not binding; owner, 2026-10-09: feature order follows development convenience): **panel-redesign** shipped as `2.0.0` (2026-10-11) and diff --git a/internal/backup/backup.go b/internal/backup/backup.go index 9c8a585..d5055bf 100644 --- a/internal/backup/backup.go +++ b/internal/backup/backup.go @@ -15,7 +15,8 @@ // 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 +// (architecture.md § Persistence), or if the archive was unpacked in part +// (ContentsName), 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 @@ -25,6 +26,7 @@ package backup import ( "archive/tar" + "bytes" "compress/gzip" "database/sql" "encoding/json" @@ -113,12 +115,15 @@ var excludedFromArchive = map[string]bool{ "tls": true, "log": true, ManifestName: true, + ContentsName: true, + contentsCheckedName: true, } // Create writes a gzip-compressed tar backup to w. Archive layout: // // data/manifest.json, data/selfpost.db, data/ // docker-compose.yml, .env, certs/... +// data/contents.json // // Extract the archive into an empty project directory, then docker compose up. func Create(w io.Writer, p Params) error { @@ -140,6 +145,7 @@ func Create(w io.Writer, p Params) error { gz := gzip.NewWriter(w) tw := tar.NewWriter(gz) + a := &archive{tw: tw} manifest := Manifest{ Format: FormatFull, @@ -150,21 +156,26 @@ func Create(w io.Writer, p Params) error { if err != nil { return fmt.Errorf("backup: encode manifest: %w", err) } - if err := writeTarBytes(tw, DataArchivePrefix+ManifestName, 0o600, manifestJSON); err != nil { + if err := writeTarBytes(a, DataArchivePrefix+ManifestName, 0o600, manifestJSON); err != nil { return err } // The consistent SQLite snapshot, under the canonical filename the panel // opens on start (the live file and its WAL/SHM are excluded from the walk). - if err := writeTarFile(tw, DataArchivePrefix+"selfpost.db", 0o640, snapshot); err != nil { + if err := writeTarFile(a, DataArchivePrefix+"selfpost.db", 0o640, snapshot); err != nil { return err } - if err := addTree(tw, p.DataDir, DataArchivePrefix, excludedFromArchive, skipQueue, false); err != nil { + if err := addTree(a, p.DataDir, DataArchivePrefix, excludedFromArchive, skipQueue, false); err != nil { return err } - if err := addDeployFiles(tw, p.DeployRoot, p.OnWarn); err != nil { + if err := addDeployFiles(a, p.DeployRoot, p.OnWarn); err != nil { + return err + } + + // Last, so that only an archive unpacked to its end has it. + if err := a.writeContents(); err != nil { return err } @@ -249,7 +260,7 @@ func UnreadableCert(err error, deployRoot string) (name string, ok bool) { return CertsDirName + "/" + filepath.ToSlash(rel), true } -func addDeployFiles(tw *tar.Writer, deployRoot string, onWarn func(string)) error { +func addDeployFiles(a *archive, deployRoot string, onWarn func(string)) error { for _, name := range []string{ComposeFileName, EnvFileName} { src := filepath.Join(deployRoot, name) info, err := os.Stat(src) @@ -259,7 +270,7 @@ func addDeployFiles(tw *tar.Writer, deployRoot string, onWarn func(string)) erro if !info.Mode().IsRegular() { return fmt.Errorf("backup: deploy file %s is not a regular file", name) } - if err := writeTarFile(tw, name, info.Mode().Perm(), src); err != nil { + if err := writeTarFile(a, name, info.Mode().Perm(), src); err != nil { return err } } @@ -286,19 +297,19 @@ func addDeployFiles(tw *tar.Writer, deployRoot string, onWarn func(string)) erro // are links again, relative, into archive/ (eff-certbot docs, "Where are // my certificates?"; assumptions C-Q4, C-A1). The archive carries the // files they lead to, under the links' names. - return addTree(tw, certsDir, CertsDirName+"/", nil, nil, true) + return addTree(a, certsDir, CertsDirName+"/", nil, nil, true) } // addTree walks root and adds every regular file (and directory, to preserve -// empty ones and modes) to tw under archivePrefix + path relative to root. +// empty ones and modes) to a under archivePrefix + path relative to root. // When exclude is non-nil, top-level names relative to root are skipped; skip, -// when non-nil, leaves out whatever it says. With a nil tw nothing is written: +// when non-nil, leaves out whatever it says. With a nil a nothing is written: // every file is opened and closed, which is how Check finds what cannot be read. // Non-regular, non-directory entries (symlinks, sockets) are skipped - except, // with follow, a symbolic link to a regular file, which is archived as that // file under the link's name; a link that leads nowhere this process can // reach is an error, as an unreadable file is. -func addTree(tw *tar.Writer, root, archivePrefix string, exclude map[string]bool, skip skipFunc, follow bool) error { +func addTree(a *archive, root, archivePrefix string, exclude map[string]bool, skip skipFunc, follow bool) error { return filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { if err != nil { return err @@ -334,7 +345,7 @@ func addTree(tw *tar.Writer, root, archivePrefix string, exclude map[string]bool } switch { case d.IsDir(): - if tw == nil { + if a == nil { return nil } hdr := &tar.Header{ @@ -343,9 +354,9 @@ func addTree(tw *tar.Writer, root, archivePrefix string, exclude map[string]bool Mode: int64(info.Mode().Perm()), ModTime: info.ModTime(), } - return tw.WriteHeader(hdr) + return a.tw.WriteHeader(hdr) case info.Mode().IsRegular(): - err := writeTarFile(tw, name, info.Mode().Perm(), path) + err := writeTarFile(a, name, info.Mode().Perm(), path) if errors.Is(err, fs.ErrNotExist) { // A writer's temporary file, renamed into place between // the listing and the open: not part of /data's state. @@ -360,7 +371,7 @@ func addTree(tw *tar.Writer, root, archivePrefix string, exclude map[string]bool if !target.Mode().IsRegular() { return nil } - return writeTarFile(tw, name, target.Mode().Perm(), path) + return writeTarFile(a, name, target.Mode().Perm(), path) default: return nil } @@ -368,21 +379,14 @@ func addTree(tw *tar.Writer, root, archivePrefix string, exclude map[string]bool } // writeTarBytes writes an in-memory file entry. -func writeTarBytes(tw *tar.Writer, name string, mode int64, data []byte) error { - hdr := &tar.Header{ +func writeTarBytes(a *archive, name string, mode int64, data []byte) error { + return a.writeFile(&tar.Header{ Typeflag: tar.TypeReg, Name: name, Mode: mode, Size: int64(len(data)), ModTime: time.Now().UTC(), - } - if err := tw.WriteHeader(hdr); err != nil { - return fmt.Errorf("backup: write header %s: %w", name, err) - } - if _, err := tw.Write(data); err != nil { - return fmt.Errorf("backup: write %s: %w", name, err) - } - return nil + }, bytes.NewReader(data)) } // writeTarFile streams a file from disk into the archive under name. @@ -394,13 +398,13 @@ func writeTarBytes(tw *tar.Writer, name string, mode int64, data []byte) error { // the queue are not walked), and a rename leaves the file open here as it // was: the archive holds the old file or the new one, whole (assumptions // C-Q3). -func writeTarFile(tw *tar.Writer, name string, mode fs.FileMode, srcPath string) error { +func writeTarFile(a *archive, name string, mode fs.FileMode, srcPath string) error { f, err := os.Open(srcPath) if err != nil { return fmt.Errorf("backup: open %s: %w", srcPath, err) } defer f.Close() - if tw == nil { + if a == nil { return nil } @@ -408,20 +412,13 @@ func writeTarFile(tw *tar.Writer, name string, mode fs.FileMode, srcPath string) if err != nil { return fmt.Errorf("backup: stat %s: %w", srcPath, err) } - hdr := &tar.Header{ + return a.writeFile(&tar.Header{ Typeflag: tar.TypeReg, Name: name, Mode: int64(mode.Perm()), Size: info.Size(), ModTime: info.ModTime(), - } - if err := tw.WriteHeader(hdr); err != nil { - return fmt.Errorf("backup: write header %s: %w", name, err) - } - if _, err := io.Copy(tw, f); err != nil { - return fmt.Errorf("backup: copy %s: %w", name, err) - } - return nil + }, f) } // snapshotDB produces a consistent copy of the SQLite database at dbPath using @@ -463,10 +460,13 @@ func snapshotDB(dbPath string) (path string, cleanup func(), err error) { // CheckRestore enforces the backup version guard (architecture.md § // Persistence). If manifestPath exists (a backup was extracted into the data // 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, and restored is true so the -// caller can heal drifted daemon maps once. Absence of the manifest is the +// start, telling the operator which image tag to use; and the data directory +// must hold every file the archive recorded at its end, as the archive holds +// it, or the panel refuses to start on an archive unpacked in part (see +// ContentsName). Then restored is true so the caller can heal drifted daemon +// maps once. The manifest stays until the caller has done that and calls +// ConsumeRestore: a start that fails on the way leaves it, and the next start +// is a restore again (audit 2026-10-12, M1). 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) @@ -489,10 +489,24 @@ func CheckRestore(manifestPath, binaryVersion string) (restored bool, err error) "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 false, fmt.Errorf("backup: consume restore manifest: %w", err) + if err := checkContents(filepath.Dir(manifestPath)); err != nil { + return false, err } return true, nil } + +// ConsumeRestore ends a restore once its first start has done what a restore +// start does: the manifest is removed, so later starts (and in-place +// upgrades) are not gated by it, and then the record of the checked files. A +// start stopped between the two leaves the record alone, which nothing reads +// without a manifest and no backup carries. +func ConsumeRestore(manifestPath string) error { + if err := os.Remove(manifestPath); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("backup: consume restore manifest: %w", err) + } + checked := filepath.Join(filepath.Dir(manifestPath), contentsCheckedName) + if err := os.Remove(checked); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("backup: consume restore manifest: %w", err) + } + return nil +} diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index fce4964..63c7d3f 100644 --- a/internal/backup/backup_test.go +++ b/internal/backup/backup_test.go @@ -4,6 +4,8 @@ import ( "archive/tar" "bytes" "compress/gzip" + "crypto/sha256" + "encoding/hex" "encoding/json" "fmt" "io" @@ -200,9 +202,33 @@ func TestCheckRestoreNoManifestIsNormalStart(t *testing.T) { } } -func TestCheckRestoreMatchConsumesManifest(t *testing.T) { +// unpackData writes the data directory of a Create archive into a new +// directory and returns the manifest's path there. +func unpackData(t *testing.T, archive []byte) string { + t.Helper() dir := t.TempDir() - path := writeManifest(t, dir, FormatFull, "1.0.0") + for name, body := range readArchive(t, archive) { + if rel, ok := strings.CutPrefix(name, DataArchivePrefix); ok { + writeFile(t, filepath.Join(dir, filepath.FromSlash(rel)), body) + } + } + return filepath.Join(dir, ManifestName) +} + +func createArchive(t *testing.T, version string) []byte { + t.Helper() + dataDir, dbPath, deployRoot := seedDataDir(t) + var buf bytes.Buffer + if err := Create(&buf, Params{DataDir: dataDir, DBPath: dbPath, Version: version, DeployRoot: deployRoot}); err != nil { + t.Fatalf("Create: %v", err) + } + return buf.Bytes() +} + +// A matching restore keeps its manifest until the start that runs it is done +// with it (audit 2026-10-12, M1): CheckRestore reports, ConsumeRestore ends. +func TestCheckRestoreMatchKeepsTheManifestUntilConsumed(t *testing.T) { + path := unpackData(t, createArchive(t, "1.0.0")) restored, err := CheckRestore(path, "1.0.0") if err != nil { t.Fatalf("CheckRestore matching = %v, want nil", err) @@ -210,8 +236,145 @@ func TestCheckRestoreMatchConsumesManifest(t *testing.T) { 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) + if _, err := os.Stat(path); err != nil { + t.Fatalf("CheckRestore consumed the manifest before the start was done with it: %v", err) + } + if err := ConsumeRestore(path); err != nil { + t.Fatalf("ConsumeRestore: %v", err) + } + for _, name := range []string{ManifestName, ContentsName, contentsCheckedName} { + if _, err := os.Stat(filepath.Join(filepath.Dir(path), name)); !os.IsNotExist(err) { + t.Errorf("%s left after ConsumeRestore (stat err = %v)", name, err) + } + } + if err := ConsumeRestore(path); err != nil { + t.Errorf("ConsumeRestore on a consumed restore = %v, want nil", err) + } +} + +// The archive's last entry records the data directory's files - every one, +// with its size and digest - and nothing of the deploy files, which the +// operator edits before the first start. +func TestCreateEndsWithTheRecordOfItsData(t *testing.T) { + archive := createArchive(t, "1.0.0") + gz, err := gzip.NewReader(bytes.NewReader(archive)) + if err != nil { + t.Fatalf("gzip: %v", err) + } + tr := tar.NewReader(gz) + var last string + for { + hdr, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("tar next: %v", err) + } + last = hdr.Name + } + if last != DataArchivePrefix+ContentsName { + t.Fatalf("the last entry is %s, want %s", last, DataArchivePrefix+ContentsName) + } + + files := readArchive(t, archive) + var c Contents + if err := json.Unmarshal([]byte(files[DataArchivePrefix+ContentsName]), &c); err != nil { + t.Fatalf("contents: %v", err) + } + recorded := map[string]FileSum{} + for _, f := range c.Files { + recorded[f.Name] = f + } + for name, body := range files { + if name == DataArchivePrefix+ContentsName { + continue + } + f, ok := recorded[name] + if !strings.HasPrefix(name, DataArchivePrefix) { + if ok { + t.Errorf("the record names the deploy file %s", name) + } + continue + } + sum := sha256.Sum256([]byte(body)) + if !ok || f.Size != int64(len(body)) || f.SHA256 != hex.EncodeToString(sum[:]) { + t.Errorf("%s recorded as %+v, want size %d sha256 %x", name, f, len(body), sum) + } + delete(recorded, name) + } + for name := range recorded { + t.Errorf("the record names %s, which the archive does not hold", name) + } +} + +// Without the archive's last entry, or with a file that is not the one it +// holds, a restore is refused, and the manifest stays for the next try. +func TestCheckRestoreRefusesAnArchiveUnpackedInPart(t *testing.T) { + sasldb := func(body string) func(string) { + return func(dir string) { writeFile(t, filepath.Join(dir, "sasl", "sasldb2"), body) } + } + for _, c := range []struct { + name string + break_ func(dir string) + want string + }{ + {"no record", func(dir string) { os.Remove(filepath.Join(dir, ContentsName)) }, "contents.json, its last entry, is not there"}, + {"a record cut", func(dir string) { writeFile(t, filepath.Join(dir, ContentsName), `{"format":"selfpost-backup-con`) }, "is not a record"}, + {"a file missing", func(dir string) { os.Remove(filepath.Join(dir, "sasl", "sasldb2")) }, "data/sasl/sasldb2 is missing"}, + {"a file cut", sasldb("SASL"), "data/sasl/sasldb2 is not the file"}, + {"a file altered", sasldb("SASLDC"), "data/sasl/sasldb2 is not the file"}, + } { + t.Run(c.name, func(t *testing.T) { + path := unpackData(t, createArchive(t, "1.0.0")) + c.break_(filepath.Dir(path)) + restored, err := CheckRestore(path, "1.0.0") + if err == nil || !strings.Contains(err.Error(), c.want) || !strings.Contains(err.Error(), "not unpacked whole") { + t.Fatalf("CheckRestore = %v, want a refusal saying %q", err, c.want) + } + if restored { + t.Error("a refused restore reported a restore") + } + if _, err := os.Stat(path); err != nil { + t.Errorf("the refusal removed the manifest: %v", err) + } + }) + } +} + +// A record names files of the data directory only; one that names another +// path is refused, not followed. +func TestCheckRestoreRefusesARecordOutsideTheDataDirectory(t *testing.T) { + for _, name := range []string{"data/../.env", "data/", "docker-compose.yml", "data//sasl/sasldb2", "data/./x", "data/.."} { + path := unpackData(t, createArchive(t, "1.0.0")) + body, _ := json.Marshal(Contents{Format: FormatContents, Files: []FileSum{{Name: name}}}) + writeFile(t, filepath.Join(filepath.Dir(path), ContentsName), string(body)) + if _, err := CheckRestore(path, "1.0.0"); err == nil || !strings.Contains(err.Error(), "not a file of the data directory") { + t.Errorf("a record naming %q = %v, want it refused", name, err) + } + } +} + +// The files are compared once per restore: a start that failed after the +// check had the database open, and the next start must not take the changes +// it made for damage. A different archive unpacked over the directory since +// is compared anew. +func TestCheckRestoreComparesTheFilesOncePerRestore(t *testing.T) { + path := unpackData(t, createArchive(t, "1.0.0")) + dir := filepath.Dir(path) + if _, err := CheckRestore(path, "1.0.0"); err != nil { + t.Fatalf("first CheckRestore: %v", err) + } + writeFile(t, filepath.Join(dir, "selfpost.db"), "changed by the start that failed") + if _, err := CheckRestore(path, "1.0.0"); err != nil { + t.Fatalf("the start after a failed one compared the files again: %v", err) + } + + // Another archive's manifest, its record lost on the way. + b, _ := json.Marshal(Manifest{Format: FormatFull, Version: "1.0.0", CreatedAt: "another time"}) + writeFile(t, path, string(b)) + if _, err := CheckRestore(path, "1.0.0"); err == nil || !strings.Contains(err.Error(), "not unpacked whole") { + t.Fatalf("another archive's manifest over a checked restore = %v, want it compared anew and refused", err) } } diff --git a/internal/backup/contents.go b/internal/backup/contents.go new file mode 100644 index 0000000..de435f3 --- /dev/null +++ b/internal/backup/contents.go @@ -0,0 +1,175 @@ +package backup + +import ( + "archive/tar" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "path" + "path/filepath" + "strings" +) + +// FormatContents identifies the record of what an archive's data directory +// holds. +const FormatContents = "selfpost-backup-contents" + +// ContentsName is the record of the data directory's files, the last entry of +// every archive (data/contents.json). The manifest is the first entry and +// says a restore is under way; this one says the archive was unpacked to its +// end. A download cut short, or a tar that stopped at a damaged block, leaves +// a data directory with the manifest and without this file, and the panel +// refuses to start on it instead of opening half a database (audit +// 2026-10-12, stage 3). +const ContentsName = "contents.json" + +// contentsCheckedName is ContentsName once a start has checked the files +// against it. The check is made once: the start that made it opens the +// database, and from then on the files are the panel's to change, so a start +// after a failed one (manifest still there) must not compare them again. +const contentsCheckedName = "contents.checked.json" + +// Contents is the record written as ContentsName. +type Contents struct { + Format string `json:"format"` + Files []FileSum `json:"files"` +} + +// FileSum is one regular file of the data directory as the archive holds it, +// named as in the archive (data/...). +type FileSum struct { + Name string `json:"name"` + Size int64 `json:"size"` + SHA256 string `json:"sha256"` +} + +// archive is the tar stream a backup is written to, and the record of the +// data directory's files written to it so far. A nil *archive writes +// nothing: every file is opened and closed, which is how Check finds what +// cannot be read. +// +// Only the data directory is recorded. The deploy files are the operator's to +// edit before the first start - the guide sends them to .env for a new +// hostname and to docker-compose.yml for the tag - and the data directory is +// what the panel opens. +type archive struct { + tw *tar.Writer + files []FileSum +} + +// writeFile writes one regular file's header and content, read from r, and +// records it when it belongs to the data directory. +func (a *archive) writeFile(hdr *tar.Header, r io.Reader) error { + if err := a.tw.WriteHeader(hdr); err != nil { + return fmt.Errorf("backup: write header %s: %w", hdr.Name, err) + } + h := sha256.New() + n, err := io.Copy(io.MultiWriter(a.tw, h), r) + if err != nil { + return fmt.Errorf("backup: copy %s: %w", hdr.Name, err) + } + if strings.HasPrefix(hdr.Name, DataArchivePrefix) { + a.files = append(a.files, FileSum{Name: hdr.Name, Size: n, SHA256: hex.EncodeToString(h.Sum(nil))}) + } + return nil +} + +// writeContents ends the archive's data with the record of what it holds. +func (a *archive) writeContents() error { + body, err := json.MarshalIndent(Contents{Format: FormatContents, Files: a.files}, "", " ") + if err != nil { + return fmt.Errorf("backup: encode contents: %w", err) + } + return writeTarBytes(&archive{tw: a.tw}, DataArchivePrefix+ContentsName, 0o600, body) +} + +// errUnpackedPart is what a restore that is not whole is refused with. +func errUnpackedPart(detail string) error { + return fmt.Errorf("backup: the archive was not unpacked whole (%s) - it was cut short or is damaged; nothing was opened. Unpack it again, from an intact copy, into an empty directory", detail) +} + +// checkContents compares the data directory with the archive's record, once +// per restore (see contentsCheckedName). +func checkContents(dataDir string) error { + checked := filepath.Join(dataDir, contentsCheckedName) + if c, err := readContents(checked); err == nil { + // A start before this one checked the files. It is the same restore + // when the manifest is still the one that record holds; a different + // archive unpacked over the directory since is checked anew. + if verifyFiles(dataDir, c, DataArchivePrefix+ManifestName) == nil { + return nil + } + } else if !errors.Is(err, os.ErrNotExist) { + return err + } + + path := filepath.Join(dataDir, ContentsName) + c, err := readContents(path) + if errors.Is(err, os.ErrNotExist) { + return errUnpackedPart(DataArchivePrefix + ContentsName + ", its last entry, is not there") + } + if err != nil { + return err + } + if err := verifyFiles(dataDir, c, ""); err != nil { + return err + } + if err := os.Rename(path, checked); err != nil { + return fmt.Errorf("backup: mark the restored files checked: %w", err) + } + return nil +} + +// readContents reads a record of the archive's files. +func readContents(path string) (Contents, error) { + var c Contents + data, err := os.ReadFile(path) + if err != nil { + return c, err + } + if err := json.Unmarshal(data, &c); err != nil || c.Format != FormatContents { + return c, errUnpackedPart(filepath.Base(path) + " is not a record of the archive's files") + } + return c, nil +} + +// verifyFiles compares the files c records with those under dataDir: each +// present, of its size, with its digest. A non-empty only limits it to the +// file of that name. +func verifyFiles(dataDir string, c Contents, only string) error { + found := false + for _, f := range c.Files { + if only != "" && f.Name != only { + continue + } + found = true + rel, ok := strings.CutPrefix(f.Name, DataArchivePrefix) + if !ok || rel == "" || path.Clean(rel) != rel || rel == ".." || strings.HasPrefix(rel, "../") || path.IsAbs(rel) { + return errUnpackedPart("the record names " + f.Name + ", which is not a file of the data directory") + } + file, err := os.Open(filepath.Join(dataDir, filepath.FromSlash(rel))) + if errors.Is(err, os.ErrNotExist) { + return errUnpackedPart(f.Name + " is missing") + } + if err != nil { + return fmt.Errorf("backup: check %s: %w", f.Name, err) + } + h := sha256.New() + n, err := io.Copy(h, file) + file.Close() + if err != nil { + return fmt.Errorf("backup: check %s: %w", f.Name, err) + } + if n != f.Size || hex.EncodeToString(h.Sum(nil)) != f.SHA256 { + return errUnpackedPart(f.Name + " is not the file the archive holds") + } + } + if only != "" && !found { + return errUnpackedPart("the record does not name " + only) + } + return nil +} diff --git a/internal/web/handlers/handlers_import_test.go b/internal/web/handlers/handlers_import_test.go index a963402..c65062b 100644 --- a/internal/web/handlers/handlers_import_test.go +++ b/internal/web/handlers/handlers_import_test.go @@ -3,6 +3,7 @@ package handlers import ( "bytes" "encoding/json" + "html" "mime/multipart" "net/http" "net/http/httptest" @@ -10,6 +11,7 @@ import ( "testing" "github.com/mixeme/selfpost/internal/domain" + "github.com/mixeme/selfpost/internal/secretfile" "github.com/mixeme/selfpost/internal/store" "github.com/mixeme/selfpost/internal/web/auth" ) @@ -116,3 +118,57 @@ func TestDomainImportRefusedCreatesNothing(t *testing.T) { t.Fatalf("the refused import created %v", domains) } } + +// A domain export cut short or altered on the way is refused with what is +// wrong with it, before anything is created (audit 2026-10-12, stage 3). +func TestDomainImportOfADamagedFileCreatesNothing(t *testing.T) { + h, st := inboundHandlers(t) + root := userWith(t, st, "root", store.RoleGlobal, store.Reach{}) + plain, _ := json.Marshal(domain.DomainExport{ + Format: domain.FormatDomainExport, Domain: "example.com", DKIMSelector: "mail", + Applications: []domain.AppExport{{Login: "app1", AddressMode: store.AddressModeWildcard, Password: "imported-password"}}, + }) + const password = "a-long-enough-password" + var sealed bytes.Buffer + w, err := secretfile.NewWriter(&sealed, secretfile.TypeDomainExport, password) + if err != nil { + t.Fatal(err) + } + w.Write(plain) + w.Close() + flipped := bytes.Clone(sealed.Bytes()) + flipped[len(flipped)-20] ^= 1 + + for _, c := range []struct { + name, file, password string + body []byte + want string + }{ + {"plain, cut", "example.com.json", "", plain[:len(plain)/2], "That file is not a valid SelfPost domain export."}, + {"encrypted, cut", "example.com.spde", password, sealed.Bytes()[:sealed.Len()-10], "That file is damaged or incomplete."}, + {"encrypted, altered", "example.com.spde", password, flipped, "Wrong password, or the file has been altered since it was exported."}, + } { + t.Run(c.name, func(t *testing.T) { + var body bytes.Buffer + mw := multipart.NewWriter(&body) + fw, _ := mw.CreateFormFile("file", c.file) + fw.Write(c.body) + mw.WriteField("import_password", c.password) + mw.Close() + req := httptest.NewRequest("POST", "/server/backup/import", &body) + req.Header.Set("Content-Type", mw.FormDataContentType()) + req = auth.RequestWithPrincipal(req, root) + rec := httptest.NewRecorder() + h.HandleImportDomain(rec, req) + if rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), html.EscapeString(c.want)) { + t.Fatalf("import = %d, want 400 saying %q:\n%s", rec.Code, c.want, rec.Body.String()) + } + if domains, _ := st.ListDomains(); len(domains) != 0 { + t.Fatalf("the refused import created %v", domains) + } + if _, err := st.GetApplicationByLogin("app1"); err == nil { + t.Fatal("the refused import created the application") + } + }) + } +} diff --git a/test/e2e/restore_test.go b/test/e2e/restore_test.go index d1dee90..60f2067 100644 --- a/test/e2e/restore_test.go +++ b/test/e2e/restore_test.go @@ -70,7 +70,7 @@ func testFullBackupRestore(t *testing.T, sc *scenario) { if err != nil { t.Fatalf("unpack the backup into the empty project directory: %v", err) } - for _, want := range []string{"docker-compose.yml", ".env", "data/manifest.json", "data/selfpost.db", "certs/fullchain.pem", "certs/privkey.pem"} { + for _, want := range []string{"docker-compose.yml", ".env", "data/manifest.json", "data/selfpost.db", "data/contents.json", "certs/fullchain.pem", "certs/privkey.pem"} { if !names[want] { t.Fatalf("the backup has no %s (it has %d entries)", want, len(names)) } @@ -93,12 +93,15 @@ func testFullBackupRestore(t *testing.T, sc *scenario) { t.Fatal(err) } - // The manifest guards the first start only and is removed by it. + // The manifest guards the first start only and is removed by it, with the + // record of the archive's files that start compared /data with - after + // the entrypoint had set owners and modes and the daemons had started, + // which is what this run shows the comparison survives. if err := waitFor("manifest.json to be removed by the first start", 30*time.Second, 500*time.Millisecond, func() (bool, error) { - out, _ := h.execIn("selfpost", "sh", "-c", "test -e /data/manifest.json && echo present || echo gone") - return strings.TrimSpace(out) == "gone", fmt.Errorf("manifest.json is %s", strings.TrimSpace(out)) + out, _ := h.execIn("selfpost", "sh", "-c", "ls /data/manifest.json /data/contents.json /data/contents.checked.json 2>/dev/null; true") + return strings.TrimSpace(out) == "", fmt.Errorf("still there: %s", strings.TrimSpace(out)) }); err != nil { - t.Fatal(err) + t.Fatalf("%v\n%s", err, logTail(h.logs("selfpost"), "backup")) } p, err := newPanelClient()