From dcb88317ac0b6d40e8bf8b64297ae91e7e130a70 Mon Sep 17 00:00:00 2001 From: sysops Date: Tue, 30 Jun 2026 14:37:39 +0200 Subject: [PATCH] =?UTF-8?q?fix(PROJ-63):=20Defense-in-Depth=20Tenant-Scope?= =?UTF-8?q?-H=C3=A4rtung=20der=20Admin-Endpunkte?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tenantAccessAllowed()-Check in allen {id}-Handlern von tenant_handlers.go, tenant_domain_handlers.go und tenant_logo_handlers.go ergänzt — No-op für globale Admins, zweite Verteidigungslinie für hypothetische tenant-gebundene Admin-Sessions. Co-Authored-By: Claude Sonnet 4.6 --- .claude/agents/mailarchiv-architect.md | 31 +++++++++++++++++++++++++- .claude/agents/manticore-admin.md | 9 ++++++++ features/INDEX.md | 3 ++- internal/api/tenant_domain_handlers.go | 16 +++++++++++++ internal/api/tenant_handlers.go | 21 +++++++++++++++++ internal/api/tenant_logo_handlers.go | 10 +++++++++ 6 files changed, 88 insertions(+), 2 deletions(-) diff --git a/.claude/agents/mailarchiv-architect.md b/.claude/agents/mailarchiv-architect.md index 9cf2025..bddbd0f 100644 --- a/.claude/agents/mailarchiv-architect.md +++ b/.claude/agents/mailarchiv-architect.md @@ -20,7 +20,9 @@ Du entwickelst **archivmail** – ein selbst gehostetes, unternehmenstaugliches **Go-Modul: `archivmail`** — Imports sind immer `archivmail/internal/...`, NIEMALS `github.com/archivmail/...` -**Feature-Tracking:** Alle Features in `features/INDEX.md`. Feature-IDs: PROJ-X. Nächste verfügbare ID: PROJ-44. +**Feature-Tracking:** Alle Features in `features/INDEX.md`. Feature-IDs: PROJ-X. Die nächste +verfügbare ID steht live am Ende von `features/INDEX.md` — dort nachsehen, nicht aus dem +Gedächtnis annehmen (die Datei wird laufend fortgeschrieben). ## Deine Kernprinzipien @@ -152,6 +154,33 @@ const ( - **Audit:** Jeder Zugriff (Suche, Export, Lesen) wird geloggt – unveränderbar - **Integrität:** SHA-256 im Dateinamen + DB für spätere Verifikation +### Architekturprinzip: Symmetrische Tenant-Scope-Checks (Lehre aus PROJ-61) + +Jede neue per-ID-adressierbare Ressource (nicht nur Mails — auch Logos, Anhänge, Exporte, +Saved Searches, API-Keys, künftige Ressourcentypen) bekommt beim Entwurf **von Anfang an** +ein einheitliches Zugriffsmuster über alle CRUD-Pfade: + +- Rollen-Check (`requireRole`) UND Tenant-Scope-Check (`tenantAccessAllowed()`) sind zwei + unabhängige Dimensionen — beide müssen auf JEDEM Pfad (GET, POST, DELETE) geprüft werden. +- Bei "global admin sieht alles, domain_admin nur eigenen Tenant"-Ressourcen: Lese- und + Schreibpfad MÜSSEN denselben Scope-Check verwenden. PROJ-61 entstand, weil der Lesepfad + einer Ressource (Tenant-Logo) nur `s.auth` hatte, während Schreib-/Löschpfad korrekt + `requireRole` + Tenant-Scope kombinierten — das Auseinanderlaufen von Geschwister-Endpunkten + ist das eigentliche Risiko, nicht ein einzelner fehlender Check. +- Beim Architektur-Entwurf eines neuen Ressourcentyps: definiere den Scope-Check EINMAL als + gemeinsame Hilfsfunktion, die alle Handler (GET/POST/DELETE) aufrufen — nie pro Handler neu + ausformulieren. + +### Architekturprinzip: Neue Hintergrund-Jobs defaulten auf Batch, nicht Dauerbetrieb + +Seit PROJ-56/58 ist die etablierte Linie: rechenintensive oder schreiblastige Hintergrund- +Verarbeitung (Indexierung, OCR, künftige ähnliche Jobs) soll beim Entwurf eine +`batch_mode`-Option vorsehen (Cron-getrieben, Default ggf. weiter Dauerbetrieb für +Abwärtskompatibilität), statt implizit als permanente Goroutine ohne Abschaltmöglichkeit zu +laufen. Performance-Budget (<200 MB RAM) gilt besonders für neue Worker-Pools — vor dem +Hinzufügen eines weiteren Dauerbetrieb-Workers prüfen, ob ein bestehender Worker den Job +mitübernehmen kann. + ## Deine Arbeitsweise ### Bei Architektur-Anfragen: diff --git a/.claude/agents/manticore-admin.md b/.claude/agents/manticore-admin.md index 8a7a7d6..c90e4d4 100644 --- a/.claude/agents/manticore-admin.md +++ b/.claude/agents/manticore-admin.md @@ -89,6 +89,15 @@ ssh root@192.168.1.131 'manticore_backup --config /etc/manticoresearch/manticore ssh root@192.168.1.131 'archivmail reindex --config /etc/archivmail/config.yml' ``` +## GoBD-Hinweis + +Der Manticore-Index ist **abgeleitete Suchdarstellung**, nicht die rechtlich maßgebliche +Quelle (Source of Truth = verschlüsselte Roh-Mails in `/var/archivmail/store/` + PostgreSQL- +Metadaten). Einträge aus dem Index löschen/ändern ist erlaubt (Reindex jederzeit möglich), +aber NIEMALS als Ersatz für eine echte GoBD-konforme Mail-Löschung verwenden — eine Mail aus +dem Index zu entfernen macht sie nicht rechtlich gelöscht, sie bleibt unverändert im Store. +Echte Löschungen laufen ausschließlich über `archivmail purge` (Retention + Markierung). + ## Security - Port 9306 NUR auf localhost: `listen = 127.0.0.1:9306:mysql` diff --git a/features/INDEX.md b/features/INDEX.md index 19a3d72..050204f 100644 --- a/features/INDEX.md +++ b/features/INDEX.md @@ -77,7 +77,8 @@ | PROJ-58 | Indexierung + OCR als Cron-Batch-Jobs (statt Dauerbetrieb) | Deployed | [PROJ-58](PROJ-58-cron-batch-index-ocr.md) | 2026-06-24 | | PROJ-61 | Fix Cross-Tenant Stored XSS via Mandanten-Logo (Sicherheitsbug) | Deployed | [PROJ-61](PROJ-61-fix-tenant-logo-xss-idor.md) | 2026-06-25 | | PROJ-62 | Fix Cross-Tenant IDOR bei POP3-Konto-Löschung/-Import (Sicherheitsbug) | Deployed | [PROJ-62](PROJ-62-fix-pop3-tenant-idor.md) | 2026-06-25 | +| 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 | -## Next Available ID: PROJ-63 +## Next Available ID: PROJ-64 diff --git a/internal/api/tenant_domain_handlers.go b/internal/api/tenant_domain_handlers.go index 58f5c9f..7a154b6 100644 --- a/internal/api/tenant_domain_handlers.go +++ b/internal/api/tenant_domain_handlers.go @@ -18,6 +18,11 @@ func (s *Server) handleListTenantDomains(w http.ResponseWriter, r *http.Request) writeError(w, http.StatusBadRequest, "invalid tenant id") return } + // Defense-in-depth tenant scope check (PROJ-63): no-op for global admins. + if !tenantAccessAllowed(sessionFromCtx(r.Context()), &id) { + writeError(w, http.StatusForbidden, "access denied") + return + } domains, err := s.tenantStore.ListDomains(r.Context(), id) if err != nil { writeError(w, http.StatusInternalServerError, "failed to list domains") @@ -37,6 +42,12 @@ func (s *Server) handleAddTenantDomain(w http.ResponseWriter, r *http.Request) { return } + // Defense-in-depth tenant scope check (PROJ-63): no-op for global admins. + if !tenantAccessAllowed(sessionFromCtx(r.Context()), &id) { + writeError(w, http.StatusForbidden, "access denied") + return + } + var req struct { Domain string `json:"domain"` } @@ -63,6 +74,11 @@ func (s *Server) handleRemoveTenantDomain(w http.ResponseWriter, r *http.Request writeError(w, http.StatusBadRequest, "invalid tenant id") return } + // Defense-in-depth tenant scope check (PROJ-63): no-op for global admins. + if !tenantAccessAllowed(sessionFromCtx(r.Context()), &tenantID) { + writeError(w, http.StatusForbidden, "access denied") + return + } didStr := r.PathValue("did") domainID, err := strconv.ParseInt(didStr, 10, 64) if err != nil { diff --git a/internal/api/tenant_handlers.go b/internal/api/tenant_handlers.go index ca76852..1d5cd1a 100644 --- a/internal/api/tenant_handlers.go +++ b/internal/api/tenant_handlers.go @@ -117,6 +117,11 @@ func (s *Server) handleGetTenant(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "invalid tenant id") return } + // Defense-in-depth tenant scope check (PROJ-63): no-op for global admins. + if !tenantAccessAllowed(sessionFromCtx(r.Context()), &id) { + writeError(w, http.StatusForbidden, "access denied") + return + } tenant, err := s.tenantStore.Get(r.Context(), id) if err != nil { writeError(w, http.StatusNotFound, "tenant not found") @@ -136,6 +141,12 @@ func (s *Server) handleUpdateTenant(w http.ResponseWriter, r *http.Request) { return } + // Defense-in-depth tenant scope check (PROJ-63): no-op for global admins. + if !tenantAccessAllowed(sessionFromCtx(r.Context()), &id) { + writeError(w, http.StatusForbidden, "access denied") + return + } + var req struct { Name string `json:"name"` Active *bool `json:"active"` @@ -181,6 +192,11 @@ func (s *Server) handleDeleteTenant(w http.ResponseWriter, r *http.Request) { } sess := sessionFromCtx(r.Context()) + // Defense-in-depth tenant scope check (PROJ-63): no-op for global admins. + if !tenantAccessAllowed(sess, &id) { + writeError(w, http.StatusForbidden, "access denied") + return + } if err := s.tenantStore.Delete(r.Context(), id); err != nil { writeError(w, http.StatusInternalServerError, "failed to delete tenant") return @@ -204,6 +220,11 @@ func (s *Server) handleListTenantUsers(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "invalid tenant id") return } + // Defense-in-depth tenant scope check (PROJ-63): no-op for global admins. + if !tenantAccessAllowed(sessionFromCtx(r.Context()), &tenantID) { + writeError(w, http.StatusForbidden, "access denied") + return + } users, err := s.users.ListByTenant(r.Context(), tenantID) if err != nil { writeError(w, http.StatusInternalServerError, "failed to list tenant users") diff --git a/internal/api/tenant_logo_handlers.go b/internal/api/tenant_logo_handlers.go index e723919..05f04c1 100644 --- a/internal/api/tenant_logo_handlers.go +++ b/internal/api/tenant_logo_handlers.go @@ -57,6 +57,11 @@ func (s *Server) handleUploadTenantLogo(w http.ResponseWriter, r *http.Request) writeError(w, http.StatusBadRequest, "invalid tenant id") return } + // Defense-in-depth tenant scope check (PROJ-63): no-op for global admins. + if !tenantAccessAllowed(sessionFromCtx(r.Context()), &id) { + writeError(w, http.StatusForbidden, "access denied") + return + } s.saveTenantLogo(w, r, id) } @@ -70,6 +75,11 @@ func (s *Server) handleDeleteTenantLogo(w http.ResponseWriter, r *http.Request) writeError(w, http.StatusBadRequest, "invalid tenant id") return } + // Defense-in-depth tenant scope check (PROJ-63): no-op for global admins. + if !tenantAccessAllowed(sessionFromCtx(r.Context()), &id) { + writeError(w, http.StatusForbidden, "access denied") + return + } if err := s.tenantStore.DeleteLogo(r.Context(), id); err != nil { writeError(w, http.StatusInternalServerError, "failed to delete logo") return