fix: Final-Review-Findings (GUI-Login, FK-Kaskaden, Validierung, Duplikat-Recheck, Views)

F1: GUI-Login-Hash gegen $-Korruption gehaertet. .env-Werte einfach-gequotet
    geschrieben + Sanity-Check nach Sourcing. Zusaetzlich systemd-Ebene:
    podman escapt $->$$ in Environment=, systemd 252 kollabiert das nicht ->
    sed-Post-Processing der Unit-Dateien. Live-.env: GUI-Passwort rotiert.
F2: Explizite Dependent-Deletes gegen FK-500 (delete_scenario/loan), 409 bei
    bestaetigtem Import-Delete. Test-Engine erzwingt PRAGMA foreign_keys=ON.
F3: Enum/Range-Validierung (Literal + Field ge/le) fuer Recurring, Loan,
    Modifier inkl. PATCH-Pfade.
F4: Confirm nur fuer draft-Statements (sonst 409) + Dedup-Re-Check gegen
    bestaetigte Buchungen vor dem Promoten.
F5: v_balance_history/v_balance_total addieren pro Konto den opening_balance
    des fruehesten bestaetigten Statements (approved deviation vom Plan-SQL).
F6: patch_scenario Namenskollision -> 409; create_transaction unbekanntes
    Konto -> 404 (statt 500).
F7: Generierte systemd-Unit-Dateien chmod 600 (enthalten Env-Secrets).
F8: Globaler htmx:responseError-Handler in base.html.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
2026-07-17 20:11:31 +02:00
parent b3b40a5982
commit cf9282c716
10 changed files with 265 additions and 31 deletions

View File

@@ -53,15 +53,26 @@ GRAFANA_DASHBOARDS_DIR="$FINANCE_DIR/grafana/dashboards"
# unchanged, so re-running this script never rotates secrets under existing
# data.
if [ ! -f "$ENV_FILE" ]; then
# Compute all values into variables first, then write them SINGLE-QUOTED.
# The GUI password hash has the form salt$digest; if written unquoted, the
# later `set -a; . "$ENV_FILE"` would shell-expand the "$digest" part and
# corrupt the hash, breaking GUI login. Single quotes make every value a
# literal string on sourcing.
GUI_PASSWORD=$(openssl rand -base64 12)
POSTGRES_PASSWORD_VAL=$(openssl rand -hex 16)
FINANCE_READ_PASSWORD_VAL=$(openssl rand -hex 16)
FB_API_KEY_VAL=$(openssl rand -hex 32)
FB_SESSION_SECRET_VAL=$(openssl rand -hex 32)
FB_GUI_PASSWORD_HASH_VAL=$(python3 -c "import hashlib,secrets,sys;s=secrets.token_hex(16);pw=sys.argv[1];print(s+'\$'+hashlib.pbkdf2_hmac('sha256',pw.encode(),s.encode(),200000).hex())" "$GUI_PASSWORD")
GRAFANA_ADMIN_PASSWORD_VAL=$(openssl rand -base64 12)
cat > "$ENV_FILE" <<EOF
POSTGRES_PASSWORD=$(openssl rand -hex 16)
FINANCE_READ_PASSWORD=$(openssl rand -hex 16)
FB_API_KEY=$(openssl rand -hex 32)
FB_SESSION_SECRET=$(openssl rand -hex 32)
FB_GUI_USER=admin
FB_GUI_PASSWORD_HASH=$(python3 -c "import hashlib,secrets;s=secrets.token_hex(16);print(s+'\$'+hashlib.pbkdf2_hmac('sha256',b'$GUI_PASSWORD',s.encode(),200000).hex())")
GRAFANA_ADMIN_PASSWORD=$(openssl rand -base64 12)
POSTGRES_PASSWORD='$POSTGRES_PASSWORD_VAL'
FINANCE_READ_PASSWORD='$FINANCE_READ_PASSWORD_VAL'
FB_API_KEY='$FB_API_KEY_VAL'
FB_SESSION_SECRET='$FB_SESSION_SECRET_VAL'
FB_GUI_USER='admin'
FB_GUI_PASSWORD_HASH='$FB_GUI_PASSWORD_HASH_VAL'
GRAFANA_ADMIN_PASSWORD='$GRAFANA_ADMIN_PASSWORD_VAL'
EOF
chmod 600 "$ENV_FILE"
echo "NEU ERZEUGT: GUI-Login admin / $GUI_PASSWORD (jetzt notieren!)"
@@ -69,6 +80,16 @@ EOF
fi
set -a; . "$ENV_FILE"; set +a
# Sanity check: the GUI password hash must survive shell sourcing intact. An
# unquoted value containing a literal '$' gets mangled by parameter expansion,
# which would silently break GUI login. Require the canonical salt$digest form
# (32 hex chars, '$', 64 hex chars).
if ! printf '%s' "$FB_GUI_PASSWORD_HASH" | grep -Eq '^[0-9a-f]{32}\$[0-9a-f]{64}$'; then
echo "ERROR: FB_GUI_PASSWORD_HASH is malformed after sourcing $ENV_FILE." >&2
echo " All values in $ENV_FILE must be single-quoted." >&2
exit 1
fi
# Stop existing systemd-managed pod if present, to avoid conflicts on rerun
echo "Stopping systemd-managed pod 'pod-$POD_NAME.service' if it exists..."
if systemctl --user list-units --type=service --all 2>/dev/null | \
@@ -169,6 +190,24 @@ cd "$USER_SYSTEMD_DIR"
podman generate systemd --name --new --files "$POD_NAME"
echo "Generated systemd service files (rc=$?)"
# The --new units embed the full `podman run` commands, including container
# environment secrets (POSTGRES_PASSWORD, GF_SECURITY_ADMIN_PASSWORD,
# FB_GUI_PASSWORD_HASH, ...). Post-process each generated unit file:
# 1. chmod 600 so only the owner can read the embedded secrets.
# 2. Collapse '$$' -> '$' on Environment= lines. `podman generate systemd`
# escapes every '$' as '$$' (correct for ExecStart command lines, where
# systemd performs variable expansion). But systemd does NOT un-escape
# '$$' inside Environment= *values* - it passes them through literally.
# The GUI password hash has the form salt$digest, so without this fix the
# container receives salt$$digest and GUI login fails. Restrict the
# substitution to Environment= lines so ExecStart escaping stays intact.
for svc in "pod-${POD_NAME}.service" "container-${DB_CTR_NAME}.service" \
"container-${API_CTR_NAME}.service" "container-${GRAFANA_CTR_NAME}.service"; do
[ -f "$svc" ] || continue
chmod 600 "$svc"
sed -i '/^Environment=/ s/\$\$/$/g' "$svc"
done
# Stop & remove live pod and containers
podman pod stop --ignore --time 15 "$POD_NAME"
podman pod rm -f --ignore "$POD_NAME"

View File

@@ -1,17 +1,33 @@
from sqlalchemy import text
from sqlalchemy.engine import Engine
# Per-account starting balance: the opening_balance of each account's earliest
# confirmed statement (if any). Used to seed the cumulative sums below so the
# Grafana balance charts show the real account balance, not just the running
# sum of imported transactions (which would start at zero and ignore the
# balance that existed before the first imported statement).
_ACCOUNT_BASE = """
SELECT DISTINCT ON (account_id) account_id, opening_balance
FROM statements
WHERE status = 'confirmed' AND opening_balance IS NOT NULL
ORDER BY account_id, period_start NULLS LAST, id
"""
VIEWS: dict[str, str] = {
"v_balance_history": """
"v_balance_history": f"""
SELECT t.booking_date AS day, a.name AS account,
SUM(SUM(t.amount)) OVER (PARTITION BY a.id
ORDER BY t.booking_date) AS balance
COALESCE(b.opening_balance, 0)
+ SUM(SUM(t.amount)) OVER (PARTITION BY a.id
ORDER BY t.booking_date) AS balance
FROM transactions t JOIN accounts a ON a.id = t.account_id
LEFT JOIN ({_ACCOUNT_BASE}) b ON b.account_id = a.id
WHERE t.status = 'confirmed'
GROUP BY a.id, a.name, t.booking_date""",
"v_balance_total": """
GROUP BY a.id, a.name, t.booking_date, b.opening_balance""",
"v_balance_total": f"""
SELECT booking_date AS day,
SUM(SUM(amount)) OVER (ORDER BY booking_date) AS balance
(SELECT COALESCE(SUM(opening_balance), 0)
FROM ({_ACCOUNT_BASE}) s)
+ SUM(SUM(amount)) OVER (ORDER BY booking_date) AS balance
FROM transactions WHERE status = 'confirmed' GROUP BY booking_date""",
"v_monthly_by_category": """
SELECT date_trunc('month', t.booking_date) AS month,
@@ -30,4 +46,13 @@ def create_views(engine: Engine) -> None:
return
with engine.begin() as conn:
for name, body in VIEWS.items():
conn.execute(text(f"CREATE OR REPLACE VIEW {name} AS {body}"))
# CREATE OR REPLACE VIEW fails if the output column set changed
# (name/type/order). Fall back to DROP + CREATE in that case so a
# redeploy always applies the current definition.
try:
with conn.begin_nested():
conn.execute(
text(f"CREATE OR REPLACE VIEW {name} AS {body}"))
except Exception:
conn.execute(text(f"DROP VIEW IF EXISTS {name} CASCADE"))
conn.execute(text(f"CREATE VIEW {name} AS {body}"))

View File

@@ -115,6 +115,8 @@ def confirm(statement_id: int, session: Session = Depends(get_session)):
stmt = session.get(Statement, statement_id)
if stmt is None:
raise HTTPException(404, "Import nicht gefunden")
if stmt.status != "draft":
raise HTTPException(409, "Import ist nicht im Entwurfsstatus")
txs = session.execute(
select(Transaction).where(
Transaction.statement_id == statement_id,
@@ -123,6 +125,19 @@ def confirm(statement_id: int, session: Session = Depends(get_session)):
)
).scalars().all()
for tx in txs:
# Re-check against currently confirmed transactions: another import
# confirmed between preview and now may have introduced the same
# booking. Mark late duplicates and skip promoting them.
clash = session.execute(
select(Transaction).where(
Transaction.dedup_hash == tx.dedup_hash,
Transaction.status == "confirmed",
Transaction.id != tx.id,
)
).scalar()
if clash is not None:
tx.is_duplicate = True
continue
tx.status = "confirmed"
stmt.status = "confirmed"
session.commit()
@@ -135,6 +150,8 @@ def delete_import(statement_id: int, session: Session = Depends(get_session)):
stmt = session.get(Statement, statement_id)
if stmt is None:
raise HTTPException(404, "Import nicht gefunden")
if stmt.status == "confirmed":
raise HTTPException(409, "Bestätigter Import kann nicht gelöscht werden")
drafts = session.execute(
select(Transaction).where(
Transaction.statement_id == statement_id,

View File

@@ -1,15 +1,17 @@
from datetime import date
from decimal import Decimal
from typing import Literal
from fastapi import APIRouter, Depends, HTTPException
from pydantic import BaseModel, ConfigDict
from sqlalchemy import select
from pydantic import BaseModel, ConfigDict, Field
from sqlalchemy import delete, select
from sqlalchemy.orm import Session
from app.auth import require_auth
from app.db import get_session
from app.engine.loans import loan_schedule
from app.models.tables import Category, Loan, PlannedItem, RecurringItem
from app.models.tables import (Category, Loan, PlannedItem, RecurringItem,
ScenarioLoan)
from app.services.suggestions import suggest_recurring
router = APIRouter(prefix="/api", tags=["planning"],
@@ -20,8 +22,8 @@ router = APIRouter(prefix="/api", tags=["planning"],
class RecurringIn(BaseModel):
name: str
amount: Decimal
rhythm: str
due_day: int
rhythm: Literal["monthly", "quarterly", "yearly"]
due_day: int = Field(ge=1, le=31)
start_date: date | None = None
end_date: date | None = None
category_id: int | None = None
@@ -35,8 +37,8 @@ class RecurringOut(RecurringIn):
class RecurringPatch(BaseModel):
name: str | None = None
amount: Decimal | None = None
rhythm: str | None = None
due_day: int | None = None
rhythm: Literal["monthly", "quarterly", "yearly"] | None = None
due_day: int | None = Field(default=None, ge=1, le=31)
start_date: date | None = None
end_date: date | None = None
category_id: int | None = None
@@ -170,7 +172,7 @@ class LoanIn(BaseModel):
annual_rate_pct: Decimal
term_months: int
payout_date: date
repayment_type: str = "annuity"
repayment_type: Literal["annuity", "bullet"] = "annuity"
class LoanOut(LoanIn):
@@ -184,7 +186,7 @@ class LoanPatch(BaseModel):
annual_rate_pct: Decimal | None = None
term_months: int | None = None
payout_date: date | None = None
repayment_type: str | None = None
repayment_type: Literal["annuity", "bullet"] | None = None
class InstallmentOut(BaseModel):
@@ -227,6 +229,10 @@ def delete_loan(loan_id: int, session: Session = Depends(get_session)):
loan = session.get(Loan, loan_id)
if loan is None:
raise HTTPException(404, "Kredit nicht gefunden")
# Remove scenario assignments first: no DB-level cascade exists, so a
# dangling ScenarioLoan row would raise a foreign-key violation (500 on
# Postgres).
session.execute(delete(ScenarioLoan).where(ScenarioLoan.loan_id == loan_id))
session.delete(loan)
session.commit()

View File

@@ -1,16 +1,17 @@
from datetime import date
from decimal import Decimal
from typing import Literal
from fastapi import APIRouter, Depends, HTTPException
from pydantic import BaseModel, ConfigDict
from sqlalchemy import select
from sqlalchemy import delete, select
from sqlalchemy.orm import Session
from app.auth import require_auth
from app.config import get_settings
from app.db import get_session
from app.models.tables import (Loan, ProjectionPoint, Scenario, ScenarioLoan,
ScenarioModifier)
from app.models.tables import (Loan, ProjectionPoint, ProjectionResult,
Scenario, ScenarioLoan, ScenarioModifier)
from app.services.projection_service import run_projection
router = APIRouter(prefix="/api/scenarios", tags=["scenarios"],
@@ -37,9 +38,9 @@ class ScenarioPatch(BaseModel):
class ModifierIn(BaseModel):
target_type: str
target_type: Literal["category", "recurring"]
target_id: int
kind: str
kind: Literal["percent", "absolute", "remove"]
value: Decimal = Decimal("0")
@@ -90,7 +91,13 @@ def create_scenario(data: ScenarioIn, session: Session = Depends(get_session)):
def patch_scenario(scenario_id: int, data: ScenarioPatch,
session: Session = Depends(get_session)):
scenario = _get_scenario(session, scenario_id)
for key, value in data.model_dump(exclude_unset=True).items():
fields = data.model_dump(exclude_unset=True)
if "name" in fields and fields["name"] != scenario.name:
clash = session.execute(
select(Scenario).where(Scenario.name == fields["name"])).scalar()
if clash is not None:
raise HTTPException(409, "Szenario existiert bereits")
for key, value in fields.items():
setattr(scenario, key, value)
session.commit()
session.refresh(scenario)
@@ -100,6 +107,16 @@ def patch_scenario(scenario_id: int, data: ScenarioPatch,
@router.delete("/{scenario_id}", status_code=204)
def delete_scenario(scenario_id: int, session: Session = Depends(get_session)):
scenario = _get_scenario(session, scenario_id)
# No DB-level cascades exist, so remove dependent rows explicitly to avoid
# foreign-key violations (would surface as a 500 on Postgres).
session.execute(delete(ProjectionPoint).where(
ProjectionPoint.scenario_id == scenario_id))
session.execute(delete(ProjectionResult).where(
ProjectionResult.scenario_id == scenario_id))
session.execute(delete(ScenarioLoan).where(
ScenarioLoan.scenario_id == scenario_id))
session.execute(delete(ScenarioModifier).where(
ScenarioModifier.scenario_id == scenario_id))
session.delete(scenario)
session.commit()

View File

@@ -8,7 +8,7 @@ from sqlalchemy.orm import Session
from app.auth import require_auth
from app.db import get_session
from app.models.tables import Category, Transaction
from app.models.tables import Account, Category, Transaction
from app.parsers.validate import dedup_hash
from app.services.categorize import apply_rules
@@ -79,6 +79,8 @@ def list_transactions(
@router.post("", response_model=TransactionOut, status_code=201)
def create_transaction(data: TransactionIn, session: Session = Depends(get_session)):
if session.get(Account, data.account_id) is None:
raise HTTPException(404, "Konto nicht gefunden")
h = dedup_hash(data.account_id, data.booking_date, data.amount, data.purpose)
existing = session.execute(
select(Transaction).where(Transaction.dedup_hash == h)).scalar()

View File

@@ -21,5 +21,10 @@
<main>
{% block content %}{% endblock %}
</main>
<script>
document.body.addEventListener('htmx:responseError', function (e) {
alert('Fehler: ' + (e.detail.xhr.responseText || e.detail.xhr.status));
});
</script>
</body>
</html>

View File

@@ -1,6 +1,6 @@
import pytest
from fastapi.testclient import TestClient
from sqlalchemy import create_engine
from sqlalchemy import create_engine, event
from sqlalchemy.orm import Session
from sqlalchemy.pool import StaticPool
@@ -14,6 +14,16 @@ def db():
"sqlite://", poolclass=StaticPool,
connect_args={"check_same_thread": False},
)
# SQLite does not enforce foreign keys unless told to per-connection. Turn
# enforcement on so the tests catch FK violations (missing cascades) the
# same way Postgres would in production.
@event.listens_for(engine, "connect")
def _fk_pragma(dbapi_conn, _):
cur = dbapi_conn.cursor()
cur.execute("PRAGMA foreign_keys=ON")
cur.close()
Base.metadata.create_all(engine)
with Session(engine) as session:
yield session

View File

@@ -0,0 +1,93 @@
"""Tests fuer die Final-Review-Findings F2, F3, F6."""
from datetime import date
from decimal import Decimal
from app.models.tables import (ProjectionPoint, ProjectionResult, Scenario,
Statement, Transaction)
H = {"Authorization": "Bearer test-key"}
def _account(client, iban="DE01"):
return client.post("/api/accounts", headers=H, json={
"bank": "DKB", "iban": iban, "name": "G", "type": "giro"}).json()
# ----------------------------------------------------------------- F2 cascades
def test_delete_projected_scenario_removes_dependent_rows(client, db):
sc = client.post("/api/scenarios", headers=H, json={"name": "Basis"}).json()
sid = sc["id"]
r = client.post(f"/api/scenarios/{sid}/project", headers=H,
params={"horizon_days": 10, "start_date": "2026-07-15"})
assert r.status_code == 200
assert db.query(ProjectionPoint).filter_by(scenario_id=sid).count() > 0
assert db.get(ProjectionResult, sid) is not None
assert client.delete(f"/api/scenarios/{sid}", headers=H).status_code == 204
db.expire_all()
assert db.get(Scenario, sid) is None
assert db.query(ProjectionPoint).filter_by(scenario_id=sid).count() == 0
assert db.get(ProjectionResult, sid) is None
def test_delete_loan_assigned_to_scenario(client):
loan = client.post("/api/loans", headers=H, json={
"name": "K1", "principal": "5000.00", "annual_rate_pct": "6.0",
"term_months": 48, "payout_date": "2026-07-20"}).json()
sc = client.post("/api/scenarios", headers=H, json={"name": "Kredit"}).json()
assert client.post(f"/api/scenarios/{sc['id']}/loans/{loan['id']}",
headers=H).status_code == 204
assert client.delete(f"/api/loans/{loan['id']}", headers=H).status_code == 204
def test_delete_confirmed_import_returns_409(client, db):
acc = _account(client)
db.add(Statement(filename="s.pdf", bank="dkb", account_id=acc["id"],
status="confirmed"))
db.commit()
sid = db.query(Statement).one().id
assert client.delete(f"/api/imports/{sid}", headers=H).status_code == 409
# --------------------------------------------------------------- F3 validation
def test_recurring_invalid_rhythm_422(client):
r = client.post("/api/recurring", headers=H, json={
"name": "X", "amount": "-1.00", "rhythm": "weekly", "due_day": 1})
assert r.status_code == 422
def test_recurring_due_day_zero_422(client):
r = client.post("/api/recurring", headers=H, json={
"name": "X", "amount": "-1.00", "rhythm": "monthly", "due_day": 0})
assert r.status_code == 422
def test_loan_invalid_repayment_type_422(client):
r = client.post("/api/loans", headers=H, json={
"name": "K", "principal": "1000.00", "annual_rate_pct": "3.0",
"term_months": 12, "payout_date": "2026-08-01",
"repayment_type": "balloon"})
assert r.status_code == 422
def test_modifier_invalid_kind_422(client):
sc = client.post("/api/scenarios", headers=H, json={"name": "S"}).json()
r = client.post(f"/api/scenarios/{sc['id']}/modifiers", headers=H, json={
"target_type": "category", "target_id": 1, "kind": "double",
"value": "10"})
assert r.status_code == 422
# ------------------------------------------------------- F6 409/404 statt 500
def test_patch_scenario_rename_clash_409(client):
client.post("/api/scenarios", headers=H, json={"name": "A"})
b = client.post("/api/scenarios", headers=H, json={"name": "B"}).json()
r = client.patch(f"/api/scenarios/{b['id']}", headers=H, json={"name": "A"})
assert r.status_code == 409
def test_create_transaction_unknown_account_404(client):
r = client.post("/api/transactions", headers=H, json={
"account_id": 9999, "booking_date": "2026-07-01",
"amount": "-10.00", "purpose": "X", "counterparty": ""})
assert r.status_code == 404

View File

@@ -48,6 +48,26 @@ def test_upload_preview_confirm(client, fake_parse, tmp_path, monkeypatch):
assert len(client.get("/api/transactions", headers=H).json()) == 1
def test_confirm_rechecks_duplicates_across_two_drafts(client, fake_parse, tmp_path, monkeypatch):
# Beide Auszuege werden importiert, bevor einer bestaetigt ist -> zum
# Importzeitpunkt ist keine Buchung ein Duplikat (dedup prueft nur gegen
# bestaetigte). Erst beim zweiten Confirm faellt auf, dass die Buchung nun
# bereits bestaetigt existiert. Der Re-Check muss sie ueberspringen, sonst
# wird dieselbe Buchung doppelt bestaetigt.
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()
sid1 = _upload(client, "a1.pdf").json()["id"]
sid2 = _upload(client, "a2.pdf").json()["id"]
assert client.post(f"/api/imports/{sid1}/confirm", headers=H).status_code == 200
assert client.post(f"/api/imports/{sid2}/confirm", headers=H).status_code == 200
# Zweites Confirm darf die Buchung nicht erneut bestaetigen.
assert len(client.get("/api/transactions", headers=H).json()) == 1
# Zweites Confirm eines bereits bestaetigten Imports -> 409.
assert client.post(f"/api/imports/{sid1}/confirm", headers=H).status_code == 409
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"))