fix(review): Intensiv-Review-Findings bis Low (Security/DSGVO/UI-UX/Tests) (#12)

5 parallele Reviews (Security, DSGVO, Backend-Code, Frontend-UI/UX,
Testabdeckung). Alle Findings bis einschliesslich LOW umgesetzt.

Backend:
- PATCH /me/profile: exclude_unset (kein NULL-Ueberschreiben, Datenverlust)
- GET /me/export: Feedback + Edit-State + Preset-Geometrie ergaenzt (Art. 15/20)
- marketplace fork kopiert geometry; report_count via FOR UPDATE (Lost-Update)
- delete_image: 502 statt 500, Janitor raeumt failed; confirm idempotent
- Marketplace-Preview nur fuer ready-Bilder; Pending-Upload-Soft-Quota (429)
- Modell-Indizes gespiegelt; 002-Downgrade-Datenqualitaet kommentiert

Frontend:
- Geteilte Modal-Komponente (role/aria-modal/Escape/Focus-Trap/-Restore)
  fuer Marketplace-Detail, PresetDialog, ShortcutCheatsheet, BatchApplyModal;
  ExportDialog Escape+role
- Library-Bild-Loeschen zweistufig (Datenverlust-Schutz)
- Bypass-Shortcut-Text, Umlaut-Fixes, Login-Fehler sichtbar, Compare-Race,
  Smart-Suggestion-Hinweis, Account-Lade-Hinweis, aria-labels/aria-controls

DSGVO-Texte: Matomo-DNT relativiert, tfhub.dev/kaggle.com + buy-me-a-coffee
offengelegt, Feedback-Datenkategorie + Retention, RAW-EXIF-Hinweis.

Infra: security_headers-Snippet versioniert; CSP-Trade-off dokumentiert.

Tests: +20 Backend (AuthZ-Negativ, Moderation, S3-Fehlerpfad, Honeypot,
Bounds, Merge-Gruppen, Quota), +Frontend (Modal-A11y, exifStrip-Gate,
transform-Degenerate). backend 182 / frontend 381 / lint / tsc / build gruen.

Co-authored-by: Manuel Rödig <m.roedig@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: 3 people

backend/alembic/versions/002_keycloak.py

@@ -53,6 +53,11 @@ def downgrade() -> None:
     op.create_index("idx_refresh_tokens_token_hash", "refresh_tokens", ["token_hash"])
 
     # users: password_hash wieder rein, keycloak_sub raus
+    # ACHTUNG: Dieser Downgrade ist NICHT datenneutral. Bestehende User
+    # bekommen einen leeren password_hash (server_default="") — Keycloak-only-
+    # Accounts haben kein lokales Passwort, das wiederhergestellt werden
+    # koennte. Der Reverse-Pfad ist nur fuer leere/Dev-DBs gedacht; in Prod
+    # nie ausfuehren.
     op.add_column(
         "users",
         sa.Column("password_hash", sa.String(length=255), nullable=False, server_default=""),

backend/app/config.py

@@ -32,6 +32,10 @@ class Settings(BaseSettings):
     garage_s3_secret_access_key: str = ""
     presigned_url_expires_in: int = 900
     max_image_size_bytes: int = 200 * 1024 * 1024  # 200 MB
+    # Soft-Quota: max. gleichzeitig offene (pending) Uploads pro User. Begrenzt
+    # zusammen mit Rate-Limit + Janitor das Anlegen vieler Zombie-Rows/Objekte
+    # (DB-/Bucket-DoS gegen den gemeinsamen Bucket).
+    max_pending_uploads_per_user: int = 50
 
     cors_origin: str = "http://localhost:5173"
 

backend/app/janitor.py

@@ -1,11 +1,17 @@
-"""Janitor — periodische Cleanup-Tasks fuer pending Uploads.
+"""Janitor — periodische Cleanup-Tasks fuer haengengebliebene Uploads.
 
 Pre-Signed-URLs haben keine Content-Length-Range-Constraint, also kann
 ein Browser einen `init_upload` aufrufen und den `confirm` nie senden.
 Die DB-Row und ein potentiell schon hochgeladenes S3-Object bleiben
 dann zurueck (Bucket-Bloat). Dieser Janitor raeumt sie ab einem
 Schwellenalter (Default: 15 min) weg.
 
+Geraeumt werden zwei Zustaende:
+- `pending`: nie bestaetigte Uploads (Browser hat confirm nie geschickt).
+- `failed`: DELETE-Versuche, bei denen das S3-Delete fehlschlug und die
+  Row als `failed` markiert wurde (Object evtl. noch da). Sonst bliebe ein
+  verwaistes Object ohne DB-Referenz dauerhaft liegen.
+
 Das Modul stellt eine reine async-Funktion `prune_pending_uploads()`
 bereit; aufgerufen wird sie von `backend/scripts/janitor.py` (Cron).
 """
@@ -26,7 +32,7 @@
 
 @dataclass
 class PruneResult:
-    candidates: int          # gefundene pending-Rows aelter als TTL
+    candidates: int          # gefundene pending/failed-Rows aelter als TTL
     storage_deleted: int     # erfolgreich aus S3 entfernt
     storage_errors: int      # S3-Fehler (best effort, DB-Row bleibt drin)
     db_deleted: int          # tatsaechlich aus DB entfernt
@@ -39,7 +45,8 @@ async def prune_pending_uploads(
     ttl: timedelta = PENDING_TTL,
     now: datetime | None = None,
 ) -> PruneResult:
-    """Loescht pending Uploads aelter als ttl aus S3 und DB.
+    """Loescht haengengebliebene Uploads (pending + failed) aelter als ttl
+    aus S3 und DB.
 
     Reihenfolge: erst S3-Object weg, dann DB-Row. Wenn S3 wirft, bleibt
     die DB-Row stehen (naechster Lauf versucht's wieder). Das ist
@@ -49,7 +56,7 @@ async def prune_pending_uploads(
     threshold = (now or datetime.now(timezone.utc)) - ttl
     result = await db.execute(
         select(Image).where(
-            Image.upload_state == "pending",
+            Image.upload_state.in_(("pending", "failed")),
             Image.created_at < threshold,
         )
     )

backend/app/models.py

@@ -53,6 +53,10 @@ class Preset(Base):
             "visibility IN ('private','public')",
             name="ck_presets_visibility",
         ),
+        # In Migration 001 angelegt — hier gespiegelt, damit Modell == Schema
+        # und `alembic revision --autogenerate` den Index nicht faelschlich
+        # als "zu droppen" erkennt.
+        Index("idx_presets_user_id", "user_id"),
     )
 
     id: Mapped[UUID] = mapped_column(PG_UUID(as_uuid=True), primary_key=True, default=uuid4)
@@ -107,6 +111,9 @@ class Image(Base):
             "upload_state IN ('pending','ready','failed')",
             name="ck_images_upload_state",
         ),
+        # In Migration 003 angelegt — hier gespiegelt (Modell == Schema).
+        Index("idx_images_user_id", "user_id"),
+        Index("idx_images_state", "upload_state"),
     )
 
     id: Mapped[UUID] = mapped_column(

backend/app/routers/auth.py

@@ -16,7 +16,7 @@
 from app.auth import current_user
 from app.database import get_db
 from app.keycloak_admin import delete_user as kc_delete_user
-from app.models import Image, Preset, PresetReport, User
+from app.models import Feedback, Image, ImageEdit, Preset, PresetReport, User
 from app.rate_limit import limiter
 from app.schemas import (
     CAMEL_BASE_CONFIG,
@@ -48,8 +48,15 @@ async def update_profile(
     user: User = Depends(current_user),
     db: AsyncSession = Depends(get_db),
 ) -> ProfileOut:
-    user.handle = payload.handle
-    user.bio = payload.bio
+    # PATCH ist partiell: nur tatsaechlich gesendete Felder anfassen, sonst
+    # loescht ein PATCH, der z.B. nur bio schickt, stillschweigend den handle
+    # (= oeffentlicher Marketplace-Creator-Name). exclude_unset trennt
+    # "nicht gesendet" sauber von "explizit auf null gesetzt".
+    data = payload.model_dump(exclude_unset=True)
+    if "handle" in data:
+        user.handle = data["handle"]
+    if "bio" in data:
+        user.bio = data["bio"]
     try:
         await db.commit()
     except IntegrityError:
@@ -137,6 +144,10 @@ class ImageExport(BaseModel):
     confirmed_at: datetime | None
     download_url: str
     download_url_expires_in: int
+    # Persistierter Editor-Bearbeitungsstand (C1) — User-erzeugter Inhalt,
+    # gehoert in einen vollstaendigen Art.-15/20-Export. None, wenn das Bild
+    # nie im Editor bearbeitet/gespeichert wurde.
+    edit_state: dict | None = None
 
 
 class PresetExport(BaseModel):
@@ -147,6 +158,9 @@ class PresetExport(BaseModel):
     name: str
     adjustments: dict
     masks: list
+    # Crop/Straighten/Lens-Geometrie ist User-erzeugter Inhalt (Migration
+    # 009) und gehoert in den Export.
+    geometry: dict | None
     visibility: str
     genre: str | None
     description: str | None
@@ -169,6 +183,19 @@ class ReportExport(BaseModel):
     created_at: datetime
 
 
+class FeedbackExport(BaseModel):
+    """Vom User abgegebenes Feedback. Der Message-Text ist freier User-Inhalt
+    und auskunfts-/uebertragbarkeitspflichtig (Art. 15 + 20) — und bleibt bei
+    Loeschung nur anonymisiert erhalten, ist also gerade vorher exportwichtig."""
+    model_config = CAMEL_OUT_CONFIG
+    id: UUID
+    kind: str
+    message: str
+    page: str | None
+    status: str
+    created_at: datetime
+
+
 class MeExport(BaseModel):
     model_config = CAMEL_BASE_CONFIG
     id: UUID
@@ -179,6 +206,7 @@ class MeExport(BaseModel):
     presets: list[PresetExport]
     images: list[ImageExport]
     submitted_reports: list[ReportExport]
+    feedbacks: list[FeedbackExport]
 
 
 @router.get("/me/export", response_model=MeExport)
@@ -202,8 +230,20 @@ async def export_me(
     images_result = await db.execute(
         select(Image).where(Image.user_id == user.id).order_by(Image.created_at)
     )
+    image_rows = images_result.scalars().all()
+
+    # Edit-States der eigenen Bilder in einer Query (image_id -> state).
+    edits_by_image: dict[UUID, dict] = {}
+    if image_rows:
+        edit_rows = await db.execute(
+            select(ImageEdit).where(
+                ImageEdit.image_id.in_([img.id for img in image_rows])
+            )
+        )
+        edits_by_image = {e.image_id: e.state for e in edit_rows.scalars().all()}
+
     images: list[ImageExport] = []
-    for img in images_result.scalars().all():
+    for img in image_rows:
         url, expires = storage.presign_get(img.bucket_key)
         images.append(ImageExport(
             id=img.id,
@@ -215,6 +255,7 @@ async def export_me(
             confirmed_at=img.confirmed_at,
             download_url=url,
             download_url_expires_in=expires,
+            edit_state=edits_by_image.get(img.id),
         ))
 
     reports_result = await db.execute(
@@ -224,6 +265,15 @@ async def export_me(
     )
     reports = [ReportExport.model_validate(r) for r in reports_result.scalars().all()]
 
+    feedback_result = await db.execute(
+        select(Feedback)
+        .where(Feedback.user_id == user.id)
+        .order_by(Feedback.created_at)
+    )
+    feedbacks = [
+        FeedbackExport.model_validate(f) for f in feedback_result.scalars().all()
+    ]
+
     return MeExport(
         id=user.id,
         email=user.email,
@@ -233,4 +283,5 @@ async def export_me(
         presets=presets,
         images=images,
         submitted_reports=reports,
+        feedbacks=feedbacks,
     )

backend/app/routers/images.py

@@ -4,7 +4,7 @@
 
 from fastapi import APIRouter, Depends, HTTPException, Query, Request, status
 from fastapi.concurrency import run_in_threadpool
-from sqlalchemy import select
+from sqlalchemy import func, select
 from sqlalchemy.ext.asyncio import AsyncSession
 
 from app.auth import current_user
@@ -96,6 +96,24 @@ async def init_upload(
             detail=f"size_bytes ueber Maximum {settings.max_image_size_bytes}.",
         )
 
+    # Soft-Quota gegen Zombie-Pending-Rows: ein Client koennte bis zum
+    # Rate-Limit pending Uploads anlegen, die erst der Janitor (>15 min) wegraeumt.
+    pending_count = (
+        await db.execute(
+            select(func.count())
+            .select_from(Image)
+            .where(Image.user_id == user.id, Image.upload_state == "pending")
+        )
+    ).scalar_one()
+    if pending_count >= settings.max_pending_uploads_per_user:
+        raise HTTPException(
+            status_code=status.HTTP_429_TOO_MANY_REQUESTS,
+            detail=(
+                "Zu viele offene Uploads — bitte bestehende bestaetigen oder "
+                "kurz warten."
+            ),
+        )
+
     image_id = uuid4()
     bucket_key = storage.make_key(user.id, image_id)
 
@@ -133,6 +151,11 @@ async def confirm_upload(
         raise HTTPException(status_code=404, detail="Image nicht gefunden.")
     _ensure_owns_key(image, user)
 
+    # Bereits bestaetigt -> idempotent zurueckgeben, ohne erneute S3-Roundtrips
+    # (HEAD + Magic-Byte-Read) und ohne confirmed_at zu ueberschreiben.
+    if image.upload_state == "ready":
+        return ImageOut.model_validate(image)
+
     try:
         # boto3 ist blockierend -> Threadpool, sonst steht der Event-Loop.
         size = await run_in_threadpool(storage.head, image.bucket_key)
@@ -308,12 +331,19 @@ async def delete_image(
 
     try:
         await run_in_threadpool(storage.delete, image.bucket_key)
-    except Exception:
-        # S3-Fehler: DB-Row als 'failed' markieren, kein Hard-Fail —
-        # erneute DELETE-Calls funktionieren idempotent.
+    except Exception as exc:
+        # S3-Fehler: DB-Row als 'failed' markieren statt sie zu loeschen —
+        # sonst entstuende ein verwaistes S3-Object ohne DB-Referenz, das nie
+        # wieder aufgeraeumt wird. Der Janitor sammelt 'failed'-Rows aelter als
+        # TTL nachtraeglich ein. Kontrolliertes 502 (statt nacktem 500), damit
+        # der Client einen Retry-Hinweis bekommt; erneute DELETE-Calls
+        # funktionieren idempotent.
         image.upload_state = "failed"
         await db.commit()
-        raise
+        raise HTTPException(
+            status_code=status.HTTP_502_BAD_GATEWAY,
+            detail="Speicher-Backend nicht erreichbar — bitte erneut versuchen.",
+        ) from exc
 
     await db.delete(image)
     await db.commit()

backend/app/routers/marketplace.py

@@ -67,7 +67,10 @@ def _preview_url(
     db_session: AsyncSession,  # noqa: ARG001 — Hook fuer spaetere Caches
     image: Image | None,
 ) -> str | None:
-    if image is None:
+    # Nur bestaetigte Uploads als Vorschau ausliefern. Ein pending/failed
+    # Object koennte (vor confirm) beliebige, nicht magic-byte-gepruefte Bytes
+    # enthalten — die werden im Marketplace same-origin im <img> gerendert.
+    if image is None or image.upload_state != "ready":
         return None
     url, _ = storage.presign_get(image.bucket_key)
     return url
@@ -237,6 +240,10 @@ async def fork_marketplace_preset(
         name=name,
         adjustments=dict(src.adjustments),
         masks=list(src.masks),
+        # Geometrie (Crop/Straighten/Lens) ist Teil des Preset-Inhalts und
+        # muss mitkopiert werden — sonst verliert der Fork stillschweigend
+        # die Crop/Lens-Einstellungen des Originals.
+        geometry=dict(src.geometry) if src.geometry else None,
         visibility="private",
         # Genre/Description/Preview NICHT mitkopieren — der Fork ist privat.
     )
@@ -271,19 +278,28 @@ async def report_marketplace_preset(
         await db.rollback()
         raise HTTPException(status_code=409, detail="Du hast dieses Preset bereits gemeldet.")
 
-    # Auto-Hide-Trigger.
+    # Auto-Hide-Trigger. Die Preset-Row sperren (FOR UPDATE), bevor wir den
+    # denormalisierten report_count read-modify-write aktualisieren — sonst
+    # ueberschreiben zwei parallele Reports verschiedener Reporter den Counter
+    # mit einem veralteten Snapshot (Lost-Update), und die Auto-Hide-Schwelle
+    # kann verpasst werden, obwohl genug echte Reports existieren.
+    locked = (
+        await db.execute(
+            select(Preset).where(Preset.id == preset.id).with_for_update()
+        )
+    ).scalar_one()
     count = (
         await db.execute(
             select(func.count())
             .select_from(PresetReport)
-            .where(PresetReport.preset_id == preset.id)
+            .where(PresetReport.preset_id == locked.id)
         )
     ).scalar_one()
-    preset.report_count = int(count)
-    if count >= REPORT_AUTOHIDE_THRESHOLD and preset.visibility == "public":
-        preset.visibility = "private"
-        preset.published_at = None
+    locked.report_count = int(count)
+    if count >= REPORT_AUTOHIDE_THRESHOLD and locked.visibility == "public":
+        locked.visibility = "private"
+        locked.published_at = None
         logger.warning(
-            "preset %s auto-hidden after %d reports", preset.id, count
+            "preset %s auto-hidden after %d reports", locked.id, count
         )
     await db.commit()