diff --git a/backend/.env.example b/backend/.env.example index e50041b..c4403a5 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -66,6 +66,16 @@ AIDND_DEMO_TURNS_PER_DAY= # Local (single-user) installs are always treated as power users. AIDND_POWER_USERS= +# --- Guest retention (only active when AIDND_MULTI_USER=1) --- +# Every first visit mints a guest account, so a public demo collects one row +# per visitor. A guest with no activity for this many days is deleted along +# with its scenarios, adventures and actions. Registered accounts are never +# touched. Default: 5. Set 0 to keep guests forever. +AIDND_GUEST_RETENTION_DAYS= +# How often a running process re-checks. The sweep also runs once at startup, +# which is what actually fires on hosts that sleep. Default: 6 +AIDND_CLEANUP_INTERVAL_HOURS= + # The AI endpoint/API key/model are NOT env vars — they are configured at # runtime in the app's Settings page and stored (encrypted) in the database. # diff --git a/backend/app/cleanup.py b/backend/app/cleanup.py new file mode 100644 index 0000000..762ca3b --- /dev/null +++ b/backend/app/cleanup.py @@ -0,0 +1,158 @@ +"""Retention policy for throwaway guest accounts. + +In multi-user mode every first visit mints a `users` row (GET /api/auth/me), +so a public demo accumulates one account per curious visitor — most of whom +never come back, each leaving behind whatever scenarios, adventures, actions +and memories they generated. This drops guests that have gone quiet for +AIDND_GUEST_RETENTION_DAYS (default 5) along with everything they made. + +Why this is safe to run unattended: + +- Only rows with `is_guest` AND `email IS NULL` are ever touched, and both + clauses are checked rather than either alone. Registering upgrades the row + in place (is_guest -> False), so a guest who signs up keeps everything; + local mode's implicit single user is also is_guest=False. +- Idle time is COALESCE(last_seen_at, created_at). `auth._touch` only writes + last_seen_at once an hour, and a guest minted by /auth/me has NULL until + its *second* request, so created_at is the honest floor for a brand-new + visitor — without the coalesce those rows look infinitely old. +- Nothing a guest owns is reachable by anyone else: `is_public` is an + output-only field (see schemas.ScenarioBase), so only seeded scenarios — + which have user_id NULL and are therefore outside this filter entirely — + are shared. Deleting a guest can't take content away from another user. + +Why one Core DELETE instead of an ORM cascade: `db.delete(user)` would SELECT +every adventure, action, memory and story card into Python purely to delete +them, which on Neon is exactly the egress pattern that has already cost this +project once. Every foreign key from users downwards is ON DELETE CASCADE +(users -> scenarios/adventures/scripts/settings -> actions/memories/cards), so +the database does the whole graph in one statement and ships back a row count. + +No index is added for the scan: the sweep runs a handful of times a day +against a table with at most a few thousand rows, which is not worth a +migration and the schema surface that comes with it. +""" + +import asyncio +import logging +import os +from datetime import datetime, timedelta + +from sqlalchemy import delete, func +from sqlalchemy.orm import Session +from starlette.concurrency import run_in_threadpool + +from . import auth, models +from .database import SessionLocal + +logger = logging.getLogger(__name__) + + +def _int_env(name: str, default: int) -> int: + try: + return int(os.environ.get(name, "").strip() or default) + except ValueError: + logger.warning("%s is not an integer; using %d.", name, default) + return default + + +# Days of inactivity before a guest account is dropped. 0 or less disables the +# policy entirely, for a deployment that would rather keep everything. +RETENTION_DAYS = _int_env("AIDND_GUEST_RETENTION_DAYS", 5) + +# How often a long-lived process re-checks. Hours, not minutes: nothing here is +# time-critical, and on Render's free tier the service sleeps and cold-starts +# often enough that the startup sweep does most of the work by itself. +SWEEP_INTERVAL_SECONDS = _int_env("AIDND_CLEANUP_INTERVAL_HOURS", 6) * 3600 + + +def enabled() -> bool: + """Guests only exist in multi-user mode, so local runs skip the sweep + rather than pointing a DELETE at a database that has nothing to collect.""" + return auth.MULTI_USER and RETENTION_DAYS > 0 + + +def delete_stale_guests(db: Session, *, now: datetime | None = None) -> int: + """Delete guests idle for RETENTION_DAYS or more. Returns the row count. + + The caller owns error handling; `sweep` is the safe wrapper. + """ + if RETENTION_DAYS <= 0: + return 0 + # Stored timestamps are naive UTC on both backends (SQLite drops tzinfo; + # Postgres columns are TIMESTAMP WITHOUT TIME ZONE with the session pinned + # to UTC in database.py). Match that exactly so the comparison can't hinge + # on how a given dialect renders an aware value. + reference = now or models.utcnow() + cutoff = reference.replace(tzinfo=None) - timedelta(days=RETENTION_DAYS) + + stmt = ( + delete(models.User) + .where( + models.User.is_guest.is_(True), + models.User.email.is_(None), + func.coalesce(models.User.last_seen_at, models.User.created_at) < cutoff, + ) + # Without this, "auto" can't evaluate coalesce in Python and falls back + # to fetching every matching primary key first — a second round trip + # for nothing, since this session holds no User objects to synchronize. + .execution_options(synchronize_session=False) + ) + removed = db.execute(stmt).rowcount or 0 + db.commit() + return removed + + +def sweep() -> int: + """One pass, with its own session. Never raises: a failed cleanup must not + be able to take the app down (same rule as seeding).""" + if not enabled(): + return 0 + db = SessionLocal() + try: + removed = delete_stale_guests(db) + if removed: + logger.info( + "Cleaned up %d guest account(s) idle for %d+ days.", + removed, + RETENTION_DAYS, + ) + return removed + except Exception: + db.rollback() + logger.exception("Guest cleanup failed; continuing without it.") + return 0 + finally: + db.close() + + +async def _sweep_loop() -> None: + while True: + # Blocking DB work: keep it off the event loop, which is also serving + # SSE turn streams. + await run_in_threadpool(sweep) + await asyncio.sleep(SWEEP_INTERVAL_SECONDS) + + +def start_sweeper() -> asyncio.Task | None: + """Kick off the periodic sweep; None when the policy is off.""" + if not enabled(): + logger.info("Guest cleanup disabled (multi_user=%s, retention_days=%d).", + auth.MULTI_USER, RETENTION_DAYS) + return None + logger.info( + "Guest cleanup on: deleting guests idle %d+ days, every %d hour(s).", + RETENTION_DAYS, + SWEEP_INTERVAL_SECONDS // 3600, + ) + return asyncio.create_task(_sweep_loop()) + + +async def stop_sweeper(task: asyncio.Task | None) -> None: + if task is None: + return + task.cancel() + try: + await task + except asyncio.CancelledError: + pass diff --git a/backend/app/main.py b/backend/app/main.py index a59052a..3156656 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -1,4 +1,5 @@ import os +from contextlib import asynccontextmanager from pathlib import Path from fastapi import FastAPI @@ -6,6 +7,7 @@ from fastapi.middleware.cors import CORSMiddleware from fastapi.staticfiles import StaticFiles from starlette.exceptions import HTTPException as StarletteHTTPException +from . import cleanup from .auth import MULTI_USER from .database import engine from .limits import BodySizeLimitMiddleware @@ -24,6 +26,18 @@ CORS_ORIGINS = [ if o.strip() ] or ["http://localhost:5173", "http://127.0.0.1:5173"] +@asynccontextmanager +async def lifespan(_app: FastAPI): + # Sweeps once on boot, then on an interval. Booting is the reliable + # trigger on Render's free tier, where the service sleeps after ~15 + # minutes and a long-running timer rarely gets to fire. + sweeper = cleanup.start_sweeper() + try: + yield + finally: + await cleanup.stop_sweeper(sweeper) + + # The interactive API docs stay local-only: in multi-user mode they just hand # strangers a map of the API surface. app = FastAPI( @@ -31,6 +45,7 @@ app = FastAPI( docs_url=None if MULTI_USER else "/docs", redoc_url=None, openapi_url=None if MULTI_USER else "/openapi.json", + lifespan=lifespan, ) app.add_middleware( diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 53f4c20..9aa43ce 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -3,7 +3,7 @@ import re from fastapi import APIRouter, Depends, HTTPException, Request, Response from sqlalchemy.orm import Session -from .. import auth, limits, models, schemas, security +from .. import auth, cleanup, limits, models, schemas, security from ..database import get_db from .settings import get_settings @@ -34,6 +34,10 @@ def me_payload(user: models.User, db: Session) -> dict: "is_guest": user.is_guest, # Trusted testers: unmetered demo turns, plus the AI Chat scratchpad. "power_user": auth.is_power_user(user), + # How long an idle guest is kept before cleanup deletes it (None when + # the policy is off). Served rather than hardcoded in the UI so the + # number a guest is shown is the number actually enforced. + "guest_retention_days": cleanup.RETENTION_DAYS if cleanup.enabled() else None, "demo": { "enabled": auth.demo_enabled(), "using_demo": cfg.using_demo, diff --git a/backend/tests/test_guest_cleanup.py b/backend/tests/test_guest_cleanup.py new file mode 100644 index 0000000..67a72df --- /dev/null +++ b/backend/tests/test_guest_cleanup.py @@ -0,0 +1,210 @@ +"""Guest retention policy — app/cleanup.py. + +Covers the two things that matter: that idle guests and their whole data +graph actually go, and that nothing else ever does. + + python -m pytest tests/test_guest_cleanup.py -v +""" +import os +import tempfile +from datetime import timedelta + +_tmp = tempfile.NamedTemporaryFile(suffix=".db", delete=False) +_tmp.close() +os.environ["AIDND_DB_PATH"] = _tmp.name +os.environ.pop("AIDND_DATABASE_URL", None) +os.environ.pop("DATABASE_URL", None) + +import pytest +from sqlalchemy import create_engine, event +from sqlalchemy.orm import sessionmaker + +from app import cleanup, models +from app.database import Base +from app.migrations import bootstrap + + +@pytest.fixture() +def db(tmp_path): + engine = create_engine(f"sqlite:///{tmp_path/'t.db'}", connect_args={"check_same_thread": False}) + + @event.listens_for(engine, "connect") + def _fk(dbapi_connection, _record): + # The whole policy leans on ON DELETE CASCADE; SQLite ignores every + # one of them unless this is set (same as database.py does). + cur = dbapi_connection.cursor() + cur.execute("PRAGMA foreign_keys=ON") + cur.close() + + bootstrap(engine) + Base.metadata.create_all(bind=engine) + session = sessionmaker(bind=engine, autoflush=False, expire_on_commit=False)() + yield session + session.close() + + +NOW = models.utcnow().replace(tzinfo=None) + + +def make_user(db, *, days_idle=None, days_old=0, guest=True, email=None): + """A user last seen `days_idle` ago (None = never seen, only created).""" + user = models.User( + is_guest=guest, + email=email, + password_hash=None if email is None else "x", + created_at=NOW - timedelta(days=days_old), + last_seen_at=None if days_idle is None else NOW - timedelta(days=days_idle), + ) + db.add(user) + db.commit() + return user + + +def sweep(db): + return cleanup.delete_stale_guests(db, now=NOW) + + +def alive(db, user_id): + # A count, not db.get: the sweep deletes with synchronize_session=False, so + # the session's identity map still holds the object and db.get would answer + # from memory without ever asking the database. + return db.query(models.User).filter(models.User.id == user_id).count() == 1 + + +# ---------- what goes ---------- + +def test_deletes_guest_idle_past_the_window(db): + user = make_user(db, days_idle=6) + assert sweep(db) == 1 + assert not alive(db, user.id) + + +def test_keeps_guest_inside_the_window(db): + user = make_user(db, days_idle=4) + assert sweep(db) == 0 + assert alive(db, user.id) + + +def test_boundary_is_not_yet_stale(db): + # Exactly 5 days survives; the comparison is strict. + user = make_user(db, days_idle=cleanup.RETENTION_DAYS) + assert sweep(db) == 0 + assert alive(db, user.id) + + +def test_never_seen_guest_falls_back_to_created_at(db): + """last_seen_at is NULL until a guest's second request (auth._touch runs + hourly), so a coalesce-less query would delete brand-new visitors.""" + fresh = make_user(db, days_idle=None, days_old=0) + stale = make_user(db, days_idle=None, days_old=9) + assert sweep(db) == 1 + assert alive(db, fresh.id) + assert not alive(db, stale.id) + + +def test_recent_visit_beats_an_old_created_at(db): + # A long-standing guest who came back yesterday stays. + user = make_user(db, days_idle=1, days_old=90) + assert sweep(db) == 0 + assert alive(db, user.id) + + +# ---------- what must never go ---------- + +def test_spares_registered_users(db): + """Registering upgrades the guest row in place, so an idle account here is + a real user with real data — the whole point of signing up.""" + user = make_user(db, days_idle=400, guest=False, email="a@b.com") + assert sweep(db) == 0 + assert alive(db, user.id) + + +def test_spares_the_local_mode_user(db): + # email NULL but is_guest False: local mode's implicit owner of everything. + user = make_user(db, days_idle=400, guest=False) + assert sweep(db) == 0 + assert alive(db, user.id) + + +def test_spares_a_guest_flagged_row_that_has_an_email(db): + # Shouldn't exist, but both clauses are checked so it can't be collected. + user = make_user(db, days_idle=400, guest=True, email="odd@b.com") + assert sweep(db) == 0 + assert alive(db, user.id) + + +def test_leaves_seeded_public_scenarios_alone(db): + """Seeded demo content has user_id NULL, so it is outside the filter.""" + seeded = models.Scenario(user_id=None, is_public=True, title="Demo") + db.add(seeded) + make_user(db, days_idle=30) + db.commit() + assert sweep(db) == 1 + assert db.query(models.Scenario).filter(models.Scenario.id == seeded.id).count() == 1 + + +def test_disabled_when_retention_is_zero(db, monkeypatch): + monkeypatch.setattr(cleanup, "RETENTION_DAYS", 0) + user = make_user(db, days_idle=999) + assert sweep(db) == 0 + assert alive(db, user.id) + + +def test_enabled_requires_multi_user(monkeypatch): + from app import auth + monkeypatch.setattr(auth, "MULTI_USER", False) + assert cleanup.enabled() is False + monkeypatch.setattr(auth, "MULTI_USER", True) + monkeypatch.setattr(cleanup, "RETENTION_DAYS", 5) + assert cleanup.enabled() is True + monkeypatch.setattr(cleanup, "RETENTION_DAYS", 0) + assert cleanup.enabled() is False + + +# ---------- the cascade ---------- + +def test_deletes_the_whole_data_graph(db): + """One DELETE has to take the adventure, its actions and memories, the + story cards and the settings row with it — nothing is loaded into Python, + so if the FK cascade isn't reaching, rows are silently orphaned (or the + statement errors) rather than tidied.""" + user = make_user(db, days_idle=30) + scenario = models.Scenario(user_id=user.id, title="S") + db.add(scenario) + db.commit() + adventure = models.Adventure(user_id=user.id, scenario_id=scenario.id, title="A") + db.add(adventure) + db.commit() + db.add_all([ + models.Action(adventure_id=adventure.id, index=0, type="ai", text="t"), + models.Memory(adventure_id=adventure.id, text="m", source_start=0, source_end=0), + models.StoryCard(adventure_id=adventure.id, name="c"), + models.Settings(user_id=user.id), + ]) + db.commit() + + assert sweep(db) == 1 + + for model in (models.Scenario, models.Adventure, models.Action, + models.Memory, models.StoryCard, models.Settings): + assert db.query(model).count() == 0, f"{model.__name__} rows survived" + + +def test_one_users_cleanup_does_not_touch_another(db): + keeper = make_user(db, days_idle=1) + keep_adv = models.Adventure(user_id=keeper.id, title="mine") + goner = make_user(db, days_idle=30) + db.add_all([keep_adv, models.Adventure(user_id=goner.id, title="theirs")]) + db.commit() + + assert sweep(db) == 1 + remaining = db.query(models.Adventure).all() + assert [a.title for a in remaining] == ["mine"] + + +def test_sweep_swallows_errors(monkeypatch): + """A broken cleanup must not take the app down (same rule as seeding).""" + monkeypatch.setattr(cleanup.auth, "MULTI_USER", True) + monkeypatch.setattr(cleanup, "delete_stale_guests", + lambda *a, **k: (_ for _ in ()).throw(RuntimeError("boom"))) + assert cleanup.sweep() == 0 diff --git a/docs/GUIDE.md b/docs/GUIDE.md index ca64d86..3921315 100644 --- a/docs/GUIDE.md +++ b/docs/GUIDE.md @@ -732,6 +732,21 @@ guest survives with no re-parenting and no migration step. Three kinds of row sh users table: local (email NULL, not guest), guest (email NULL, guest), registered (email set). +**Guests expire; accounts don't.** One row per curious visitor adds up, so `cleanup.py` +deletes guests idle for `AIDND_GUEST_RETENTION_DAYS` (default 5) — measured as +`COALESCE(last_seen_at, created_at)`, because `_touch` only writes `last_seen_at` hourly +and a guest minted by `/auth/me` has NULL until its second request. The filter requires +both `is_guest` *and* `email IS NULL`, so upgrading in place is also how you opt out of +expiry. It runs once at startup (the reliable trigger on a host that sleeps) and then +every few hours. + +It's a single Core `DELETE`, not `db.delete(user)`: the ORM path would SELECT every +adventure, action and memory into Python purely to delete them, and the FK graph is +`ON DELETE CASCADE` from `users` all the way down, so the database can do the whole graph +in one statement. Nothing a guest owns is visible to anyone else either — `is_public` is +output-only, so shared content is exactly the seeded scenarios, which have `user_id NULL` +and never match the filter. + ## 3.2 The shared demo key The demo lets people play with no signup and no API key, on a key the server pays for. That @@ -768,7 +783,7 @@ Everything derives from one server-side secret (`AIDND_SECRET_KEY`). | Thing | Mechanism | |---|---| | Passwords | `hashlib.scrypt`, N=2^14, r=8, p=1, per-password salt, constant-time compare. Stdlib, so no extra dependency. | -| Sessions | `v1..`, no expiry — long-lived guest sessions are the point. | +| Sessions | `v1..`, no expiry — long-lived guest sessions are the point. A cookie can outlive a swept guest row; that resolves to a 401, which the frontend already turns into a fresh session. | | Stored LLM API keys | Fernet (AES) encryption at rest, key derived from the secret, `enc:` prefix so legacy plaintext rows are recognisable and migratable. | The secret auto-generates into a file next to the database for local installs (zero config), diff --git a/docs/guide.html b/docs/guide.html index 2135066..2c1126a 100644 --- a/docs/guide.html +++ b/docs/guide.html @@ -1094,6 +1094,22 @@ load. Registering sets email and password_hash on that — so every adventure they played as a guest survives with no re-parenting and no migration step. Three kinds of row share the users table: local, guest, and registered.

+

Guests expire; accounts don't. One row per curious visitor adds up, so +cleanup.py deletes guests idle for AIDND_GUEST_RETENTION_DAYS +(default 5) — measured as COALESCE(last_seen_at, created_at), since +_touch only writes last_seen_at hourly and a freshly minted guest +has NULL until its second request. The filter requires both is_guest +and email IS NULL, so upgrading in place is also how you opt out of +expiry. It sweeps once at startup — the reliable trigger on a host that sleeps — and then +every few hours.

+ +

It's a single Core DELETE, not db.delete(user): the ORM path +would SELECT every adventure, action and memory into Python purely to delete them, and the +foreign keys are ON DELETE CASCADE from users all the way down, so +the database does the whole graph in one statement. Nothing a guest owns is visible to +anyone else either — is_public is output-only, so shared content is exactly the +seeded scenarios, which have user_id NULL and never match the filter.

+

3.2The shared demo key

The demo lets people play with no signup and no API key, on a key the server pays for. That is a @@ -1133,7 +1149,7 @@ successful turn.

ThingMechanism Passwordshashlib.scrypt, N=2¹⁴, r=8, p=1, per-password salt, constant-time compare. Stdlib, so no extra dependency. - Sessionsv1.<user_id>.<HMAC-SHA256>, no expiry — long-lived guest sessions are the point. + Sessionsv1.<user_id>.<HMAC-SHA256>, no expiry — long-lived guest sessions are the point. A cookie can outlive a swept guest row; that resolves to a 401, which the frontend already turns into a fresh session. Stored LLM API keysFernet encryption at rest, key derived from the secret, enc: prefix so legacy plaintext rows are recognisable and migratable. diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index 93d59ee..6d07100 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -68,7 +68,12 @@ export default function App() {
{me.is_guest ? ( <> - Playing as guest — sign up to keep your adventures + + Playing as guest — sign up to keep your adventures + @@ -84,7 +89,8 @@ export default function App() { {authMode && ( - setAuthMode(null)} onAuthed={onAuthed} /> + setAuthMode(null)} onAuthed={onAuthed} + retentionDays={me?.guest_retention_days} /> )} ) diff --git a/frontend/src/components.jsx b/frontend/src/components.jsx index 7dd53aa..df64be7 100644 --- a/frontend/src/components.jsx +++ b/frontend/src/components.jsx @@ -234,7 +234,7 @@ export function PlaceholderModal({ title, names, onSubmit, onCancel }) { // Phase 8: register/login for the hosted multi-user mode. `onAuthed(me)` gets // the fresh /auth/me payload after success. -export function AuthModal({ mode: initialMode, onClose, onAuthed }) { +export function AuthModal({ mode: initialMode, onClose, onAuthed, retentionDays }) { const [mode, setMode] = useState(initialMode || 'register') const [email, setEmail] = useState('') const [password, setPassword] = useState('') @@ -283,6 +283,12 @@ export function AuthModal({ mode: initialMode, onClose, onAuthed }) { {registering ? 'Everything you’ve played as a guest stays with your new account, and you can pick it up from any device.' : 'Log in to reach your adventures.'} + {/* Guest data really is deleted, so say so where the decision is + being made. The window comes from the server (see cleanup.py) so + it can't drift from what's enforced. */} + {registering && retentionDays ? ( + <> Guest adventures are deleted after {retentionDays} days without a visit. + ) : null}

diff --git a/render.yaml b/render.yaml index 572f8e2..a365227 100644 --- a/render.yaml +++ b/render.yaml @@ -51,4 +51,11 @@ services: - key: AIDND_POWER_USERS sync: false + # Guest retention: one account is minted per first-time visitor, so idle + # ones are collected (with their adventures) to keep the free-tier + # Postgres from filling with abandoned demo data. Registered accounts are + # never touched. 0 would disable it. + - key: AIDND_GUEST_RETENTION_DAYS + value: "5" + # CORS is unset on purpose: the SPA is served same-origin by FastAPI.