diff --git a/PROGRESS.md b/PROGRESS.md index ff91186..e489967 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -88,6 +88,8 @@ | D6-b | ARCH-023 service_container.initialize 'unvollständig': Plugin-Services registrieren sich selbst bei on_activate (bewusstes Design) | ✅ Verifiziertes No-Op — Finding war Design-Missverständnis; dokumentiert in test-bugs.md | 3934aea | | E7-a | CI als hartes Gate (E7): ruff über app/ hatte 105 Findings (77 auto-fixable + 27 manuell); darunter 8 echte F821-NameError-Produktionsbugs (stream_chat in external_api mit falscher Call-Signatur, uuid_mod vor lokalem Import, UserTenant ×3 in automation/plugin, user_id in tasks delete-audit, timedelta in workflows/engine, Any ×5 in unified_search/contracts) + py311-inkompatibles type-Statement in step_handlers | ✅ Alle behoben: Auto-Fixes + manuelle Fixes; ruff exit=0 über app/; create_app OK (559 routes); Verifikation unified_tasks+automation+phase_g_workflows 85/89 grün (4 Failures = bekannter Vorbestand BUG-099 workstream) | — | | E7-b | Forgejo Actions: ci.yml existiert (.forgejo/workflows/ci.yml, trigger push/PR main), aber 0 Läufe bisher (total_count=0) — Runner-Konfiguration auf Server-Seite zu prüfen; Branch-Protection 'Merge nur bei grün' ist Forgejo-Server-Einstellung | ⏳ Dokumentiert für Server-Admin: Actions-Runner aktivieren + Branch-Protection setzen; Pipeline-Inhalt ist vollständig (15 Checks) | — | +| E1-a | E1 Audit-Vollständigkeit: Lücken-Analyse — 349 mutierende Endpoints, 59 Dateien ohne JEDE Audit-Referenz (AGENTS.md-Verstoß 'jede Mutation erzeugt Audit-Eintrag') | ✅ AuditMiddleware als systematisches Safety-Net implementiert (app/core/middleware.py): loggt alle erfolgreichen POST/PATCH/DELETE mit Session-basierter user/tenant-Attribuierung, entity_type aus Pfad, source=middleware in changes; Skip-Liste für auth/health/errors/audit/external; best-effort (Audit-Fehler brechen Requests nie); registriert in main.py | — | +| E1-b | E1 Beweis: Dedizierter Test test_audit_middleware.py — POST auf /api/v1/saved-views (Route OHNE explizites log_audit) erzeugt Audit-Zeile mit source=middleware | ✅ Test grün; Regressionssmoke test_permissions+test_audit_middleware 23/23 grün; ruff clean; dabei log_audit-details-Schwäche entdeckt (details-Parameter wird nicht persistiert — nur changes) und Middleware entsprechend auf changes umgestellt | — | **Block D ABGESCHLOSSEN** (D1–D6) — D1: alle 9 Ziel-Suites grün; D2: DateTime/SQLITE-001; D3: ARCH-051/055/056/057 + systemischer Permission-Resolver-Bug + conftest-pgvector; D4: Security-Triage (ARCH-027 verifiziert, BUG-019 = 0 echte Secrets, BUG-020 kein fixbares Finding); D5: Scanner-Triage (api_contracts -75%, plugins -100%, 371 Fehlalarme eliminiert); D6: ai_copilot deprecated + ARCH-023 No-Op. Offene Follow-ups dokumentiert (~12 echte API-Bugs aus D5, IMAP-Mocking für Mail-Tests). Nächster Block: E (Production-Härtung). diff --git a/app/core/middleware.py b/app/core/middleware.py index a00d0ad..c6b3b74 100644 --- a/app/core/middleware.py +++ b/app/core/middleware.py @@ -3,6 +3,8 @@ from __future__ import annotations import logging +import re +import uuid as uuid_mod from fastapi import Request, status from starlette.middleware.base import BaseHTTPMiddleware @@ -137,3 +139,97 @@ class CSRFMiddleware(BaseHTTPMiddleware): pass return await call_next(request) + + +class AuditMiddleware(BaseHTTPMiddleware): + """Safety-net audit trail for ALL successful mutating requests. + + AGENTS.md requires every mutation to produce an audit entry. Explicit + ``log_audit`` calls in routes/services remain the detail layer (entity ids, + change diffs); this middleware guarantees a baseline entry for mutations + that lack one, marked with ``source=middleware`` in ``details``. + + Best-effort by design: audit failures never break the request. + """ + + _MUTATING = {"POST", "PUT", "PATCH", "DELETE"} + _SKIP_PREFIXES = ( + "/api/v1/auth", + "/api/v1/health", + "/api/v1/errors", + "/api/v1/audit", + "/api/v1/external", + ) + + async def dispatch(self, request: Request, call_next): + response = await call_next(request) + + if request.method not in self._MUTATING: + return response + if response.status_code < 200 or response.status_code >= 300: + return response + path = request.url.path + if any(path.startswith(p) for p in self._SKIP_PREFIXES): + return response + + try: + await self._write_entry(request, path, response.status_code) + except Exception: + logging.getLogger(__name__).debug( + "AuditMiddleware: failed to write baseline entry for %s %s", request.method, path + ) + return response + + @staticmethod + def _derive_entity_type(path: str) -> str: + """Derive an entity_type from the second URL segment.""" + parts = [p for p in path.split("/") if p] + # /api/v1//... -> resource; singularize naive trailing 's' + resource = parts[2] if len(parts) > 2 and parts[0] == "api" and parts[1] == "v1" else (parts[0] if parts else "unknown") + return resource[:-1] if len(resource) > 3 and resource.endswith("s") else resource + + async def _write_entry(self, request: Request, path: str, status_code: int) -> None: + from app.core.audit import log_audit + from app.core.auth import get_redis, get_session_data + from app.core.db import create_db_session + + # Attribute via the Redis session (same source as CSRFMiddleware) — + # FastAPI dependencies run after middleware, so request.state is empty here. + settings = get_settings() + session_id = request.cookies.get(settings.session_cookie_name) + if not session_id: + return # unauthenticated — nothing to attribute + redis = get_redis() + session_data = await get_session_data(redis, session_id) + if not session_data: + return + tenant_raw = session_data.get("tenant_id") + user_raw = session_data.get("user_id") + if not tenant_raw: + return + + action_map = {"POST": "create", "PATCH": "update", "PUT": "update", "DELETE": "delete"} + entity_id: uuid_mod.UUID | None = None + parts = [p for p in path.split("/") if p] + if parts and re.fullmatch(r"[0-9a-fA-F-]{36}", parts[-1]): + try: + entity_id = uuid_mod.UUID(parts[-1]) + except ValueError: + entity_id = None + + async with create_db_session(uuid_mod.UUID(tenant_raw)) as db: + await log_audit( + db, + uuid_mod.UUID(tenant_raw), + uuid_mod.UUID(user_raw) if user_raw else None, + action_map.get(request.method, request.method.lower()), + self._derive_entity_type(path), + entity_id, + changes={ + "source": "middleware", + "method": request.method, + "path": path, + "status": status_code, + }, + ) + await db.commit() diff --git a/app/main.py b/app/main.py index fffa3d4..fe697a0 100644 --- a/app/main.py +++ b/app/main.py @@ -23,7 +23,11 @@ logger = logging.getLogger(__name__) from app.config import get_settings # noqa: E402 from app.core.db import close_engine, get_engine # noqa: E402 from app.core.error_codes import ApiError, build_error_response # noqa: E402 -from app.core.middleware import CSRFMiddleware, SecurityHeadersMiddleware # noqa: E402 +from app.core.middleware import ( # noqa: E402 + AuditMiddleware, + CSRFMiddleware, + SecurityHeadersMiddleware, +) from app.core.monitoring import record_error, record_request # noqa: E402 from app.core.rate_limit import GeneralRateLimitMiddleware # noqa: E402 from app.core.resilience import CircuitBreakerMiddleware # noqa: E402 @@ -472,6 +476,7 @@ def create_app() -> FastAPI: ) app.add_middleware(CSRFMiddleware) app.add_middleware(SecurityHeadersMiddleware) + app.add_middleware(AuditMiddleware) app.add_middleware(GeneralRateLimitMiddleware) app.add_middleware(RequestLoggingMiddleware) app.add_middleware(CircuitBreakerMiddleware) diff --git a/tests/test_audit_middleware.py b/tests/test_audit_middleware.py new file mode 100644 index 0000000..3718092 --- /dev/null +++ b/tests/test_audit_middleware.py @@ -0,0 +1,48 @@ +"""Tests for AuditMiddleware (Block E / E1). + +Proves the AGENTS.md requirement "every mutation produces an audit entry" +for routes that lack an explicit ``log_audit`` call. +""" + +from __future__ import annotations + +import uuid + +import pytest +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from app.models.audit import AuditLog +from tests.conftest import ORIGIN_HEADER, login_client, seed_tenant_and_users + + +@pytest.mark.asyncio +async def test_post_without_explicit_audit_creates_middleware_entry( + client, db_session: AsyncSession +): + """POST to a route without explicit log_audit still yields an audit row.""" + seed = await seed_tenant_and_users(db_session) + await login_client(client, "admin@tenanta.com") + + # Mutating request on a route WITHOUT explicit log_audit: + # create a saved view via core route (no log_audit in saved_views.py) + view_name = f"audit-mw-{uuid.uuid4().hex[:8]}" + resp = await client.post( + "/api/v1/saved-views", + json={"name": view_name, "entity_type": "contact", "config": {}}, + headers=ORIGIN_HEADER, + ) + assert resp.status_code in (200, 201), resp.text + + # A middleware-written entry must exist with source=middleware in changes + rows = ( + await db_session.execute(select(AuditLog).where(AuditLog.tenant_id == seed["tenant_a"].id)) + ).scalars().all() + mw_rows = [ + r for r in rows if r.changes and isinstance(r.changes, dict) and r.changes.get("source") == "middleware" + ] + assert len(mw_rows) > 0, "expected at least one middleware-sourced audit entry" + latest = max(mw_rows, key=lambda r: r.timestamp) + assert latest.changes["method"] == "POST" + assert latest.changes["path"].startswith("/api/v1/") + assert latest.action in ("create", "update", "delete")