feat(e1): AuditMiddleware als systematisches Safety-Net — alle erfolgreichen POST/PATCH/DELETE erzeugen Audit-Eintrag (Session-basierte user/tenant-Attribuierung, entity_type aus Pfad, source=middleware in changes); schließt Lücke von 349 mutierenden Endpoints in 59 Dateien ohne Audit; Skip-Liste auth/health/errors/audit/external; best-effort; Beweistest test_audit_middleware.py grün (POST ohne explizites log_audit → Audit-Zeile); Regressionssmoke 23/23 grün
This commit is contained in:
@@ -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 |
|
| 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-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) | — |
|
| 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).
|
**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).
|
||||||
|
|
||||||
|
|||||||
@@ -3,6 +3,8 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
|
import re
|
||||||
|
import uuid as uuid_mod
|
||||||
|
|
||||||
from fastapi import Request, status
|
from fastapi import Request, status
|
||||||
from starlette.middleware.base import BaseHTTPMiddleware
|
from starlette.middleware.base import BaseHTTPMiddleware
|
||||||
@@ -137,3 +139,97 @@ class CSRFMiddleware(BaseHTTPMiddleware):
|
|||||||
pass
|
pass
|
||||||
|
|
||||||
return await call_next(request)
|
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>/... -> 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()
|
||||||
|
|||||||
+6
-1
@@ -23,7 +23,11 @@ logger = logging.getLogger(__name__)
|
|||||||
from app.config import get_settings # noqa: E402
|
from app.config import get_settings # noqa: E402
|
||||||
from app.core.db import close_engine, get_engine # 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.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.monitoring import record_error, record_request # noqa: E402
|
||||||
from app.core.rate_limit import GeneralRateLimitMiddleware # noqa: E402
|
from app.core.rate_limit import GeneralRateLimitMiddleware # noqa: E402
|
||||||
from app.core.resilience import CircuitBreakerMiddleware # 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(CSRFMiddleware)
|
||||||
app.add_middleware(SecurityHeadersMiddleware)
|
app.add_middleware(SecurityHeadersMiddleware)
|
||||||
|
app.add_middleware(AuditMiddleware)
|
||||||
app.add_middleware(GeneralRateLimitMiddleware)
|
app.add_middleware(GeneralRateLimitMiddleware)
|
||||||
app.add_middleware(RequestLoggingMiddleware)
|
app.add_middleware(RequestLoggingMiddleware)
|
||||||
app.add_middleware(CircuitBreakerMiddleware)
|
app.add_middleware(CircuitBreakerMiddleware)
|
||||||
|
|||||||
@@ -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")
|
||||||
Reference in New Issue
Block a user