fix(PROJ-74): Test-/Vet-Signatur-Drift beheben, go build/vet/test wieder grün

archivmail-export nutzte noch die alte storage.New(path string)-Signatur
statt storage.Config (fehlender Keyfile/Compress hätte Rohbytes statt
Klartext-EML exportiert). Toten Self-Assignment-Code in storage.go entfernt.
Testdateien (storage, audit, api, userstore, auth) an aktuelle Signaturen
angeglichen; auth-Tests liefen bisher gegen einen SQLite-Pfad statt Postgres-
DSN und wurden auf das TEST_DATABASE_URL-Schema-Isolationsmuster der übrigen
Pakete umgestellt. api_test.go las den Login-Token noch aus dem JSON-Body
statt aus dem httpOnly-Cookie (Auth-Contract-Drift).

TestParseMissingDate an tatsächliches Verhalten angepasst: der Parser lässt
das Datum bewusst als Zero-Value, der time.Now()-Fallback sitzt in der
Storage-Schicht — damit bleibt nachvollziehbar ob ein Datum aus der Mail
stammt oder vom Archiv gesetzt wurde (GoBD).

Verifiziert auf 192.168.1.132: go build/vet/test ./... komplett grün,
kein Skip (Postgres + Manticore erreichbar).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019j28kGcaJAhBnrYX34hGdt
This commit is contained in:
sysops
2026-08-05 14:07:46 +02:00
co-authored by Claude Sonnet 5
parent 88cdc3eb3e
commit 075afa005a
11 changed files with 247 additions and 34 deletions
+7 -1
View File
@@ -36,11 +36,17 @@ func main() {
os.Exit(1)
}
mailStore, err := storage.New(cfg.Storage.StorePath)
mailStore, err := storage.New(storage.Config{
Dir: cfg.Storage.StorePath,
Keyfile: cfg.Storage.Keyfile,
DSN: cfg.Database.DSN(),
CompressEnabled: cfg.Storage.Compress,
})
if err != nil {
logger.Error("storage init failed", "err", err)
os.Exit(1)
}
defer mailStore.Close()
indexBackend := cfg.Index.Backend
if indexBackend == "" {
+1 -1
View File
@@ -89,7 +89,7 @@
| PROJ-71 | TLS-Pflicht (optional) für eingehenden SMTP-BCC-Journaling-Kanal | Deployed | [PROJ-71](PROJ-71-smtp-require-tls.md) | 2026-07-08 |
| PROJ-72 | Fix Superadmin kann Passwort/Rolle von Superadmin-Peers nicht ändern (Sicherheitsbug) | Deployed | [PROJ-72](PROJ-72-fix-superadmin-peer-patch.md) | 2026-07-27 |
| PROJ-73 | Restliche Crash-Härtung (Upload-Job-Status bei Panic, fehlende nil-Checks) | In Review | [PROJ-73](PROJ-73-restliche-crash-haertung.md) | 2026-08-05 |
| PROJ-74 | Vorbestehende Test-/Vet-Signatur-Drift beheben (go vet/test wieder komplett grün) | Planned | [PROJ-74](PROJ-74-test-suite-signatur-drift.md) | 2026-08-05 |
| PROJ-74 | Vorbestehende Test-/Vet-Signatur-Drift beheben (go vet/test wieder komplett grün) | In Review | [PROJ-74](PROJ-74-test-suite-signatur-drift.md) | 2026-08-05 |
| PROJ-75 | DB-Performance-Audit (Query-Index-Nutzung, Fan-out, pgxpool-Tuning) | Planned | [PROJ-75](PROJ-75-db-performance-audit.md) | 2026-08-05 |
| PROJ-76 | Mail-HTML-Sanitizing schließt CSS-url()/link/srcset nicht ein (Tracking-Pixel-Umgehung) | Planned | [PROJ-76](PROJ-76-mail-html-sanitizing-luecken.md) | 2026-08-05 |
| PROJ-77 | Admin-Tab-Bundle-Optimierung (dynamic import statt 19 statische Imports) | Planned | [PROJ-77](PROJ-77-admin-tabs-dynamic-import.md) | 2026-08-05 |
@@ -68,6 +68,8 @@ Alle Pfade, die eine Zugriffsprüfung machen (Search/Export/OCR), fallen bei
Lokal kein `go build` möglich (kein Go-Toolchain), nur statische Prüfung.
Deployed auf 132 am 2026-08-05.
## Acceptance Criteria
- [x] Upload-Job zeigt nach einem simulierten Panic im Verarbeitungspfad
@@ -0,0 +1,154 @@
---
id: PROJ-74
title: Vorbestehende Test-/Vet-Signatur-Drift beheben (go vet/test wieder komplett grün)
status: In Review
created: 2026-08-05
---
## Problem
Bei der Verifikation von PROJ-73-Vorarbeiten auf 192.168.1.132
(`go build ./... && go vet ./... && go test ./...`) traten mehrere
vorbestehende, vom aktuellen Crash-Härtung-Fix unabhängige Fehler auf:
- `cmd/archivmail-export/main.go:39``storage.New` wird mit einem
`string` statt `storage.Config` aufgerufen. `storage.New`-Signatur wurde
in einem früheren Commit geändert, dieses CLI-Tool nicht nachgezogen.
Build schlägt komplett fehl.
- `internal/storage/storage.go:508` — Self-Assignment, von `go vet`
gemeldet.
- `internal/storage/storage_test.go`, `internal/audit/audit_test.go`,
`internal/api/api_test.go`, `internal/userstore/userstore_test.go`
Testdateien passen nicht mehr zu aktuellen Signaturen (`storage.New`,
`os.Discard`, `UpsertLDAPUser`). Build der Testpakete schlägt fehl.
- `pkg/mailparser/parser_test.go:98` (`TestParseMissingDate`) — Testfall
erwartet Fallback auf `time.Now()` bei fehlendem Date-Header, aktuelles
Verhalten weicht ab. Nicht Teil der Multipart-Depth/strings.Builder-
Änderungen aus PROJ-73-Vorarbeiten.
- `internal/auth`-Tests — `userstore.New: cannot parse .../users.db`,
vermutlich Umgebungs-/Test-DSN-Problem statt Code-Bug.
Solange diese Drift besteht, ist `go vet ./...` / `go test ./...` insgesamt
rot und liefert kein verlässliches Gesamtbild mehr — jede künftige
Verifikation muss den Diff manuell gegen die bekannten Altlasten abgleichen.
## Lösung (Vorschlag)
1. `archivmail-export`: `storage.New`-Aufruf auf aktuelle `storage.Config`-
Signatur anpassen (Config-Feld statt reinem String-Pfad).
2. `storage.go:508` Self-Assignment beheben.
3. Testdateien (`storage_test.go`, `audit_test.go`, `api_test.go`,
`userstore_test.go`) auf aktuelle Signaturen (`storage.New`,
`os.Discard`, `UpsertLDAPUser`) nachziehen.
4. `TestParseMissingDate` klären: ist der Test oder das aktuelle
Date-Fallback-Verhalten falsch? Danach eins von beiden anpassen.
5. `internal/auth`-Test-DSN-Problem auf 132 diagnostizieren (evtl. reines
Testumgebungs-Setup-Problem, kein Code-Bug).
## Implementation Notes
Umgesetzt (lokal, ohne Build/Test — kein Go-Toolchain auf der Workstation;
Verifikation erfolgt separat auf 192.168.1.132).
### 1. `cmd/archivmail-export/main.go:39`
`storage.New(cfg.Storage.StorePath)``storage.New(storage.Config{Dir, Keyfile,
DSN, CompressEnabled})`, analog zu `cmd/archivmail/cmd_export.go:88-93`.
Zusätzlich `defer mailStore.Close()` ergänzt (der Store hält jetzt einen
pgxpool, der bisherige Aufruf kannte noch keine DB-Verbindung).
Ohne `Keyfile`/`Compress` hätte das Tool verschlüsselte bzw. gzip-komprimierte
Mails als Rohbytes exportiert — der Fix stellt das alte Verhalten
("exportiere Klartext-EML") wieder her.
### 2. `internal/storage/storage.go:~506` Self-Assignment
`pm.MessageID = pm.MessageID` war echter toter Code (Kommentar im Original:
"no-op; thread resolved inside insertMeta"). Der komplette `if pm.InReplyTo != ""
|| len(pm.References) > 0`-Block hatte keinerlei Effekt und wurde entfernt; der
erklärende Kommentar bleibt als Doku über `resolveThreadID` stehen. Kein
Verhaltensunterschied.
### 3. Testdateien an aktuelle Signaturen angeglichen
- `internal/storage/storage_test.go`: `storage.New(dir)`
`storage.New(storage.Config{Dir: dir})` (5x); `store.Save(raw, time.Now())`
`store.Save(context.Background(), raw, time.Now(), nil)` (Signatur ist jetzt
`Save(ctx, raw, _ time.Time, tenantID *int64)`); Import `context` ergänzt.
- `internal/audit/audit_test.go`: `os.Discard``io.Discard` (4x), Import `io`
ergänzt (`os` bleibt für `os.Getenv`/`os.Open` nötig). `audit.New`,
`Log`, `Query` waren unverändert.
- `internal/api/api_test.go`: `os.Discard``io.Discard` + Import `io`;
`storage.New(filepath.Join(dir,"store"))`
`storage.New(storage.Config{Dir: ...})`. `api.New`, `auth.New`,
`userstore.Create` gegen den Produktionscode geprüft — unverändert.
- `internal/userstore/userstore_test.go`: `UpsertLDAPUser(username, email, role)`
`UpsertLDAPUser(username, email, role, nil)` (4. Parameter `tenantID *int64`,
`nil` = tenant-loser LDAP-User, entspricht dem alten Verhalten).
### 4. `TestParseMissingDate` — Entscheidung: Test angepasst, Code bleibt
Aktuelles Verhalten in `pkg/mailparser/parser.go:188-191`: Wenn der Date-Header
fehlt oder durch keinen der Fallback-Layouts parsbar ist, bleibt `pm.Date` der
Zero-Value (expliziter Kommentar: "Leave pm.Date as zero — storage will use DB
DEFAULT NOW()"). Der `time.Now()`-Fallback ist bewusst in die Storage-Schicht
gewandert: `internal/storage/storage.go:998-1000` (`insertMeta`) und
`:1034-1036` (`SaveMeta`) setzen `received_at = time.Now()`, wenn `pm.Date`
zero ist.
Das ist die sinnvollere Aufteilung und wird deshalb **nicht** zurückgedreht:
Der Parser bildet die Mail ab, wie sie ist, und erfindet kein Datum — dadurch
bleibt für jeden Konsumenten unterscheidbar, ob ein Datum aus der Mail stammt
oder vom Archiv gesetzt wurde (GoBD-Nachvollziehbarkeit). Ein `time.Now()`
direkt im Parser würde diese Information unwiederbringlich verschlucken.
Der Test prüft nun `p.Date.IsZero() == true` und dokumentiert die Aufteilung
inkl. Verweis auf die Storage-Fallbacks.
### 5. `internal/auth`-Tests — Ursache: veralteter SQLite-DSN
Kein Produktionscode-Bug. `internal/auth/auth_test.go:13` rief
`userstore.New(filepath.Join(t.TempDir(), "users.db"))` auf — ein Relikt aus der
SQLite-Zeit. `userstore.New` erwartet seit der PostgreSQL-Migration einen
pgx-DSN (`userstore.go:71`, `pgxpool.New`), weshalb pgx den Dateipfad mit
"cannot parse .../users.db" ablehnte.
Fix: `newTestAuth` nutzt jetzt dasselbe Muster wie `userstore_test.go` /
`api_test.go``TEST_DATABASE_URL` + pro Test isoliertes Schema
(`authtest_<testname>`, `search_path=`-Suffix am DSN), `t.Skip` wenn
`TEST_DATABASE_URL` nicht gesetzt ist, Cleanup per `DROP SCHEMA ... CASCADE`.
`TestHasRole` ist reine Logik und läuft weiterhin ohne PostgreSQL.
### Geänderte Dateien
- `cmd/archivmail-export/main.go`
- `internal/storage/storage.go`
- `internal/storage/storage_test.go`
- `internal/audit/audit_test.go`
- `internal/api/api_test.go`
- `internal/userstore/userstore_test.go`
- `internal/auth/auth_test.go`
- `pkg/mailparser/parser_test.go`
### Verifikation auf 132 (2026-08-05)
Mit `TEST_DATABASE_URL` gegen die lokale Postgres-Instanz auf 132 (eigenes
Schema pro Testlauf, per `DROP SCHEMA CASCADE` aufgeräumt) und erreichbarem
Manticore: `go build ./...`, `go vet ./...` und `go test ./...` liefen
komplett grün, alle Pakete PASS, keine Skips.
Zwei zusätzliche Fixes waren nötig, über die ursprüngliche Analyse hinaus:
- `internal/api/api_test.go` — ungenutzter Import `"net/http"` entfernt
(von `go vet` gemeldet).
- `internal/api/api_test.go` Helper `login()` las den JWT bisher aus dem
JSON-Response-Body (`resp["token"]`), was zu `nil`-Panic führte. Die
echte API liefert den Token nur noch als httpOnly-Cookie
(`archivmail_session`, siehe `internal/api/auth_handlers.go:81-99`) —
Auth-Contract-Drift, unabhängig von PROJ-73/74. Test liest den Token
jetzt aus `w.Result().Cookies()`.
## Acceptance Criteria
- [x] `CGO_ENABLED=0 go build ./...` auf 132 fehlerfrei (alle Pakete
inkl. `archivmail-export`).
- [x] `go vet ./...` ohne Meldungen.
- [x] `go test ./...` komplett grün (oder verbleibende Fails explizit als
bekannt/erwartet dokumentiert, nicht stillschweigend).
+10 -6
View File
@@ -4,8 +4,8 @@ import (
"bytes"
"context"
"encoding/json"
"io"
"log/slog"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
@@ -33,9 +33,9 @@ type testEnv struct {
func newTestEnv(t *testing.T) *testEnv {
t.Helper()
dir := t.TempDir()
logger := slog.New(slog.NewTextHandler(os.Discard, nil))
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
store, err := storage.New(filepath.Join(dir, "store"))
store, err := storage.New(storage.Config{Dir: filepath.Join(dir, "store")})
if err != nil {
t.Fatal(err)
}
@@ -133,9 +133,13 @@ func (e *testEnv) login(t *testing.T, username, password string) string {
if w.Code != 200 {
t.Fatalf("login %s: status %d, body: %s", username, w.Code, w.Body.String())
}
var resp map[string]interface{}
json.Unmarshal(w.Body.Bytes(), &resp)
return resp["token"].(string)
for _, c := range w.Result().Cookies() {
if c.Name == "archivmail_session" {
return c.Value
}
}
t.Fatalf("login %s: no session cookie in response, body: %s", username, w.Body.String())
return ""
}
// ---- Tests ----
+5 -4
View File
@@ -4,6 +4,7 @@ import (
"bufio"
"context"
"encoding/json"
"io"
"log/slog"
"os"
"path/filepath"
@@ -75,7 +76,7 @@ func newTestAudit(t *testing.T) *audit.Logger {
}
schemaDSN := dsn + sep + "search_path=" + schema
logger := slog.New(slog.NewTextHandler(os.Discard, nil))
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
l, err := audit.New(schemaDSN, t.TempDir(), logger)
if err != nil {
t.Fatalf("audit.New: %v", err)
@@ -218,7 +219,7 @@ func TestQueryPagination(t *testing.T) {
// DELETE on audit_log through the normal application connection.
func TestImmutableTrigger(t *testing.T) {
dsn, schemaDSN := testDSN(t)
logger := slog.New(slog.NewTextHandler(os.Discard, nil))
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
l, err := audit.New(schemaDSN, t.TempDir(), logger)
if err != nil {
t.Fatalf("audit.New: %v", err)
@@ -255,7 +256,7 @@ func TestImmutableTrigger(t *testing.T) {
func TestFileLogging(t *testing.T) {
_, schemaDSN := testDSN(t)
logPath := filepath.Join(t.TempDir(), "audit.log")
logger := slog.New(slog.NewTextHandler(os.Discard, nil))
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
l, err := audit.New(schemaDSN, logPath, logger)
if err != nil {
t.Fatalf("audit.New: %v", err)
@@ -297,7 +298,7 @@ func TestFileLoggingUnwritableContinues(t *testing.T) {
_, schemaDSN := testDSN(t)
// A directory cannot be opened for writing → file logging disabled.
logPath := t.TempDir()
logger := slog.New(slog.NewTextHandler(os.Discard, nil))
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
l, err := audit.New(schemaDSN, logPath, logger)
if err != nil {
t.Fatalf("audit.New should not fail on unwritable path: %v", err)
+44 -3
View File
@@ -1,20 +1,61 @@
package auth_test
import (
"path/filepath"
"context"
"os"
"strings"
"testing"
"github.com/jackc/pgx/v5"
"archivmail/internal/auth"
"archivmail/internal/userstore"
)
// newTestAuth builds an auth.Manager on top of a schema-isolated PostgreSQL
// userstore. userstore.New expects a PostgreSQL DSN (pgxpool) — the former
// SQLite file path ("users.db") made pgx fail with "cannot parse ...".
func newTestAuth(t *testing.T) (*auth.Manager, *userstore.Store) {
t.Helper()
store, err := userstore.New(filepath.Join(t.TempDir(), "users.db"))
dsn := os.Getenv("TEST_DATABASE_URL")
if dsn == "" {
t.Skip("TEST_DATABASE_URL not set — skipping (needs PostgreSQL)")
}
schema := "authtest_" + strings.ToLower(strings.ReplaceAll(t.Name(), "/", "_"))
if len(schema) > 63 {
schema = schema[:63]
}
ctx := context.Background()
conn, err := pgx.Connect(ctx, dsn)
if err != nil {
t.Fatalf("connect: %v", err)
}
if _, err := conn.Exec(ctx, "CREATE SCHEMA IF NOT EXISTS "+schema); err != nil {
conn.Close(ctx)
t.Fatalf("create schema: %v", err)
}
conn.Close(ctx)
sep := "?"
if strings.Contains(dsn, "?") {
sep = "&"
}
schemaDSN := dsn + sep + "search_path=" + schema
store, err := userstore.New(schemaDSN)
if err != nil {
t.Fatalf("userstore.New: %v", err)
}
t.Cleanup(func() { store.Close() })
t.Cleanup(func() {
store.Close()
conn2, _ := pgx.Connect(context.Background(), dsn)
if conn2 != nil {
conn2.Exec(context.Background(), "DROP SCHEMA "+schema+" CASCADE")
conn2.Close(context.Background())
}
})
// Seed a test user
store.Create(userstore.CreateUserRequest{
+3 -4
View File
@@ -503,10 +503,9 @@ func (s *Store) Save(ctx context.Context, raw []byte, _ time.Time, tenantID *int
}
if parseErr == nil {
// PROJ-38: resolve thread before inserting
if pm.InReplyTo != "" || len(pm.References) > 0 {
pm.MessageID = pm.MessageID // no-op; thread resolved inside insertMeta
}
// PROJ-38: resolve thread before inserting.
// In-Reply-To/References werden innerhalb von resolveThreadID
// ausgewertet, hier ist keine Vorverarbeitung nötig.
threadID := s.resolveThreadID(ctx, pm)
if err := s.insertMeta(ctx, id, pm, len(raw), tenantID, storageID, threadID); err != nil {
// Race: another goroutine inserted via Message-ID UNIQUE conflict.
+11 -10
View File
@@ -2,6 +2,7 @@ package storage_test
import (
"bytes"
"context"
"os"
"path/filepath"
"testing"
@@ -12,13 +13,13 @@ import (
func TestSaveAndLoad(t *testing.T) {
dir := t.TempDir()
store, err := storage.New(dir)
store, err := storage.New(storage.Config{Dir: dir})
if err != nil {
t.Fatalf("New: %v", err)
}
raw := []byte("From: alice@example.com\r\nSubject: Test\r\n\r\nHello World")
id, err := store.Save(raw, time.Now())
id, err := store.Save(context.Background(), raw, time.Now(), nil)
if err != nil {
t.Fatalf("Save: %v", err)
}
@@ -37,17 +38,17 @@ func TestSaveAndLoad(t *testing.T) {
func TestDeduplication(t *testing.T) {
dir := t.TempDir()
store, err := storage.New(dir)
store, err := storage.New(storage.Config{Dir: dir})
if err != nil {
t.Fatal(err)
}
raw := []byte("From: alice@example.com\r\n\r\nDuplicate test")
id1, err := store.Save(raw, time.Now())
id1, err := store.Save(context.Background(), raw, time.Now(), nil)
if err != nil {
t.Fatal(err)
}
id2, err := store.Save(raw, time.Now())
id2, err := store.Save(context.Background(), raw, time.Now(), nil)
if err != nil {
t.Fatal(err)
}
@@ -68,13 +69,13 @@ func TestDeduplication(t *testing.T) {
func TestDelete(t *testing.T) {
dir := t.TempDir()
store, err := storage.New(dir)
store, err := storage.New(storage.Config{Dir: dir})
if err != nil {
t.Fatal(err)
}
raw := []byte("From: alice@example.com\r\n\r\nDelete me")
id, _ := store.Save(raw, time.Now())
id, _ := store.Save(context.Background(), raw, time.Now(), nil)
if err := store.Delete(id); err != nil {
t.Fatalf("Delete: %v", err)
@@ -86,7 +87,7 @@ func TestDelete(t *testing.T) {
func TestStats(t *testing.T) {
dir := t.TempDir()
store, err := storage.New(dir)
store, err := storage.New(storage.Config{Dir: dir})
if err != nil {
t.Fatal(err)
}
@@ -97,7 +98,7 @@ func TestStats(t *testing.T) {
[]byte("From: c@x.com\r\n\r\nMail 3"),
}
for _, m := range mails {
store.Save(m, time.Now())
store.Save(context.Background(), m, time.Now(), nil)
}
stats, err := store.Stats()
@@ -114,7 +115,7 @@ func TestStats(t *testing.T) {
func TestStorageDirectoryCreation(t *testing.T) {
dir := filepath.Join(t.TempDir(), "nested", "path")
_, err := storage.New(dir)
_, err := storage.New(storage.Config{Dir: dir})
if err != nil {
t.Fatalf("New with nested path: %v", err)
}
+2 -2
View File
@@ -323,7 +323,7 @@ func TestCleanExpiredTokens(t *testing.T) {
func TestUpsertLDAPUser(t *testing.T) {
s := newTestStore(t)
u, err := s.UpsertLDAPUser("ldapuser", "ldap@corp.com", userstore.RoleAuditor)
u, err := s.UpsertLDAPUser("ldapuser", "ldap@corp.com", userstore.RoleAuditor, nil)
if err != nil {
t.Fatalf("UpsertLDAPUser: %v", err)
}
@@ -332,7 +332,7 @@ func TestUpsertLDAPUser(t *testing.T) {
}
// Second upsert should update, not duplicate
u2, err := s.UpsertLDAPUser("ldapuser", "ldap@corp.com", userstore.RoleAuditor)
u2, err := s.UpsertLDAPUser("ldapuser", "ldap@corp.com", userstore.RoleAuditor, nil)
if err != nil {
t.Fatalf("UpsertLDAPUser second: %v", err)
}
+8 -3
View File
@@ -93,8 +93,13 @@ func TestParseMissingDate(t *testing.T) {
if err != nil {
t.Fatalf("Parse: %v", err)
}
// Should fall back to time.Now(), so should not be zero
if p.Date.IsZero() {
t.Error("Date should fall back to now, not zero")
// Der Parser erfindet bewusst kein Datum: fehlt/unparsbar ist der
// Date-Header, bleibt pm.Date der Zero-Value (parser.go:188-191).
// Den Fallback auf "jetzt" setzt erst die Storage-Schicht beim
// Schreiben von received_at (storage.go:998-1000, 1034-1036).
// So bleibt im Parser unterscheidbar, ob ein Datum aus der Mail
// stammt oder vom Archiv gesetzt wurde (GoBD-Nachvollziehbarkeit).
if !p.Date.IsZero() {
t.Errorf("Date should stay zero when Date header is missing, got %v", p.Date)
}
}