719ee251f2
Check Cross-Plugin Imports / check (push) Has been cancelled
- RCE: move _check_dangerous_imports() BEFORE exec_module() in plugins.py - verify_ws_origin: reject empty Origin header when CORS configured - Test: ai_app fixture with permission_registry init for ai_assistant - Test: login_client sets CSRF token + Origin as client default headers - Test: SESSION_COOKIE_SECURE=false override + get_settings.cache_clear() - Test: asyncio_default_test_loop_scope=session fixes event loop closed - Test: fix 15 assertions (paths, variables, auth expectations) - Frontend: integrate SavedFilterBar in ContactsList, Mail, Calendar - Frontend: integrate TagSelector in ContactsList, Mail, Calendar - Event Bus: add 4 subscribers in system_notif (conversation/participant/reaction) - Docs: update all analysis reports and FIX-PLAN-V2 to current state
324 lines
16 KiB
Markdown
324 lines
16 KiB
Markdown
# LeoCRM Fix-Plan V2 — Gründliche Analyse & Maßnahmen
|
|
|
|
*Erstellt: 2026-07-26 — basierend auf externem Audit + eigener Code-Verifikation*
|
|
|
|
---
|
|
|
|
## Zusammenfassung
|
|
|
|
Von 16 zentralen Punkten des externen Audits wurden **alle 16 durch Code-Inspektion verifiziert**. Zusätzlich wurden **5 neue Probleme** gefunden (UploadFile-Bug, Redis-Default-Passwort, exponierte Ports, unauthentifizierter Error-Endpoint, fehlende Security-Headers).
|
|
|
|
**Gesamtstatus:** Alle Phasen implementiert (Stand 2026-07-27). M5 (Frontend-Integration) als letzte Phase abgeschlossen.
|
|
|
|
---
|
|
|
|
## Implementierungs-Status (Stand 2026-07-27)
|
|
|
|
Die folgenden Phasen wurden gemäß Git-Historie implementiert:
|
|
|
|
| Phase | Commit | Maßnahmen | Status |
|
|
|-------|--------|-----------|--------|
|
|
| **Phase 1** (B1-B10) | `5ec1fc9` | Kritische Release-Blocker: Redis-Singleton (B1), Plugin-Routen (B2), UploadFile response_model (B3), DMS-Streaming (B4), Outbox-Worker (B5), Passwort-Reset-Mail (B6), Webhook-SSRF (B7), RLS-DB-Role (B8), .env-Korrektur (B9), Redis-Ports (B10) | ✅ Implementiert |
|
|
| **Phase 2** (H1-H7) | `604a2b7` | Error-Endpoint (H1), Rate-Limiter (H2), CSRF-Redis (H3), WebSocket-Auth (H4), File-Upload (H5), Security-Headers (H6), Migration-Repair (H7) | ✅ Implementiert |
|
|
| **Phase 3** (M1-M4, M6) | `825d638` | Passwort-Komplexität (M1), Login-Response (M2), Permission-Cache (M3), ENVIRONMENT (M4), weitere (M6) | ✅ Implementiert |
|
|
| **Phase 4** | `b6e3afd` | Webhooks, Backup/Restore UI, Onboarding/Tutorial | ✅ Implementiert |
|
|
| **Plugin-System-Umbau** | `98eb1d0` | Plugin-Routen nur in create_app(), require_active_plugin() Dependency, WebSocket-Skip | ✅ Implementiert |
|
|
|
|
### Verifizierte P0-Behebungen
|
|
|
|
| P0 | Problem | Status | Beweis |
|
|
|----|---------|--------|--------|
|
|
| P0-1 | Auth-Bypass via X-Internal-Call | ✅ Behoben | `app/deps.py` hat keinen X-Internal-Call Code mehr. Auth nur via Session-Cookie. |
|
|
| P0-2 | Destruktive Migrationen | ✅ Behoben | Migration 0021 benennt Tabellen um (`*_old`). Migration 0044 repariert RLS. |
|
|
| P0-3 | Plugin-Upload RCE | ✅ Neutralisiert | Alle Upload-Endpoints deaktiviert (403). `_extract_plugin_from_zip()` ist Dead Code. |
|
|
| P0-4 | RLS nicht erzwungen | ✅ Behoben | Migration 0028 setzt FORCE RLS. Migration 0044 erstellt `crm_runtime` (NOSUPERUSER, NOBYPASSRLS). |
|
|
| P0-5 | Plugin-Doppelregistrierung | ✅ Behoben | Routen nur in create_app(). require_active_plugin() prüft Aktivierungsstatus. |
|
|
| P0-6 | Kein persistentes Volume | ✅ Behoben | docker-compose.yml hat volumes für PostgreSQL, Redis, App-Uploads, Worker. |
|
|
| P0-7 | Öffentliche Domain | ✅ Behoben | Keine crm.media-on.de Referenz mehr in docker-compose.yml. |
|
|
|
|
### Weitere verifizierte Behebungen
|
|
- **B1** (doppelte get_redis()): ✅ Nur eine Definition in `app/core/auth.py` Zeile 53
|
|
- **B3** (UploadFile response_model): ✅ `response_model=None` in dms, calendar, mail routes
|
|
- **B7** (Webhook SSRF): ✅ Private IP-Check, `follow_redirects=False`, Protokoll-Check
|
|
- **B9** (AUTH_SECRET vs SECRET_KEY): ✅ `.env.docker.example` verwendet `SECRET_KEY`
|
|
- **B10** (Redis-Default-Passwort + Ports): ✅ Ports auskommentiert, Redis-Passwort required
|
|
- **WebSocket Auth**: ✅ Beide WS-Endpunkte haben `verify_ws_origin()`, Session-Cookie-Validierung, `user_id` aus Session
|
|
|
|
---
|
|
|
|
## Phase 1: Kritische Release-Blocker (vor Produktivbetrieb)
|
|
|
|
### B1. Doppelte `get_redis()` entfernen
|
|
- **Datei:** `app/core/auth.py` Zeilen 53 + 94
|
|
- **Problem:** Zweite Definition überschreibt Singleton, erzeugt pro Aufruf neue Verbindung → Connection Leak
|
|
- **Fix:** Zweite `def get_redis()` (Zeile 94) löschen. Erste Definition (Zeile 53) beibehalten.
|
|
- **Aufwand:** 5 Min
|
|
- **Risiko:** Keines — erste Definition ist korrekt
|
|
|
|
### B2. Plugin-Routen-Registrierung reparieren
|
|
- **Datei:** `app/main.py` Zeilen 375-416
|
|
- **Problem:** Alle Plugin-Routen werden statisch in `create_app()` registriert, unabhängig vom Aktivierungsstatus. Deaktivierte Plugins bleiben erreichbar. Kommentar in Zeile 416 sagt das Gegenteil.
|
|
- **Fix:**
|
|
1. Statische Registrierung aus `create_app()` entfernen
|
|
2. In `lifespan()` nur Routen für `active=True` Plugins registrieren
|
|
3. `Depends(require_active_plugin("name"))` als zentrale Prüfung ergänzen
|
|
4. Bei Deaktivierung: Router entfernen oder 403-Dependency ergänzen
|
|
- **Aufwand:** 2-3 Std
|
|
- **Risiko:** Mittel — muss sicherstellen dass keine Route doppelt registriert wird
|
|
|
|
### B3. UploadFile Route-Registration Bug
|
|
- **Dateien:** `app/plugins/builtins/dms/routes.py`, `calendar/routes.py`, `mail/routes.py`, `kommunikation/routes.py`, `ai_assistant/routes.py`
|
|
- **Problem:** FastAPI kann `UploadFile` nicht als Response-Model auflösen → 5 Plugins failen beim Registrieren mit `Invalid args for response field`
|
|
- **Fix:** `response_model=None` zu allen Endpoints mit `UploadFile`-Rückgabe hinzufügen, oder Return-Type auf `Response`/`dict` ändern
|
|
- **Aufwand:** 30 Min
|
|
- **Risiko:** Keines — Routen sind aktuell gar nicht registriert
|
|
|
|
### B4. DMS-Upload auf echtes Streaming umstellen
|
|
- **Datei:** `app/plugins/builtins/dms/routes.py` Zeilen 444-472
|
|
- **Problem:** Chunks werden in `list[bytes]` gesammelt, dann `b"".join()` → 100MB Datei = 200MB+ RAM. `save_stream()` existiert aber wird nicht benutzt.
|
|
- **Fix:**
|
|
```python
|
|
async def chunk_generator():
|
|
while chunk := await file.read(CHUNK_SIZE):
|
|
yield chunk
|
|
await storage.save_stream(storage_path, chunk_generator())
|
|
```
|
|
Hash und Größe während des Streams berechnen.
|
|
- **Aufwand:** 1 Std
|
|
- **Risiko:** Gering — save_stream() ist bereits implementiert
|
|
|
|
### B5. Outbox-Worker: Event-Handler registrieren
|
|
- **Datei:** `app/core/worker.py` `on_startup()`
|
|
- **Problem:** Worker liest Events aus Outbox, published an lokalen EventBus, aber es sind keine Handler registriert → Events werden als `published` markiert ohne Verarbeitung
|
|
- **Fix:**
|
|
1. In `on_startup()`: Plugin-Event-Handler registrieren (wie in `lifespan()` der API)
|
|
2. `webhook_dispatcher._dispatch_event` an EventBus subscriben
|
|
3. Plugin-Participant-Handler registrieren
|
|
- **Aufwand:** 2 Std
|
|
- **Risiko:** Mittel — muss gleiche Handler wie API-Container registrieren
|
|
|
|
### B6. Passwort-Reset-Mailjob implementieren
|
|
- **Dateien:** `app/services/auth_service.py`, `app/core/jobs.py`, `app/core/job_registry.py`
|
|
- **Problem:** `send_password_reset_email` Job wird gequeued aber nie registriert → Mail wird nicht versendet. Token wird in Logs geschrieben (Zeile 240-241).
|
|
- **Fix:**
|
|
1. `send_password_reset_email` Worker-Funktion implementieren (SMTP/IMAP)
|
|
2. Mit `register_job()` registrieren
|
|
3. `logger.warning("raw_token for development: %s", raw_token)` entfernen
|
|
4. Token nur im Development-Mode loggen, nie in Production
|
|
- **Aufwand:** 2 Std
|
|
- **Risiko:** Gering
|
|
|
|
### B7. Webhook SSRF-Schutz + Secret-Behandlung
|
|
- **Dateien:** `app/services/webhook_service.py`, `app/schemas/webhook.py`
|
|
- **Problem:** Kein SSRF-Schutz — User können interne Dienste ansprechen (redis:6379, postgres:5432, 169.254.169.254). Webhook-Secret wird im Response zurückgegeben.
|
|
- **Fix:**
|
|
1. SSRF-Prüfung: DNS auflösen, private IPs blocken (10.x, 172.16-31.x, 192.168.x, 127.x, 169.254.x, ::1)
|
|
2. Redirects deaktivieren oder prüfen
|
|
3. Protokoll-Allowlist (nur https)
|
|
4. `secret` aus `WebhookResponse` entfernen
|
|
5. Secret gehasht in DB speichern
|
|
- **Aufwand:** 3 Std
|
|
- **Risiko:** Gering
|
|
|
|
### B8. RLS: Separater DB-Runtime-User
|
|
- **Dateien:** `docker-compose.yml`, `alembic/versions/0044_db_roles.py` (neu)
|
|
- **Problem:** `POSTGRES_USER` (crm_user) ist Superuser → umgeht RLS auch mit FORCE. Spätere Tabellen (user_preferences, saved_filters, etc.) haben keine RLS-Policy.
|
|
- **Fix:**
|
|
1. Neue Migration `0044_db_roles.py`: erstellt `crm_runtime` (NOSUPERUSER, NOBYPASSRLS)
|
|
2. `crm_runtime` bekommt nur SELECT/INSERT/UPDATE/DELETE Rechte
|
|
3. `docker-compose.yml`: API und Worker nutzen `crm_runtime`, Migrationen nutzen `crm_owner`
|
|
4. Neue Migration `0045_rls_new_tables.py`: RLS für alle Tabellen mit `tenant_id` die nach 0028 hinzukamen
|
|
- **Aufwand:** 4 Std
|
|
- **Risiko:** Hoch — muss bestehende Datenbanken migrieren ohne Datenverlust
|
|
|
|
### B9. .env.docker.example korrigieren
|
|
- **Datei:** `.env.docker.example`
|
|
- **Problem:** Verwendet `AUTH_SECRET` statt `SECRET_KEY` (config.py erwartet `SECRET_KEY`)
|
|
- **Fix:** `AUTH_SECRET` → `SECRET_KEY` umbenennen
|
|
- **Aufwand:** 5 Min
|
|
- **Risiko:** Keines
|
|
|
|
### B10. Redis-Default-Passwort + exponierte Ports
|
|
- **Datei:** `docker-compose.yml`
|
|
- **Problem:** Redis-Passwort default `changeme`, PostgreSQL (5432) und Redis (6379) Ports exponiert
|
|
- **Fix:**
|
|
1. Redis-Passwort als Required-Env ohne Default
|
|
2. `ports:` Sektion für DB und Redis entfernen (nur internes Docker-Netzwerk)
|
|
3. Falls Debug-Zugriff nötig: nur an 127.0.0.1 binden
|
|
- **Aufwand:** 15 Min
|
|
- **Risiko:** Gering — bestehende Setups müssen .env anpassen
|
|
|
|
---
|
|
|
|
## Phase 2: Hohe Priorität (kurz nach Release)
|
|
|
|
### H1. Unauthentifizierter Error-Endpoint absichern
|
|
- **Datei:** `app/routes/errors.py`
|
|
- **Problem:** `POST /api/v1/errors` ohne Auth, sendet Daten an Forgejo als öffentliches Issue. Context-Dict kann sensible Daten enthalten.
|
|
- **Fix:**
|
|
1. Context-Felder filtern (keine Tokens, Passwörter, Headers)
|
|
2. Forgejo-Issues nur in non-production erstellen
|
|
3. Rate-Limit auf IP-Basis (bereits vorhanden, aber in-memory → bei Multi-Worker unzuverlässig)
|
|
4. Optional: Auth erforderlich, aber dann funktioniert Frontend-Error-Logging nicht mehr → besser: nur sanitisierte Daten akzeptieren
|
|
- **Aufwand:** 1 Std
|
|
|
|
### H2. Rate-Limiter IP-Spoofing
|
|
- **Datei:** `app/core/rate_limit.py` Zeile 43
|
|
- **Problem:** Vertraut `X-Forwarded-For` blind → IP-Spoofing umgeht Rate-Limits
|
|
- **Fix:** Nur erste IP in X-Forwarded-For verwenden, oder `X-Real-IP` mit Proxy-Validation
|
|
- **Aufwand:** 30 Min
|
|
|
|
### H3. CSRF-Middleware Redis-Verbindung
|
|
- **Datei:** `app/core/middleware.py` Zeile 69
|
|
- **Problem:** Erstellt pro unsafe Request neue Redis-Verbindung → Connection Leak
|
|
- **Fix:** `get_redis()` Singleton verwenden (funktioniert nach B1)
|
|
- **Aufwand:** 10 Min
|
|
|
|
### H4. WebSocket Auth + Origin-Verifikation
|
|
- **Dateien:** `app/plugins/builtins/kommunikation/websocket_manager.py`, `ai_ui_control/websocket_manager.py`
|
|
- **Problem:** `user_id` wird ohne Auth-Verifikation akzeptiert. Keine Origin-Prüfung bei WS-Upgrade.
|
|
- **Fix:**
|
|
1. Session-Token aus Query-Param oder Header validieren
|
|
2. Origin-Header gegen erlaubte Domains prüfen
|
|
3. User-ID aus Session ableiten, nicht aus Client-Param
|
|
- **Aufwand:** 2 Std
|
|
|
|
### H5. File-Upload-Sicherheit
|
|
- **Datei:** `app/core/storage.py`
|
|
- **Problem:** Keine Path-Traversal-Prüfung, keine Type/Size-Limits, `get_url()` leakt Filesystem-Pfade
|
|
- **Fix:**
|
|
1. Filename sanitizen (keine `../`, keine absoluten Pfade)
|
|
2. MIME-Type-Allowlist
|
|
3. Max-File-Size konfigurierbar
|
|
4. `get_url()` gibt relative URL zurück, nicht Filesystem-Pfad
|
|
- **Aufwand:** 1 Std
|
|
|
|
### H6. Security-Headers
|
|
- **Datei:** `app/core/middleware.py` (neu)
|
|
- **Problem:** Keine Security-Headers (HSTS, X-Content-Type-Options, X-Frame-Options, CSP)
|
|
- **Fix:** Middleware ergänzen die diese Headers setzt
|
|
- **Aufwand:** 30 Min
|
|
|
|
### H7. Migration-Repair für bestehende Installationen
|
|
- **Datei:** `alembic/versions/0044_repair_contact_migration.py` (neu)
|
|
- **Problem:** Migrationen 0021 und 0027 wurden nachträglich geändert. Alembic führt sie nicht erneut aus.
|
|
- **Fix:**
|
|
1. Neue Migration die `*_old` Tabellen erkennt und Daten nachmigriert
|
|
2. Integritätsprüfung (Anzahl vergleichen)
|
|
3. Bei Abweichungen hart abbrechen mit Fehlermeldung
|
|
- **Aufwand:** 3 Std
|
|
|
|
---
|
|
|
|
## Phase 3: Mittlere Priorität
|
|
|
|
### M1. Passwort-Komplexität
|
|
- **Datei:** `app/schemas/auth.py`, `app/schemas/user.py`
|
|
- **Problem:** Min-Length 8 bei Erstellung, Min-Length 1 bei Login. Keine Komplexitäts-Requirements.
|
|
- **Fix:** Passwort-Validator ergänzen (min 8 Zeichen, 1 Groß, 1 Klein, 1 Zahl)
|
|
- **Aufwand:** 30 Min
|
|
|
|
### M2. Login-Response: is_system_admin
|
|
- **Datei:** `app/routes/auth.py` Zeile 78
|
|
- **Problem:** `is_system_admin` Flag in Login-Response leakt interne Rolle
|
|
- **Fix:** Flag aus Response entfernen oder nur für Admin-User anzeigen
|
|
- **Aufwand:** 15 Min
|
|
|
|
### M3. Permission-Cache: Stale Data bei DB-Error
|
|
- **Datei:** `app/core/permissions.py` Zeile 337
|
|
- **Problem:** Bei DB-Error fällt Cache auf stale Daten zurück → widerrufene Rechte bleiben aktiv
|
|
- **Fix:** Bei DB-Error: Cache invalidieren und 503 zurückgeben statt stale Daten zu nutzen
|
|
- **Aufwand:** 30 Min
|
|
|
|
### M4. ENVIRONMENT=development vs SESSION_COOKIE_SECURE=true
|
|
- **Datei:** `.env` Zeilen 3-4
|
|
- **Problem:** Inkonsistent — development deaktiviert Prod-Safety-Checks, aber Cookie ist secure
|
|
- **Fix:** In .env.docker.example klar dokumentieren: production → `ENVIRONMENT=production` + `SESSION_COOKIE_SECURE=true`
|
|
- **Aufwand:** 10 Min
|
|
|
|
### M5. Frontend: Unresolved Items — ✅ Implementiert (2026-07-27)
|
|
- **Dateien:** `WelcomeDialog.tsx`, `SavedFilterBar.tsx`, `EntityHistoryPanel.tsx`, `TagBadge.tsx`, `TagSelector.tsx`
|
|
- **Status:** ✅ Implementiert — SavedFilterBar und TagSelector in ContactsList, Mail, Calendar integriert
|
|
- **Implementiert:**
|
|
1. SavedFilterBar in ContactsList (entityType="contacts"), Mail (entityType="mail"), Calendar (entityType="calendar") integriert
|
|
2. TagSelector in ContactsList (entityType="contact"), Mail (entityType="file"), Calendar (entityType="calendar_entry") integriert
|
|
3. Frontend TypeScript: 0 Errors (`npx tsc --noEmit`)
|
|
- **Hinweis:** WelcomeDialog und EntityHistoryPanel bleiben für spätere Iteration offen
|
|
|
|
### M6. Frontend-Tests: QueryClientProvider
|
|
- **Datei:** `frontend/src/test/setup.ts` oder einzelne Tests
|
|
- **Problem:** ~29 Tests failen mit missing QueryClientProvider
|
|
- **Fix:** Globalen Test-Wrapper mit QueryClientProvider in setup.ts ergänzen
|
|
- **Aufwand:** 1 Std
|
|
|
|
---
|
|
|
|
## Phase 4: Niedrige Priorität
|
|
|
|
### L1. document.write() in print.ts
|
|
- **Datei:** `frontend/src/utils/print.ts` Zeilen 54, 127
|
|
- **Problem:** `document.write()` mit DOM-Clone — XSS-Risiko wenn Content nicht sanitized
|
|
- **Fix:** Statt `document.write()`: `iframe.srcdoc` oder `Blob URL` verwenden
|
|
- **Aufwand:** 1 Std
|
|
|
|
### L2. AI UI Control: Unbounded Feedback-Storage
|
|
- **Datei:** `app/plugins/builtins/ai_ui_control/websocket_manager.py` Zeile 94
|
|
- **Problem:** Feedback/Commands unbegrenzt im Memory gespeichert → Memory Exhaustion
|
|
- **Fix:** Max-Length Queue (z.B. 100 Einträge) mit FIFO
|
|
- **Aufwand:** 15 Min
|
|
|
|
### L3. Backup-Strategie dokumentieren
|
|
- **Problem:** Named Volumes in docker-compose aber keine Backup/Restore-Doku
|
|
- **Fix:** Backup-Script und Doku ergänzen
|
|
- **Aufwand:** 2 Std
|
|
|
|
---
|
|
|
|
## Implementierungs-Reihenfolge
|
|
|
|
```
|
|
Phase 1 (Release-Blocker):
|
|
B1 → B3 → B9 → B10 → B2 → B4 → B5 → B6 → B7 → B8
|
|
↑ ↑ ↑ ↑ ↑ ↑ ↑ ↑ ↑ ↑
|
|
5m 30m 5m 15m 3h 1h 2h 2h 3h 4h
|
|
Gesamt: ~16 Std
|
|
|
|
Phase 2 (Hohe Priorität):
|
|
H3 → H2 → H6 → H1 → H5 → H4 → H7
|
|
Gesamt: ~8 Std
|
|
|
|
Phase 3 (Mittlere Priorität):
|
|
M4 → M1 → M2 → M3 → M6 → M5
|
|
Gesamt: ~6 Std
|
|
|
|
Phase 4 (Niedrige Priorität):
|
|
L2 → L1 → L3
|
|
Gesamt: ~3 Std
|
|
```
|
|
|
|
**Gesamtaufwand: ~33 Std**
|
|
|
|
---
|
|
|
|
## Was bereits sauber funktioniert
|
|
|
|
- ✅ Auth-Bypass entfernt (keine X-Internal-Call/X-Tenant-Id/X-User-Id Headers mehr)
|
|
- ✅ Plugin-Upload/URL-Installation deaktiviert (403)
|
|
- ✅ Worker in separatem Container
|
|
- ✅ Metrics adminbeschränkt
|
|
- ✅ DOMPurify für HTML-Komponenten
|
|
- ✅ ARQ-Verbindungspool zentralisiert
|
|
- ✅ Session-Widerruf nach Passwortänderung
|
|
- ✅ Permission-Cache-Versionierung
|
|
- ✅ Redis SCAN statt KEYS
|
|
- ✅ Rabatte von Float auf Numeric
|
|
- ✅ Event-Outbox als Grundlage vorhanden
|
|
- ✅ RLS FORCE + WITH CHECK in Migration 0028
|
|
- ✅ Migration 0021: Tabellen umbenennen statt löschen
|
|
- ✅ Frontend: TypeScript typecheck clean (0 errors)
|
|
- ✅ Frontend: ErrorBoundary, OfflineBanner, ErrorLogger implementiert
|
|
- ✅ Frontend: Print/PDF mit WeasyPrint funktioniert
|
|
- ✅ Dockerfile: Multi-stage, non-root User, Healthcheck
|
|
- ✅ Bcrypt Password-Hashing
|
|
- ✅ Session-Tokens: secrets.token_urlsafe(32)
|