From b2f75495deedf215a4134040da7d82dc949afc0c Mon Sep 17 00:00:00 2001 From: Agent Zero Date: Fri, 18 Sep 2026 08:46:27 +0200 Subject: [PATCH] =?UTF-8?q?fix(security):=20F20=20(Astra=20P1)=20=E2=80=94?= =?UTF-8?q?=20pauschaler=20Boot-GRANT=20entfernt,=20DELETE-Rechte=20als=20?= =?UTF-8?q?Migration=200145=20festgeschrieben?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vorher: prestart.sh fuehrte bei JEDEM Container-Start GRANT DELETE ON ALL TABLES fuer crm_api/crm_auth/crm_worker aus — und hob damit Migration 0100 auf, die DELETE auf 12 sensiblen Tabellen (audit_log, api_tokens, password_reset_tokens, tenants, ...) gezielt entzogen hatte. Der Blanket-Grant war ein BUG-030-Workaround (User-DELETE 500), der den Schutz seit jedem Start zerstoerte. Fix: - Migration 0145 (0145_delete_grants_converged): deterministischer Sollzustand — REVOKE DELETE auf geschuetzten Tabellen von beiden Runtime-Rollen (audit_log, api_tokens, password_reset_tokens, plugin_allowlist, plugin_migrations, tenants, tenant_plugin_activation); GRANT DELETE auf legitime Runtime-Loeschungen (users, user_tenants, sessions, plugins, notification_types) NUR fuer crm_api; crm_worker erhaelt kein DELETE auf geschuetzten Tabellen. - prestart.sh: Blanket-GRANT-Block entfernt, durch dokumentierenden Verweis auf 0145 ersetzt. - audit.py Retention-Route: Delete laeuft ueber Migrations-Session-Factory (Table-Owner) statt Request-DB — Runtime-Rollen koennen Auditdaten schreiben aber NIEMALS loeschen (Astra-Abnahme). Gleiches Muster wie Plugin-Uninstall. Abnahme (Astra): API und Worker koennen Auditdaten schreiben, aber nicht loeschen — erfuellt (audit_log DELETE von crm_api/crm_worker entzogen, Retention als dokumentierte Wartungsoperation ueber Owner-Session). Verifikation: Migration-Syntax OK, ruff clean, alembic heads = genau 0145, prestart bash -n OK, test_audit_architecture_fixes + test_user_service 30/30 (Logout-Session-Delete, User-DELETE, Audit-Pfade alle intakt). Bekannte Grenze (ehrlich): Kuenftige Plugin-Tabellen brauchen ihre DELETE-Rechte in der jeweiligen Migration statt im Boot-Skript — sync_plugin_schema.py vergibt KEINE GRANTs (verifiziert), deshalb ist das Default-Privilege-Problem in S2 (F18 Schema-Verantwortung) adressiert. --- .../versions/0145_delete_grants_converged.py | 80 +++++++++++++++++++ app/routes/audit.py | 21 +++-- prestart.sh | 40 +++------- 3 files changed, 106 insertions(+), 35 deletions(-) create mode 100644 alembic/versions/0145_delete_grants_converged.py diff --git a/alembic/versions/0145_delete_grants_converged.py b/alembic/versions/0145_delete_grants_converged.py new file mode 100644 index 0000000..1d7c45b --- /dev/null +++ b/alembic/versions/0145_delete_grants_converged.py @@ -0,0 +1,80 @@ +"""Converged DELETE grants (F20/Astra). + +Removes the effect of the blanket ``GRANT DELETE ON ALL TABLES`` that +prestart.sh applied on every boot — which silently undid migration +0100's protections on every container start. + +Documented target state: + +Runtime-legitimate DELETEs (crm_api only): + - users, user_tenants (user deletion on last membership, BUG-030) + - sessions (logout session invalidation) + - plugins, notification_types (plugin uninstall + registry sync) + +Protected — DELETE stays REVOKED from crm_api AND crm_worker: + - audit_log (Astra acceptance: API/Worker write, never delete) + - api_tokens (revoke is an UPDATE on revoked_at) + - password_reset_tokens (consumption is an UPDATE on used_at) + - plugin_allowlist, plugin_migrations (install/migration path only — + plugin_migrations rows are deleted via the migration factory) + - tenants (never deleted at runtime) + - tenant_plugin_activation (deactivation is an UPDATE) + +crm_worker receives no DELETE on any protected table (workers never +delete users, sessions or plugin rows). + +Revision ID: 0145 +Revises: 0144 +""" + +from alembic import op + +revision = "0145" +down_revision = "0144" +branch_labels = None +depends_on = None + +# Tables where runtime DELETE is a documented, legitimate operation (crm_api) +RUNTIME_DELETE_TABLES = [ + "users", + "user_tenants", + "sessions", + "plugins", + "notification_types", +] + +# Tables where DELETE must stay revoked from BOTH runtime roles (0100 + F20) +PROTECTED_TABLES = [ + "audit_log", + "api_tokens", + "password_reset_tokens", + "plugin_allowlist", + "plugin_migrations", + "tenants", + "tenant_plugin_activation", +] + + + +def upgrade() -> None: + # 1. Re-assert 0100's revocations — production DBs have lived with the + # blanket boot grant, so revoke first for a deterministic baseline. + for table in PROTECTED_TABLES: + op.execute(f"REVOKE DELETE ON TABLE {table} FROM crm_api;") + op.execute(f"REVOKE DELETE ON TABLE {table} FROM crm_worker;") + + # 2. Grant the runtime-legitimate DELETEs to crm_api (BUG-030 stays + # fixed, logout keeps working, plugin management keeps working). + for table in RUNTIME_DELETE_TABLES: + op.execute(f"GRANT DELETE ON TABLE {table} TO crm_api;") + + + +def downgrade() -> None: + # Best-effort inverse: revoke the runtime grants, re-grant the + # protected tables (matching the pre-F20 blanket state). + for table in RUNTIME_DELETE_TABLES: + op.execute(f"REVOKE DELETE ON TABLE {table} FROM crm_api;") + for table in PROTECTED_TABLES: + op.execute(f"GRANT DELETE ON TABLE {table} TO crm_api;") + op.execute(f"GRANT DELETE ON TABLE {table} TO crm_worker;") diff --git a/app/routes/audit.py b/app/routes/audit.py index 737a903..7202ee9 100644 --- a/app/routes/audit.py +++ b/app/routes/audit.py @@ -165,15 +165,24 @@ async def audit_retention_cleanup( """Delete audit log entries older than retention_days. Admin only. Default retention: 365 days. + + F20 (Astra): runtime roles (crm_api/crm_worker) must NOT be able to + delete audit data. The delete runs via the migration session factory + (table owner) instead of the request ``db`` — a documented maintenance + operation, same pattern as plugin uninstall. """ tenant_id = uuid.UUID(current_user["tenant_id"]) cutoff = datetime.now(UTC) - timedelta(days=retention_days) - q = delete(AuditLog).where( - AuditLog.tenant_id == tenant_id, - AuditLog.timestamp < cutoff, - ) - result = await db.execute(q) - await db.commit() + from app.core.db import get_migration_session_factory + + factory = get_migration_session_factory() + async with factory() as mig_db: + q = delete(AuditLog).where( + AuditLog.tenant_id == tenant_id, + AuditLog.timestamp < cutoff, + ) + result = await mig_db.execute(q) + await mig_db.commit() return {"deleted": result.rowcount, "retention_days": retention_days, "cutoff": cutoff.isoformat()} diff --git a/prestart.sh b/prestart.sh index af51feb..f9cab5e 100644 --- a/prestart.sh +++ b/prestart.sh @@ -70,35 +70,17 @@ PYEOF python3 /tmp/set_role_passwords.py rm -f /tmp/set_role_passwords.py -# Grant DELETE on all tables to crm_api, crm_auth, crm_worker (BUG-030 fix) -echo "[prestart] Granting DELETE on all tables to crm_api, crm_auth, crm_worker..." -python3 -c " -import asyncio, os -from sqlalchemy.ext.asyncio import create_async_engine -from sqlalchemy import text - -async def grant_delete(): - db_url = os.environ.get('MIGRATION_DATABASE_URL', os.environ.get('DATABASE_URL', '')) - if not db_url: - print('[prestart] No DATABASE_URL found, skipping GRANT DELETE') - return - engine = create_async_engine(db_url) - try: - async with engine.begin() as conn: - for role in ['crm_api', 'crm_auth', 'crm_worker']: - try: - await conn.execute(text(f'GRANT DELETE ON ALL TABLES IN SCHEMA public TO {role}')) - print(f'[prestart] GRANT DELETE to {role} OK') - except Exception as e: - print(f'[prestart] WARNING: Could not GRANT DELETE to {role}: {e}') - print('[prestart] GRANT DELETE complete.') - except Exception as e: - print(f'[prestart] WARNING: Could not GRANT DELETE: {e}') - finally: - await engine.dispose() - -asyncio.run(grant_delete()) -" +# F20 (Astra P1): the blanket "GRANT DELETE ON ALL TABLES" block that used +# to run here was removed — it silently undid migration 0100's targeted +# DELETE revocations on EVERY container start (audit_log, api_tokens, +# password_reset_tokens, tenants, ...). +# The documented target state now lives in migration 0145 +# (0145_delete_grants_converged.py): runtime DELETEs only where the app +# legitimately deletes rows (users, user_tenants, sessions, plugins, +# notification_types) and DELETE revoked from crm_api/crm_worker on all +# protected tables. Audit-log retention runs via the migration session +# factory (app/routes/audit.py), plugin uninstall via the migration +# factory as before. # Set crm_runtime password if RUNTIME_DB_PASSWORD is set (legacy support) if [ -n "$RUNTIME_DB_PASSWORD" ]; then