diff --git a/cmd/archivmail/cmd_backup.go b/cmd/archivmail/cmd_backup.go index 5a5d6d8..10699d7 100644 --- a/cmd/archivmail/cmd_backup.go +++ b/cmd/archivmail/cmd_backup.go @@ -202,6 +202,14 @@ func copyTreePreservingHardlinks(src, dst string) (linked, copied int, err error stat, ok := info.Sys().(*syscall.Stat_t) if ok { if existing, dup := seen[stat.Ino]; dup { + // os.Link fails with EEXIST if target is already occupied — happens + // on a -force restore where the destination isn't empty (BUG-2, + // PROJ-66 QA). Remove any stale file first; harmless when the + // destination is empty already (os.Remove on a non-existent path + // is ignored below). + if rmErr := os.Remove(target); rmErr != nil && !os.IsNotExist(rmErr) { + return fmt.Errorf("hardlink %s -> %s: remove stale target: %w", existing, target, rmErr) + } if err := os.Link(existing, target); err != nil { return fmt.Errorf("hardlink %s -> %s: %w", existing, target, err) } diff --git a/cmd/archivmail/cmd_import.go b/cmd/archivmail/cmd_import.go index 909c95d..34696ea 100644 --- a/cmd/archivmail/cmd_import.go +++ b/cmd/archivmail/cmd_import.go @@ -307,7 +307,7 @@ Commands: ocr-reprocess OCR für Anhänge nachholen (alle oder pro Mandant/Status) index-pending Ungeindexte Mails nachindexieren (cron-fähig, PROJ-58 batch_mode) backup Store, Keyfile, PostgreSQL und Config konsistent sichern (PROJ-66) - restore Backup aus `archivmail backup` zurückspielen (PROJ-66) + restore Backup aus 'archivmail backup' zurückspielen (PROJ-66) update Auf neueste Version aktualisieren (führt update.sh aus) status Healthcheck für DB, Manticore und Storage version Version anzeigen diff --git a/features/INDEX.md b/features/INDEX.md index 7ad4c36..ecd51aa 100644 --- a/features/INDEX.md +++ b/features/INDEX.md @@ -81,7 +81,7 @@ | PROJ-63 | Defensive Tenant-Scope-Härtung der Tenant-Verwaltungs-Endpunkte (FUND-2) | Deployed | [PROJ-63](PROJ-63-harden-tenant-admin-scope.md) | 2026-06-25 | | PROJ-64 | Session-Invalidation bei Passwort-Change + Datei-Permissions-Härtung (Security-Audit) | Deployed | [PROJ-64](PROJ-64-session-invalidation-file-permissions.md) | 2026-07-03 | | PROJ-65 | Physische Tenant-Trennung im Storage-Layer | Deployed | [PROJ-65](PROJ-65-physische-tenant-trennung.md) | 2026-07-04 | -| PROJ-66 | Backup-Strategie für Store, Keyfile, PostgreSQL (Produktiv + Teilproduktiv) | In Review | [PROJ-66](PROJ-66-backup-strategie.md) | 2026-07-04 | +| PROJ-66 | Backup-Strategie für Store, Keyfile, PostgreSQL (Produktiv + Teilproduktiv) | In Progress | [PROJ-66](PROJ-66-backup-strategie.md) | 2026-07-04 | diff --git a/features/PROJ-66-backup-strategie.md b/features/PROJ-66-backup-strategie.md index 1cb289f..c4becc4 100644 --- a/features/PROJ-66-backup-strategie.md +++ b/features/PROJ-66-backup-strategie.md @@ -1,6 +1,6 @@ # PROJ-66: Backup-Strategie für archivmail (Produktiv + Teilproduktiv) -**Status:** In Review +**Status:** In Review (BUG-1/BUG-2 aus QA-Runde 1 gefixt, Re-Test steht aus) **Erstellt:** 2026-07-04 ## Problem / Ausgangslage @@ -522,3 +522,101 @@ Speicherplatz-Mehrverbrauch). blockierend bei nicht-leerem Store. - `-dest`-Ziel für einen produktiven Cron-Eintrag muss noch vom Nutzer festgelegt werden, bevor die auskommentierte Cron-Zeile aktiviert wird. + +## QA Test Results (2026-07-04, Testserver 132) + +**Getestet gegen Commit f3a7dea (lokal, nicht gepusht/deployt).** Build und +Funktionstests auf 132 in isoliertem Testbereich (synthetischer Store mit +PROJ-65-Hardlinks unter `/tmp/qa66-*`, eigene Test-DB `archivmail_qa66_test`). +Produktion (60.595 Store-Dateien, DB `archivmail`) nachweislich unberührt — +kein Zugriff auf echte Store-Dateien oder die echte DB, nach dem Test verifiziert. + +**Gesamtergebnis: QA NICHT BESTANDEN** — 1 Critical (build-breaking) + 1 High +(-force funktionslos). Nach Fix beider Bugs erneut testen. + +| # | Testpunkt | Ergebnis | +|---|-----------|----------| +| 1 | `go build ./cmd/archivmail/` | **FAIL** (BUG-1, build-breaking) | +| 2 | Backup-Lauf gegen Test-Config | PASS (nur mit QA-Workaround-Patch) | +| 3 | postgres.dump/store/keyfile/config.yml vorhanden + Store-Hardlinks (Inode-Gleichheit, link count 2) | PASS | +| 4 | Restore-Roundtrip auf isoliertes Ziel: Store + Hardlinks + pg_restore (DB-Zeilen korrekt), Keyfile 0600 | PASS | +| 5 | `-force`-Schutz: ohne `-force` blockiert (5a) / mit `-force` läuft durch (5b) | 5a PASS / **5b FAIL** (BUG-2) | +| 6 | Rotation `-keep 2` über 3 Läufe → nur 2 Verzeichnisse | PASS | +| 7 | Ungültiges/nicht beschreibbares `-dest` → sauberer Abbruch, keine Rotation guter Backups | PASS | +| 8 | `backup` ohne `-dest` → Fehlerabbruch | PASS | + +Zusätzlich verifiziert: pg_dump-Fehlerfall (falsche DB) lässt partiellen +Backup-Ordner stehen und rotiert **keine** guten alten Backups weg (Lehre aus +PROJ-58) — PASS. + +### BUG-1 (Severity: CRITICAL, Priorität: sofort) — Binary kompiliert nicht + +`cmd/archivmail/cmd_import.go:310` bricht den Build. Die Hilfetext-Zeile für +`restore` wurde im Commit f3a7dea in den mit Backticks begrenzten +Raw-String-Literal von `printHelp()` (`fmt.Printf` mit Backtick-String, geschlossen +mit `` `, AppVersion) ``) eingefügt, enthält aber selbst Backticks um +`` `archivmail backup` ``. Diese beenden das Raw-String-Literal vorzeitig: + +``` +cmd/archivmail/cmd_import.go:310:30: syntax error: unexpected name archivmail in argument list; possibly missing comma or ) +``` + +Repro: `CGO_ENABLED=0 go build -buildvcs=false -o /tmp/x ./cmd/archivmail/` +→ scheitert. **Das gesamte Backend (nicht nur backup/restore) ist nicht +baubar/deploybar, solange dieser Fehler besteht.** Für die restlichen QA-Punkte +wurde in einem Wegwerf-Build-Verzeichnis die Zeile behelfsweise auf einfache +Anführungszeichen geändert (kein Fix am Repo/Commit — Bug besteht unverändert). +Fix-Vorschlag (Backend Developer): Backticks in Zeile 310 durch einfache +Anführungszeichen ersetzen oder die Zeile ohne Inline-Code-Markup formulieren. + +### BUG-2 (Severity: HIGH, Priorität: hoch) — `restore -force` schlägt bei nicht-leerem Store mit Hardlinks fehl + +`-force` soll laut Spec/AC einen bereits befüllten Store überschreiben. Tatsächlich +bricht der Restore ab, sobald der Store Hardlinks enthält (PROJ-65-Tenant-Struktur — +der Normalfall): + +``` +restore: store restore failed: hardlink /…/ab/abcdef123.bin -> /…/tenant_1/abcdef123.bin: + link …: file exists +``` + +Ursache: `copyTreePreservingHardlinks()` (`cmd_backup.go`) ruft `os.Link()` für die +zweite Inode-Referenz auf, ohne dass das Zielverzeichnis vorher geleert wird. Bei +`-force` überschreibt `copyFile()` reguläre Dateien zwar via `O_TRUNC`, aber `os.Link()` +scheitert an einer bereits existierenden Zieldatei (`EEXIST`). Damit ist `-force` +für genau den Anwendungsfall funktionslos, für den es gedacht ist (Restore über +einen vorhandenen, mit Tenant-Hardlinks befüllten Store). Repro: einmal restoren, +dann erneut mit `-force` auf dasselbe Ziel → Fehler. +Fix-Vorschlag (Backend Developer): bei `-force` das Zielverzeichnis vor dem +Restore leeren, oder in `copyTreePreservingHardlinks()` vor `os.Link()`/`copyFile()` +ein vorhandenes Ziel entfernen (`os.Remove(target)`, `ErrNotExist` ignorieren). +Hinweis: `-force` ohne Hardlinks (nur reguläre Dateien) läuft dank `O_TRUNC` durch — +der Fehler tritt nur bei der zweiten+ Inode-Referenz auf. + +### Nicht getestet / Hinweise +- Nur die CLI-Kommandos wurden getestet, nicht die offenen ACs (Cron/Timer AC 1, + physische Trennung AC 3, Verschlüsselung AC 4, Restore-Verifikation via echtem + `reconcile` AC 6, Alerting AC 7, Runbook AC 8, Bitwarden AC 10, PBS AC 11) — + diese sind laut Implementation Notes bewusst noch offen. +- `pg_restore --clean --if-exists` wurde nur gegen eine leere Test-DB geprüft; + Idempotenz über eine bereits befüllte DB nicht separat getestet. +- Testartefakte (`/tmp/qa66-*`, Test-DB `archivmail_qa66_test`, Wegwerf-Build + `/root/archivmail-qa66`) nach Testende entfernt und Entfernung verifiziert. + +## Fixes nach QA-Runde 1 (2026-07-04) + +- **BUG-1 (Critical, build-breaking):** `cmd/archivmail/cmd_import.go:310` + enthielt Backticks um `` `archivmail backup` `` innerhalb des Raw-String- + Literals von `printHelp()`, was den String vorzeitig schloss + (`syntax error: unexpected name archivmail in argument list`). Fix: + Backticks durch einfache Anführungszeichen ersetzt. +- **BUG-2 (High):** `-force` bei `restore` scheiterte an bereits vorhandenen + Hardlink-Zieldateien (`os.Link: file exists`) — für den Normalfall (Restore + gegen einen PROJ-65-Tenant-Hardlink-Bestand) war `-force` damit + funktionslos. Fix in `copyTreePreservingHardlinks()`: vor `os.Link` wird + eine bereits vorhandene Zieldatei entfernt (`os.Remove`, `os.IsNotExist` + wird ignoriert — im Normalfall ist das Ziel leer und der Remove ist ein + No-Op). + +Re-Test von Punkt 1 (Build) und Punkt 5b (`-force` gegen befüllten Store) +steht aus.