236 lines
14 KiB
Markdown
236 lines
14 KiB
Markdown
|
|
# Audit Fix-Plan — LeoCRM Architektur-Audit
|
||
|
|
|
||
|
|
**Basis:** `docs/audit-consolidated-errors.md` (153 eindeutige Fehler)
|
||
|
|
**Datum:** 2026-08-15
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Prinzipien
|
||
|
|
|
||
|
|
1. **P0 zuerst** — Runtime Crashes müssen sofort behoben werden
|
||
|
|
2. **Kein neues Architekturmodell** — vorhandene Interfaces nutzen
|
||
|
|
3. **Deklarierte Plugin→Plugin-Abhängigkeiten sind erlaubt**
|
||
|
|
4. **Minimal focused changes** — bestehenden Style beibehalten
|
||
|
|
5. **Jeder Fix wird verifiziert** — Tests laufen, App startet
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Phase 1: P0 Runtime Crashes (8 Fixes, ~2h)
|
||
|
|
|
||
|
|
### 1.1 hooks.py:83 — unregister() _filters 2-tuple CRASH
|
||
|
|
**Datei:** `app/core/hooks.py:83`
|
||
|
|
**Änderung:** `register_filter` speichert 3-tuple `(priority, callback, owner_tag)`, aber `unregister()` unpackt nur 2-tuple. Fix: `unregister()` auf 3-tuple unpacking aktualisieren.
|
||
|
|
**Verifikation:** `pytest tests/test_hooks.py -v`
|
||
|
|
|
||
|
|
### 1.2 trigger_dispatcher.py:127 — AutomationDefinition nicht importiert
|
||
|
|
**Datei:** `app/core/trigger_dispatcher.py:127`
|
||
|
|
**Änderung:** `AutomationDefinition` wird in Query verwendet aber nie importiert. Fix: Import hinzufügen (via Contract oder lazy import).
|
||
|
|
**Verifikation:** `pytest tests/test_trigger_core.py -v`
|
||
|
|
|
||
|
|
### 1.3 contacts/plugin.py:88-90 — clear_actions() statt unregister_actions_by_owner()
|
||
|
|
**Datei:** `app/plugins/builtins/contacts/plugin.py:88-90`
|
||
|
|
**Änderung:** `clear_actions()` entfernt Hooks ALLER Plugins. Fix: `unregister_actions_by_owner("contacts")` verwenden.
|
||
|
|
**Verifikation:** `pytest tests/test_contacts_lifecycle.py -v`
|
||
|
|
|
||
|
|
### 1.4-1.8 Test P0s — check_permission mocked + hardcoded DB credential
|
||
|
|
**Dateien:** `tests/test_external_agent_api.py`, `tests/test_graph_rag.py`, `tests/test_agent_memory.py`, `tests/test_marketplace.py`, `tests/test_cross_tenant_standalone.py`
|
||
|
|
**Änderung:**
|
||
|
|
- 4 Dateien: `patch("check_permission", return_value=True)` entfernen, echte Permission-Setup verwenden
|
||
|
|
- 1 Datei: Hardcoded DB credential durch env var ersetzen
|
||
|
|
**Verifikation:** `pytest tests/test_external_agent_api.py tests/test_graph_rag.py tests/test_agent_memory.py tests/test_marketplace.py tests/test_cross_tenant_standalone.py -v`
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Phase 2: P1 Funktionale Fehler (27 Fixes, ~6h)
|
||
|
|
|
||
|
|
### 2.1 Backend P1s (10 Fixes)
|
||
|
|
|
||
|
|
| # | Datei | Problem | Fix |
|
||
|
|
|---|-------|---------|-----|
|
||
|
|
| 1 | `app/services/attachment_service.py:48` | DmsFile type hint not imported | Import via Contract |
|
||
|
|
| 2 | `app/core/restore_registry.py:113-195` | register_default_entities noch Plugin-Entities | Nur Contact registrieren |
|
||
|
|
| 3 | `app/core/history_hooks.py:140` | register_default_history_hooks noch Contact | Nur Contact registrieren |
|
||
|
|
| 4 | `app/plugins/builtins/mail/plugin.py` | on_deactivate fehlt unregister_actions_by_owner | Hinzufügen |
|
||
|
|
| 5 | `app/routes/saved_views.py:62` | pattern validation hardcoded | Gegen ENTITY_MODELS validieren |
|
||
|
|
| 6 | `app/routes/saved_filters.py:62` | pattern validation hardcoded | Gegen ENTITY_MODELS validieren |
|
||
|
|
| 7 | `app/plugins/builtins/mail/plugin.py:194-200` | on_deactivate fehlt restore + history unregister | Hinzufügen |
|
||
|
|
| 8 | `app/plugins/builtins/mcp_client/models.py:35-36` | datetime.utcnow (naive) | datetime.now(UTC) |
|
||
|
|
| 9 | `frontend/src/api/tags.ts:12` | EntityType hardcoded, inkonsistent | Dynamisch aus Backend holen |
|
||
|
|
| 10 | `frontend/src/components/common/ProtectedRoute.tsx:22-24` | Fail-open bei leeren permissions | Block access bis permissions geladen |
|
||
|
|
|
||
|
|
### 2.2 Test P1s (26 Fixes)
|
||
|
|
|
||
|
|
| # | Datei(en) | Problem | Fix |
|
||
|
|
|---|----------|---------|-----|
|
||
|
|
| 1-2 | `test_ai_copilot.py:45-47,77-79` | Tests silently pass bei 403 | Exakte status codes asserten |
|
||
|
|
| 3-4 | `test_user_service.py:38-39,82-100` | Non-deterministic, try/except pass | Exakte assertions, echte Fehler testen |
|
||
|
|
| 5-7 | `test_backup_service.py:37-38,50-51,63` | Overly permissive assertions | Exakte status codes |
|
||
|
|
| 8-9 | `test_mcp_server.py:62,127` | `assert success in (True, False)` | Exakte assertions |
|
||
|
|
| 10 | `test_saved_filters.py` | Missing cross-tenant isolation + RBAC | Tests hinzufügen |
|
||
|
|
| 11 | `test_tasks.py` | Missing cross-tenant isolation + RBAC | Tests hinzufügen |
|
||
|
|
| 12 | `test_dashboard.py` | Missing cross-tenant isolation + RBAC | Tests hinzufügen |
|
||
|
|
| 13 | `test_custom_fields.py` | Missing cross-tenant isolation + RBAC | Tests hinzufügen |
|
||
|
|
| 14 | `test_calendar.py` | Missing cross-tenant isolation test | Test hinzufügen |
|
||
|
|
| 15 | `test_workflows.py` | Missing RBAC + tenant isolation tests | Tests hinzufügen |
|
||
|
|
| 16 | `test_notifications.py` | Missing tenant isolation + RBAC tests | Tests hinzufügen |
|
||
|
|
| 17 | `test_companies.py` | Missing visibility filter test | Test hinzufügen |
|
||
|
|
| 18 | `test_contacts.py` | Missing visibility filter test | Test hinzufügen |
|
||
|
|
| 19 | `test_dms.py + test_dms_coverage.py + test_dms_errors.py` | Missing cross-tenant isolation tests | Tests hinzufügen |
|
||
|
|
| 20 | `test_entity_links.py` | Missing cross-tenant test | Test hinzufügen |
|
||
|
|
| 21 | `test_tags.py` | Missing cross-tenant + RBAC test | Test hinzufügen |
|
||
|
|
| 22 | `mail/plugin.py:194-217` (P1-47) | on_deactivate FEHLT restore unregister | `get_restore_registry().unregister("mail")` hinzufügen |
|
||
|
|
| 23 | `mcp_client/models.py:35-36` (P1-58) | Naive datetime `utcnow` | `datetime.now(UTC)` |
|
||
|
|
| 24 | `frontend/src/api/tags.ts:12` (P1-31) | EntityType hardcoded, inkonsistent | Dynamisch aus Backend holen |
|
||
|
|
| 25 | `frontend/src/components/common/ProtectedRoute.tsx:22-24` (P1-F1) | Fail-open bei leeren permissions | Block access bis permissions geladen |
|
||
|
|
| 26 | `mail/plugin.py:194-200` (P1-30) | on_deactivate fehlt restore + history unregister | Hinzufügen |
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Phase 3: P2 Architekturfehler (69 Fixes, ~12h)
|
||
|
|
|
||
|
|
### 3.1 Backend P2s (23 Fixes)
|
||
|
|
|
||
|
|
**Core/Plugin-Grenze (8 Fixes):**
|
||
|
|
- `entity_permission_service.py:59-61` — Contact hardcoded in ENTITY_MODELS → über ContactsPlugin registrieren
|
||
|
|
- `sensitive_data.py:83-98` — DATA_EXPOSURE_POLICY Contact-spezifisch → Plugin deklarierbar
|
||
|
|
- `sensitive_data.py:24-48` — SENSITIVE_FIELDS Contact/Mail-spezifisch → Plugin deklarierbar
|
||
|
|
- `permission_registry.py:86-122` — CORE_FIELD_DEFINITIONS Contact-spezifisch → Plugin deklarierbar
|
||
|
|
- `deps.py:21-36` — _WRITE_PERMISSIONS hardcoded Plugin-Perms → Nur Core-Perms
|
||
|
|
- `roles.py:27-50` — SYSTEM_PERMISSIONS hardcoded Plugin-Perms → Dynamisch aus Registry
|
||
|
|
- `routes/webhooks.py:26,49` — Falsche permissions (automation statt webhooks) → Korrigieren
|
||
|
|
- `routes/dashboard.py:14,61-93` — Hardcoded Contact counts → Plugin-contributable
|
||
|
|
|
||
|
|
**Generische Services (5 Fixes):**
|
||
|
|
- `routes/import_export.py:40` — entity_type default 'companies' hardcoded → Dynamisch
|
||
|
|
- `schemas/address.py:9` — pattern="^contact$" hardcoded → Dynamisch
|
||
|
|
- `tags/schemas.py:27,33,39` — entity_type pattern hardcoded → Dynamisch
|
||
|
|
- `entity_links/schemas.py:9` — entity_type pattern hardcoded → Dynamisch
|
||
|
|
- `unified_search` — 3 separate hardcoded entity maps → Eine Registry
|
||
|
|
|
||
|
|
**Deprecated Notifications (3 Fixes):**
|
||
|
|
- `workflow_service.py:13` — importiert deprecated Notification → post_system_message
|
||
|
|
- `routes/users.py:15` — importiert create_notification → post_system_message
|
||
|
|
- `workflows/engine.py:122-130` — erstellt Notification model direkt → post_system_message
|
||
|
|
|
||
|
|
**Plugin Lifecycle (4 Fixes):**
|
||
|
|
- `restore_registry.py` — register_default_entities veraltet → Entfernen oder dokumentieren
|
||
|
|
- `history_hooks.py` — register_default_history_hooks veraltet → Entfernen oder dokumentieren
|
||
|
|
- `base.py:81` — unregister_all_for_plugin nutzt __self__ Heuristik → owner_tag nutzen
|
||
|
|
- `report_generator/plugin.py:9` — top-level import of jobs module → lazy import in on_activate
|
||
|
|
|
||
|
|
**Architektur (5 Fixes):**
|
||
|
|
- `forgejo_error_reporter/models.py:13` — Eigenes Base statt app.core.db.Base → app.core.db.Base nutzen
|
||
|
|
- `saved_views.py:60` — require_permission("contacts:read") hardcoded → Dynamisch
|
||
|
|
- `saved_filters.py:60` — require_permission("contacts:read") hardcoded → Dynamisch
|
||
|
|
- `worker.py:169` — direkter Import `unified_search.provider_registry.auto_register_providers` → `auto_register_providers` im UnifiedSearchContract exponieren und via Contract nutzen
|
||
|
|
- `worker.py:280` — direkter Import `forgejo_error_reporter.service.report_error_to_forgejo` → ForgejoErrorReporterContract nutzen (wie main.py/errors.py)
|
||
|
|
|
||
|
|
### 3.2 Frontend P2s (25 Fixes)
|
||
|
|
|
||
|
|
**Hardcoded Entity-Types (8 Fixes):**
|
||
|
|
- `routes/index.tsx:17-70` — 54 hardcoded Plugin-Routes → Dynamisch aus Manifesten
|
||
|
|
- `search.ts:40-47` — ENTITY_URL_MAP hardcoded → Dynamisch aus Backend
|
||
|
|
- `CommandPalette.tsx:12-39` — TYPE_LABELS/ICONS hardcoded → Dynamisch
|
||
|
|
- `GlobalSearchResults.tsx:15-22` — TYPE_LABELS hardcoded (dupliziert) → Gemeinsame Konstante
|
||
|
|
- `tags.ts:12` — EntityType hardcoded → Dynamisch aus Backend
|
||
|
|
- `Trash.tsx:17` — ENTITY_TYPES hardcoded → Dynamisch
|
||
|
|
- `CustomFields.tsx:40-43` — ENTITY_OPTIONS hardcoded → Dynamisch
|
||
|
|
- `ImportWizard.tsx:45-48 + ExportPanel.tsx:13-16` — ENTITY_OPTIONS dupliziert → Gemeinsame Konstante
|
||
|
|
|
||
|
|
**Hardcoded Field Definitions (6 Fixes):**
|
||
|
|
- `FilterPanel.tsx:22-80` — FIELD_DEFS hardcoded → Aus Backend/Manifest holen
|
||
|
|
- `SortPanel.tsx:22-69` — SORT_FIELDS hardcoded (dupliziert) → Gemeinsame Konstante
|
||
|
|
- `GroupPanel.tsx:22-68` — GROUP_FIELDS hardcoded (dupliziert) → Gemeinsame Konstante
|
||
|
|
- `MailFilterPanel.tsx:22-44` — FIELD_DEFS hardcoded → Aus Backend holen
|
||
|
|
- `MailSortPanel.tsx:19-27` — SORT_FIELDS hardcoded (dupliziert) → Gemeinsame Konstante
|
||
|
|
- `MailGroupPanel.tsx:19-29` — GROUP_FIELDS hardcoded (dupliziert) → Gemeinsame Konstante
|
||
|
|
|
||
|
|
**Deprecated Notification API (2 Fixes):**
|
||
|
|
- `NotificationDropdown.tsx` — Nutzt alte /notifications API → Communication API
|
||
|
|
- `NotificationBell.tsx:13` — Nutzt useUnreadNotificationCount von /notifications → Communication API
|
||
|
|
|
||
|
|
**Hardcoded Options (9 Fixes):**
|
||
|
|
- `Settings.tsx:18-29` — 10 hardcoded settings nav items → Dynamisch
|
||
|
|
- `SettingsWebhooks.tsx:38-56` — AVAILABLE_EVENTS hardcoded, 6 nicht existent → Aus Backend holen
|
||
|
|
- `WorkflowEditor.tsx:16-27` — triggerEventOptions hardcoded, 4 nicht existent → Aus Backend holen
|
||
|
|
- `SettingsUsers.tsx:19-25` — LEGACY_ROLES hardcoded → Aus /roles API holen
|
||
|
|
- `SettingsMenuOrder.tsx:34-46` — DEFAULT_ORDER hardcoded → Aus Backend holen
|
||
|
|
- `AgentDashboard.tsx:50-61` — commonModels hardcoded → Aus /ai/providers API holen
|
||
|
|
- `DashboardWidgetLoader.tsx:11-21` — widgetRegistry hardcoded → Dynamisch aus Manifesten
|
||
|
|
- `ContactList.tsx:28-80` — ALL_COLUMNS hardcoded → Aus Backend/Manifest holen
|
||
|
|
- `MailFolderTree.tsx:22-43` — FOLDER_NAME_MAP hardcoded → i18n keys
|
||
|
|
|
||
|
|
### 3.3 Test P2s (21 Fixes)
|
||
|
|
|
||
|
|
- 3x Invalid bcrypt hash `$2b$12$testhash` → `hash_password("TestPass123!")` (`test_cross_tenant_security.py:98`, `test_cross_tenant_security_v2.py:132`, `test_cross_tenant_standalone.py:73`)
|
||
|
|
- 2x `password_hash="dummy"` → Proper bcrypt hash (`test_workspaces.py:42`, `test_api_tokens.py:33`)
|
||
|
|
- 3x Random UUIDs für non-existent entities → Echte Entity-IDs aus DB (`test_tags.py:138,165,194`)
|
||
|
|
- 1x is_system_admin bypass → Echte Permission-Setup (`test_ai_proactive.py:146-150`)
|
||
|
|
- 1x Wildcard permissions `*: *` → Echte Permissions (`test_commands.py:140-148`)
|
||
|
|
- 1x API token in plaintext → Token nicht in Response asserten (`test_mcp_client.py:41`)
|
||
|
|
- 3x Hardcoded DB URLs → env vars (`test_cross_tenant_security.py:40`, `test_cross_tenant_security_v2.py:47-56`, `test_cross_tenant_standalone.py:33`)
|
||
|
|
- 3x Hardcoded SECRET_KEY → env var/conftest (`test_cross_tenant_security_v2.py:34-37`, `test_cross_tenant_standalone.py:18-21`, `test_no_legacy_tenant_var.py:17-20`)
|
||
|
|
- 1x Variable naming mismatch → Korrigieren (`test_entity_links.py:105`)
|
||
|
|
- 4x Manually constructed user → Echte Auth verwenden (`test_external_agent_api.py:92-101`, `test_graph_rag.py`, `test_agent_memory.py`, `test_marketplace.py`)
|
||
|
|
- 1x PermissionLevelBadge hardcoded → i18n (`frontend/src/pages/SettingsRechte.tsx:37-75`)
|
||
|
|
- 1x categoryLabels/modelOptions hardcoded → Aus Backend (`frontend/src/pages/ProactiveAISettings.tsx:3-17`)
|
||
|
|
- 1x ENTITY_OPTIONS hardcoded → Aus Backend (`frontend/src/pages/CustomFields.tsx:40-43`)
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Phase 4: P3 Code Quality (49 Fixes, ~4h)
|
||
|
|
|
||
|
|
### 4.1 Backend P3s (20 Fixes)
|
||
|
|
- Dead Code entfernen (restore_registry `register_default_entities`, history_hooks `register_default_history_hooks`, registry `_mounted_routes`)
|
||
|
|
- Deprecated notification imports ersetzen (`entity_permission_service.py:30`, `schemas/common.py:22-58`)
|
||
|
|
- Duplicate imports entfernen (`ai_copilot_service.py:18-19`, `ai_proactive/services.py:26-27`)
|
||
|
|
- Private Attribut Zugriff ersetzen (`custom_fields.py:42` → `list_discovered()` statt `registry._plugins`)
|
||
|
|
- `__import__` inline durch proper import ersetzen (`entity_attachment.py:45-49`)
|
||
|
|
- Duplicate field definitions entfernen (`system_settings.py:44-54`)
|
||
|
|
- Unified Search hardcoded entity maps konsolidieren (`search_engine.py:19-24`, `lifecycle.py:22-27`, `jobs.py:16-21` → eine Registry)
|
||
|
|
- Plugin→Plugin direkte Contracts-Imports vereinheitlichen (`ai_assistant/plugin.py:96-98`, `system_notif/plugin.py:161`, `graph_rag/plugin.py:44,57` → Contract-Registry nutzen)
|
||
|
|
|
||
|
|
### 4.2 Frontend P3s (30 Fixes)
|
||
|
|
- `import * as LucideIcons` durch ICON_MAP ersetzen (2 Dateien)
|
||
|
|
- `any` type casts durch proper types ersetzen (~15 Dateien)
|
||
|
|
- inline styles durch Tailwind classes ersetzen (3 Dateien)
|
||
|
|
- hardcoded German strings durch i18n t() ersetzen (~10 Dateien)
|
||
|
|
- redundante regex vor DOMPurify entfernen
|
||
|
|
- leere DmsTrash Implementation vervollständigen
|
||
|
|
|
||
|
|
### 4.3 Test P3s (8 Fixes)
|
||
|
|
- Duplicate fixtures/decorators entfernen
|
||
|
|
- sys.path.insert durch conftest/pytest config ersetzen
|
||
|
|
- Unused imports entfernen
|
||
|
|
- Potential KeyError fixen
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Verifikation nach jeder Phase
|
||
|
|
|
||
|
|
1. `python -m pytest -v --tb=short` — alle Tests grün
|
||
|
|
2. `cd frontend && npx tsc --noEmit` — TypeScript kompiliert
|
||
|
|
3. `python -c "from app.main import app; print(len(app.routes))"` — App startet
|
||
|
|
4. `python scripts/check_cross_plugin_imports.py` — 0 Verstöße
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Aufwandsschätzung
|
||
|
|
|
||
|
|
| Phase | Fixes | Aufwand | Priorität |
|
||
|
|
|-------|-------|---------|-----------|
|
||
|
|
| 1 — P0 | 8 | ~2h | Sofort |
|
||
|
|
| 2 — P1 | 27 | ~6h | Hoch |
|
||
|
|
| 3 — P2 | 69 | ~12h | Mittel |
|
||
|
|
| 4 — P3 | 49 | ~4h | Niedrig |
|
||
|
|
| **Total** | **153** | **~24h** | |
|
||
|
|
|
||
|
|
## Reihenfolge
|
||
|
|
|
||
|
|
1. **Phase 1** — P0 Runtime Crashes (sofort, blockiert alles)
|
||
|
|
2. **Phase 2** — P1 Funktionale Fehler (nach P0)
|
||
|
|
3. **Phase 3** — P2 Architekturfehler (nach P1, kann parallel)
|
||
|
|
4. **Phase 4** — P3 Code Quality (nach P3, kann parallel)
|
||
|
|
|
||
|
|
Nach jeder Phase: Tests laufen, App startet, Cross-Plugin-Checker 0 Verstöße.
|