fix: Upload-Sanitizing, atomare Import-Reihenfolge, idempotentes UNBEKANNT-Konto

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

finance/app/routers/imports.py

@@ -1,5 +1,6 @@
 from datetime import date
 from decimal import Decimal
+from pathlib import Path
 
 from fastapi import APIRouter, Depends, HTTPException, UploadFile
 from pydantic import BaseModel, ConfigDict
@@ -55,10 +56,13 @@ class PreviewOut(BaseModel):
 
 @router.post("/upload", response_model=StatementOut, status_code=201)
 def upload(file: UploadFile, session: Session = Depends(get_session)):
+    safe_name = Path(file.filename or "upload.pdf").name
+    if not safe_name or not safe_name.lower().endswith(".pdf"):
+        raise HTTPException(400, "Nur PDF-Dateien")
     settings = get_settings()
     inbox_dir = settings.inbox_dir
     inbox_dir.mkdir(parents=True, exist_ok=True)
-    dest = inbox_dir / file.filename
+    dest = inbox_dir / safe_name
     with dest.open("wb") as f:
         f.write(file.file.read())
     stmt = process_pdf(session, dest)

finance/app/services/importer.py

@@ -12,12 +12,11 @@ from app.services.categorize import apply_rules
 
 
 def _find_or_create_account(session: Session, bank: str, iban: str | None, filename: str) -> Account:
-    if iban:
+    if not iban:
+        iban = f"UNBEKANNT-{filename}"
     acc = session.execute(select(Account).where(Account.iban == iban)).scalar()
     if acc is not None:
         return acc
-    else:
-        iban = f"UNBEKANNT-{filename}"
     acc = Account(bank=bank, iban=iban, name=iban, type="giro")
     session.add(acc)
     session.flush()
@@ -89,11 +88,19 @@ def process_pdf(session: Session, path: Path) -> Statement:
     apply_rules(session, drafts)
 
     stmt.status = "draft"
+
+    # Invariant: a committed draft implies the PDF left the inbox. Move the
+    # file before committing so a failed move can never leave a committed
+    # draft with the PDF still sitting in the inbox.
+    uploads_dir = settings.uploads_dir
+    uploads_dir.mkdir(parents=True, exist_ok=True)
+    try:
+        path.replace(uploads_dir / filename)
+    except OSError:
+        session.rollback()
+        raise
+
     session.commit()
     session.refresh(stmt)
 
-    uploads_dir = settings.uploads_dir
-    uploads_dir.mkdir(parents=True, exist_ok=True)
-    path.replace(uploads_dir / filename)
-
     return stmt

finance/tests/conftest.py

@@ -5,6 +5,7 @@ from sqlalchemy.orm import Session
 from sqlalchemy.pool import StaticPool
 
 from app.db import Base, get_session
+import app.models  # noqa: F401  ensures all ORM tables are registered on Base.metadata
 
 
 @pytest.fixture

finance/tests/test_import_api.py

@@ -46,3 +46,28 @@ def test_upload_preview_confirm(client, fake_parse, tmp_path, monkeypatch):
     assert prev2["duplicates"] == 1
     client.post(f"/api/imports/{sid2}/confirm", headers=H)
     assert len(client.get("/api/transactions", headers=H).json()) == 1
+
+
+def test_upload_sanitizes_path_traversal_filename(client, fake_parse, tmp_path, monkeypatch):
+    monkeypatch.setenv("FB_INBOX_DIR", str(tmp_path / "inbox"))
+    monkeypatch.setenv("FB_UPLOADS_DIR", str(tmp_path / "uploads"))
+    from app.config import get_settings
+    get_settings.cache_clear()
+    r = client.post("/api/imports/upload", headers=H,
+                    files={"file": ("../evil.pdf", b"%PDF-fake", "application/pdf")})
+    assert r.status_code == 201
+    # Der Dateiname darf keine Pfad-Anteile aus dem Client uebernehmen: nur
+    # der Basisname landet im Uploads-Verzeichnis, nichts entkommt nach oben.
+    assert not (tmp_path / "evil.pdf").exists()
+    assert (tmp_path / "uploads" / "evil.pdf").exists()
+    assert not (tmp_path / "inbox" / "evil.pdf").exists()
+
+
+def test_upload_rejects_non_pdf(client, tmp_path, monkeypatch):
+    monkeypatch.setenv("FB_INBOX_DIR", str(tmp_path / "inbox"))
+    monkeypatch.setenv("FB_UPLOADS_DIR", str(tmp_path / "uploads"))
+    from app.config import get_settings
+    get_settings.cache_clear()
+    r = client.post("/api/imports/upload", headers=H,
+                    files={"file": ("x.txt", b"not a pdf", "text/plain")})
+    assert r.status_code == 400