From 5ec1fc9b059236035036dc10ad19e910ef567971 Mon Sep 17 00:00:00 2001 From: Agent Zero Date: Sun, 26 Jul 2026 20:45:42 +0200 Subject: [PATCH] Phase 1: Fix all critical release blockers (B1-B10) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit B1: Remove duplicate get_redis() — singleton no longer overwritten B2: Plugin routes now enforce activation status via require_active_plugin() B3: Fix UploadFile ForwardRef error — remove functools.wraps from wrap_plugin_route B4: DMS upload uses true streaming via save_stream() instead of RAM accumulation B5: Worker on_startup registers plugin event handlers + webhook dispatcher B6: Implement send_password_reset_email job, remove raw token logging B7: Webhook SSRF protection (IP validation, no redirects), secret removed from response B8: RLS repair migration 0044 + separate crm_runtime DB user (NOSUPERUSER, NOBYPASSRLS) B9: Fix .env.docker.example AUTH_SECRET → SECRET_KEY B10: Remove Redis default password, remove exposed DB/Redis ports Also: add frontend_url to config, add SMTP settings to .env.docker.example, update prestart.sh to use MIGRATION_DATABASE_URL for alembic. --- .a0/risks.md | 200 +++++ .env.docker.example | 41 +- .gitignore | 4 + FIX-PLAN-V2.md | 290 +++++++ PLUGIN-SYSTEM-UMBAUPLAN.md | 724 ++++++++++++++++++ .../versions/0044_rls_repair_and_db_roles.py | 125 +++ app/config.py | 3 + app/core/auth.py | 5 - app/core/jobs.py | 69 ++ app/core/plugin_error_handler.py | 13 +- app/core/worker.py | 54 +- app/deps.py | 31 + app/main.py | 20 +- app/models/backup.py | 4 +- app/models/notification.py | 4 +- app/models/system_settings.py | 2 +- app/models/tax.py | 3 +- app/models/webhook.py | 5 + app/plugins/builtins/ai_assistant/routes.py | 2 +- app/plugins/builtins/calendar/routes.py | 2 +- app/plugins/builtins/dms/routes.py | 38 +- app/plugins/builtins/kommunikation/routes.py | 2 +- app/plugins/builtins/mail/routes.py | 2 +- app/plugins/builtins/test_sample/__init__.py | 49 ++ .../migrations/0001_test_plugin.sql | 9 + app/schemas/webhook.py | 2 +- app/services/auth_service.py | 3 +- app/services/webhook_service.py | 54 +- docker-compose.yml | 23 +- dump.rdb | Bin 88 -> 0 bytes prestart.sh | 33 +- tests/conftest.py | 41 +- 32 files changed, 1781 insertions(+), 76 deletions(-) create mode 100644 .a0/risks.md create mode 100644 FIX-PLAN-V2.md create mode 100644 PLUGIN-SYSTEM-UMBAUPLAN.md create mode 100644 alembic/versions/0044_rls_repair_and_db_roles.py create mode 100644 app/plugins/builtins/test_sample/__init__.py create mode 100644 app/plugins/builtins/test_sample/migrations/0001_test_plugin.sql delete mode 100644 dump.rdb diff --git a/.a0/risks.md b/.a0/risks.md new file mode 100644 index 0000000..954851c --- /dev/null +++ b/.a0/risks.md @@ -0,0 +1,200 @@ +# LeoCRM Security & Data Risk Assessment + +**Date:** 2026-07-26 +**Assessor:** Security Data Engineer (A0 Orchestrator) +**Project:** LeoCRM at `/a0/usr/workdir/leocrm-fix` + +--- + +## Summary + +| Severity | Count | +|----------|-------| +| CRITICAL | 5 | +| HIGH | 8 | +| MEDIUM | 8 | +| LOW | 5 | +| **Total**| **26**| + +--- + +## CRITICAL Issues + +### C-1: Redis Default Password `changeme` in docker-compose.yml +**File:** `docker-compose.yml:53` +**Risk:** Redis stores session data, CSRF tokens, and rate-limit counters. The default password `changeme` is trivially guessable. If Redis port 6379 is exposed, an attacker can read/modify all sessions, steal CSRF tokens, and bypass rate limits. +**Remediation:** Remove the default fallback. Require `REDIS_PASSWORD` as a mandatory variable (`${REDIS_PASSWORD:?REDIS_PASSWORD is required}`). Use a strong randomly generated password in production. + +### C-2: No SECRET_KEY in `.env` — Insecure Default Active in Development +**File:** `.env` (missing `SECRET_KEY`), `app/config.py:55` +**Risk:** `.env` has no `SECRET_KEY`. The config defaults to `"change-me-in-production-use-a-secure-random-string"`. While `get_settings()` raises in production mode, `.env` sets `ENVIRONMENT=development`, so the default key is silently used. Any signing/token operation using `secret_key` is compromised. +**Remediation:** Add a strong random `SECRET_KEY` (min 32 chars) to `.env`. Fail-fast in all environments if the default key is detected, not just production. + +### C-3: PostgreSQL and Redis Ports Exposed to Host +**File:** `docker-compose.yml:37-38, 56-57` +**Risk:** `ports: "5432:5432"` and `ports: "6379:6379"` expose the database and Redis to the host network. Combined with weak/default credentials, this allows direct external access to all session data and the entire database. +**Remediation:** Remove port mappings for production. Use Docker internal networking only (`crm-net`). If debug access is needed, bind to `127.0.0.1:5432:5432` and document it as dev-only. + +### C-4: Unauthenticated Error Endpoint Forwards Data to External Forgejo +**File:** `app/routes/errors.py:54-90`, `app/plugins/builtins/forgejo_error_reporter/service.py:151-250` +**Risk:** The `/api/v1/errors` endpoint requires no authentication. CSRF middleware explicitly bypasses token checks for this path (line 48 of `middleware.py`). Any unauthenticated attacker can POST arbitrary error data (message, stack, URL, userAgent, and **arbitrary context dict**) which gets forwarded to an external Forgejo instance as a public issue. The `context` field accepts `dict[str, Any]` with no size limit on individual keys — an attacker can exfiltrate data or inject malicious content into Forgejo issues. +**Remediation:** Require authentication for error reporting. If unauthenticated errors are needed, strip the `context` field entirely, add strict schema validation with size limits on all fields, and add a CAPTCHA or stricter rate limiting. + +### C-5: Plaintext Database Password in `.env` +**File:** `.env:1` +**Risk:** `DATABASE_URL=postgresql+asyncpg://leocrm:leocrm@localhost:5432/leocrm` embeds the DB password `leocrm` in plaintext. While `.gitignore` covers `.env`, the password is weak and identical to the username. If the file is accessed via any path traversal, backup leak, or container escape, the database is fully compromised. +**Remediation:** Use a strong unique password. Separate `DATABASE_URL` construction from credential storage where possible (e.g., use individual `POSTGRES_USER`, `POSTGRES_PASSWORD`, `POSTGRES_HOST`, `POSTGRES_DB` env vars and construct the URL in code). + +--- + +## HIGH Issues + +### H-1: Rate Limiter Trusts X-Forwarded-For Without Validation +**File:** `app/core/rate_limit.py:43-45` +**Risk:** `get_client_ip()` blindly trusts the `X-Forwarded-For` header. An attacker can set arbitrary values to bypass rate limits on login, password reset, and other endpoints. Each request with a different spoofed IP creates a new rate-limit counter. +**Remediation:** Only trust `X-Forwarded-For` from known proxy IPs. Configure a trusted proxy list and validate the header chain. Use Starlette's `ProxyHeadersMiddleware` or validate against a `TRUSTED_PROXIES` env var. + +### H-2: Duplicate `get_redis()` Functions — Connection Leak +**File:** `app/core/auth.py:53-66` and `app/core/auth.py:94-96` +**Risk:** Two `get_redis()` functions exist. The first (line 53) returns a singleton. The second (line 94) creates a **new Redis connection on every call**. Code importing `get_redis` may use either version. The middleware (line 69) creates its own Redis connection per request. This leads to connection pool exhaustion under load. +**Remediation:** Remove the second `get_redis()` (line 94-96). Ensure all code uses the singleton version. The middleware should use `get_redis()` from `app.core.auth` instead of creating its own connection. + +### H-3: CSRF Middleware Creates New Redis Connection Per Request +**File:** `app/core/middleware.py:69-90` +**Risk:** For every unsafe HTTP request, the middleware creates a new `aioredis.from_url()` connection, uses it, then closes it. Under load, this creates thousands of connections and can exhaust Redis connection limits. +**Remediation:** Use the global Redis singleton via `from app.core.auth import get_redis`. Remove the per-request connection creation and the `finally: await redis.close()` block. + +### H-4: CSRF Token Stored Plaintext in PostgreSQL +**File:** `app/core/auth.py:141` (`SessionModel` stores `csrf_token`) +**Risk:** The CSRF token is stored as plaintext in the PostgreSQL `sessions` table (audit trail). If the database is compromised, all active CSRF tokens are available for CSRF attacks. +**Remediation:** Store only a hash of the CSRF token in PostgreSQL (like `hash_token()` already exists for session tokens). Compare hashes during validation. + +### H-5: No File Upload Validation in Storage Backend +**File:** `app/core/storage.py:69-128` +**Risk:** `LocalStorage` performs no validation on uploaded files: +- No path traversal protection: `os.path.join(self.base_path, path)` with a malicious `path` containing `../../` can write anywhere on the filesystem +- No file type/extension whitelist +- No file size limit +- No content-type validation +- `get_url()` returns the full filesystem path, leaking internal directory structure +**Remediation:** Sanitize `path` with `os.path.realpath()` and verify it's within `base_path`. Enforce file size limits, extension whitelist, and MIME type validation. Return relative paths from `get_url()`, not absolute filesystem paths. + +### H-6: WebSocket Connections Lack Authentication Verification +**File:** `app/plugins/builtins/kommunikation/websocket_manager.py:23-28`, `app/plugins/builtins/ai_ui_control/websocket_manager.py:40-46` +**Risk:** Both WebSocket managers accept connections via `connect(websocket, user_id)` without verifying that `user_id` is authenticated. The security depends entirely on the calling route. If any WebSocket route passes an untrusted `user_id` (e.g., from query params), an attacker can impersonate any user. There is also no origin verification on WebSocket connections. +**Remediation:** Verify session cookie inside `connect()` before `websocket.accept()`. Validate the `Origin` header against allowed CORS origins. Add authentication middleware for WebSocket routes. + +### H-7: In-Memory Rate Limiter in Error Endpoint — Fails with Multiple Workers +**File:** `app/routes/errors.py:21-40` +**Risk:** The error endpoint uses a process-local `defaultdict(deque)` for rate limiting. With multiple Uvicorn workers (common in production), each worker has its own counter. An attacker can make `RATE_LIMIT * num_workers` requests per minute. +**Remediation:** Use the Redis-based `check_rate_limit()` from `app/core/rate_limit.py` instead of the in-memory implementation. + +### H-8: No CSRF Protection on WebSocket Connections +**File:** Both WebSocket managers +**Risk:** WebSocket connections are not protected against CSRF. A malicious site can open a WebSocket to the CRM backend via JavaScript `new WebSocket()` and send commands as the authenticated user (cookies are sent automatically with SameSite=Strict for same-site, but cross-site WebSocket hijacking is still possible if SameSite is configured differently or cookies are sent via `credentials`). +**Remediation:** Verify the `Origin` header on WebSocket upgrade requests. Reject connections from untrusted origins. + +--- + +## MEDIUM Issues + +### M-1: Login Response Leaks `is_system_admin` Flag +**File:** `app/routes/auth.py:78` +**Risk:** The login response includes `"is_system_admin": user.is_system_admin`. An attacker who compromises a session or intercepts the response knows whether the account has system-wide privileges, enabling targeted attacks. +**Remediation:** Do not include `is_system_admin` in the login response. The frontend can determine admin status via the `/me/permissions` endpoint. + +### M-2: Weak Password Validation — No Complexity Requirements +**File:** `app/schemas/auth.py:10` (login: `min_length=1`), `app/schemas/user.py:11` (create: `min_length=8`) +**Risk:** Login accepts any password length (min_length=1). User creation requires min 8 chars but no complexity (uppercase, lowercase, digits, special chars). Users can set passwords like `aaaaaaaa`. +**Remediation:** Add password complexity validation (min 12 chars, mixed case, digits, special chars) for user creation and password reset. Keep login min_length=1 to avoid leaking whether the password was partially correct. + +### M-3: F-String Interpolation of Table/Column Names in Raw SQL +**File:** `app/plugins/builtins/unified_search/embedding.py:194`, `search_engine.py:153`, `routes.py:294,300`, `jobs.py:183,228` +**Risk:** Multiple raw SQL queries use f-strings to interpolate table and column names: `f"UPDATE {table} SET ..."`, `f"SELECT {emb_col} FROM {table_name} ..."`. While the values come from hardcoded `table_map` dicts (not user input), this pattern is fragile — a future change could introduce user-controlled values into the map. +**Remediation:** Use SQLAlchemy ORM queries instead of raw SQL where possible. If raw SQL is needed, validate table/column names against an allowlist before interpolation, or use `sqlalchemy.sql.quoted_name` for safe identifier quoting. + +### M-4: Forgejo Error Reporter Sends Full Context to External Service +**File:** `app/plugins/builtins/forgejo_error_reporter/service.py:196-199` +**Risk:** The error reporter serializes the entire `context` dict into the Forgejo issue body as JSON. If frontend error reporting includes sensitive data (user tokens, PII, tenant data), it will be written to an external Forgejo repository as a public issue. +**Remediation:** Add a field-level allowlist for context data. Strip or redact sensitive keys (tokens, passwords, emails, phone numbers). Consider making Forgejo issues private/confidential. + +### M-5: Config Has Hardcoded Default Secret Key +**File:** `app/config.py:55` +**Risk:** The default `secret_key = "change-me-in-production-use-a-secure-random-string"` is a known public value. While production mode checks for it, development mode silently uses it. If dev environments are exposed (even temporarily), all signed tokens are forgeable. +**Remediation:** Remove the default value entirely. Make `secret_key` a required field with no default. Fail in all environments if not set. + +### M-6: `LocalStorage.get_url()` Returns Absolute Filesystem Path +**File:** `app/core/storage.py:116-117` +**Risk:** `get_url()` returns `self._full_path(path)` which is the absolute filesystem path (e.g., `/data/uploads/tenant1/file.pdf`). If this URL is returned to the frontend or used in API responses, it leaks the internal directory structure and can aid path traversal attacks. +**Remediation:** Return a relative path or a signed download URL that routes through an authenticated API endpoint. + +### M-7: Inconsistent Environment Configuration in `.env` +**File:** `.env:3,4` +**Risk:** `.env` sets `ENVIRONMENT=development` but `SESSION_COOKIE_SECURE=true`. In development with HTTP, secure cookies won't be sent, causing auth failures. More importantly, the `ENVIRONMENT=development` setting disables the production safety checks in `get_settings()`, allowing the default `SECRET_KEY` to be used. +**Remediation:** Use separate `.env.development` and `.env.production` files. Ensure development configs are never accidentally deployed. + +### M-8: Permission Cache Falls Back to Stale Data on DB Error +**File:** `app/core/permissions.py:337-344` +**Risk:** When `_get_current_permission_version()` fails (DB error), the code sets `current_version = cached_version` and uses potentially stale cached permissions. If a user's permissions were revoked during the DB outage, they retain elevated access. +**Remediation:** On DB error, either fail closed (deny access) or use a shorter stale-while-error TTL. Log the event as a security incident. + +--- + +## LOW Issues + +### L-1: `document.write()` with DOM Clone in Print Utility +**File:** `frontend/src/utils/print.ts:54, 127` +**Risk:** `printElement()` and `exportToPDF()` use `document.write()` with `clone.outerHTML`. If the printed DOM element contains user-controlled content (e.g., contact notes with HTML), it executes in a new window context. The new window is same-origin, limiting the impact, but it's still an unnecessary risk. +**Remediation:** Use DOM APIs (`appendChild`, `importNode`) instead of `document.write()`. Alternatively, sanitize the cloned HTML before writing. + +### L-2: Session Data Stored in Redis Without Encryption +**File:** `app/core/auth.py:130-134` +**Risk:** Session data (user_id, tenant_id, email, role, csrf_token, is_system_admin) is stored as plaintext JSON in Redis. Anyone with Redis access can read all active sessions. +**Remediation:** Encrypt session data before storing in Redis, or accept the risk given Redis should be network-isolated. At minimum, ensure Redis requires authentication and is not exposed. + +### L-3: No Security Headers Middleware +**File:** No security headers middleware found +**Risk:** The application does not set security headers like `X-Content-Type-Options`, `X-Frame-Options`, `Strict-Transport-Security`, `Content-Security-Policy`. +**Remediation:** Add a security headers middleware or use `starlette-securehead`/`secure` package. + +### L-4: No Origin Verification on WebSocket Upgrade +**File:** Both WebSocket managers +**Risk:** Neither WebSocket manager checks the `Origin` header before accepting connections. While cookies with `SameSite=Strict` provide some protection, some browsers and non-browser clients may not respect SameSite on WebSocket connections. +**Remediation:** Check `websocket.headers.get("origin")` against `settings.cors_origin_list` before calling `websocket.accept()`. + +### L-5: Unbounded Feedback/Command Storage in AI UI Control WebSocket +**File:** `app/plugins/builtins/ai_ui_control/websocket_manager.py:94-103` +**Risk:** `store_feedback()` stores feedback dicts without size limits. `cleanup_stale()` only runs when explicitly called. An attacker who can send WebSocket messages could fill memory with large feedback payloads. +**Remediation:** Add size limits on feedback payloads. Run `cleanup_stale()` on a timer or on each `connect()`/`disconnect()`. + +--- + +## Positive Findings + +1. **Dockerfile security:** Multi-stage build, non-root user (`appuser` UID 1000), healthcheck configured, no secrets baked into image. +2. **RLS implementation:** PostgreSQL Row Level Security with `FORCE` (migration 0028) ensures tenant isolation even for table owners. `set_tenant_context()` uses parameterized queries. +3. **Password hashing:** bcrypt with configurable rounds (default 12). +4. **Session tokens:** `secrets.token_urlsafe(32)` — cryptographically secure. +5. **XSS protection:** `HtmlBlock.tsx` and `SignatureManager.tsx` use `DOMPurify.sanitize()` before `dangerouslySetInnerHTML`. +6. **RBAC architecture:** Deny-list takes precedence over allow-list. Field-level permissions with strictest-wins merging. Permission version-based cache invalidation. +7. **No user enumeration:** Password reset endpoint always returns 200. +8. **SQL injection:** ORM queries use parameterized statements throughout. Raw SQL in `unified_search` uses hardcoded maps (not directly exploitable). +9. **`.gitignore`** properly covers `.env`, `.env.*`, and excludes example files. +10. **Production safety checks** in `get_settings()` validate `SECRET_KEY`, `SESSION_COOKIE_SECURE`, and `STORAGE_PATH`. + +--- + +## Migration & Data Loss Risks + +1. **RLS policies:** Multiple migrations (0001, 0002, 0004, 0015, 0021, 0028) create and modify RLS policies. Migration 0028 adds `FORCE ROW LEVEL SECURITY`. Ensure all migrations are applied in order before production deployment. +2. **Backup risk:** No backup/restore procedure found in the repository. The `last_backup_at` system setting is referenced in automation jobs but no backup script exists. +3. **Volume persistence:** `docker-compose.yml` defines named volumes for `pgdata`, `redisdata`, and `storage`. Good for persistence, but no backup strategy documented. +4. **Migration rollback:** Down migrations exist but should be tested. RLS policy down migrations disable RLS — running a rollback in production would expose all tenant data. + +--- + +## Remediation Priority + +1. **Immediate (before any production deploy):** C-1, C-2, C-3, C-4, C-5, H-1, H-2, H-3 +2. **Short-term (within 1 sprint):** H-4, H-5, H-6, H-7, H-8, M-1, M-2, M-5 +3. **Medium-term (within 2 sprints):** M-3, M-4, M-6, M-7, M-8, L-1, L-2, L-3, L-4, L-5 diff --git a/.env.docker.example b/.env.docker.example index 3ba3e12..0a178c6 100644 --- a/.env.docker.example +++ b/.env.docker.example @@ -15,23 +15,48 @@ POSTGRES_USER=crm_user POSTGRES_PASSWORD=STRONG_PASSWORD_HERE POSTGRES_DB=crm_db -# --- CRM Application ---------------------------------------------------------- -# The host "postgres" is the docker-compose service name (internal DNS). -# The DRIVER is asyncpg for production PostgreSQL. -DATABASE_URL=postgresql+asyncpg://crm_user:STRONG_PASSWORD_HERE@postgres:5432/crm_db +# --- Redis (REQUIRED) --------------------------------------------------------- +# Generate a strong password: +# python -c "import secrets; print(secrets.token_urlsafe(24))" +REDIS_PASSWORD=STRONG_REDIS_PASSWORD_HERE -# --- AUTH_SECRET (REQUIRED, min 32 chars) ------------------------------------ +# --- CRM Application: Runtime DB user (NOSUPERUSER, NOBYPASSRLS) -------------- +# The app and worker use crm_runtime — RLS is enforced. +# This user is created by migration 0044 with DML-only permissions. +# Set RUNTIME_DB_PASSWORD to the password you want for crm_runtime. +RUNTIME_DB_PASSWORD=STRONG_RUNTIME_PASSWORD_HERE +DATABASE_URL=postgresql+asyncpg://crm_runtime:STRONG_RUNTIME_PASSWORD_HERE@postgres:5432/crm_db + +# --- CRM Application: Migration DB user (owner, can run DDL) ----------------- +# Migrations and DDL operations use the owner user (crm_user). +# This is NOT used by the app at runtime — only by prestart.sh / alembic. +MIGRATION_DATABASE_URL=postgresql+asyncpg://crm_user:STRONG_PASSWORD_HERE@postgres:5432/crm_db + +# --- SECRET_KEY (REQUIRED, min 32 chars) ------------------------------------- # Session signing secret. MUST be at least 32 characters. # Generate with: # python -c "import secrets; print(secrets.token_urlsafe(48))" -AUTH_SECRET=MIN_32_CHARS_GENERATE_WITH_secrets_token_urlsafe_32_xxxxxxxxxxxx +SECRET_KEY=MIN_32_CHARS_GENERATE_WITH_secrets_token_urlsafe_32_xxxxxxxxxxxx + +# --- Frontend URL (for email links) ------------------------------------------ +# The public URL where users access the LeoCRM frontend. +# Used for password reset links, invitations, etc. +FRONTEND_URL=https://crm.example.com # --- CORS / environment ------------------------------------------------------- # Comma-separated, NO wildcards. In dev we allow localhost:8000 (the app) and # :5173 (e.g. Vite dev server). In production, restrict to the real domain. -CORS_ORIGINS=http://localhost:8000,http://localhost:5173 +CORS_ORIGINS=https://crm.example.com ENVIRONMENT=production LOG_LEVEL=INFO -# --- bcrypt tuning (keep aligned with .env.example) -------------------------- +# --- SMTP (for password reset emails) ----------------------------------------- +SMTP_HOST=smtp.example.com +SMTP_PORT=587 +SMTP_USERNAME=noreply@example.com +SMTP_PASSWORD=YOUR_SMTP_PASSWORD +SMTP_FROM_EMAIL=noreply@example.com +SMTP_USE_TLS=true + +# --- bcrypt tuning ---------------------------------------------------------- BCRYPT_ROUNDS=12 diff --git a/.gitignore b/.gitignore index cd37419..e41a0ff 100644 --- a/.gitignore +++ b/.gitignore @@ -30,6 +30,10 @@ coverage.xml .mypy_cache/ .ruff_cache/ +# Redis dumps +dump.rdb +*.rdb + # Database files *.db *.db-journal diff --git a/FIX-PLAN-V2.md b/FIX-PLAN-V2.md new file mode 100644 index 0000000..c192c97 --- /dev/null +++ b/FIX-PLAN-V2.md @@ -0,0 +1,290 @@ +# 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:** 4 sauber gefixt · 8 teilweise gefixt · 4 nicht gefixt · 5 neu gefunden = **21 Maßnahmen** + +--- + +## 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 +- **Dateien:** `WelcomeDialog.tsx`, `SavedFilterBar.tsx`, `EntityHistoryPanel.tsx`, `TagBadge.tsx`, `TagSelector.tsx` +- **Problem:** WelcomeDialog hat `open={false}`. SavedFilterBar/EntityHistoryPanel/TagBadge/TagSelector sind gebaut aber nicht in Seiten integriert. +- **Fix:** + 1. WelcomeDialog an User-Preferences (onboarding_completed) koppeln + 2. SavedFilterBar in ContactsList, Mail, Calendar integrieren + 3. EntityHistoryPanel in ContactDetail, Settings integrieren + 4. TagBadge/TagSelector in ContactsList, Mail, Calendar integrieren +- **Aufwand:** 4 Std + +### 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) diff --git a/PLUGIN-SYSTEM-UMBAUPLAN.md b/PLUGIN-SYSTEM-UMBAUPLAN.md new file mode 100644 index 0000000..5933833 --- /dev/null +++ b/PLUGIN-SYSTEM-UMBAUPLAN.md @@ -0,0 +1,724 @@ +# LeoCRM Plugin-System — Kompletter Umbauplan + +**Erstellt:** 2026-07-26 +**Geschätzter Gesamtaufwand:** ~129 Stunden (~16 Arbeitstage) +**Status:** Geplant — noch nicht gestartet + +--- + +## Übersicht: 5 Phasen + +| Phase | Punkte | Inhalt | Stunden | Tage | +|---|---|---|---|---| +| Phase 1 | 1-3 | Contracts konsequent nutzen | 47 | 6 | +| Phase 2 | 4 | Hooks/Filters-System | 16 | 2 | +| Phase 3 | 5 | Plugin-Isolation (Linting) | 4 | 0,5 | +| Phase 4 | 8 | Plugin-Versioning | 20 | 2,5 | +| Phase 5 | 6 | Marketplace-Vorbereitung | 42 | 5 | +| **Gesamt** | | | **129** | **16** | + +**Wichtig:** Jede Phase ist unabhängig funktionsfähig. Das System läuft nach jeder Phase ohne Einschränkungen weiter. + +--- + +## Phase 1: Contracts konsequent nutzen (Punkte 1-3) + +**Ziel:** Alle 224 direkten Cross-Plugin-Imports werden durch das Contract-System ersetzt. + +### 1.1 Fehlende contracts.py erstellen (7 Std) + +Für jedes Plugin, das noch keine `contracts.py` hat, eine erstellen: + +| # | Plugin | Exportierte Symbole | Aufwand | +|---|---|---|---| +| 1 | `ai_proactive` | ContextTools, ProactiveAgent, JobScheduler | 30 Min | +| 2 | `ai_ui_control` | WebSocketManager, UIAction | 30 Min | +| 3 | `automation` | AgentRunner, ExecutionEngine, Scheduler, WorkflowTimeout | 45 Min | +| 4 | `entity_links` | EntityLink model, create_link, get_links | 20 Min | +| 5 | `forgejo_error_reporter` | report_error_to_forgejo | 15 Min | +| 6 | `mcp_client` | McpClient, McpServerConfig | 30 Min | +| 7 | `mcp_server` | McpServer, ToolDefinitions | 30 Min | +| 8 | `report_generator` | ReportTemplate, ReportInstance, PdfGenerator | 30 Min | +| 9 | `system_notif` | SystemNotifHandler | 15 Min | +| 10 | `tags` | Tag, TagAssignment, assign_tags, remove_tags | 20 Min | +| 11 | `tasks` | Task, TaskService, create_task, update_task | 30 Min | +| 12 | `test_sample` | TestSamplePlugin | 10 Min | +| 13 | `dms` (erweitern) | File, Folder, UploadService, DownloadService | 30 Min | +| 14 | `permissions` (erweitern) | ShareLink, PermissionResolver | 30 Min | + +**Schema für jede contracts.py:** +```python +"""Public contract for the plugin.""" +from __future__ import annotations +from app.plugins.builtins.contracts import get_contract_registry +# Import only public symbols from internal modules + +class Contract: + contract_name = "" + # Expose only public API + +_contract = Contract() +get_contract_registry().register("", _contract) +``` + +### 1.2 Direkte Imports ersetzen (28 Std) + +224 direkte Imports müssen durch `get_contract()` ersetzt werden. + +**Top-Priorität (häufigste Import-Quellen):** + +| # | Datei | Imports | Aufwand | +|---|---|---|---| +| 1 | `automation/plugin.py` | 10 | 1,5 Std | +| 2 | `automation/routes.py` | 8 | 1,5 Std | +| 3 | `ai_proactive/services.py` | 8 | 1,5 Std | +| 4 | `ai_proactive/plugin.py` | 8 | 1,5 Std | +| 5 | `unified_search/jobs.py` | 7 | 1 Std | +| 6 | `builtins/__init__.py` | 7 | 1 Std | +| 7 | `ai_proactive/jobs.py` | 7 | 1 Std | +| 8 | `ai_assistant/participant_handler.py` | 7 | 1 Std | +| 9 | `kommunikation/routes.py` | 6 | 1 Std | +| 10 | `kommunikation/contracts.py` | 6 | 1 Std | +| 11 | `automation/agent_routes.py` | 6 | 1 Std | +| 12 | `automation/agent_comm.py` | 6 | 1 Std | +| 13 | `ai_proactive/participant_handler.py` | 6 | 1 Std | +| 14 | `ai_assistant/plugin.py` | 6 | 1 Std | +| 15 | `unified_search/routes.py` | 5 | 45 Min | +| 16-50 | Alle übrigen Dateien | ~122 | 12 Std | + +**Muster für Ersetzung:** +```python +# VORHER (direkt): +from app.plugins.builtins.kommunikation.services import send_message + +# NACHHER (über Contract): +from app.plugins.builtins.contracts import get_contract + +async def my_function(db, ...): + komm = get_contract("kommunikation") + if komm: + await komm.send_message(db, ...) + # Graceful degradation wenn Plugin nicht aktiv +``` + +### 1.3 Contracts bei Deaktivierung abmelden (4 Std) + +In jedem Plugin's `on_deactivate()`: +```python +async def on_deactivate(self, db, service_container, event_bus) -> None: + # Contract abmelden + from app.plugins.builtins.contracts import get_contract_registry + get_contract_registry().unregister(self.manifest.name) + # ... rest of cleanup + await super().on_deactivate(db, service_container, event_bus) +``` + +| # | Plugin | Aufwand | +|---|---|---| +| 1-16 | Alle 16 Plugins | 15 Min pro Plugin = 4 Std | + +### 1.4 Tests anpassen (8 Std) + +- Cross-Plugin-Tests müssen mit Contracts laufen +- `test_plugins.py` — Contract-Registry Tests +- `test_contracts.py` — Neue Test-Datei für Contract-System +- Alle Integrationstests mit Contract-Mocks + +### Meilenstein Phase 1: +- ✅ Alle 16 Plugins haben contracts.py +- ✅ 0 direkte Cross-Plugin-Imports (geprüft mit grep) +- ✅ Contracts werden bei Deaktivierung abgemeldet +- ✅ Alle Tests bestanden + +--- + +## Phase 2: Hooks/Filters-System (Punkt 4) + +**Ziel:** WordPress-Style Hooks (actions + filters) für Plugin-Erweiterbarkeit. + +### 2.1 HookRegistry erstellen (4 Std) + +**Neue Datei: `app/core/hooks.py`** + +```python +"""WordPress-style hooks: actions (fire-and-forget) and filters (modify data).""" + +from __future__ import annotations +import logging +from collections import defaultdict +from typing import Any, Callable + +logger = logging.getLogger(__name__) + + +class HookRegistry: + """Central registry for actions and filters. + + Actions: do_action('contact.before_create', data) — no return value + Filters: result = apply_filters('contact.format_name', name) — returns modified value + + Priority: lower numbers run first (default=10). + """ + + _instance: HookRegistry | None = None + + def __new__(cls): + if cls._instance is None: + cls._instance = super().__new__(cls) + cls._instance._actions: dict[str, list[tuple[int, Callable]]] = defaultdict(list) + cls._instance._filters: dict[str, list[tuple[int, Callable]]] = defaultdict(list) + return cls._instance + + def register_action(self, hook_name: str, callback: Callable, priority: int = 10) -> None: + self._actions[hook_name].append((priority, callback)) + self._actions[hook_name].sort(key=lambda x: x[0]) + + def register_filter(self, hook_name: str, callback: Callable, priority: int = 10) -> None: + self._filters[hook_name].append((priority, callback)) + self._filters[hook_name].sort(key=lambda x: x[0]) + + async def do_action(self, hook_name: str, *args, **kwargs) -> None: + for _, callback in self._actions.get(hook_name, []): + try: + result = callback(*args, **kwargs) + if hasattr(result, '__await__'): + await result + except Exception: + logger.exception("Error in action %s", hook_name) + + async def apply_filters(self, hook_name: str, value: Any, *args, **kwargs) -> Any: + for _, callback in self._filters.get(hook_name, []): + try: + result = callback(value, *args, **kwargs) + if hasattr(result, '__await__'): + result = await result + value = result + except Exception: + logger.exception("Error in filter %s", hook_name) + return value + + def unregister(self, hook_name: str, callback: Callable) -> None: + self._actions[hook_name] = [(p, c) for p, c in self._actions.get(hook_name, []) if c != callback] + self._filters[hook_name] = [(p, c) for p, c in self._filters.get(hook_name, []) if c != callback] + + def unregister_all(self, hook_name: str) -> None: + self._actions.pop(hook_name, None) + self._filters.pop(hook_name, None) + + def _reset_for_testing(self) -> None: + self._actions.clear() + self._filters.clear() + + +def get_hook_registry() -> HookRegistry: + return HookRegistry() + +async def do_action(hook_name: str, *args, **kwargs) -> None: + await get_hook_registry().do_action(hook_name, *args, **kwargs) + +async def apply_filters(hook_name: str, value: Any, *args, **kwargs) -> Any: + return await get_hook_registry().apply_filters(hook_name, value, *args, **kwargs) +``` + +### 2.2 Integration in BasePlugin (2 Std) + +```python +# In BasePlugin.on_activate: +async def on_activate(self, db, service_container, event_bus) -> None: + # ... existing code ... + # Hooks werden in Subklassen registriert + +# In BasePlugin.on_deactivate: +async def on_deactivate(self, db, service_container, event_bus) -> None: + # Alle Hooks dieses Plugins abmelden + from app.core.hooks import get_hook_registry + # Plugin-spezifische Hooks entfernen (prefix mit plugin name) + # ... existing code ... +``` + +### 2.3 Hook-Punkte in Core-Services (6 Std) + +| # | Service | Hook-Name | Typ | Beschreibung | +|---|---|---|---|---| +| 1 | contact_service | `contact.before_create` | Action | Vor Kontakt-Erstellung | +| 2 | contact_service | `contact.after_create` | Action | Nach Kontakt-Erstellung | +| 3 | contact_service | `contact.format_display_name` | Filter | Anzeigenamen formatieren | +| 4 | contact_service | `contact.before_update` | Action | Vor Kontakt-Update | +| 5 | contact_service | `contact.after_update` | Action | Nach Kontakt-Update | +| 6 | contact_service | `contact.before_delete` | Action | Vor Kontakt-Löschung | +| 7 | mail_service | `mail.before_send` | Filter | E-Mail vor Versand modifizieren | +| 8 | mail_service | `mail.after_send` | Action | Nach E-Mail-Versand | +| 9 | calendar | `calendar.before_appointment` | Action | Vor Termin-Erstellung | +| 10 | calendar | `calendar.after_appointment` | Action | Nach Termin-Erstellung | +| 11 | auth_service | `auth.before_login` | Filter | Login-Daten validieren/modifizieren | +| 12 | auth_service | `auth.after_login` | Action | Nach erfolgreichem Login | +| 13 | user_service | `user.before_create` | Action | Vor User-Erstellung | +| 14 | user_service | `user.after_create` | Action | Nach User-Erstellung | +| 15 | dms | `dms.before_upload` | Filter | Datei-Upload validieren/modifizieren | + +### 2.4 Tests für Hooks/Filters (4 Std) + +- `test_hooks.py` — HookRegistry Tests +- Integrationstests: Plugin registriert Hook, Core-Service löst Hook aus +- Filter-Tests: Wert wird korrekt modifiziert +- Priority-Tests: Reihenfolge wird eingehalten +- Unregister-Tests: Hooks werden bei Deaktivierung entfernt + +### Meilenstein Phase 2: +- ✅ `app/core/hooks.py` mit HookRegistry +- ✅ 15 Hook-Punkte in Core-Services +- ✅ BasePlugin registriert/unregistriert Hooks automatisch +- ✅ Tests bestanden + +--- + +## Phase 3: Plugin-Isolation (Punkt 5) + +**Ziel:** Direkte Cross-Plugin-Imports werden durch Linting verhindert. + +### 3.1 Linting-Regel erstellen (2 Std) + +**Neue Datei: `.ruff/rules/no_cross_plugin_imports.py`** + +```python +"""Ruff rule: forbid direct imports from app.plugins.builtins.* (except contracts).""" + +# Erlaubt: +# from app.plugins.builtins.contracts import get_contract +# from app.plugins.builtins..contracts import ... +# +# Verboten: +# from app.plugins.builtins..services import ... +# from app.plugins.builtins..models import ... +# from app.plugins.builtins..routes import ... +``` + +### 3.2 CI/CD Integration (1 Std) + +- `ruff check` in GitHub Actions / Forgejo CI +- Pre-commit Hook für lokale Entwicklung +- Fehler bei direkten Cross-Plugin-Imports + +### 3.3 Ausnahmen definieren (1 Std) + +- `conftest.py` — Tests dürfen direkt importieren +- `app/plugins/builtins/__init__.py` — Plugin-Discovery +- `app/plugins/registry.py` — Registry darf importieren + +### Meilenstein Phase 3: +- ✅ Linting-Regel aktiv +- ✅ CI/CD prüft bei jedem Commit +- ✅ 0 direkte Cross-Plugin-Imports (automatisch erzwungen) + +--- + +## Phase 4: Plugin-Versioning (Punkt 8) + +**Ziel:** Vollständige Versionsverwaltung mit SemVer, Rollback und Kompatibilitäts-Check. + +### 4.1 SemVer-Vergleich (3 Std) + +**Neue Datei: `app/plugins/semver.py`** + +```python +"""Semantic version comparison for plugin versions.""" + +from dataclasses import dataclass +import re + +@dataclass +class SemVer: + major: int + minor: int + patch: int + prerelease: str = "" + + @classmethod + def parse(cls, version: str) -> "SemVer": + match = re.match(r"(\d+)\.(\d+)\.(\d+)(?:-(.+))?", version) + if not match: + raise ValueError(f"Invalid semver: {version}") + return cls(int(match[1]), int(match[2]), int(match[3]), match[4] or "") + + def __lt__(self, other): ... + def __eq__(self, other): ... + def __le__(self, other): ... + def __gt__(self, other): ... + + def is_breaking_change(self, other: "SemVer") -> bool: + return self.major != other.major + + def is_compatible_with(self, min_version: "SemVer") -> bool: + return self >= min_version +``` + +**Änderung in `registry.py`:** +```python +# VORHER: String-Vergleich +if record.version != plugin.manifest.version: + +# NACHHER: SemVer-Vergleich +old_ver = SemVer.parse(record.version) +new_ver = SemVer.parse(plugin.manifest.version) +if old_ver != new_ver: + if new_ver < old_ver: + # Downgrade — nur mit Rollback-Migration + ... +``` + +### 4.2 Rollback-Migrationen (6 Std) + +**Erweiterung des Migration-Systems:** + +```python +# MigrationRunner erweitern: +async def run_migration_down(self, db, plugin_name, migration_filename): + """Run rollback (down) migration.""" + # Suche _down.sql oder parse DOWNGRADE-Block + +async def rollback_to_version(self, db, plugin_name, target_version: str): + """Rollback plugin to a specific version.""" + # 1. Finde alle Migrationen nach target_version + # 2. Führe sie in umgekehrter Reihenfolge aus + # 3. Aktualisiere DB-Version +``` + +**Migration-Datei-Format:** +```sql +-- 0001_initial.sql +-- UP: +CREATE TABLE ...; +-- DOWN: +DROP TABLE ... CASCADE; +``` + +Oder separate Dateien: +- `0001_initial_up.sql` +- `0001_initial_down.sql` + +### 4.3 Version-Kompatibilitäts-Check (3 Std) + +**Manifest-Erweiterung:** +```python +class PluginManifest(BaseModel): + # ... existing fields ... + min_app_version: str = Field( + default="0.0.0", + description="Minimum LeoCRM version required" + ) +``` + +**Check bei Installation:** +```python +async def install(self, db, name): + plugin = self.get_plugin(name) + # Check app version compatibility + app_version = SemVer.parse(settings.app_version) + min_version = SemVer.parse(plugin.manifest.min_app_version) + if app_version < min_version: + raise ValueError( + f"Plugin '{name}' requires LeoCRM >= {plugin.manifest.min_app_version}, " + f"but current version is {settings.app_version}" + ) +``` + +### 4.4 Update-Benachrichtigung im Frontend (4 Std) + +**Backend:** +- `GET /api/v1/plugins/updates` — Liste Plugins mit verfügbarer neuer Version +- Vergleich mit Marketplace-Registry (wenn verfügbar) oder lokaler Version + +**Frontend:** +- Badge im Plugin-Settings: "Update verfügbar (1.2.0 → 1.3.0)" +- Update-Button: Löst Update aus (führt neue Migrationen aus) +- Changelog-Anzeige (optional) + +### 4.5 Tests (4 Std) + +- `test_semver.py` — SemVer-Vergleich, Parse, Edge Cases +- `test_versioning.py` — Upgrade, Downgrade, Kompatibilitäts-Check +- `test_rollback.py` — Rollback-Migrationen +- Integrationstests: Version-Update löst Migrationen aus + +### Meilenstein Phase 4: +- ✅ SemVer-Vergleich statt String-Vergleich +- ✅ Rollback-Migrationen funktionieren +- ✅ min_app_version wird geprüft +- ✅ Frontend zeigt Update-Benachrichtigungen +- ✅ Tests bestanden + +--- + +## Phase 5: Marketplace-Vorbereitung (Punkt 6) + +**Ziel:** Code so vorbereiten, dass ein Marketplace nur noch gebaut werden muss — ohne Systemänderungen. + +**Wichtig:** Funktioniert auch OHNE Marketplace — Built-in Plugins laufen normal weiter. + +### 5.1 Externe Plugin-Discovery (6 Std) + +**Erweiterung `registry.py`:** + +```python +class PluginRegistry: + + def discover_all(self) -> list[str]: + """Discover built-in AND external plugins.""" + discovered = self.discover_builtins() + discovered.extend(self.discover_external()) + return discovered + + def discover_external(self) -> list[str]: + """Discover plugins from external plugins/ directory.""" + external_dir = Path(settings.external_plugins_path or "plugins") + if not external_dir.exists(): + return [] + + discovered = [] + for plugin_dir in external_dir.iterdir(): + if not plugin_dir.is_dir() or plugin_dir.name.startswith("_"): + continue + # Look for plugin.py or __init__.py with BasePlugin subclass + plugin_file = plugin_dir / "plugin.py" + if not plugin_file.exists(): + continue + # Import and register + import sys + sys.path.insert(0, str(external_dir)) + try: + module = importlib.import_module(f"{plugin_dir.name}.plugin") + # ... find BasePlugin subclass ... + finally: + sys.path.remove(str(external_dir)) + return discovered +``` + +### 5.2 Plugin-Signatur-Validierung (8 Std) + +**Neue Datei: `app/plugins/signature.py`** + +```python +"""Plugin signature verification for external plugins.""" + +from pathlib import Path +import hashlib +import hmac + +# Ed25519 oder HMAC-SHA256 Signatur + +class PluginSignature: + """Verify plugin package signatures.""" + + @staticmethod + def verify_signature(zip_path: Path, signature: bytes, public_key: bytes) -> bool: + """Verify Ed25519 signature of plugin ZIP.""" + # 1. Read ZIP content + # 2. Compute hash + # 3. Verify signature with public key + pass + + @staticmethod + def compute_hash(zip_path: Path) -> bytes: + """Compute SHA-256 hash of plugin ZIP.""" + pass + + @staticmethod + def sign_plugin(zip_path: Path, private_key: bytes) -> bytes: + """Sign a plugin ZIP (for plugin authors).""" + pass +``` + +### 5.3 Plugin-Allowlist (4 Std) + +**Neue Alembic-Migration: `0044_plugin_allowlist.py`** + +```python +# Tabelle: plugin_allowlist +# - id: UUID +# - plugin_name: VARCHAR(80) +# - allowed_hash: VARCHAR(64) # SHA-256 +# - allowed_signature: TEXT # Ed25519 signature +# - added_by: UUID (user) +# - created_at: TIMESTAMPTZ +# - is_active: BOOLEAN +``` + +### 5.4 Plugin-Metadata-Erweiterung (4 Std) + +**Manifest-Erweiterung:** +```python +class PluginManifest(BaseModel): + # ... existing fields ... + author: str = Field(default="", description="Plugin author") + author_email: str = Field(default="", description="Author contact") + homepage: str = Field(default="", description="Plugin homepage URL") + license: str = Field(default="MIT", description="License") + min_app_version: str = Field(default="0.0.0") + icon: str = Field(default="", description="Icon URL or emoji") + screenshots: list[str] = Field(default_factory=list) + changelog: str = Field(default="", description="Changelog URL or text") + tags: list[str] = Field(default_factory=list, description="Marketplace categories") + price: float = Field(default=0.0, description="Price (0 = free)") +``` + +### 5.5 Plugin-Download-Endpoint (4 Std) + +**Neue Route: `POST /api/v1/plugins/install-marketplace`** + +```python +@router.post("/install-marketplace") +async def install_from_marketplace( + body: MarketplaceInstall, + db: AsyncSession = Depends(get_db), + current_user: dict = Depends(require_permission("plugins:configure")), +): + """Install a plugin from the marketplace. + + 1. Download ZIP from marketplace URL + 2. Verify signature against allowlist + 3. Validate manifest + 4. Check dangerous imports + 5. Validate migration SQL + 6. Install (migrations + DB record) + 7. Activate (optional) + """ + # 1. Download + async with httpx.AsyncClient() as client: + resp = await client.get(body.url) + zip_data = resp.content + + # 2. Verify signature + if not PluginSignature.verify_signature(zip_data, body.signature, public_key): + raise HTTPException(403, "Invalid plugin signature") + + # 3-6. Validate and install + # ... (reuse existing validation + install logic) +``` + +### 5.6 Plugin-Update-Check (4 Std) + +```python +@router.get("/updates") +async def check_plugin_updates( + db: AsyncSession = Depends(get_db), + current_user: dict = Depends(require_permission("plugins:read")), +): + """Check for available plugin updates from marketplace.""" + # 1. Query marketplace registry (if configured) + # 2. Compare versions with installed plugins + # 3. Return list of available updates +``` + +### 5.7 Plugin-Quarantine (4 Std) + +```python +async def _quarantine_plugin(zip_path: Path) -> Path: + """Extract plugin to temp dir, validate, then move to plugins/ dir. + + 1. Extract to /tmp/plugin_upload_/ + 2. Validate manifest exists + 3. Check dangerous imports + 4. Validate migration SQL + 5. Check signature + 6. If all OK: move to plugins/ dir + 7. If any fail: delete temp dir, raise error + """ +``` + +### 5.8 Tests (8 Std) + +- `test_marketplace.py` — Download, Verify, Install Flow +- `test_signature.py` — Signatur-Validierung +- `test_allowlist.py` — Allowlist-Management +- `test_quarantine.py` — Quarantine-Validierung +- `test_external_discovery.py` — Externe Plugin-Discovery +- Integrationstests: Vollständiger Marketplace-Flow + +### Meilenstein Phase 5: +- ✅ Externe Plugins können entdeckt werden +- ✅ Signatur-Validierung funktioniert +- ✅ Allowlist schützt vor nicht autorisierten Plugins +- ✅ Marketplace-Endpoint ist vorbereitet (deaktiviert bis Marketplace live) +- ✅ Plugin-Upload bleibt deaktiviert +- ✅ Built-in Plugins laufen ohne Marketplace +- ✅ Tests bestanden + +--- + +## Zeitplan + +``` +Woche 1 (Tag 1-5): Phase 1 — Contracts (Teil 1: contracts.py + Imports) +Woche 2 (Tag 6-8): Phase 1 — Contracts (Teil 2: Deaktivierung + Tests) + (Tag 9-10): Phase 2 — Hooks/Filters-System +Woche 3 (Tag 11): Phase 3 — Plugin-Isolation + (Tag 12-14): Phase 4 — Plugin-Versioning +Woche 4 (Tag 15-19): Phase 5 — Marketplace-Vorbereitung + (Tag 20): Puffer / Bugfixes / Doku +``` + +### Abhängigkeiten +``` +Phase 1 (Contracts) ──→ Phase 3 (Isolation: Linting braucht Contracts als Ausnahme) + │ + └──→ Phase 2 (Hooks: unabhängig, kann parallel) + │ + └──→ Phase 4 (Versioning: braucht Contracts für min_app_version) + │ + └──→ Phase 5 (Marketplace: braucht alles) +``` + +### Parallelisierungsmöglichkeiten +- Phase 1 und Phase 2 können **parallel** laufen (verschiedene Entwickler) +- Phase 3 kann erst nach Phase 1 starten +- Phase 4 kann nach Phase 1 starten +- Phase 5 kann erst nach Phase 1+4 starten + +--- + +## Risiken + +| Risiko | Wahrscheinlichkeit | Auswirkung | Mitigation | +|---|---|---|---| +| Contract-Refactoring bricht bestehende Funktionalität | Mittel | Hoch | Tests nach jedem Plugin, schrittweise Migration | +| Hooks/Filters verändern Core-Verhalten | Niedrig | Mittel | Tests für alle Hook-Punkte, Priority-System | +| Externe Plugin-Discovery hat Sicherheitslücken | Mittel | Hoch | Signatur-Validierung, Quarantine, Allowlist | +| SemVer-Parse-Fehler bei bestehenden Versionen | Niedrig | Niedrig | Fallback auf String-Vergleich | +| Rollback-Migrationen löschen Daten | Mittel | Hoch | Bestätigungs-Prompt, Backup vor Rollback | + +--- + +## Erfolgskriterien + +Nach Abschluss aller 5 Phasen: + +1. ✅ **0 direkte Cross-Plugin-Imports** (grep-verifiziert, linting-enforced) +2. ✅ **Alle 16 Plugins haben contracts.py** mit klarer öffentlicher API +3. ✅ **Contracts werden bei Deaktivierung abgemeldet** +4. ✅ **Hooks/Filters-System** mit 15+ Hook-Punkten in Core-Services +5. ✅ **Plugin-Isolation** durch Linting-Regeln erzwungen +6. ✅ **SemVer-Vergleich** statt String-Vergleich +7. ✅ **Rollback-Migrationen** für alle Plugins verfügbar +8. ✅ **min_app_version** wird bei Installation geprüft +9. ✅ **Update-Benachrichtigung** im Frontend +10. ✅ **Marketplace-Endpoint** vorbereitet (deaktiviert) +11. ✅ **Signatur-Validierung** für externe Plugins +12. ✅ **Allowlist** schützt vor nicht autorisierten Plugins +13. ✅ **Externe Plugin-Discovery** funktioniert +14. ✅ **Alle Tests bestanden** +15. ✅ **Built-in Plugins laufen ohne Marketplace** + +--- + +## Dokumentation + +Nach Abschluss jeder Phase: +- `docs/plugin-system/phase-N.md` — Was wurde gemacht, was geändert +- `docs/plugin-system/contracts-api.md` — Contract-API Referenz +- `docs/plugin-system/hooks-api.md` — Hooks/Filters Referenz +- `docs/plugin-system/marketplace-api.md` — Marketplace-API Referenz +- `docs/plugin-system/plugin-development-guide.md` — Wie man ein Plugin entwickelt + +--- + +**Dieser Plan ist vollständig. Alle Aufgaben, Aufwände, Abhängigkeiten und Risiken sind erfasst.** diff --git a/alembic/versions/0044_rls_repair_and_db_roles.py b/alembic/versions/0044_rls_repair_and_db_roles.py new file mode 100644 index 0000000..5d47189 --- /dev/null +++ b/alembic/versions/0044_rls_repair_and_db_roles.py @@ -0,0 +1,125 @@ +"""RLS repair + separate DB runtime user. + +Revision ID: 0044 +Revises: 0043 +Created: 2026-07-26 + +This migration: +1. Re-discovers ALL tenant-scoped tables and ensures RLS is enabled + with FORCE + WITH CHECK (covers tables added after migration 0028). +2. Creates a separate ``crm_runtime`` role with NOSUPERUSER and + NOBYPASSRLS so the application cannot bypass RLS. +3. Grants only DML permissions (SELECT/INSERT/UPDATE/DELETE) to + ``crm_runtime`` on all tenant-scoped tables. + +IMPORTANT: After this migration, the application's DATABASE_URL must +use ``crm_runtime`` (not the superuser) for API and worker containers. +Migration/DDL operations continue to use the owner user (crm_user). +""" + +from alembic import op +import sqlalchemy as sa +import logging + +logger = logging.getLogger(__name__) + +revision = "0044" +down_revision = "0043" +branch_labels = None +depends_on = None + + +def _discover_tenant_tables(conn) -> list[str]: + """Return all table names in the public schema that have a tenant_id column.""" + result = conn.execute( + sa.text( + "SELECT table_name FROM information_schema.columns " + "WHERE table_schema = 'public' AND column_name = 'tenant_id' " + "ORDER BY table_name" + ) + ) + return [row[0] for row in result] + + +def _discover_existing_policies(conn, table_name: str) -> list[str]: + """Return all policy names on *table_name* that contain 'tenant' or 'isolation'.""" + result = conn.execute( + sa.text( + "SELECT policyname FROM pg_policies " + "WHERE schemaname = 'public' AND tablename = :t " + "AND (policyname LIKE '%tenant%' OR policyname LIKE '%isolation%')" + ), + {"t": table_name}, + ) + return [row[0] for row in result] + + +def upgrade() -> None: + conn = op.get_bind() + + # ── 1. RLS Repair: ensure all tenant tables have RLS + WITH CHECK ── + tenant_tables = _discover_tenant_tables(conn) + logger.info("RLS repair: discovered %d tenant-scoped tables: %s", len(tenant_tables), tenant_tables) + + for table_name in tenant_tables: + # Enable RLS + op.execute(f'ALTER TABLE "{table_name}" ENABLE ROW LEVEL SECURITY') + # Force RLS (applies to table owner too) + op.execute(f'ALTER TABLE "{table_name}" FORCE ROW LEVEL SECURITY') + + # Drop existing tenant policies + existing_policies = _discover_existing_policies(conn, table_name) + for policy_name in existing_policies: + op.execute(f'DROP POLICY IF EXISTS "{policy_name}" ON "{table_name}"') + logger.info("Dropped policy %s on %s", policy_name, table_name) + + # Create unified tenant isolation policy with WITH CHECK + op.execute( + f'CREATE POLICY tenant_isolation ON "{table_name}" ' + f"USING (tenant_id = current_setting('app.tenant_id', true)::uuid) " + f"WITH CHECK (tenant_id = current_setting('app.tenant_id', true)::uuid)" + ) + logger.info("Created/updated tenant_isolation policy on %s (USING + WITH CHECK)", table_name) + + # ── 2. Create crm_runtime role (NOSUPERUSER, NOBYPASSRLS) ── + # Use DO block for idempotent creation + op.execute( + sa.text( + "DO $$ " + "BEGIN " + " IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'crm_runtime') THEN " + " CREATE ROLE crm_runtime LOGIN NOSUPERUSER NOCREATEDB NOCREATEROLE " + " NOREPLICATION NOBYPASSRLS; " + " END IF; " + "END $$;" + ) + ) + logger.info("Ensured crm_runtime role exists (NOSUPERUSER, NOBYPASSRLS)") + + # ── 3. Grant DML permissions to crm_runtime on all tenant tables ── + for table_name in tenant_tables: + op.execute( + f'GRANT SELECT, INSERT, UPDATE, DELETE ON "{table_name}" TO crm_runtime' + ) + + # Grant usage on sequences (for SERIAL/IDENTITY columns) + op.execute("GRANT USAGE, SELECT ON ALL SEQUENCES IN SCHEMA public TO crm_runtime") + + logger.info("Granted DML permissions to crm_runtime on %d tables", len(tenant_tables)) + + +def downgrade() -> None: + conn = op.get_bind() + + # Revoke permissions from crm_runtime + tenant_tables = _discover_tenant_tables(conn) + for table_name in tenant_tables: + op.execute(f'REVOKE SELECT, INSERT, UPDATE, DELETE ON "{table_name}" FROM crm_runtime') + op.execute("REVOKE USAGE, SELECT ON ALL SEQUENCES IN SCHEMA public FROM crm_runtime") + + # Drop crm_runtime role + op.execute("DROP ROLE IF EXISTS crm_runtime") + logger.info("Dropped crm_runtime role") + + # Note: RLS policies are NOT reverted here to avoid weakening security. + # Migration 0028's downgrade handles the original set of tables. diff --git a/app/config.py b/app/config.py index 3972b5d..dfc571a 100644 --- a/app/config.py +++ b/app/config.py @@ -57,6 +57,9 @@ class Settings(BaseSettings): # CORS cors_origins: str = "http://localhost:5173,http://localhost:3000" + # Frontend URL for email links (password reset, invitations, etc.) + frontend_url: str = "http://localhost:5173" + # Rate Limiting rate_limit_login_max: int = 5 rate_limit_login_window: int = 900 # 15 min diff --git a/app/core/auth.py b/app/core/auth.py index 635ea10..c2195dd 100644 --- a/app/core/auth.py +++ b/app/core/auth.py @@ -91,11 +91,6 @@ def hash_token(token: str) -> str: return hashlib.sha256(token.encode()).hexdigest() -def get_redis() -> aioredis.Redis: - """Get a Redis client instance.""" - return aioredis.from_url(get_settings().redis_url, decode_responses=True) - - async def create_session( db: AsyncSession, redis: aioredis.Redis, diff --git a/app/core/jobs.py b/app/core/jobs.py index f3dad7c..aca10c4 100644 --- a/app/core/jobs.py +++ b/app/core/jobs.py @@ -80,3 +80,72 @@ async def get_job_status(job_id: str) -> dict[str, Any] | None: "start_time": job_info.start_time.isoformat() if job_info.start_time else None, "finish_time": job_info.finish_time.isoformat() if job_info.finish_time else None, } + + +# ── Password Reset Email Job ───────────────────────────────────────────────── + +async def send_password_reset_email( + ctx: dict[str, Any], + *, + user_id: str, + email: str, + raw_token: str, + expires_at: str, +) -> None: + """Send a password reset email via SMTP. + + This is an ARQ worker function. It is registered with the job registry + so the worker can execute it when the auth service enqueues it. + """ + import aiosmtplib + from email.mime.text import MIMEText + from email.mime.multipart import MIMEMultipart + + settings = get_settings() + + # Build the reset URL + reset_url = f"{settings.frontend_url.rstrip('/')}/reset-password?token={raw_token}" + + # Build the email + msg = MIMEMultipart("alternative") + msg["From"] = settings.smtp_from_email + msg["To"] = email + msg["Subject"] = "LeoCRM — Passwort zurücksetzen" + + text_body = ( + f"Sie haben angefordert, Ihr Passwort zurückzusetzen.\n\n" + f"Klicken Sie auf den folgenden Link, um ein neues Passwort zu setzen:\n" + f"{reset_url}\n\n" + f"Dieser Link ist gültig bis {expires_at}.\n\n" + f"Falls Sie diese Anfrage nicht gestellt haben, können Sie diese\n" + f"E-Mail ignorieren. Ihr Passwort bleibt unverändert.\n" + ) + html_body = ( + f"" + f"

Passwort zurücksetzen

" + f"

Sie haben angefordert, Ihr Passwort zurückzusetzen.

" + f"

Passwort jetzt zurücksetzen

" + f"

Dieser Link ist gültig bis {expires_at}.

" + f"

Falls Sie diese Anfrage nicht gestellt haben, können Sie diese " + f"E-Mail ignorieren. Ihr Passwort bleibt unverändert.

" + f"" + ) + msg.attach(MIMEText(text_body, "plain", "utf-8")) + msg.attach(MIMEText(html_body, "html", "utf-8")) + + # Send via SMTP + await aiosmtplib.send( + msg, + hostname=settings.smtp_host, + port=settings.smtp_port, + username=settings.smtp_username, + password=settings.smtp_password, + start_tls=settings.smtp_use_tls, + ) + logger.info("Password reset email sent to %s for user %s", email, user_id) + + +# Register the job so the worker can find it +from app.core.job_registry import register_job # noqa: E402 + +register_job("send_password_reset_email", send_password_reset_email) diff --git a/app/core/plugin_error_handler.py b/app/core/plugin_error_handler.py index 45907a5..669775d 100644 --- a/app/core/plugin_error_handler.py +++ b/app/core/plugin_error_handler.py @@ -1,14 +1,19 @@ """Plugin error isolation wrapper.""" import logging import functools +from fastapi import UploadFile as _UploadFile # noqa: F401 — needed for ForwardRef resolution from fastapi.responses import JSONResponse logger = logging.getLogger(__name__) def wrap_plugin_route(handler): - """Decorator that isolates plugin route errors and returns structured JSON.""" - @functools.wraps(handler) + """Decorator that isolates plugin route errors and returns structured JSON. + + Does NOT use functools.wraps to avoid copying __annotations__ and + __wrapped__ — FastAPI would otherwise try to resolve + ``ForwardRef('UploadFile')`` from the original handler's signature. + """ async def wrapper(*args, **kwargs): try: return await handler(*args, **kwargs) @@ -18,4 +23,8 @@ def wrap_plugin_route(handler): status_code=500, content={'detail': f'Plugin error: {exc}', 'code': 'plugin_error'} ) + # Preserve identity for debugging but NOT __wrapped__ or __annotations__ + wrapper.__name__ = getattr(handler, '__name__', 'wrapper') + wrapper.__module__ = getattr(handler, '__module__', __name__) + wrapper.__qualname__ = getattr(handler, '__qualname__', 'wrapper') return wrapper diff --git a/app/core/worker.py b/app/core/worker.py index 2525f2b..e862094 100644 --- a/app/core/worker.py +++ b/app/core/worker.py @@ -90,11 +90,59 @@ def _get_redis_settings() -> RedisSettings: async def on_startup(ctx: dict[str, Any]) -> None: """Called when worker starts.""" logger.info("ARQ worker starting...") + + # Initialize Redis singleton (same as API lifespan) + from app.core.auth import init_redis + await init_redis() + + # Initialize service container + from app.core.service_container import get_container + container = get_container() + await container.initialize() + + # Initialize plugin registry and discover built-in plugins + from app.plugins.registry import get_registry + from app.core.db import get_engine + from app.core.event_bus import get_event_bus + from app.core.webhook_dispatcher import register_webhook_event_handlers + from sqlalchemy import select as sa_select + from app.models.plugin import Plugin as PluginModel + from sqlalchemy.ext.asyncio import async_sessionmaker + + registry = get_registry() + registry.initialize(get_engine(), app=None) + registry.discover_builtins() + + event_bus = get_event_bus() + async_session = async_sessionmaker(get_engine(), expire_on_commit=False) + + # Activate plugins that are marked active in DB (register event handlers) + async with async_session() as db: + for name in registry.resolve_load_order(): + plugin = registry.get_plugin(name) + if plugin is None: + continue + result = await db.execute( + sa_select(PluginModel).where(PluginModel.name == name) + ) + plugin_record = result.scalar_one_or_none() + if plugin_record is None or not plugin_record.active: + continue + try: + await plugin.on_activate(db, container, event_bus) + logger.info(f"Worker: activated plugin {name}") + except Exception as exc: + logger.error(f"Worker: failed to activate plugin {name}: {exc}") + await db.commit() + + # Register webhook dispatcher on the event bus + register_webhook_event_handlers(event_bus) + logger.info("Worker: webhook event handlers registered") + # Register search providers (normally done by app startup) try: - from app.core.db import get_session_factory from app.plugins.builtins.unified_search.provider_registry import auto_register_providers - factory = get_session_factory() + factory = async_session async with factory() as db: await auto_register_providers(db) logger.info("Search providers registered for worker") @@ -105,6 +153,8 @@ async def on_startup(ctx: dict[str, Any]) -> None: async def on_shutdown(ctx: dict[str, Any]) -> None: """Called when worker shuts down.""" logger.info("ARQ worker shutting down...") + from app.core.auth import close_redis + await close_redis() # --------------------------------------------------------------------------- diff --git a/app/deps.py b/app/deps.py index 50a3a32..8573f00 100644 --- a/app/deps.py +++ b/app/deps.py @@ -224,3 +224,34 @@ async def get_current_user_id( ) -> uuid.UUID: """Extract user_id from current user session.""" return uuid.UUID(current_user["user_id"]) + + +def require_active_plugin(plugin_name: str): + """FastAPI dependency factory: require that a plugin is active. + + Returns 403 if the plugin is not active in the permission registry. + This allows routes to be registered at app creation time while + enforcing activation status at request time. + """ + async def _check( + current_user: dict[str, Any] = Depends(get_current_user), + ) -> dict[str, Any]: + from app.core.permission_registry import get_permission_registry + try: + registry = get_permission_registry() + if not registry.is_plugin_active(plugin_name): + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail={ + "detail": f"Plugin '{plugin_name}' is not active", + "code": "plugin_inactive", + }, + ) + except HTTPException: + raise + except Exception: + # If registry not initialized yet, allow request (startup race) + pass + return current_user + + return _check diff --git a/app/main.py b/app/main.py index 90e7b38..1172b8d 100644 --- a/app/main.py +++ b/app/main.py @@ -6,7 +6,7 @@ import time import traceback from contextlib import asynccontextmanager -from fastapi import FastAPI, HTTPException, Request +from fastapi import FastAPI, HTTPException, Request, Depends from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import FileResponse, JSONResponse from fastapi.staticfiles import StaticFiles @@ -208,6 +208,11 @@ async def lifespan(app: FastAPI): init_permission_registry(active_plugin_names) logger.info("Permission registry initialized with %d active plugins", len(active_plugin_names)) + # Register webhook dispatcher on the event bus + from app.core.webhook_dispatcher import register_webhook_event_handlers + register_webhook_event_handlers(event_bus) + logger.info("Webhook event handlers registered") + # Register field definitions from active plugins only from app.core.permission_registry import get_permission_registry for name in active_plugin_names: @@ -368,9 +373,10 @@ def create_app() -> FastAPI: app.include_router(errors.router) # ── Register plugin routes for all built-in plugins ── - # Routes are registered here (before app start); activation status - # is enforced at runtime via require_permission and plugin checks. + # Routes are registered at app creation time so OpenAPI docs are complete. + # Activation status is enforced per-request via require_active_plugin(). import importlib + from app.deps import require_active_plugin # Discover all built-in plugin modules and register their routes plugin_modules = [ "app.plugins.builtins.tags", @@ -399,6 +405,7 @@ def create_app() -> FastAPI: for attr_name in dir(mod): attr = getattr(mod, attr_name) if isinstance(attr, type) and hasattr(attr, "manifest") and hasattr(attr.manifest, "routes"): + plugin_name = getattr(attr.manifest, "name", mod_name.split(".")[-1]) for route_def in attr.manifest.routes: try: router_module = importlib.import_module(route_def.module) @@ -407,13 +414,16 @@ def create_app() -> FastAPI: for route in router.routes: if hasattr(route, 'endpoint'): route.endpoint = wrap_plugin_route(route.endpoint) - app.include_router(router) + # Add active-plugin check as a router-level dependency + app.include_router( + router, + dependencies=[Depends(require_active_plugin(plugin_name))], + ) except Exception as exc: logger.error(f"Failed to register route {route_def.module}.{route_def.router_attr}: {exc}") break except Exception as exc: logger.error(f"Failed to register plugin routes for {mod_name}: {exc}") - # Do NOT register plugin routes here — lifespan() handles it for active plugins only # ── Serve frontend static files (SPA) ────────────────────────────── # Mount built frontend assets (JS, CSS, images) diff --git a/app/models/backup.py b/app/models/backup.py index e356749..9f78451 100644 --- a/app/models/backup.py +++ b/app/models/backup.py @@ -5,7 +5,7 @@ from __future__ import annotations import uuid from datetime import datetime -from sqlalchemy import BigInteger, DateTime, String, Text +from sqlalchemy import BigInteger, DateTime, String, Text, func from sqlalchemy.dialects.postgresql import UUID as PGUUID from sqlalchemy.orm import Mapped, mapped_column @@ -39,7 +39,7 @@ class Backup(Base, TenantMixin): PGUUID(as_uuid=True), nullable=True ) created_at: Mapped[datetime] = mapped_column( - DateTime(timezone=True), nullable=False, default=datetime.utcnow + DateTime(timezone=True), nullable=False, server_default=func.now() ) completed_at: Mapped[datetime | None] = mapped_column( DateTime(timezone=True), nullable=True, default=None diff --git a/app/models/notification.py b/app/models/notification.py index 62b713f..82143e7 100644 --- a/app/models/notification.py +++ b/app/models/notification.py @@ -35,7 +35,7 @@ class Notification(Base, TenantMixin): user_id: Mapped[uuid.UUID] = mapped_column( PGUUID(as_uuid=True), ForeignKey("users.id", ondelete="CASCADE"), nullable=False, index=True ) - type: Mapped[str] = mapped_column(String(20), nullable=False) + type: Mapped[str] = mapped_column(String(100), nullable=False) title: Mapped[str] = mapped_column(String(200), nullable=False) body: Mapped[str | None] = mapped_column(Text, nullable=True) read_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) @@ -83,5 +83,5 @@ class NotificationPreference(Base, TenantMixin): ForeignKey("users.id", ondelete="CASCADE"), nullable=False, ) - type_key: Mapped[str] = mapped_column(String(20), nullable=False) + type_key: Mapped[str] = mapped_column(String(100), nullable=False) is_enabled: Mapped[bool] = mapped_column(Boolean, nullable=False, default=True) diff --git a/app/models/system_settings.py b/app/models/system_settings.py index 83027ea..a588e9c 100644 --- a/app/models/system_settings.py +++ b/app/models/system_settings.py @@ -6,6 +6,7 @@ import uuid from datetime import datetime from sqlalchemy import DateTime, ForeignKey, Index, Integer, String +from sqlalchemy.dialects.postgresql import JSONB from sqlalchemy.dialects.postgresql import UUID as PGUUID from sqlalchemy.orm import Mapped, mapped_column @@ -51,5 +52,4 @@ class SystemSettings(Base, TenantMixin): theme_font_family: Mapped[str] = mapped_column(String(100), nullable=False, default="Inter") theme_border_radius: Mapped[str] = mapped_column(String(20), nullable=False, default="0.5rem") # Automation plugin settings (JSONB) - from sqlalchemy.dialects.postgresql import JSONB automation_config: Mapped[dict | None] = mapped_column(JSONB, nullable=True) diff --git a/app/models/tax.py b/app/models/tax.py index 93c53c3..a842cb1 100644 --- a/app/models/tax.py +++ b/app/models/tax.py @@ -4,6 +4,7 @@ from __future__ import annotations import uuid from datetime import datetime +from decimal import Decimal from sqlalchemy import Boolean, DateTime, Index, Numeric, String from sqlalchemy.dialects.postgresql import UUID as PGUUID @@ -25,6 +26,6 @@ class TaxRate(Base, TenantMixin): PGUUID(as_uuid=True), primary_key=True, default=uuid.uuid4 ) name: Mapped[str] = mapped_column(String(100), nullable=False) - rate: Mapped[float] = mapped_column(Numeric(5, 2), nullable=False) + rate: Mapped[Decimal] = mapped_column(Numeric(5, 2), nullable=False) is_default: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) country: Mapped[str | None] = mapped_column(String(2), nullable=True) diff --git a/app/models/webhook.py b/app/models/webhook.py index 6f51f6a..5929b05 100644 --- a/app/models/webhook.py +++ b/app/models/webhook.py @@ -41,3 +41,8 @@ class Webhook(Base, TenantMixin): updated_by: Mapped[uuid.UUID | None] = mapped_column( PGUUID(as_uuid=True), nullable=True ) + + @property + def has_secret(self) -> bool: + """Return True if a webhook secret is set (never expose the secret itself).""" + return self.secret is not None and len(self.secret) > 0 diff --git a/app/plugins/builtins/ai_assistant/routes.py b/app/plugins/builtins/ai_assistant/routes.py index d4e35cd..e708771 100644 --- a/app/plugins/builtins/ai_assistant/routes.py +++ b/app/plugins/builtins/ai_assistant/routes.py @@ -698,7 +698,7 @@ ATTACHMENT_DIR = Path(os.environ.get("STORAGE_PATH", "/data/storage")) / "ai_att MAX_ATTACHMENT_SIZE = 25 * 1024 * 1024 # 25MB -@router.post("/sessions/{session_id}/attachments", dependencies=[Depends(require_permission("ai:write"))]) +@router.post("/sessions/{session_id}/attachments", response_model=None, dependencies=[Depends(require_permission("ai:write"))]) async def upload_attachment( session_id: str, file: UploadFile, diff --git a/app/plugins/builtins/calendar/routes.py b/app/plugins/builtins/calendar/routes.py index a63aa28..a4be812 100644 --- a/app/plugins/builtins/calendar/routes.py +++ b/app/plugins/builtins/calendar/routes.py @@ -820,7 +820,7 @@ async def ics_feed( return Response(content=ics_content, media_type="text/calendar") -@router.post("/calendar/import", dependencies=[Depends(require_permission("calendar:write"))]) +@router.post("/calendar/import", response_model=None, dependencies=[Depends(require_permission("calendar:write"))]) async def import_ics( file: UploadFile = File(...), calendar_id: str | None = None, diff --git a/app/plugins/builtins/dms/routes.py b/app/plugins/builtins/dms/routes.py index dba2a43..a6c9850 100644 --- a/app/plugins/builtins/dms/routes.py +++ b/app/plugins/builtins/dms/routes.py @@ -417,7 +417,7 @@ async def delete_folder( # ─── Files ─── -@router.post("/files/upload", status_code=status.HTTP_201_CREATED, dependencies=[Depends(require_permission("dms:write"))]) +@router.post("/files/upload", status_code=status.HTTP_201_CREATED, response_model=None, dependencies=[Depends(require_permission("dms:write"))]) async def upload_file( file: UploadFile = File(...), folder_id: str | None = Form(None), @@ -441,35 +441,35 @@ async def upload_file( if folder_result.scalar_one_or_none() is None: raise HTTPException(404, detail={"detail": "Folder not found", "code": "not_found"}) - # Stream file in chunks — avoid loading entire file into RAM + # Stream file to storage — avoid loading entire file into RAM import hashlib CHUNK_SIZE = 1024 * 1024 # 1MB chunks sha256 = hashlib.sha256() file_size = 0 - chunks: list[bytes] = [] - while True: - chunk = await file.read(CHUNK_SIZE) - if not chunk: - break - file_size += len(chunk) - if file_size > MAX_FILE_SIZE: - raise HTTPException( - 413, detail={"detail": "File too large (max 100MB)", "code": "file_too_large"} - ) - sha256.update(chunk) - chunks.append(chunk) - - content_hash = sha256.hexdigest() + async def chunk_stream(): + nonlocal file_size + while True: + chunk = await file.read(CHUNK_SIZE) + if not chunk: + break + file_size += len(chunk) + if file_size > MAX_FILE_SIZE: + raise HTTPException( + 413, detail={"detail": "File too large (max 100MB)", "code": "file_too_large"} + ) + sha256.update(chunk) + yield chunk # Create file record file_id = uuid.uuid4() storage_path = _file_storage_path(tenant_id, file_id) - # Save file via storage backend + # Save file via storage backend (true streaming, no RAM accumulation) storage = get_storage_backend() - await storage.save(storage_path, b"".join(chunks)) - del chunks # Free memory + await storage.save_stream(storage_path, chunk_stream()) + + content_hash = sha256.hexdigest() mime_type = file.content_type or "application/octet-stream" diff --git a/app/plugins/builtins/kommunikation/routes.py b/app/plugins/builtins/kommunikation/routes.py index 3f46192..8f9e859 100644 --- a/app/plugins/builtins/kommunikation/routes.py +++ b/app/plugins/builtins/kommunikation/routes.py @@ -338,7 +338,7 @@ async def delete_msg( # ─── Attachments ─── -@router.post("/messages/{message_id}/attachments", dependencies=[Depends(require_permission("comm:write"))]) +@router.post("/messages/{message_id}/attachments", response_model=None, dependencies=[Depends(require_permission("comm:write"))]) async def upload_attachment( message_id: str, file: UploadFile = File(...), diff --git a/app/plugins/builtins/mail/routes.py b/app/plugins/builtins/mail/routes.py index e15b13b..1f46212 100644 --- a/app/plugins/builtins/mail/routes.py +++ b/app/plugins/builtins/mail/routes.py @@ -722,7 +722,7 @@ async def sync_folder( # ─── Attachment Upload (F-MAIL-04) ─── -@router.post("/upload-attachment") +@router.post("/upload-attachment", response_model=None) async def upload_attachment( file: UploadFile = File(...), db: AsyncSession = Depends(get_db), diff --git a/app/plugins/builtins/test_sample/__init__.py b/app/plugins/builtins/test_sample/__init__.py new file mode 100644 index 0000000..9395844 --- /dev/null +++ b/app/plugins/builtins/test_sample/__init__.py @@ -0,0 +1,49 @@ +"""Test sample plugin for LeoCRM plugin system testing.""" + +from __future__ import annotations + +from typing import Any + +from app.plugins.base import BasePlugin +from app.plugins.manifest import PluginManifest + + +class TestSamplePlugin(BasePlugin): + """A sample plugin for testing the plugin lifecycle.""" + + manifest = PluginManifest( + name="test_sample", + version="1.0.0", + display_name="Test Sample Plugin", + description="A sample plugin for testing install/activate/deactivate/uninstall lifecycle.", + dependencies=[], + routes=[], + events=["contact.created"], + migrations=["0001_test_plugin.sql"], + permissions=[], + ) + + def __init__(self) -> None: + super().__init__() + self.install_called = False + self.activate_called = False + self.deactivate_called = False + self.uninstall_called = False + self.event_log: list[dict[str, Any]] = [] + + async def on_install(self, db, service_container) -> None: + self.install_called = True + + async def on_activate(self, db, service_container, event_bus) -> None: + self.activate_called = True + await super().on_activate(db, service_container, event_bus) + + async def on_deactivate(self, db, service_container, event_bus) -> None: + self.deactivate_called = True + await super().on_deactivate(db, service_container, event_bus) + + async def on_uninstall(self, db, service_container) -> None: + self.uninstall_called = True + + async def on_contact_created(self, payload: dict[str, Any]) -> None: + self.event_log.append({"event": "contact.created", "payload": payload}) diff --git a/app/plugins/builtins/test_sample/migrations/0001_test_plugin.sql b/app/plugins/builtins/test_sample/migrations/0001_test_plugin.sql new file mode 100644 index 0000000..ea60d00 --- /dev/null +++ b/app/plugins/builtins/test_sample/migrations/0001_test_plugin.sql @@ -0,0 +1,9 @@ +-- Test sample plugin migration: creates a test table with tenant_id +CREATE TABLE IF NOT EXISTS test_sample_items ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + tenant_id UUID NOT NULL REFERENCES tenants(id) ON DELETE CASCADE, + name VARCHAR(100) NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE INDEX IF NOT EXISTS ix_test_sample_items_tenant ON test_sample_items(tenant_id); diff --git a/app/schemas/webhook.py b/app/schemas/webhook.py index 49eec94..6949abb 100644 --- a/app/schemas/webhook.py +++ b/app/schemas/webhook.py @@ -39,7 +39,7 @@ class WebhookResponse(BaseModel): tenant_id: uuid.UUID url: str events: list[str] - secret: str | None = None + has_secret: bool = False # Only indicate if a secret is set, never return it is_active: bool = True retry_count: int = 3 timeout_seconds: int = 30 diff --git a/app/services/auth_service.py b/app/services/auth_service.py index 9a3ac92..44122e8 100644 --- a/app/services/auth_service.py +++ b/app/services/auth_service.py @@ -237,8 +237,7 @@ class AuthService: except Exception: logger.warning( "ARQ enqueue failed for password reset email — " - "raw_token for development: %s", - raw_token, + "email will not be sent. Check Redis/ARQ connectivity.", exc_info=True, ) diff --git a/app/services/webhook_service.py b/app/services/webhook_service.py index 77febf7..264c558 100644 --- a/app/services/webhook_service.py +++ b/app/services/webhook_service.py @@ -4,10 +4,13 @@ from __future__ import annotations import hashlib import hmac +import ipaddress import json import logging +import socket import uuid from typing import Any +from urllib.parse import urlparse import httpx from sqlalchemy import select, delete @@ -18,6 +21,45 @@ from app.models.webhook import Webhook logger = logging.getLogger(__name__) +def _validate_webhook_url(url: str) -> None: + """Validate a webhook URL to prevent SSRF attacks. + + Blocks: + - Non-http(s) schemes + - Private/internal IP ranges (10.x, 172.16-31.x, 192.168.x, 127.x, 169.254.x, ::1) + - Hostnames that resolve to private IPs (DNS rebinding) + - Redirects (handled by httpx follow_redirects=False) + """ + parsed = urlparse(url) + + # Protocol allowlist + if parsed.scheme not in ("http", "https"): + raise ValueError(f"Webhook URL must use http or https, got: {parsed.scheme}") + + hostname = parsed.hostname + if not hostname: + raise ValueError("Webhook URL has no hostname") + + # Check if hostname is an IP address + try: + ip = ipaddress.ip_address(hostname) + if ip.is_private or ip.is_loopback or ip.is_link_local or ip.is_reserved: + raise ValueError(f"Webhook URL points to private/reserved IP: {ip}") + except ValueError as exc: + if "points to private" in str(exc) or "must use" in str(exc): + raise + # Not an IP — resolve hostname and check + try: + resolved = socket.getaddrinfo(hostname, None) + for family, _, _, _, sockaddr in resolved: + addr = sockaddr[0] + ip_obj = ipaddress.ip_address(addr) + if ip_obj.is_private or ip_obj.is_loopback or ip_obj.is_link_local or ip_obj.is_reserved: + raise ValueError(f"Webhook hostname '{hostname}' resolves to private IP: {addr}") + except socket.gaierror: + raise ValueError(f"Cannot resolve webhook hostname: {hostname}") + + async def list_webhooks( db: AsyncSession, tenant_id: uuid.UUID, @@ -151,8 +193,18 @@ async def send_webhook( signature = _sign_payload(body_bytes, webhook.secret) headers["X-Webhook-Signature"] = f"sha256={signature}" + # SSRF protection: validate URL before sending try: - async with httpx.AsyncClient(timeout=webhook.timeout_seconds) as client: + _validate_webhook_url(webhook.url) + except ValueError as exc: + logger.warning("Webhook URL validation failed for %s: %s", webhook.url, exc) + return {"success": False, "status_code": None, "error": f"URL validation failed: {exc}"} + + try: + async with httpx.AsyncClient( + timeout=webhook.timeout_seconds, + follow_redirects=False, # Prevent SSRF via redirect + ) as client: response = await client.post( webhook.url, content=body_bytes, diff --git a/docker-compose.yml b/docker-compose.yml index 6fa081a..27f74ae 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -34,8 +34,8 @@ services: PGDATA: /var/lib/postgresql/data/pgdata volumes: - pgdata:/var/lib/postgresql/data - ports: - - "5432:5432" # local-only; remove for prod-like runs + # No exposed ports — only internal Docker network access + # For local debugging, uncomment: ports: ["127.0.0.1:5432:5432"] healthcheck: test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER:-crm_user} -d ${POSTGRES_DB:-crm_db}"] interval: 10s @@ -50,13 +50,13 @@ services: image: redis:7-alpine container_name: crm-redis restart: unless-stopped - command: redis-server --requirepass ${REDIS_PASSWORD:-changeme} + command: redis-server --requirepass ${REDIS_PASSWORD:?REDIS_PASSWORD is required} volumes: - redisdata:/data - ports: - - "6379:6379" # local-only; remove for prod-like runs + # No exposed ports — only internal Docker network access + # For local debugging, uncomment: ports: ["127.0.0.1:6379:6379"] healthcheck: - test: ["CMD", "redis-cli", "-a", "${REDIS_PASSWORD:-changeme}", "ping"] + test: ["CMD", "redis-cli", "-a", "${REDIS_PASSWORD}", "ping"] interval: 10s timeout: 5s retries: 5 @@ -76,10 +76,14 @@ services: redis: condition: service_healthy environment: + # App/worker uses crm_runtime (NOSUPERUSER, NOBYPASSRLS) — RLS enforced DATABASE_URL: ${DATABASE_URL:?DATABASE_URL is required} - REDIS_URL: ${REDIS_URL:-redis://:${REDIS_PASSWORD:-changeme}@redis:6379/0} + # Migration/DDL uses crm_user (owner, can bypass RLS for DDL) + MIGRATION_DATABASE_URL: ${MIGRATION_DATABASE_URL:-postgresql+asyncpg://${POSTGRES_USER:-crm_user}:${POSTGRES_PASSWORD}@postgres:5432/${POSTGRES_DB:-crm_db}} + REDIS_URL: ${REDIS_URL:-redis://:${REDIS_PASSWORD}@redis:6379/0} SECRET_KEY: ${SECRET_KEY:?SECRET_KEY is required (min 32 chars)} CORS_ORIGINS: ${CORS_ORIGINS:-http://localhost:8000,http://localhost:5173} + FRONTEND_URL: ${FRONTEND_URL:-http://localhost:8000} ENVIRONMENT: ${ENVIRONMENT:-production} LOG_LEVEL: ${LOG_LEVEL:-INFO} SESSION_COOKIE_SECURE: ${SESSION_COOKIE_SECURE:-true} @@ -120,9 +124,12 @@ services: condition: service_healthy entrypoint: ["/app/worker.sh"] environment: + # Worker uses crm_runtime too — RLS enforced DATABASE_URL: ${DATABASE_URL:?DATABASE_URL is required} - REDIS_URL: ${REDIS_URL:-redis://:${REDIS_PASSWORD:-changeme}@redis:6379/0} + MIGRATION_DATABASE_URL: ${MIGRATION_DATABASE_URL:-postgresql+asyncpg://${POSTGRES_USER:-crm_user}:${POSTGRES_PASSWORD}@postgres:5432/${POSTGRES_DB:-crm_db}} + REDIS_URL: ${REDIS_URL:-redis://:${REDIS_PASSWORD}@redis:6379/0} SECRET_KEY: ${SECRET_KEY:?SECRET_KEY is required (min 32 chars)} + FRONTEND_URL: ${FRONTEND_URL:-http://localhost:8000} ENVIRONMENT: ${ENVIRONMENT:-production} LOG_LEVEL: ${LOG_LEVEL:-INFO} SESSION_COOKIE_SECURE: ${SESSION_COOKIE_SECURE:-true} diff --git a/dump.rdb b/dump.rdb deleted file mode 100644 index 7d06d0bfd49766d0aab16809e0e45d53f3d665ab..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 88 zcmWG?b@2=~FfcUw#aWb^l3A=H&uTqSINJ92WPm0sz6QB5VKv diff --git a/prestart.sh b/prestart.sh index ab4196f..75cce07 100755 --- a/prestart.sh +++ b/prestart.sh @@ -3,20 +3,45 @@ # prestart.sh — Container entrypoint for CRM API container # # Responsibilities: -# 1. Run Alembic DB migrations (alembic upgrade head). -# 2. Start uvicorn as PID 1 (so signals like SIGTERM are forwarded correctly). +# 1. Run Alembic DB migrations (alembic upgrade head) using the owner user. +# 2. Set the crm_runtime password (for RLS-enforced app access). +# 3. Start uvicorn as PID 1 (so signals like SIGTERM are forwarded correctly). # # Notes: # - `set -e` ensures the container crashes loudly if migrations fail. # - The ARQ worker runs in a separate container (see worker.sh / docker-compose). +# - Migrations use MIGRATION_DATABASE_URL (crm_user, owner, can bypass RLS). +# - The app uses DATABASE_URL (crm_runtime, NOSUPERUSER, NOBYPASSRLS). # ============================================================================= set -e -echo "[prestart] $(date -u +%Y-%m-%dT%H:%M:%SZ) - Running alembic upgrade head..." -alembic upgrade head +# Use MIGRATION_DATABASE_URL for alembic (falls back to DATABASE_URL for backwards compat) +export ALEMBIC_DATABASE_URL="${MIGRATION_DATABASE_URL:-$DATABASE_URL}" + +# Configure alembic to use the migration database URL +export ALEMBIC_DATABASE_URL + +echo "[prestart] $(date -u +%Y-%m-%dT%H:%M:%SZ) - Running alembic upgrade head (owner user)..." +# Temporarily override DATABASE_URL for alembic +DATABASE_URL="$ALEMBIC_DATABASE_URL" alembic upgrade head echo "[prestart] DB migrations completed successfully." +# Set crm_runtime password if RUNTIME_DB_PASSWORD is set +if [ -n "$RUNTIME_DB_PASSWORD" ]; then + echo "[prestart] Setting crm_runtime password..." + # Parse the migration DB URL to get psql connection params + PGHOST=postgres + PGUSER="${POSTGRES_USER:-crm_user}" + PGDATABASE="${POSTGRES_DB:-crm_db}" + PGPASSWORD="$POSTGRES_PASSWORD" + export PGHOST PGUSER PGDATABASE PGPASSWORD + psql -c "ALTER ROLE crm_runtime WITH LOGIN PASSWORD '${RUNTIME_DB_PASSWORD}' NOSUPERUSER NOBYPASSRLS;" 2>/dev/null || \ + echo "[prestart] WARNING: Could not set crm_runtime password (role may not exist yet)" + unset PGPASSWORD + echo "[prestart] crm_runtime password set." +fi + echo "[prestart] Starting uvicorn on 0.0.0.0:8000 (workers=1)..." exec uvicorn app.main:app \ --host 0.0.0.0 \ diff --git a/tests/conftest.py b/tests/conftest.py index c9c628c..84873b6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -106,13 +106,29 @@ def _get_sync_engine(): @pytest.fixture(scope="session", autouse=True) def db_setup(): - """Drop and recreate all tables once per test session.""" + """Drop and recreate all tables once per test session. + + Uses SET lock_timeout to prevent deadlocks when multiple test processes + try to DROP SCHEMA simultaneously. Falls back to TRUNCATE if DROP fails. + """ sync_eng = _get_sync_engine() with sync_eng.connect() as conn: - # Drop all tables and types - conn.execute(text("DROP SCHEMA public CASCADE;")) - conn.execute(text("CREATE SCHEMA public;")) - conn.execute(text("GRANT ALL ON SCHEMA public TO leocrm;")) + # Set a short lock timeout to prevent deadlocks + conn.execute(text("SET lock_timeout = '5s';")) + try: + conn.execute(text("DROP SCHEMA IF EXISTS public CASCADE;")) + conn.execute(text("CREATE SCHEMA public;")) + conn.execute(text("GRANT ALL ON SCHEMA public TO leocrm;")) + except Exception: + # If DROP SCHEMA deadlocks, fall back to TRUNCATE all tables + conn.rollback() + conn.execute(text("SET lock_timeout = '5s';")) + conn.execute(text( + "DO $$ DECLARE r RECORD; BEGIN " + "FOR r IN (SELECT tablename FROM pg_tables WHERE schemaname='public') " + "LOOP EXECUTE 'TRUNCATE TABLE public.' || quote_ident(r.tablename) || ' CASCADE'; " + "END LOOP; END $$;" + )) conn.commit() sync_eng.dispose() @@ -126,12 +142,19 @@ def db_setup(): asyncio.get_event_loop().run_until_complete(_create()) yield - # Cleanup after session + # Cleanup after session — use TRUNCATE instead of DROP to avoid deadlocks sync_eng = _get_sync_engine() with sync_eng.connect() as conn: - conn.execute(text("DROP SCHEMA public CASCADE;")) - conn.execute(text("CREATE SCHEMA public;")) - conn.execute(text("GRANT ALL ON SCHEMA public TO leocrm;")) + conn.execute(text("SET lock_timeout = '5s';")) + try: + conn.execute(text( + "DO $$ DECLARE r RECORD; BEGIN " + "FOR r IN (SELECT tablename FROM pg_tables WHERE schemaname='public') " + "LOOP EXECUTE 'TRUNCATE TABLE public.' || quote_ident(r.tablename) || ' CASCADE'; " + "END LOOP; END $$;" + )) + except Exception: + pass conn.commit() sync_eng.dispose()