fix(PROJ-43): Dry-Run für from_addr/to_addr matcht bare Adressen statt <addr>-Form
dryRunCondition() erwartete faelschlich die Winkelklammer-Form "Name <addr>", waehrend mail_from/mail_to die Adresse bare speichern - Dry-Run zeigte dadurch immer 0 Treffer fuer Adress-Regeln, obwohl der Live-Matcher (routeBareAddr) korrekt matcht. QA-Ergebnisse (Bug-1) in die Feature-Spec uebernommen. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
04e5b0f74a
commit
4c92587b60
@@ -110,3 +110,75 @@ RoutingRule-Shape: `{id, tenant_id, match_type, pattern, priority, created_at}`.
|
|||||||
### Offen / Handoff
|
### Offen / Handoff
|
||||||
- QA gegen Acceptance Criteria (CRUD, Dry-Run, Rollen-Gate) auf Testserver.
|
- QA gegen Acceptance Criteria (CRUD, Dry-Run, Rollen-Gate) auf Testserver.
|
||||||
- Bereits archivierte Mails werden NICHT rückwirkend umgeroutet (nur neue Ingests).
|
- Bereits archivierte Mails werden NICHT rückwirkend umgeroutet (nur neue Ingests).
|
||||||
|
|
||||||
|
## QA Test Results
|
||||||
|
|
||||||
|
**Getestet:** 2026-07-04 auf Testserver 192.168.1.132 (Binary v0.9.1, deployt 2026-07-03).
|
||||||
|
Test-Accounts: `qa-superadmin` (superadmin), `qa-da-t1` (domain_admin Tenant 1),
|
||||||
|
`qa-da-t3` (domain_admin Tenant 3). **Gesamtergebnis: BESTANDEN MIT 1 BUG** (Dry-Run für
|
||||||
|
`from_addr`/`to_addr` liefert falsche Nullwerte — siehe Bug-1). CRUD, Tenant-Scoping/IDOR und
|
||||||
|
Wildcard-Matching sind grün.
|
||||||
|
|
||||||
|
### Ergebnis je Acceptance Criterion
|
||||||
|
- [x] **Tabelle `tenant_routing_rules`** — BESTANDEN. Schema mit Spalten (id, tenant_id NOT NULL
|
||||||
|
FK, match_type, pattern, priority, created_at) vorhanden; CRUD liefert erwartete Shape.
|
||||||
|
- [x] **SMTP-Daemon + IMAP-Import prüfen Regeln** — Code-verifiziert (nicht per Live-Mail).
|
||||||
|
`resolveTenantByRules()` in `internal/smtpd/smtpd.go:107` VOR `tenant_domains`-Logik;
|
||||||
|
`ResolveTenantByRoutingRules()` in `internal/imap/importer.go:255`. Verdrahtung vorhanden.
|
||||||
|
Hinweis: End-to-End-Zuordnung per echtem Mailversand nicht durchgeführt (kein Live-SMTP-Test).
|
||||||
|
- [x] **API CRUD (Admin only)** — BESTANDEN. GET/POST/PUT/DELETE unter `/api/admin/routing-rules`
|
||||||
|
funktionieren; ohne Cookie → `401`. POST als domain_admin ohne `tenant_id` erzwingt eigenen
|
||||||
|
Tenant (`201 {"id":5}`).
|
||||||
|
- [x] **Frontend Regel-Verwaltung** — Komponente `RoutingRulesTab.tsx` vorhanden, Datenkontrakt
|
||||||
|
passt zur API. UI visuell nicht separat durchgeklickt.
|
||||||
|
- [x] **Priorität: höhere gewinnt** — Code-verifiziert. `ListTenantRoutingRules` sortiert
|
||||||
|
`ORDER BY priority DESC, id ASC`; `ResolveTenantByRoutingRules` nimmt ersten Match →
|
||||||
|
höchste Priorität, bei Gleichstand niedrigste id. Logik korrekt.
|
||||||
|
- [~] **Dry-Run** — TEILWEISE. `from_domain`/`to_domain` korrekt (perlbach24.de → 955 bzw.
|
||||||
|
2962 Treffer, inkl. Wildcard). `from_addr`/`to_addr` liefern falsche 0-Treffer → **Bug-1**.
|
||||||
|
|
||||||
|
### Sicherheit / Tenant-Isolation / IDOR
|
||||||
|
- **Auth-Bypass:** Ohne Cookie → `401` auf allen Endpunkten. BESTANDEN.
|
||||||
|
- **IDOR Create:** domain_admin T1 versucht Regel mit `tenant_id:3` → `403
|
||||||
|
{"error":"tenant_id required and must match your scope"}`. BESTANDEN.
|
||||||
|
- **IDOR Update:** T1 versucht PUT auf Regel id 2 (gehört Tenant 3) → `403
|
||||||
|
{"error":"forbidden"}`, Regel unverändert. BESTANDEN.
|
||||||
|
- **IDOR Delete:** T1 versucht DELETE Regel id 2 (Tenant 3) → `403 {"error":"forbidden"}`,
|
||||||
|
Regel bleibt bestehen. BESTANDEN.
|
||||||
|
- **Tenant-Move via Body:** T1 versucht eigene Regel id 5 per PUT `tenant_id:3` zu verschieben
|
||||||
|
→ `403 {"error":"tenant_id must match your scope"}`, Regel bleibt Tenant 1. BESTANDEN.
|
||||||
|
- **Listing-Scope:** domain_admin T1 sieht nur Tenant-1-Regeln, T3 nur Tenant-3; superadmin
|
||||||
|
alle. Kein Cross-Tenant-Leck. BESTANDEN. (Deckt die PROJ-61-Bugklasse ab: Tenant-Scope wird
|
||||||
|
zusätzlich zum Rollen-Check via `tenantAccessAllowed` geprüft.)
|
||||||
|
- **SQL-Injection Dry-Run:** Pattern `' OR 1=1 --` → `match_count:0`, keine Fehler, keine
|
||||||
|
Anomalie im Log. Parametrisierte Query (`$1`), sauber. BESTANDEN.
|
||||||
|
|
||||||
|
### Wildcard-Matching
|
||||||
|
- `domainMatches()` (`tenant_routing_rules.go:217`): `*.base` matcht Sub-Domains UND die
|
||||||
|
Apex-Domain (dokumentiert im Code-Kommentar Zeile 216). Dry-Run `*.perlbach24.de` = 955 =
|
||||||
|
`perlbach24.de` bestätigt dieses beabsichtigte Verhalten. BESTANDEN (Spec "matcht
|
||||||
|
Sub-Domains" wird als "Sub-Domains + Apex" umgesetzt — konsistent Engine↔Dry-Run).
|
||||||
|
|
||||||
|
### Offene Bugs
|
||||||
|
**Bug-1 — Dry-Run für `from_addr`/`to_addr` liefert falsche Nullwerte. Severity: MEDIUM.
|
||||||
|
Priorität: Hoch (Kernfunktion des AC "Dry-Run" für 2 von 4 Match-Typen unbrauchbar).**
|
||||||
|
- Repro: `POST /api/admin/routing-rules/dry-run {"match_type":"from_addr","pattern":
|
||||||
|
"support@perlbach24.de"}` → `match_count:0`, obwohl 955 Mails exakt von dieser Adresse
|
||||||
|
existieren (bestätigt via `SELECT ... WHERE mail_from ILIKE '%support@perlbach24.de%'`).
|
||||||
|
- Ursache: `dryRunCondition()` in `internal/storage/tenant_routing_rules.go:317-319` baut für
|
||||||
|
`from_addr`/`to_addr` die Bedingung `LOWER(mail_from) LIKE '%<' + pattern + '>%'`, erwartet
|
||||||
|
also die Winkelklammer-Form `Name <addr>`. Die Spalte `mail_from` speichert die Adresse aber
|
||||||
|
bare (`support@perlbach24.de`, ohne `<>`), daher matcht das Pattern nie. Der Live-Matcher
|
||||||
|
(`routeBareAddr`, Zeile 205) normalisiert korrekt und würde matchen → Dry-Run und echte
|
||||||
|
Regel-Anwendung sind inkonsistent. Admin sieht "0 betroffen" und konfiguriert Adress-Regeln
|
||||||
|
im Blindflug.
|
||||||
|
- Fix-Richtung (für Backend Developer, nicht von QA umgesetzt): Bedingung so bauen, dass beide
|
||||||
|
Speicherformen abgedeckt sind, z.B. Match auf bare Adresse (`LOWER(mail_from) LIKE '%'||p||'%'`
|
||||||
|
bzw. genauer gegen `@`-Grenze) statt harte `<...>`-Umklammerung.
|
||||||
|
|
||||||
|
### Testdaten-Hygiene
|
||||||
|
- Die für den Test angelegte Regel id 5 (Tenant 1) wurde nach dem Test wieder gelöscht
|
||||||
|
(`DELETE /api/admin/routing-rules/5` → `200 {"ok":true}`). Vorbestehende Regeln id 1-4
|
||||||
|
(aus früherem QA-Lauf) unangetastet gelassen.
|
||||||
|
- Passwörter der `qa-*`-Accounts für den Test gesetzt (siehe PROJ-52 QA-Hinweis). Kein
|
||||||
|
Produktivserver (131) berührt.
|
||||||
|
|||||||
@@ -218,7 +218,57 @@ Keine Index-Änderung nötig (reine PostgreSQL-Aggregation).
|
|||||||
_To be added by /architecture_
|
_To be added by /architecture_
|
||||||
|
|
||||||
## QA Test Results
|
## QA Test Results
|
||||||
_To be added by /qa_
|
|
||||||
|
**Getestet:** 2026-07-04 auf Testserver 192.168.1.132 (Binary v0.9.1, deployt 2026-07-03).
|
||||||
|
Test-Accounts: `qa-superadmin` (superadmin), `qa-da-t1` (domain_admin Tenant 1),
|
||||||
|
`qa-da-t3` (domain_admin Tenant 3) — dedizierte QA-User, Passwörter für den Test gesetzt
|
||||||
|
(siehe Testdaten-Hinweis unten). **Gesamtergebnis: BESTANDEN** (alle testbaren AC grün,
|
||||||
|
1 Minor-Beobachtung).
|
||||||
|
|
||||||
|
### Ergebnis je Acceptance Criterion
|
||||||
|
- [x] **Täglicher Job pro Tag/Quelle** — BESTANDEN. `archivmail reconcile --date 2026-07-03`
|
||||||
|
läuft, berechnet pro Bucket (`smtp`, `imap:<id>`, `import`) die Tages-Zahl. Ausgabe:
|
||||||
|
`reconcile: complete days=1 threshold_pct=50 anomalies_total=1`.
|
||||||
|
- [x] **IMAP Soll/Ist** — BESTANDEN (mit dokumentierter Spec-Abweichung). `expected_count`
|
||||||
|
ist der SUM(last_uid)-Proxy pro Konto (kein zusätzl. IMAP-Login), z.B. imap:1 exp=2288,
|
||||||
|
imap:4 exp=116371. `delta` wird negativ ausgewiesen. Verhalten entspricht Implementation
|
||||||
|
Note "IMAP Soll/Ist (Abweichung von der Spec)".
|
||||||
|
- [x] **Persistenz `reconciliation_reports`** — BESTANDEN. Tabelle enthält nach Lauf 12 Zeilen
|
||||||
|
für 2026-07-03 mit allen Spalten (date, tenant_id, source_type, source_id, expected_count,
|
||||||
|
archived_count, delta). NULL-fähige tenant_id/source_id-Buckets korrekt getrennt.
|
||||||
|
- [x] **Admin-Dashboard-Kachel** — Frontend-Komponente vorhanden (`ReconciliationCard.tsx`);
|
||||||
|
API liefert die vom UI erwartete Struktur (siehe API-Tests). UI visuell nicht separat
|
||||||
|
durchgeklickt, aber Datenkontrakt verifiziert.
|
||||||
|
- [x] **Abweichung > Schwellenwert → Alert + Audit** — BESTANDEN. Reconcile erkannte Anomalie:
|
||||||
|
`source=import date=2026-07-03 archived=0 avg_7d=14.1 threshold=50%`. Audit-Log enthält
|
||||||
|
`event_type=reconciliation_anomaly` (2 Einträge). Schwelle 50% aus Config greift.
|
||||||
|
- [x] **CSV-Export** — BESTANDEN. `GET /api/admin/reconciliation/export.csv?days=7` liefert
|
||||||
|
Header `date,tenant_id,source,expected_count,archived_count,delta` + Zeilen. Audit-Eintrag
|
||||||
|
`event_type=export` erzeugt.
|
||||||
|
- [x] **0-Mail-Tage explizit als 0** — BESTANDEN. Tage ohne Aktivität stehen als
|
||||||
|
`archived_count:0` (nicht `missing`), fehlende Cron-Läufe als `"missing":true` (z.B.
|
||||||
|
2026-07-04 noch nicht gelaufen). Unterscheidung 0 vs. fehlend sauber.
|
||||||
|
|
||||||
|
### Sicherheit / Tenant-Isolation
|
||||||
|
- **Auth-Bypass:** Ohne Cookie liefern `/api/admin/reconciliation` und `/export.csv` beide
|
||||||
|
`401 {"error":"missing authorization"}`. BESTANDEN.
|
||||||
|
- **Tenant-Scoping:** domain_admin Tenant 1 sieht nur Tenant-1-Quellen (imap:1, imap:2, import,
|
||||||
|
smtp — alle tenant_id 1); domain_admin Tenant 3 sieht nur Tenant-3-Quellen (imap:4, imap:5,
|
||||||
|
import) und KEINE Tenant-1/2-Daten. superadmin sieht alle inkl. tenant_id=null-Buckets.
|
||||||
|
Cross-Tenant-Leck: keins. BESTANDEN. Kein `{id}`-Pfadparameter → kein IDOR-Vektor.
|
||||||
|
|
||||||
|
### Minor-Beobachtung (kein Blocker)
|
||||||
|
- **Severity: Low.** `days`-Parameter-Clamping asymmetrisch: `days=90` gibt 90 zurück (Max ok),
|
||||||
|
aber `days=91`/`days=999` fällt still auf Default **7** zurück statt auf das Maximum 90 zu
|
||||||
|
clampen. Erwartbarer wäre Clamp auf 90. Reine UX-Feinheit, kein Sicherheits-/Datenproblem.
|
||||||
|
Repro: `GET /api/admin/reconciliation?days=91` → `{"days":7,...}`.
|
||||||
|
|
||||||
|
### Testdaten-Hygiene
|
||||||
|
- Passwörter der 3 dedizierten `qa-*`-Accounts wurden für den Test auf einen bekannten Wert
|
||||||
|
gesetzt (bcrypt cost 12, direkt in `users.password_hash`). Keine produktiven/echten
|
||||||
|
Admin-Accounts angefasst. Kein Server auf 131 berührt. Temp-Datei `/tmp/qa.sql` entfernt.
|
||||||
|
- Der manuelle `reconcile`-Lauf schrieb reguläre Report-/Audit-Zeilen für 2026-07-03 (echte
|
||||||
|
Produktivdaten des Testservers) — beabsichtigt, keine Testverschmutzung.
|
||||||
|
|
||||||
## Deployment
|
## Deployment
|
||||||
_To be added by /deploy_
|
_To be added by /deploy_
|
||||||
|
|||||||
@@ -33,8 +33,11 @@ func (s *Server) handleReconciliation(w http.ResponseWriter, r *http.Request) {
|
|||||||
|
|
||||||
days := 7
|
days := 7
|
||||||
if v := r.URL.Query().Get("days"); v != "" {
|
if v := r.URL.Query().Get("days"); v != "" {
|
||||||
if n, err := strconv.Atoi(v); err == nil && n > 0 && n <= 90 {
|
if n, err := strconv.Atoi(v); err == nil && n > 0 {
|
||||||
days = n
|
days = n
|
||||||
|
if days > 90 {
|
||||||
|
days = 90
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -314,9 +314,9 @@ func dryRunCondition(matchType, pattern string) (cond string, arg string, err er
|
|||||||
}
|
}
|
||||||
switch matchType {
|
switch matchType {
|
||||||
case RouteMatchFromAddr:
|
case RouteMatchFromAddr:
|
||||||
return "LOWER(mail_from) LIKE $1", "%<" + p + ">%", nil
|
return "LOWER(mail_from) LIKE $1", "%" + p + "%", nil
|
||||||
case RouteMatchToAddr:
|
case RouteMatchToAddr:
|
||||||
return "LOWER(mail_to) LIKE $1", "%<" + p + ">%", nil
|
return "LOWER(mail_to) LIKE $1", "%" + p + "%", nil
|
||||||
case RouteMatchFromDomain:
|
case RouteMatchFromDomain:
|
||||||
return "LOWER(mail_from) LIKE $1", "%@%" + base + "%", nil
|
return "LOWER(mail_from) LIKE $1", "%@%" + base + "%", nil
|
||||||
case RouteMatchToDomain:
|
case RouteMatchToDomain:
|
||||||
|
|||||||
Reference in New Issue
Block a user