perf+security+test: deferred audit follow-ups (health growth, first-run token, orchestrator coverage)

app/database.py

@@ -1073,13 +1073,27 @@ async def get_health_scores(days: int = 7) -> list[dict[str, Any]]:
 # --- Data Retention ---
 
 RETENTION_DAYS = 400
+# High-frequency uptime samples (check_ok / check_down, one per service every 5
+# minutes) are the dominant source of health_events growth, yet get_health_scores
+# only ever reads a bounded window. /api/health/scores caps that window at 90 days,
+# so we keep samples just past that (95d) — enough that no allowed query can
+# out-range its own samples, while still cutting the bulk sample rows ~76% versus
+# the 400-day lifecycle-event history (start/stop/restart/crash), which we keep in
+# full because those rows are rare and worth the long tail.
+HEALTH_CHECK_RETENTION_DAYS = 95
+_HEALTH_CHECK_EVENTS = ("check_ok", "check_down")
 
 
 async def purge_old_data() -> int:
-    """Delete earnings and health_events older than RETENTION_DAYS. Returns rows deleted."""
+    """Delete data past retention. Returns rows deleted.
+
+    Earnings and lifecycle health events are kept RETENTION_DAYS; the far more
+    numerous uptime-sample events are trimmed to HEALTH_CHECK_RETENTION_DAYS.
+    """
     db = await _get_db()
     try:
         cutoff = f"-{RETENTION_DAYS} days"
+        check_cutoff = f"-{HEALTH_CHECK_RETENTION_DAYS} days"
         c1 = await db.execute(
             "DELETE FROM earnings WHERE created_at < datetime('now', ?)",
             (cutoff,),
@@ -1088,7 +1102,29 @@ async def purge_old_data() -> int:
             "DELETE FROM health_events WHERE created_at < datetime('now', ?)",
             (cutoff,),
         )
+        c3 = await db.execute(
+            "DELETE FROM health_events WHERE event IN ('check_ok', 'check_down') AND created_at < datetime('now', ?)",
+            (check_cutoff,),
+        )
+        await db.commit()
+        return (c1.rowcount or 0) + (c2.rowcount or 0) + (c3.rowcount or 0)
+    finally:
+        await db.close()
+
+
+async def vacuum_database() -> None:
+    """Reclaim free pages left by retention deletes.
+
+    SQLite never shrinks the file on DELETE alone, so without a periodic VACUUM the
+    database keeps its high-water-mark size forever even as old rows are purged.
+    Run off-peak (weekly) — VACUUM rewrites the whole file and briefly locks it. We
+    commit first because VACUUM cannot run inside an open transaction, and checkpoint
+    the WAL afterwards so the freed space is actually returned to the filesystem.
+    """
+    db = await _get_db()
+    try:
         await db.commit()
-        return (c1.rowcount or 0) + (c2.rowcount or 0)
+        await db.execute("VACUUM")
+        await db.execute("PRAGMA wal_checkpoint(TRUNCATE)")
     finally:
         await db.close()

app/deps.py

@@ -11,13 +11,14 @@
 from __future__ import annotations
 
 import ipaddress
+import os
 from typing import Any
 
 from fastapi import HTTPException, Request
 from fastapi.responses import RedirectResponse
 from fastapi.templating import Jinja2Templates
 
-from app import auth
+from app import auth, setup_token
 
 __all__ = [
     "templates",
@@ -26,8 +27,15 @@
     "_require_writer",
     "_require_owner",
     "_require_private_network",
+    "_require_first_run_access",
+    "client_ip",
 ]
 
+# Opt-in: set CASHPILOT_TRUSTED_PROXY=1 only when the app sits behind exactly one
+# reverse proxy you control. X-Forwarded-For is attacker-controlled, so we ignore
+# it unless the operator asserts a trusted proxy is stripping/appending it.
+_TRUST_PROXY = os.getenv("CASHPILOT_TRUSTED_PROXY", "").strip().lower() in ("1", "true", "yes", "on")
+
 templates = Jinja2Templates(directory="app/templates")
 
 
@@ -57,13 +65,51 @@ def _require_owner(request: Request) -> dict[str, Any]:
     return user
 
 
+def client_ip(request: Request) -> str | None:
+    """Best-effort real client IP.
+
+    Behind a trusted reverse proxy (opt-in via ``CASHPILOT_TRUSTED_PROXY``) the
+    real peer is the right-most ``X-Forwarded-For`` entry — the value appended by
+    the trusted proxy, which a client cannot forge by prepending its own. Without
+    that opt-in we never trust the header and use the direct peer.
+    """
+    if _TRUST_PROXY:
+        xff = request.headers.get("x-forwarded-for", "")
+        parts = [p.strip() for p in xff.split(",") if p.strip()]
+        if parts:
+            return parts[-1]
+    return request.client.host if request.client else None
+
+
 def _require_private_network(request: Request) -> None:
-    """Block requests from public IPs (for first-run setup)."""
-    if not request.client or not request.client.host:
+    """Block requests whose real client IP is public (first-run defense in depth)."""
+    ip_str = client_ip(request)
+    if not ip_str:
         return
     try:
-        client_ip = ipaddress.ip_address(request.client.host)
+        ip = ipaddress.ip_address(ip_str)
     except ValueError:
         return
-    if not (client_ip.is_loopback or client_ip.is_private):
+    if not (ip.is_loopback or ip.is_private):
         raise HTTPException(status_code=403, detail="First-run setup only allowed from private networks")
+
+
+def _require_first_run_access(request: Request, setup_token_value: str | None = None) -> None:
+    """Gate first-run owner creation: private network AND the one-time setup token.
+
+    The network check alone is spoofable behind a reverse proxy (the peer is then
+    the proxy), so the setup token — printed to the server logs, readable only
+    with host access — is the real gate. The token is accepted from the explicit
+    argument (the registration form field) or the ``X-Setup-Token`` header.
+
+    Deliberately NOT read from the query string: a ``?setup_token=`` URL leaks the
+    secret into reverse-proxy access logs and browser history. The form field
+    (typed into the setup page) and the header keep it out of URLs.
+    """
+    _require_private_network(request)
+    token = setup_token_value or request.headers.get("x-setup-token")
+    if not setup_token.verify(token):
+        raise HTTPException(
+            status_code=403,
+            detail="First-run setup requires the setup token printed in the server logs",
+        )

app/main.py

@@ -31,7 +31,7 @@
 from pydantic import BaseModel
 from starlette.middleware.base import BaseHTTPMiddleware
 
-from app import auth, catalog, compose_generator, database, exchange_rates, fleet_key, metrics
+from app import auth, catalog, compose_generator, database, exchange_rates, fleet_key, metrics, setup_token
 
 logging.basicConfig(
     level=logging.INFO,
@@ -75,16 +75,16 @@ def _on_done(t: asyncio.Task) -> None:
 _LOGIN_WINDOW_SECONDS = 300
 
 
-def _check_login_rate(client_ip: str) -> None:
+def _check_login_rate(ip: str) -> None:
     now = monotonic()
-    attempts = _login_attempts[client_ip]
-    _login_attempts[client_ip] = [t for t in attempts if now - t < _LOGIN_WINDOW_SECONDS]
-    if len(_login_attempts[client_ip]) >= _LOGIN_MAX_ATTEMPTS:
+    attempts = _login_attempts[ip]
+    _login_attempts[ip] = [t for t in attempts if now - t < _LOGIN_WINDOW_SECONDS]
+    if len(_login_attempts[ip]) >= _LOGIN_MAX_ATTEMPTS:
         raise HTTPException(status_code=429, detail="Too many login attempts. Try again in a few minutes.")
 
 
-def _record_failed_login(client_ip: str) -> None:
-    _login_attempts[client_ip].append(monotonic())
+def _record_failed_login(ip: str) -> None:
+    _login_attempts[ip].append(monotonic())
 
 
 def _safe_json(raw: str, fallback: Any = None) -> Any:
@@ -273,6 +273,15 @@ async def _run_data_retention() -> None:
         logger.warning("Data retention error: %s", exc)
 
 
+async def _run_vacuum() -> None:
+    """Reclaim disk left by retention deletes (SQLite does not auto-shrink)."""
+    try:
+        await database.vacuum_database()
+        logger.info("Database VACUUM complete")
+    except Exception as exc:
+        logger.warning("Database VACUUM error: %s", exc)
+
+
 async def _check_stale_workers() -> None:
     """Mark workers as offline if stale, and purge workers offline > 1 hour."""
     try:
@@ -312,6 +321,22 @@ async def lifespan(app: FastAPI):
         _changed = _u.get("password_changed_at") or 0.0
         if _changed:
             auth.set_user_pwd_epoch(_u["id"], _changed)
+    # First-run setup token: while no users exist, require a one-time token
+    # (printed below) for /register so a proxy-exposed instance cannot be seized
+    # by the first public visitor. Persisted in config so it survives restarts;
+    # cleared once the owner account is created.
+    if not await database.has_any_users():
+        _tok = await database.get_config("_setup_token")
+        if not _tok:
+            _tok = setup_token.generate()
+            await database.set_config("_setup_token", _tok)
+        setup_token.set_active(_tok)
+        logger.warning(
+            "FIRST-RUN SETUP: no account exists yet. Open /register and enter this "
+            "one-time setup token to create the owner account: %s  (shown only here; "
+            "not embedded in any URL so it stays out of proxy logs and browser history)",
+            _tok,
+        )
     catalog.load_services()
     catalog.register_sighup()
 
@@ -355,6 +380,15 @@ def _on_job_event(event):
         coalesce=True,
         misfire_grace_time=300,
     )
+    scheduler.add_job(
+        _run_vacuum,
+        "interval",
+        weeks=1,
+        id="db_vacuum",
+        max_instances=1,
+        coalesce=True,
+        misfire_grace_time=300,
+    )
     scheduler.add_job(
         exchange_rates.refresh,
         "interval",
@@ -424,9 +458,11 @@ async def dispatch(self, request, call_next):
 from app.deps import (  # noqa: E402
     _login_redirect,  # noqa: F401  (re-exported for app.main.* test/router surface)
     _require_auth_api,
+    _require_first_run_access,  # noqa: F401  (re-exported for app.main.* router surface)
     _require_owner,
     _require_private_network,  # noqa: F401  (re-exported for app.main.* router surface)
     _require_writer,
+    client_ip,  # noqa: F401  (re-exported for app.main.* router surface)
     templates,  # noqa: F401  (re-exported for app.main.* router/test surface)
 )
 

app/routers/auth.py

@@ -45,7 +45,7 @@ async def do_login(
     username: str = Form(...),
     password: str = Form(...),
 ):
-    client_ip = request.client.host if request.client else "unknown"
+    client_ip = main.client_ip(request) or "unknown"
     try:
         main._check_login_rate(client_ip)
     except HTTPException:
@@ -85,6 +85,9 @@ async def page_register(request: Request, error: str = ""):
         user = main.auth.get_current_user(request)
         if not user or user.get("r") != "owner":
             return RedirectResponse("/login", status_code=303)
+    # The GET page is gated only by the network check so the operator can reach the
+    # form; the setup token is entered into a form field and verified on POST (never
+    # via URL, which would leak it into access logs / browser history).
     if is_first:
         main._require_private_network(request)
 
@@ -109,6 +112,7 @@ async def do_register(
     username: str = Form(...),
     password: str = Form(...),
     password_confirm: str = Form(...),
+    setup_token: str = Form(""),
 ):
     is_first = not await main.database.has_any_users()
 
@@ -119,7 +123,7 @@ async def do_register(
             raise HTTPException(status_code=403, detail="Only owners can add users")
 
     if is_first:
-        main._require_private_network(request)
+        main._require_first_run_access(request, setup_token)
 
     if not re.match(r"^[a-zA-Z0-9_-]{3,32}$", username):
         return main.templates.TemplateResponse(
@@ -133,6 +137,7 @@ async def do_register(
                 "button_text": "Create Account",
                 "error": "Username must be 3-32 alphanumeric characters (a-z, 0-9, _ -)",
                 "is_first": is_first,
+                "setup_token": setup_token if is_first else "",
             },
             status_code=400,
         )
@@ -149,6 +154,7 @@ async def do_register(
                 "button_text": "Create Account",
                 "error": "Passwords do not match",
                 "is_first": is_first,
+                "setup_token": setup_token if is_first else "",
             },
             status_code=400,
         )
@@ -165,6 +171,7 @@ async def do_register(
                 "button_text": "Create Account",
                 "error": "Password must be at least 10 characters",
                 "is_first": is_first,
+                "setup_token": setup_token if is_first else "",
             },
             status_code=400,
         )
@@ -182,6 +189,7 @@ async def do_register(
                 "button_text": "Create Account",
                 "error": "Username already taken",
                 "is_first": is_first,
+                "setup_token": setup_token if is_first else "",
             },
             status_code=400,
         )
@@ -191,6 +199,11 @@ async def do_register(
     hashed = main.auth.hash_password(password)
     user_id = await main.database.create_user(username, hashed, role)
 
+    if is_first:
+        # Owner now exists — retire the one-time setup token permanently.
+        await main.database.delete_config_keys(["_setup_token"])
+        main.setup_token.clear()
+
     token = main.auth.create_session_token(user_id, username, role)
     dest = "/setup" if is_first else "/"
     response = RedirectResponse(dest, status_code=303)

app/setup_token.py

@@ -0,0 +1,62 @@
+"""First-run setup-token gate.
+
+The very first account created on a fresh install becomes the ``owner``. The
+private-network check in :mod:`app.deps` is not enough on its own: behind a
+reverse proxy ``request.client`` is the *proxy* (a loopback/private address), so
+the network check always passes and the first person to reach ``/register`` from
+the public internet could seize the owner account.
+
+This module adds a proxy-independent second factor: a one-time token generated at
+startup while no users exist, printed to the container logs. Only someone who can
+read the server logs (i.e. already has host access) can complete first-run setup.
+Once the owner account is created the token is cleared and never required again
+(further users are added by an authenticated owner).
+
+The active token is held in a module global so the synchronous request guard can
+check it without a DB round-trip; it is also persisted in the ``config`` table so
+it survives restarts until consumed.
+"""
+
+from __future__ import annotations
+
+import hmac
+import secrets
+
+# The token currently required for first-run registration, or ``None`` when no
+# first-run gate is active (owner already exists, or not yet initialised).
+_active: str | None = None
+
+
+def generate() -> str:
+    """Return a fresh, URL-safe setup token."""
+    return secrets.token_urlsafe(24)
+
+
+def set_active(token: str | None) -> None:
+    """Install (or clear, with ``None``) the token required for first-run setup."""
+    global _active
+    _active = token or None
+
+
+def clear() -> None:
+    """Drop the first-run gate — called once the owner account exists."""
+    set_active(None)
+
+
+def active() -> str | None:
+    """Return the currently required setup token, or ``None`` if none is active."""
+    return _active
+
+
+def verify(provided: str | None) -> bool:
+    """Check a caller-supplied token against the active one.
+
+    Returns ``True`` when no token is active (nothing to enforce) or when the
+    supplied value matches in constant time; ``False`` otherwise.
+    """
+    current = _active
+    if current is None:
+        return True
+    if not provided:
+        return False
+    return hmac.compare_digest(current, provided)