fix(PROJ-66): Build-Fehler in printHelp() + -force überschreibt keine Hardlinks

BUG-1 (QA): Backticks in der restore-Hilfezeile schlossen das Raw-String-
Literal von printHelp() vorzeitig, Build brach komplett. Backticks durch
einfache Anführungszeichen ersetzt.

BUG-2 (QA): -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.
copyTreePreservingHardlinks() entfernt jetzt eine vorhandene Zieldatei vor
os.Link.
This commit is contained in:
sysops
2026-07-04 15:15:54 +02:00
parent 2b70895a37
commit 55131de81d
4 changed files with 109 additions and 3 deletions
+1 -1
View File
@@ -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 |
<!-- Add features above this line -->
+99 -1
View File
@@ -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.