diff --git a/CHANGELOG.md b/CHANGELOG.md index b2f4c43..1f52062 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,29 @@ navigation and console layout, and the internal seams (event dispatch, service ownership, projection) that the multi-replica work depends on. ### Added +- **The app runs on more than one replica** (#213) — every piece of cross-request state + moves into PostgreSQL, so there is no longer a single-replica constraint and no Redis + or broker to operate. WebSocket frames and config invalidation travel over + `LISTEN`/`NOTIFY` as compact id descriptors that each replica reads back and renders + for its own sockets; scheduled inject releases, triggered communications, LLM + pipelines and the nightly audit purge become durable jobs (procrastinate); rate-limit + counters become rows, so the login, registration, and reset limits no longer multiply + by the replica count; and startup migrations serialise on a Postgres advisory lock. + + Everything is published or enqueued **inside** the transaction that makes it true. + Postgres holds a `NOTIFY` until COMMIT and discards it on rollback, and a job row + commits with the state change that warranted it — so "committed but never announced" + and "committed but never enqueued" stop being possible. That closes the bug class + behind #211 (triggered communications lost on restart) and #218 (a scheduled release + stranded when a worker stood down mid-response) structurally rather than case by case, + and retires the hand-built coordination those fixes needed. + + Kubernetes manifests keep `replicas: 1` in the base for a storage reason rather than a + state one — the uploads volume is `ReadWriteOnce` and a PVC's access modes cannot be + changed in place. Clusters with an RWX StorageClass apply the new + `k8s/overlays/multi-replica` for two replicas and rolling deploys. Do not front the app + with a transaction-mode connection pooler: `LISTEN` needs a session that outlives a + transaction. - **Runtime configuration** — non-secret settings move out of env-only config and into the admin UI, following the singleton-row + cached-config pattern already used by `/admin/audit` and `/admin/proxy`. Email/SMTP, general settings (registration, token diff --git a/CLAUDE.md b/CLAUDE.md index aedd708..86056b4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -36,8 +36,11 @@ structure. Subsystem deep-dives are indexed there and live in [PLAN.md](PLAN.md) transaction; **whoever commits dispatches** (`await dispatch(session)`, session open). - Services own queries; routers authorize/call/serialize; `app/schemas/` holds only boundary-crossing models (placement rule #214). -- **Single-replica app**: all cross-request state (ws_manager, timers, rate limits, - config caches) is in-process. Read that section before adding any. +- **Multi-replica**: cross-request state goes through Postgres — `pg_bus` (LISTEN/NOTIFY) + for anything other replicas must hear, `task_queue` for anything that must happen later. + Never add in-process cross-request state; `ws_manager` (this process's own sockets) is + the one legitimate exception. Publish and enqueue **inside** the transaction that makes + the thing true, never after the commit. - New UI control classes must re-assert the 40px min-height floor (class selectors beat the global floor; a Playwright touch-target test enforces ≥40/44px). - Branching wording trap: **participants choose the path; the facilitator controls the diff --git a/PLAN.md b/PLAN.md index 81d2bc5..bb4bfa0 100644 --- a/PLAN.md +++ b/PLAN.md @@ -532,13 +532,13 @@ Design notes worth keeping. `ExerciseStateChanged` deliberately does **not** car **Self-registration is participant-only**: `RegisterRequest` no longer accepts `role` — `POST /api/auth/register` always creates a `participant` (#8). Privileged roles are assigned out-of-band (seeded or via the admin create-user endpoint below); extra body fields are ignored by pydantic. The register template no longer offers a role selector. -**Registration controls (#67)**: `REGISTRATION_ENABLED` (default `true`) gates self-service registration. When `false`, `_require_registration_enabled()` (`auth.py`) makes `POST /api/auth/register` return `403` (with an `auth.register` `deny` audit event), the `/register` UI route redirects to `/login`, and the login page hides the "Register" link (`_auth_context()` passes `registration_enabled` to the template). Independently of the toggle, registration is flood-protected by a **second `RateLimiter` singleton** `registration_rate_limiter` (`rate_limit.py`), keyed **per source IP** and counting **every** attempt (not just failures — the email is what's being created, so it can't be part of the key); over `REGISTRATION_MAX_ATTEMPTS` (5) within `REGISTRATION_LOCKOUT_SECONDS` (3600, deliberately longer than login's 300 — this throttles account creation, not password guessing) the route returns `429` + `Retry-After`. The admin path is `POST /api/users` (`users.py`, `require_admin`): an admin provisions an account with any `role`/`is_admin` (schema `AdminCreateUserRequest`), bypassing both the toggle and the rate limit, audited as `admin.user_create`. Same single-process constraint as the login limiter; tests clear both via the conftest autouse fixture. +**Registration controls (#67)**: `REGISTRATION_ENABLED` (default `true`) gates self-service registration. When `false`, `_require_registration_enabled()` (`auth.py`) makes `POST /api/auth/register` return `403` (with an `auth.register` `deny` audit event), the `/register` UI route redirects to `/login`, and the login page hides the "Register" link (`_auth_context()` passes `registration_enabled` to the template). Independently of the toggle, registration is flood-protected by a **second `RateLimiter` singleton** `registration_rate_limiter` (`rate_limit.py`), keyed **per source IP** and counting **every** attempt (not just failures — the email is what's being created, so it can't be part of the key); over `REGISTRATION_MAX_ATTEMPTS` (5) within `REGISTRATION_LOCKOUT_SECONDS` (3600, deliberately longer than login's 300 — this throttles account creation, not password guessing) the route returns `429` + `Retry-After`. The admin path is `POST /api/users` (`users.py`, `require_admin`): an admin provisions an account with any `role`/`is_admin` (schema `AdminCreateUserRequest`), bypassing both the toggle and the rate limit, audited as `admin.user_create`. Counters are rows in the UNLOGGED `rate_limit_hits` table, so the limit holds across replicas rather than multiplying by their count (#213); tests clear the table via the conftest autouse fixture. **Cookie security & CSRF**: the auth cookie is set with `Secure` (gated on `settings.cookies_secure`, default `not dev_mode`; override with `COOKIE_SECURE`). `CSRFOriginMiddleware` (`app/middleware.py`) verifies `Origin`/`Referer` for cookie-authenticated state-changing requests under `/api/` (#10). Bearer-`Authorization` requests and `/api/auth/*` are exempt. Since #264 the app's own `apiFetch` calls are cookie-authenticated (no Bearer), so their state-changing requests are covered by this `Origin`/`Referer` check — same-origin, so they pass; the Bearer exemption now only applies to non-browser API clients. Extra allowed origins via `TRUSTED_ORIGINS`. **Security headers (#77)**: `SecurityHeadersMiddleware` (`app/middleware.py`) emits the full security header set — the strict `CONTENT_SECURITY_POLICY` (`script-src 'self'`, no `unsafe-*`; `style-src` keeps `'unsafe-inline'` for dynamic `style=` attrs), `X-Frame-Options: DENY`, `X-Content-Type-Options: nosniff`, `Referrer-Policy`, a deny-all `Permissions-Policy`, `Cross-Origin-Opener-Policy`, and (only when `not dev_mode`) `Strict-Transport-Security`. `build_security_headers()` is a pure function evaluated per response (so `dev_mode` is monkeypatchable in tests). It uses **setdefault semantics** so the per-download `nosniff` on attachment downloads (#16) is preserved, not duplicated. Registered **outside** `CSRFOriginMiddleware` (so CSRF-blocked 403s still carry the headers) and **inside** `AuditContextMiddleware`. The **app is the single source of truth** — `docker/Caddyfile` and `k8s/base/caddy/configmap.yaml` set no security headers (only `nosniff` on the directly-served `/static/` location). -**Login brute-force protection**: `app/services/rate_limit.py` is an in-memory sliding-window limiter keyed by `ip:email`. After `LOGIN_MAX_ATTEMPTS` (default 5) failures within `LOGIN_LOCKOUT_SECONDS` (default 300) the login route returns `429` with `Retry-After`; a success resets the counter (#11). Oversized (>72 UTF-8 byte) bcrypt inputs and verifier value errors take this same counted failure path rather than escaping as a `500`. In-memory ⇒ single-process only (same constraint as `ws_manager`). Tests clear it via an autouse fixture. +**Login brute-force protection**: `app/services/rate_limit.py` is a sliding-window limiter keyed by `ip:email`, counting one row per attempt in the UNLOGGED `rate_limit_hits` table (#213). After `LOGIN_MAX_ATTEMPTS` (default 5) failures within `LOGIN_LOCKOUT_SECONDS` (default 300) the login route returns `429` with `Retry-After`; a success resets the counter (#11). Oversized (>72 UTF-8 byte) bcrypt inputs and verifier value errors take this same counted failure path rather than escaping as a `500`. Still *sliding*, not a fixed bucket — a fixed bucket lets an attacker spend the whole allowance at the end of one window and again at the start of the next. Attempts are counted on their own connection, so the 401's rollback cannot erase a strike, and expired rows are swept by an hourly maintenance job rather than opportunistically on a read path (#49). Tests clear it via an autouse fixture. **Trusted-proxy client IP (#36)**: `client_ip()` (`middleware.py`) returns `request.client.host` — the IP resolved by uvicorn's `ProxyHeadersMiddleware`, which rewrites the client from `X-Forwarded-For` **only** when the peer is in `--forwarded-allow-ips` / `FORWARDED_ALLOW_IPS`. This IP feeds both the audit `source_ip` and the login rate-limit key, so trusting only the Caddy hop (rather than the old hand-rolled leftmost-XFF parse) closes the spoofable brute-force bypass + audit poisoning. In k8s the Caddyfile sets `trusted_proxies static private_ranges` so Caddy preserves the Ingress's `X-Forwarded-*` chain. Launch commands pass `--proxy-headers`; the Docker/k8s images default `FORWARDED_ALLOW_IPS=*` because the app is reachable only through Caddy. Local `uvicorn --reload` (no proxy) leaves it unset so XFF is never trusted. @@ -548,21 +548,21 @@ Design notes worth keeping. `ExerciseStateChanged` deliberately does **not** car **Transactional inject and response operations (#125)**: The database is authoritative for response identity: `response(exercise_id, inject_id, user_id)` is unique, and an insert collision becomes a deterministic `409` after rollback. Inject release uses a `pending`-state compare-and-swap and performs timer cancellation, WebSocket delivery, and communication scheduling only after the winning transaction commits. Exercise creation flushes the parent and all seeded injects, then commits once; a seeding error rolls the entire unit of work back. Suggested-inject approval locks the pending suggestion and creates its Inject plus approval state in the same transaction, so a replay cannot produce a partial approval or duplicate inject. Accepting an exercise-bound participant invite consumes the token, creates the account, and enrols the member in one transaction; if the roster policy rejects enrolment, all three changes roll back so the token remains unconsumed. -**Triggered communications (#140)**: Scenario `triggers_communications` are logical scenario-node events, not per-team physical-inject events. A multi-team node can produce one Inject row per team, but each configured trigger is delivered once to the full exercise audience. The durable `(exercise_id, trigger_key)` constraint and conflict-safe insert make delayed workers/retries idempotent; `triggered_by_inject_id` retains attribution to the winning physical release. Delayed delivery uses the shared single-process schedule registry: pause cancels timers, resume reconstructs their remaining active-time delay, and startup rehydrates pending triggers from the scenario definition, released injects, lifecycle transitions, and delivery keys. +**Triggered communications (#140)**: Scenario `triggers_communications` are logical scenario-node events, not per-team physical-inject events. A multi-team node can produce one Inject row per team, but each configured trigger is delivered once to the full exercise audience. The durable `(exercise_id, trigger_key)` constraint and conflict-safe insert make delayed workers/retries idempotent; `triggered_by_inject_id` retains attribution to the winning physical release. Delayed delivery is a durable job enqueued **inside `release_inject`'s transaction** (#213), which is what closes #211: the jobs exist if and only if the release does. Nothing is cancelled on pause — the job fires, re-reads the pause-aware clock, and re-defers itself; resume and startup reconciliation re-derive anything still owed from the scenario definition, released injects, lifecycle transitions, and delivery keys. The task re-reads the message content from the scenario at delivery time, so a facilitator's edit between enqueue and delivery is honoured. **Group-aware scenario progression (#126)**: `ExerciseProgress` stores one authoritative cursor for the shared path and each scenario team; `InjectProgress` stores per-context release/resolution state. Releasing an inject snapshots its eligible enrolled-participant audience into `InjectProgress` rows, filtered by `group_id`/`target_teams`; observers, facilitators, and non-target teams never count. The first valid participant response resolves that group's row and advances its cursor in the same transaction. A team-specific physical inject mirrors that resolution into `Inject.state`/`resolved_at`; a shared inject stays released until every snapshotted context resolves. Roster enrolment, removal, and group changes lock after the first inject release so mutable membership cannot rewrite history or create an impossible responder. Scenario-node release is allowed only when at least one applicable cursor points to that node (legacy exercises without cursor rows remain operable, and independent root nodes remain valid opening choices), preventing a group from releasing both sides of one branch while still allowing intentional cross-team divergence. `GET /api/exercises/{id}/progression`, response HTTP/WS payloads, the facilitator console, timeline, export, and report all read the same cursor/resolution records; participant responses receive only their own group context. **OIDC / SSO (#25)**: adapter-based OpenID Connect (Authorization-Code + PKCE) via **Authlib**, running alongside or instead of local auth per `AUTH_MODE` (`local`|`oidc`|`both`, default both). The flow is provider-agnostic (Authlib handles discovery, PKCE `S256`, `state`/`nonce`, and JWKS ID-token validation of `iss`/`aud`/`exp`/`iat`/`nonce`); the small **adapter** layer (`app/services/oidc/`, a `key → adapter` registry) captures only per-provider claim mapping. Four providers ship: **Entra** (`entra.py`, stable `sub` + required tenant `tid`; email is verified only from an explicit `xms_edov`/`email_verified` assertion) and **Authentik / Auth0 / Okta**, which all reuse the shared **`StandardOIDCAdapter`** (`base.py`: standard `sub`/`email`/`email_verified`/`name`/groups) with only their metadata-URL construction differing in config (Authentik = base+slug, Auth0 = `https:///…`, Okta = org server or `/oauth2//…`; Auth0 roles need a namespaced custom-claim URI in `OIDC_AUTH0_ROLE_CLAIM`). Config builds one `OIDCProviderConfig` per enabled provider (`settings.enabled_oidc_providers()`); client secrets are **env-only** (`OIDC_*_CLIENT_SECRET`, never a DB column, never logged). Routes are `app/routers/oidc.py` (`/api/auth/oidc/{provider}/login|callback`); Authlib's transient handshake state lives in a short-lived Starlette `SessionMiddleware` cookie (`dt_oidc_session`, `same_site=lax`). `provision_oidc_user` (`service.py`) locks and matches returning users only on stable `(auth_provider, subject)` identity and verifies stored tenant provenance. Mutable `email`/`preferred_username` claims never auto-link: every collision is refused until an explicit safe link workflow exists. JIT creation additionally requires an IdP-asserted `email_verified` (#257) — collision handling stops an unverified address *taking* an existing row, not pre-claiming a colleague's future one, and facilitators enrol by email; the refusal is audited and `OIDC_ALLOW_UNVERIFIED_EMAIL` (env-only) reopens it for IdPs that never emit the claim. New users are JIT-created with `role_managed_by_idp=true`; the configured group→role map is synchronized on every returning login, so group removal or a missing/overage claim fails closed to participant. A role transition and token cutoff commit atomically before existing sessions are rejected and the user's live exercise sockets are closed. Operator-assigned roles (`bootstrap_admin`) and global admins are preserved as local overrides. On success the app mints the existing local session token (`create_access_token(subject=user.email, …)` + `_set_session_cookie`) so downstream authorization remains unchanged; the login page renders a "Sign in with …" button per provider. `User` stores nullable `hashed_password`, stable `auth_provider`/`subject`, optional `auth_tenant`, and role provenance; local login rejects passwordless SSO-only accounts. Events audited: `auth.oidc_login` (success/fail/deny), `auth.jit_provision`, and `auth.oidc_role_sync` — never with tokens/codes. Providers register at startup (`main` lifespan) and lazily (`ensure_registered`) so the test transport works without lifespan. -**SIEM forwarding (#24)**: the app is its own forwarder (no Vector/Fluent Bit sidecar). `app/services/siem_service.py` ships each event, off the response path, to the enabled sinks — `file` (append JSON line), `syslog` (RFC 5424 UDP/TCP), `http` (JSON POST to a Splunk HEC / Elastic / webhook endpoint, `Authorization: Bearer` from the **env-only** `SIEM_HTTP_TOKEN`, `verify` per config). `stdout` is the always-on baseline (the existing `iceberg_ttx.audit` handler) so a BYO node-level shipper can still tail it. `audit_service.emit()` gains `_ship()` which — like `_persist()` — reads routing from an **in-memory `SiemConfig` cache** (sync `emit` has no DB session) and `spawn()`s `siem_service.emit` on the running loop; each sink is `_safe`-wrapped so a dead/slow SIEM (5s timeouts) never raises or blocks the request. Routing lives in the admin-editable **`AuditSettings` singleton** (`app/models/audit_settings.py`, row id=1, no secret column) managed by `audit_settings_service.py`; the cache is loaded at startup (`main._load_siem_config`) and refreshed on every save. Admin API `app/routers/audit.py` (`/api/audit/events|settings|test`, gated by `require_admin` → real `User.is_admin` column) backs the **`/admin/audit`** page (event trail + SIEM config form + "send test event"); a UI-only `is_admin` JWT claim + `UserResponse.is_admin` gate the page shell / rail link, but the API always re-checks the DB column. Seeded from `SIEM_*` env (see `.env.example`); wired in compose + `k8s/base/configmap.yaml` (routing) + `k8s/base/secrets.yaml` (`SIEM_HTTP_TOKEN`). Single-process, like `ws_manager`/`rate_limit` (the persisted `AuditEvent` row remains the durable record on SIEM outage — but only until it is pruned; see audit retention (#251), which inverts this relationship by making forwarding the archive). **Secret-bearing sinks are pinned to env** (`sink_pinning.py`, #259): three admin-editable *host* fields decide where an env-only secret is sent — the SIEM `http_endpoint` (bearer `SIEM_HTTP_TOKEN`), `smtp_host` (SMTP AUTH with `SMTP_PASSWORD`), and `proxy_url` (credentials in its userinfo) — and each pairs with a "test" button, so re-pointing one used to mail the secret anywhere an admin chose, defeating the env-only boundary those secrets exist to hold. While the secret is set the destination's **origin** must equal the env value (path edits and clearing stay allowed); the check lives in each settings service's `validate_changes` and surfaces as 422, mirroring `llm_settings_service.validate_selection`. Every destination move — pinned or not — emits a `critical` audit event with old→new origin. +**SIEM forwarding (#24)**: the app is its own forwarder (no Vector/Fluent Bit sidecar). `app/services/siem_service.py` ships each event, off the response path, to the enabled sinks — `file` (append JSON line), `syslog` (RFC 5424 UDP/TCP), `http` (JSON POST to a Splunk HEC / Elastic / webhook endpoint, `Authorization: Bearer` from the **env-only** `SIEM_HTTP_TOKEN`, `verify` per config). `stdout` is the always-on baseline (the existing `iceberg_ttx.audit` handler) so a BYO node-level shipper can still tail it. `audit_service.emit()` gains `_ship()` which — like `_persist()` — reads routing from an **in-memory `SiemConfig` cache** (sync `emit` has no DB session) and `spawn()`s `siem_service.emit` on the running loop; each sink is `_safe`-wrapped so a dead/slow SIEM (5s timeouts) never raises or blocks the request. Routing lives in the admin-editable **`AuditSettings` singleton** (`app/models/audit_settings.py`, row id=1, no secret column) managed by `audit_settings_service.py`; the cache is loaded at startup (`main._load_siem_config`) and refreshed on every save. Admin API `app/routers/audit.py` (`/api/audit/events|settings|test`, gated by `require_admin` → real `User.is_admin` column) backs the **`/admin/audit`** page (event trail + SIEM config form + "send test event"); a UI-only `is_admin` JWT claim + `UserResponse.is_admin` gate the page shell / rail link, but the API always re-checks the DB column. Seeded from `SIEM_*` env (see `.env.example`); wired in compose + `k8s/base/configmap.yaml` (routing) + `k8s/base/secrets.yaml` (`SIEM_HTTP_TOKEN`). The cache is refreshed on every replica by `config_sync` on the `siem` scope (#213); the persisted `AuditEvent` row remains the durable record on SIEM outage — but only until it is pruned; see audit retention (#251), which inverts this relationship by making forwarding the archive). **Secret-bearing sinks are pinned to env** (`sink_pinning.py`, #259): three admin-editable *host* fields decide where an env-only secret is sent — the SIEM `http_endpoint` (bearer `SIEM_HTTP_TOKEN`), `smtp_host` (SMTP AUTH with `SMTP_PASSWORD`), and `proxy_url` (credentials in its userinfo) — and each pairs with a "test" button, so re-pointing one used to mail the secret anywhere an admin chose, defeating the env-only boundary those secrets exist to hold. While the secret is set the destination's **origin** must equal the env value (path edits and clearing stay allowed); the check lives in each settings service's `validate_changes` and surfaces as 422, mirroring `llm_settings_service.validate_selection`. Every destination move — pinned or not — emits a `critical` audit event with old→new origin. -**Audit retention & token purge (#251)**: `auditevent` and `authtoken` previously grew without bound and nothing ever deleted from either. `app/services/retention_service.py` adds one daily in-process sweep (`retention_task` = `sweep_once` then `asyncio.sleep`, so the "startup pass plus daily timer" the issue asks for is a single construct; created in `main`'s lifespan beside `heartbeat_task`, **not** awaited inline like `rehydrate_schedules` because a first pass over a legacy table is unbounded and would delay readiness). **Audit pruning is opt-in**: `AuditSettings.retention_days` (seeded from `AUDIT_RETENTION_DAYS`, edited at `/admin/audit`, bounded `0..3650` at the router) defaults to **0 = keep forever** — an upgrade must never silently destroy security records, and SIEM forwarding is only an archival path if an operator can enable a forwarder *first*. The knob deliberately lives on `AuditSettings` rather than `GeneralConfig`: its only reader is this async sweep, which already holds a session, so it needs no process-global cache and is **not** projected into `SiemConfig` (that snapshot exists solely for the sync `emit` path). `retention_days <= 0` short-circuits, which also neutralises a hand-edited negative row that would otherwise compute a cutoff in the *future* and delete everything; the cutoff is strict `<`, so a row landing exactly on it survives. Deletes run in `_PURGE_BATCH`-sized transactions capped at `_MAX_BATCHES` per sweep — one unbounded `DELETE` would be a giant transaction, a long lock window and a WAL spike, and the remainder is simply picked up next pass. **Token purging is unconditional** and needs no knob: `authtoken` is not a log but the live state behind an emailed single-use link, so the row is required while the link is live and worthless after — unused rows go 7 days past `expires_at`, used rows 24 hours past `used_at` (module constants), `consume()` already refuses both classes so nothing live races the delete, and the audit trail independently records issuance and acceptance. Migration `b3c4d5e6f7a8` adds the column (with a `server_default` — the singleton is lazily seeded and may already exist) plus `ix_authtoken_expires_at`; `expires_at` also carries `index=True` on the model because the test schema is built from SQLModel metadata, not Alembic. The sweep emits `audit.retention_purge` (severity `warning`) **only when something was deleted**, so a non-pruning deployment emits nothing ever and a pruning one emits at most one per day — destroying security records is itself security-relevant, and without it a purged window is indistinguishable from tampering; the event cannot eat itself because its `created_at` is newer than any cutoff by construction. It records no domain event and dispatches no WS frame. At shutdown both recurring loops are cancelled **before** `background.drain`: they are bare `create_task`s the drain never sees, and cancelling first lets a shutdown-time purge event's persist/forward children be drained rather than abandoned. Caveat to state plainly: once pruning is on, forwarding is the archive, and it is **best-effort with no outbox or retry** — a forwarder outage overlapping a purge window loses those events permanently. Single-process, like `ws_manager`/`rate_limit`: a second live replica would run a second concurrent sweep. `exercisestatetransition`, communications and responses are deliberately out of scope (domain data with real read paths). +**Audit retention & token purge (#251)**: `auditevent` and `authtoken` previously grew without bound and nothing ever deleted from either. `app/services/retention_service.py` adds one daily in-process sweep (`retention_task` = `sweep_once` then `asyncio.sleep`, so the "startup pass plus daily timer" the issue asks for is a single construct; created in `main`'s lifespan beside `heartbeat_task`, **not** awaited inline like `rehydrate_schedules` because a first pass over a legacy table is unbounded and would delay readiness). **Audit pruning is opt-in**: `AuditSettings.retention_days` (seeded from `AUDIT_RETENTION_DAYS`, edited at `/admin/audit`, bounded `0..3650` at the router) defaults to **0 = keep forever** — an upgrade must never silently destroy security records, and SIEM forwarding is only an archival path if an operator can enable a forwarder *first*. The knob deliberately lives on `AuditSettings` rather than `GeneralConfig`: its only reader is this async sweep, which already holds a session, so it needs no process-global cache and is **not** projected into `SiemConfig` (that snapshot exists solely for the sync `emit` path). `retention_days <= 0` short-circuits, which also neutralises a hand-edited negative row that would otherwise compute a cutoff in the *future* and delete everything; the cutoff is strict `<`, so a row landing exactly on it survives. Deletes run in `_PURGE_BATCH`-sized transactions capped at `_MAX_BATCHES` per sweep — one unbounded `DELETE` would be a giant transaction, a long lock window and a WAL spike, and the remainder is simply picked up next pass. **Token purging is unconditional** and needs no knob: `authtoken` is not a log but the live state behind an emailed single-use link, so the row is required while the link is live and worthless after — unused rows go 7 days past `expires_at`, used rows 24 hours past `used_at` (module constants), `consume()` already refuses both classes so nothing live races the delete, and the audit trail independently records issuance and acceptance. Migration `b3c4d5e6f7a8` adds the column (with a `server_default` — the singleton is lazily seeded and may already exist) plus `ix_authtoken_expires_at`; `expires_at` also carries `index=True` on the model because the test schema is built from SQLModel metadata, not Alembic. The sweep emits `audit.retention_purge` (severity `warning`) **only when something was deleted**, so a non-pruning deployment emits nothing ever and a pruning one emits at most one per day — destroying security records is itself security-relevant, and without it a purged window is indistinguishable from tampering; the event cannot eat itself because its `created_at` is newer than any cutoff by construction. It records no domain event and dispatches no WS frame. At shutdown both recurring loops are cancelled **before** `background.drain`: they are bare `create_task`s the drain never sees, and cancelling first lets a shutdown-time purge event's persist/forward children be drained rather than abandoned. Caveat to state plainly: once pruning is on, forwarding is the archive, and it is **best-effort with no outbox or retry** — a forwarder outage overlapping a purge window loses those events permanently. The sweep is a **periodic job** on the task queue (#213): procrastinate's periodic defer is unique per (task, timestamp), so exactly one replica purges each night however many are running. `exercisestatetransition`, communications and responses are deliberately out of scope (domain data with real read paths). **Outbound proxy (#97)**: corporate egress proxying for the three outbound surfaces — the **LLM** API, the **SIEM `http`** sink, and **OIDC** discovery/JWKS. `app/services/proxy.py` is a pure `resolve(cfg, url) -> dict` returning httpx kwargs, with three modes on a `ProxyMode` StrEnum: `SYSTEM` → `{"trust_env": True}` (honour `HTTP(S)_PROXY`/`NO_PROXY`; the **default**, and exactly what httpx did implicitly before this feature, so upgrades are a no-op — a `proxy: None` key here would *override* the env proxy), `NONE` → always direct, `EXPLICIT` → route via `proxy_url` unless the target host matches the no-proxy list (standard `NO_PROXY` semantics: `*`, CIDR, domain+subdomain, exact). The **bypass decision is per target URL** because an httpx client takes a single `proxy` (no per-host `mounts`). **Caller contract**: `resolve_kwargs(url)` returns `{}` when the cache is unloaded, and every call site splats `**kwargs` last — so an unloaded cache is byte-for-byte pre-feature behaviour (each wiring test has a paired "unset" case). Routing lives in the admin-editable **`ProxySettings` singleton** (`app/models/proxy_settings.py`, row id=1, `mode` a plain string column, **no credential column**) managed by `proxy_settings_service.py`; **credentials are env-only** (`PROXY_USERNAME`/`PROXY_PASSWORD`), injected into the proxy URL's userinfo at call time by `_with_credentials()` and never persisted, returned, or logged. Like `SiemConfig`, an in-memory `ProxyConfig` cache is read by the **sync** `audit_service.emit` → SIEM path; loaded at startup by `main._load_proxy_config()`, which **must run before `register_providers()`** (OIDC bakes the resolved proxy into its Authlib `client_kwargs` at registration). A save invalidates both caches that captured the old proxy at construction: `reset_provider_cache()` (LLM adapters hold a long-lived SDK client, so the proxy is resolved once against the provider's base URL — Bedrock against the real `bedrock-runtime..amazonaws.com`, **not** the Anthropic host) and `oidc_service.reset_registration()` (Authlib's `register()` overwrites its `_registry` but `create_client()` returns the **cached** client, so re-registering alone would silently keep the old proxy — the reset rebinds a fresh `OAuth()`). All three SDKs take `http_client=`, incl. `AsyncAnthropicBedrock` (no botocore special-case). Admin API `app/routers/proxy.py` (`/api/proxy/settings|targets|test`, `require_admin`) backs the **`/admin/proxy`** page. The connectivity test takes a **target label, never a URL** — `egress_targets()` builds the label→URL map server-side from the configured LLM/SIEM/OIDC endpoints, so the route is not an SSRF oracle (CodeQL flagged the earlier free-text-URL form as critical); it returns only `ok: HTTP ` or `error: `, with the exception *message* logged server-side after `_scrub()` strips the credentials an httpx error can echo from the proxy URL. The raw-socket `syslog` sink **cannot** be proxied. Seeded from `PROXY_*` env; wired in compose + `k8s/base/configmap.yaml` (routing) + `k8s/base/secrets.yaml` (credentials). **Facilitator ownership scoping (#12)**: facilitator access to **exercises** is scoped per-exercise, not global. `require_exercise_access` (read gate) and `require_exercise_owner` (mutation gate) in `access_control.py` grant access only to: the creator (`Exercise.created_by`), a **co-facilitator** (a facilitator enrolled as an `ExerciseMember` — reuses the existing membership mechanism, no new field), or a **global admin** (`User.is_admin`, assigned out-of-band like the facilitator role — never via registration). Any other facilitator gets `403` + an `authz.denied` audit event. The same gates must run before nested-resource side effects: inject deletion, assessment reads/queueing, and suggested-inject list/approve/reject routes are explicitly covered alongside injects/responses/communications/inject-comments/ws and the mutation/lifecycle/member/export routes in `exercises.py`; `GET /exercises` is filtered to owned-or-member (admins see all). **Scenarios remain a shared library** (any facilitator lists/reads/edits/exports — intentional, they're reusable templates), and `GET /users` stays facilitator-wide (it's the member-enrolment picker). `is_admin` is a real column so it survives role-preview `model_copy` and is unspoofable. -**LLM integration (pluggable providers, #26)**: The AI backend is pluggable via an adapter/registry that mirrors the OIDC pattern (`app/services/llm/`). `LLM_PROVIDER` selects the single active provider for the whole app (the AI analog of `AUTH_MODE`): `anthropic` | `bedrock` | `openai` | `ollama` | `gemini` | `none`. `base.py` holds the `LLMProvider` Protocol (`key`, `model`, `llm_model_label`, `async complete(system, cached_context, user_prompt, max_tokens)`) + a family registry (`register_adapter`/`get_adapter`); two adapters cover all five providers — `AnthropicFamilyAdapter` (`anthropic_provider.py`, family `"anthropic"`) handles direct Anthropic **and** Bedrock (same `messages.create` surface; differ only in client construction — `AsyncAnthropic` vs `AsyncAnthropicBedrock` — and the `anthropic.`-prefixed Bedrock model ID), and `OpenAICompatAdapter` (`openai_provider.py`, family `"openai"`) handles OpenAI, Ollama, and Gemini (all via the OpenAI Chat Completions surface, differing only by `base_url`/model/key). **Prompt caching** uses the GA `cache_control` block on the direct-Anthropic path without a retired `anthropic-beta` header; Bedrock omits it and the OpenAI-compat adapter concatenates the cached context into the user message. `service.py` force-imports the adapter modules (registration side-effects), then `active_provider()` builds/caches the provider from `settings.active_llm_provider()`. `config.py` resolves flat env vars (`ANTHROPIC_*`/`BEDROCK_*`/`OPENAI_*`/`OLLAMA_*`/`GEMINI_*`, `LLM_MAX_TOKENS`) into an `LLMProviderConfig`; `validate_settings()` rejects an unknown `LLM_PROVIDER` and (outside dev) a selected provider missing its credentials. Every provider's SDK is a **lazy-imported optional extra** — no LLM SDK is a core dependency, so all providers are on equal footing (`pip install '.[llm-anthropic]'` for direct Anthropic, `'.[llm-bedrock]'` pulls boto3, `'.[llm-openai]'` covers openai/ollama/gemini; `'.[llm-all]'` bundles all, and the `dev` extra includes them). An unconfigured provider never needs its SDK; the adapters raise a clear "install extra X" error if the selected provider's SDK is absent. `llm_service.py` is now provider-agnostic: `run_llm_pipeline` opens its own `AsyncSession(engine)` and re-checks `Exercise.llm_enabled` plus the response/inject/exercise relationships immediately before provider use; the manual route also requires exercise ownership and opt-in. `queue_llm_pipeline` deduplicates automatic/manual work per response within the supported single-replica model, and persisted assessments make retries no-ops. Provider results stamp `ResponseAssessment.llm_model`/`SuggestedInject.llm_model` with `provider.llm_model_label` (e.g. `"anthropic:claude-opus-4-8"`). Tests mock at the `active_provider` seam (`tests/test_llm.py`) plus per-adapter/config coverage (`tests/test_llm_providers.py`) — no real network requests. +**LLM integration (pluggable providers, #26)**: The AI backend is pluggable via an adapter/registry that mirrors the OIDC pattern (`app/services/llm/`). `LLM_PROVIDER` selects the single active provider for the whole app (the AI analog of `AUTH_MODE`): `anthropic` | `bedrock` | `openai` | `ollama` | `gemini` | `none`. `base.py` holds the `LLMProvider` Protocol (`key`, `model`, `llm_model_label`, `async complete(system, cached_context, user_prompt, max_tokens)`) + a family registry (`register_adapter`/`get_adapter`); two adapters cover all five providers — `AnthropicFamilyAdapter` (`anthropic_provider.py`, family `"anthropic"`) handles direct Anthropic **and** Bedrock (same `messages.create` surface; differ only in client construction — `AsyncAnthropic` vs `AsyncAnthropicBedrock` — and the `anthropic.`-prefixed Bedrock model ID), and `OpenAICompatAdapter` (`openai_provider.py`, family `"openai"`) handles OpenAI, Ollama, and Gemini (all via the OpenAI Chat Completions surface, differing only by `base_url`/model/key). **Prompt caching** uses the GA `cache_control` block on the direct-Anthropic path without a retired `anthropic-beta` header; Bedrock omits it and the OpenAI-compat adapter concatenates the cached context into the user message. `service.py` force-imports the adapter modules (registration side-effects), then `active_provider()` builds/caches the provider from `settings.active_llm_provider()`. `config.py` resolves flat env vars (`ANTHROPIC_*`/`BEDROCK_*`/`OPENAI_*`/`OLLAMA_*`/`GEMINI_*`, `LLM_MAX_TOKENS`) into an `LLMProviderConfig`; `validate_settings()` rejects an unknown `LLM_PROVIDER` and (outside dev) a selected provider missing its credentials. Every provider's SDK is a **lazy-imported optional extra** — no LLM SDK is a core dependency, so all providers are on equal footing (`pip install '.[llm-anthropic]'` for direct Anthropic, `'.[llm-bedrock]'` pulls boto3, `'.[llm-openai]'` covers openai/ollama/gemini; `'.[llm-all]'` bundles all, and the `dev` extra includes them). An unconfigured provider never needs its SDK; the adapters raise a clear "install extra X" error if the selected provider's SDK is absent. `llm_service.py` is now provider-agnostic: `run_llm_pipeline` opens its own `AsyncSession(engine)` and re-checks `Exercise.llm_enabled` plus the response/inject/exercise relationships immediately before provider use; the manual route also requires exercise ownership and opt-in. `queue_llm_pipeline` enqueues a job with a **queueing lock** keyed on the response, so an automatic trigger and a facilitator's manual re-trigger cannot each buy a provider call even on different replicas (#213); persisted assessments remain the backstop that makes retries no-ops. Provider results stamp `ResponseAssessment.llm_model`/`SuggestedInject.llm_model` with `provider.llm_model_label` (e.g. `"anthropic:claude-opus-4-8"`). Tests mock at the `active_provider` seam (`tests/test_llm.py`) plus per-adapter/config coverage (`tests/test_llm_providers.py`) — no real network requests. **Communications state guards**: participant outbound `send_comm` requires the exercise to be `active` (409 otherwise), consistent with response `submit` and inject-comment `create_comment` (#40). Facilitator `inject_comm` (simulated inbound) is **intentionally** unrestricted so facilitators can seed comms during `draft`/`paused` setup. @@ -598,8 +598,8 @@ row is lazily seeded from `SMTP_*`; after creation the database is authoritative while an unloaded cache derives from env for lifespan-free transports. `SMTP_PASSWORD` remains env-only and is read live for delivery. `/api/email/test` accepts no recipient and can mail only the requesting admin, returning only `ok` or an exception class. Like the -SIEM and proxy caches, this remains single-process. +SIEM and proxy caches, this is refreshed across replicas over the config bus (#213). **Sample scenarios**: `app/samples/` contains bundled JSON scenario definitions (`ransomware_response.json`, `vendor_outage.json`). `app/services/sample_service.py` lists, validates, and loads them. The settings page exposes a sample loader UI for facilitators. `get_sample_definition` validates `sample_id` against `SAMPLE_ID_RE` (`^[A-Za-z0-9_-]+$`) and asserts the resolved path stays within `SAMPLES_DIR` before reading, preventing directory traversal via the `sample_id` path param (#15); a rejected id returns `None` → the settings routes surface `404`. -**Containerized deployment**: `Dockerfile` is a two-stage build — stage 1 compiles Tailwind CSS (`pytailwindcss`), stage 2 is the Python runtime. The compiled `static/` directory is also copied to `static_src/` in the image; this path is never overridden by a volume mount and is used by entrypoint scripts (Docker Compose) and init containers (k8s) to populate shared static volumes so Caddy always serves the version matching the running image. **Reverse proxy is Caddy**: in `docker-compose.yml` Caddy (`caddy:2-alpine`) is the edge and terminates TLS itself with **automatic HTTPS** (`SITE_ADDRESS` env — a domain ⇒ Let's Encrypt, default `localhost` ⇒ internal self-signed CA; certs persist in the `caddy_data` volume). It runs **non-root** (`user: 1000:1000`, `cap_drop: ALL` + `cap_add: NET_BIND_SERVICE`, read-only rootfs) — the official image defaults to root; the caddy binary carries a `cap_net_bind_service` **file capability**, so `NET_BIND_SERVICE` must stay in the bounding set everywhere the image runs with dropped caps (compose *and* the k8s deployment) or exec of the setcap binary fails EPERM — and a one-shot `caddy-init` service chowns the root-initialised cert/config volumes (compose's equivalent of the k8s `fsGroup`). Both Caddyfiles bound a hung upstream (`dial_timeout 10s` / `response_header_timeout 60s` — headers-only, so WS sockets are unaffected), serve `/static/` with `max-age=604800, immutable` + `log_skip`, and scrub the `token` query param from access logs. In **k8s** Caddy is instead an internal plain-HTTP reverse proxy on `:8080` (`k8s/base/caddy/`), and TLS stays terminated at the cluster **Ingress** (`k8s/overlays/nginx/ingress.yaml`, cert-manager) — unchanged from the previous nginx setup — or, on EKS, at an ALB fronting a `TargetGroupBinding` (`k8s/overlays/eks/`). `docker-compose.yml` runs `app` + `postgres:17` + `caddy` on a private bridge network with named volumes for DB data, uploads, static files, and Caddy's cert/config stores. **The k8s manifests are a Kustomize base + overlays**: `k8s/base/` holds the cloud-agnostic stack (namespace, secrets, configmap, postgres StatefulSet, app Deployment, caddy Deployment, and ingress `NetworkPolicy`s in `k8s/base/networkpolicy.yaml`) and references no CRDs, so it applies on any cluster; the ingress edge is an overlay concern — `overlays/nginx/` adds a standard `Ingress`, `overlays/eks/` adds the AWS ALB `TargetGroupBinding` (its only CRD dependency, kept out of the base so generic clusters stay unaffected). Apply with `kubectl apply -k k8s/overlays/{nginx,eks}`. **Security posture (IaC review)**: all containers run non-root under a PSS-`restricted`-style `securityContext` (no priv-esc, all caps dropped, `RuntimeDefault` seccomp; app/init/caddy containers use a read-only rootfs with emptyDir/`tmpfs` writable mounts); the k8s Caddy listens on 8080 and its Service is `ClusterIP` fronted by the TLS Ingress (never a plaintext `:80` LoadBalancer); `automountServiceAccountToken: false` on every pod. Compose mirrors this (`no-new-privileges`, `cap_drop: ALL`, non-root `user:` on caddy, read-only rootfs). **Replica constraint**: the app must run as a single replica; k8s manifests enforce `replicas: 1` and `strategy: Recreate`. Redis pub/sub would fix only `ws_manager.py` fan-out — it does **not** lift the constraint on its own. Scheduled inject release and delayed triggered comms are in-process `asyncio` tasks (need a task queue); the rate limiters are per-process counters (a second replica silently multiplies the effective login/registration/reset limits); the SIEM and proxy config caches are per-process (a policy change would not reach the other replica). Full breakdown in [website/docs/deployment.md](website/docs/deployment.md). The async `asyncpg` driver is a core dependency (no separate extra; the `DATABASE_URL` may be a plain `postgresql://` URL — it is upgraded to `asyncpg` at runtime). Health probes (`app/routers/health.py`): `GET /api/health` is a DB-free unconditional 200 backing the k8s **liveness** probe (a DB outage must not restart pods — that would crash-loop through startup migrations), while `GET /api/health/ready` runs a short-timeout `SELECT 1` on the async engine and returns **503** when Postgres is unreachable, backing the k8s **readiness** probe and the compose app healthcheck so a pod with a dead DB is pulled from the endpoint set instead of serving 500s (#71). +**Containerized deployment**: `Dockerfile` is a two-stage build — stage 1 compiles Tailwind CSS (`pytailwindcss`), stage 2 is the Python runtime. The compiled `static/` directory is also copied to `static_src/` in the image; this path is never overridden by a volume mount and is used by entrypoint scripts (Docker Compose) and init containers (k8s) to populate shared static volumes so Caddy always serves the version matching the running image. **Reverse proxy is Caddy**: in `docker-compose.yml` Caddy (`caddy:2-alpine`) is the edge and terminates TLS itself with **automatic HTTPS** (`SITE_ADDRESS` env — a domain ⇒ Let's Encrypt, default `localhost` ⇒ internal self-signed CA; certs persist in the `caddy_data` volume). It runs **non-root** (`user: 1000:1000`, `cap_drop: ALL` + `cap_add: NET_BIND_SERVICE`, read-only rootfs) — the official image defaults to root; the caddy binary carries a `cap_net_bind_service` **file capability**, so `NET_BIND_SERVICE` must stay in the bounding set everywhere the image runs with dropped caps (compose *and* the k8s deployment) or exec of the setcap binary fails EPERM — and a one-shot `caddy-init` service chowns the root-initialised cert/config volumes (compose's equivalent of the k8s `fsGroup`). Both Caddyfiles bound a hung upstream (`dial_timeout 10s` / `response_header_timeout 60s` — headers-only, so WS sockets are unaffected), serve `/static/` with `max-age=604800, immutable` + `log_skip`, and scrub the `token` query param from access logs. In **k8s** Caddy is instead an internal plain-HTTP reverse proxy on `:8080` (`k8s/base/caddy/`), and TLS stays terminated at the cluster **Ingress** (`k8s/overlays/nginx/ingress.yaml`, cert-manager) — unchanged from the previous nginx setup — or, on EKS, at an ALB fronting a `TargetGroupBinding` (`k8s/overlays/eks/`). `docker-compose.yml` runs `app` + `postgres:17` + `caddy` on a private bridge network with named volumes for DB data, uploads, static files, and Caddy's cert/config stores. **The k8s manifests are a Kustomize base + overlays**: `k8s/base/` holds the cloud-agnostic stack (namespace, secrets, configmap, postgres StatefulSet, app Deployment, caddy Deployment, and ingress `NetworkPolicy`s in `k8s/base/networkpolicy.yaml`) and references no CRDs, so it applies on any cluster; the ingress edge is an overlay concern — `overlays/nginx/` adds a standard `Ingress`, `overlays/eks/` adds the AWS ALB `TargetGroupBinding` (its only CRD dependency, kept out of the base so generic clusters stay unaffected). Apply with `kubectl apply -k k8s/overlays/{nginx,eks}`. **Security posture (IaC review)**: all containers run non-root under a PSS-`restricted`-style `securityContext` (no priv-esc, all caps dropped, `RuntimeDefault` seccomp; app/init/caddy containers use a read-only rootfs with emptyDir/`tmpfs` writable mounts); the k8s Caddy listens on 8080 and its Service is `ClusterIP` fronted by the TLS Ingress (never a plaintext `:80` LoadBalancer); `automountServiceAccountToken: false` on every pod. Compose mirrors this (`no-new-privileges`, `cap_drop: ALL`, non-root `user:` on caddy, read-only rootfs). **Replicas**: the app shares every piece of cross-request state through PostgreSQL (#213) — WS fan-out and config invalidation over `LISTEN`/`NOTIFY`, scheduled releases and triggered comms as durable procrastinate jobs, rate-limit counters as rows, the retention sweep as a periodic job, and startup migrations serialised on an advisory lock. No Redis, no broker. The base manifests still ship `replicas: 1` for a storage reason rather than a state one: the uploads PVC is `ReadWriteOnce` and access modes are immutable, so `k8s/overlays/multi-replica` (2 replicas, `RollingUpdate` with `maxUnavailable: 0`, RWX uploads) is the opt-in path on clusters that have RWX storage. Do not front the app with a transaction-mode pooler — `LISTEN` needs a session that outlives a transaction. Full breakdown in [website/docs/deployment.md](website/docs/deployment.md). The async `asyncpg` driver is a core dependency (no separate extra; the `DATABASE_URL` may be a plain `postgresql://` URL — it is upgraded to `asyncpg` at runtime). Health probes (`app/routers/health.py`): `GET /api/health` is a DB-free unconditional 200 backing the k8s **liveness** probe (a DB outage must not restart pods — that would crash-loop through startup migrations), while `GET /api/health/ready` runs a short-timeout `SELECT 1` on the async engine and returns **503** when Postgres is unreachable, backing the k8s **readiness** probe and the compose app healthcheck so a pod with a dead DB is pulled from the endpoint set instead of serving 500s (#71). diff --git a/README.md b/README.md index c2e1df8..7da343c 100644 --- a/README.md +++ b/README.md @@ -207,7 +207,7 @@ Prefer plain `kubectl apply -f` over Kustomize? Apply the individual files under > > **Pod hardening**: every workload runs non-root under a PSS-`restricted`-style `securityContext` (no privilege escalation, all capabilities dropped, `RuntimeDefault` seccomp), and every container uses a read-only root filesystem — app, init, Caddy, Postgres, and the backup CronJob alike. The Postgres StatefulSet runs as uid 999 with `fsGroup: 999`, which needs a StorageClass that honours `fsGroup`. -> **Note**: The app must run as a single replica (`replicas: 1`); the manifests enforce it with `strategy: Recreate`. Every piece of cross-request state lives in the **process**, so a second replica would not share any of it: the **WebSocket manager** (needs a shared bus, e.g. Redis pub/sub), **scheduled inject release** and **delayed triggered comms** (in-process `asyncio` tasks — need a task queue such as Celery or ARQ), the **login/registration/reset rate limiters** (per-process counters, so the effective limits would multiply by the replica count), the **SIEM and proxy config caches** (an admin's change would not reach the other replica), and **in-flight LLM assessments**. See the [deployment docs](https://icebergai.github.io/IcebergTTX/deployment/) for the full table. +> **Scaling out**: the app shares all of its cross-request state through PostgreSQL, so it runs on more than one replica: WebSocket frames and config invalidation travel over `LISTEN`/`NOTIFY`, scheduled inject releases and triggered communications are durable jobs, and the rate limiters count in a table. There is no Redis and no broker to operate. The default manifests still ship `replicas: 1` because the uploads volume is `ReadWriteOnce` and a PVC's access modes cannot be changed in place; on a cluster with an RWX StorageClass, apply `k8s/overlays/multi-replica` for two replicas and rolling deploys. See the [deployment docs](https://icebergai.github.io/IcebergTTX/deployment/). ## Attachment reconciliation diff --git a/SECURITY.md b/SECURITY.md index 6a6d8a0..be103ee 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -46,5 +46,6 @@ participant reading another team's data, or an unauthenticated caller reaching a them) remain in scope. Deployment hardening (secret management, TLS termination, network policy, and the -single-replica WebSocket constraint) is the operator's responsibility; see the -deployment notes in [README.md](README.md) and [CLAUDE.md](CLAUDE.md). +storage prerequisites for running more than one replica) is the operator's +responsibility; see the deployment notes in [README.md](README.md) and +[CLAUDE.md](CLAUDE.md). diff --git a/alembic/env.py b/alembic/env.py index aafb2c8..09ff246 100644 --- a/alembic/env.py +++ b/alembic/env.py @@ -30,6 +30,7 @@ llm_settings, oidc_settings, proxy_settings, + rate_limit, report_summary, response, scenario, @@ -38,7 +39,11 @@ ) config = context.config -config.set_main_option("sqlalchemy.url", make_async_url(settings.database_url)) +# Settings are the source of truth, but an explicitly configured URL wins — that is how +# a caller migrates a database other than the app's own (the migration tests point this +# at a throwaway one; ``settings`` is bound at import and cannot be redirected). +if not config.get_main_option("sqlalchemy.url", None): + config.set_main_option("sqlalchemy.url", make_async_url(settings.database_url)) if config.config_file_name is not None: # run_migrations() runs Alembic in-process during the app lifespan, so the @@ -50,6 +55,19 @@ target_metadata = SQLModel.metadata +def include_name(name, type_, parent_names) -> bool: + """Keep procrastinate's own tables out of autogenerate and ``alembic check``. + + The task queue's schema is owned by the library and installed verbatim by its own + revision (#213), so it is deliberately absent from ``SQLModel.metadata``. Without + this filter every autogenerate would propose dropping it, and the ``alembic check`` + that gates CI would fail on a database that is in fact correct. + """ + if type_ == "table" and name is not None: + return not name.startswith("procrastinate_") + return True + + def run_migrations_offline() -> None: context.configure( url=config.get_main_option("sqlalchemy.url"), @@ -57,18 +75,36 @@ def run_migrations_offline() -> None: literal_binds=True, dialect_opts={"paramstyle": "named"}, compare_type=True, + include_name=include_name, ) with context.begin_transaction(): context.run_migrations() +# Any 64-bit constant works; this one is arbitrary and only has to stay stable, since +# two processes agree on a lock by using the same number. +_MIGRATION_LOCK_ID = 8_213_100_213 + + def _do_run_migrations(connection) -> None: context.configure( connection=connection, target_metadata=target_metadata, compare_type=True, + include_name=include_name, ) with context.begin_transaction(): + # Serialise migrations across replicas (#213). Every replica migrates on startup, + # so a rollout would otherwise run several `alembic upgrade head` concurrently + # against one database — two processes creating the same table is a crash loop, + # not a race you can retry past. Whoever loses the lock waits, then finds the + # schema already at head and applies nothing. + # + # Taken *inside* Alembic's transaction, and an xact lock rather than a session + # one, for two reasons: it is released automatically however the migration ends, + # and issuing any statement before this block would autobegin a second + # transaction that Alembic never commits — silently discarding every migration. + connection.exec_driver_sql(f"SELECT pg_advisory_xact_lock({_MIGRATION_LOCK_ID})") context.run_migrations() diff --git a/alembic/versions/c7d8e9f0a1b2_add_procrastinate_schema.py b/alembic/versions/c7d8e9f0a1b2_add_procrastinate_schema.py new file mode 100644 index 0000000..0d97eec --- /dev/null +++ b/alembic/versions/c7d8e9f0a1b2_add_procrastinate_schema.py @@ -0,0 +1,58 @@ +"""Install the procrastinate task-queue schema (#213). + +The DDL is procrastinate's own, read from the installed package rather than copied here, +so upgrading the library is a new revision wrapping its migration script instead of a +hand-merged diff of ours. + +Revision ID: c7d8e9f0a1b2 +Revises: b3c4d5e6f7a8 +""" + +import inspect +from collections.abc import Sequence + +from sqlalchemy.util import await_only + +from alembic import op + +revision: str = "c7d8e9f0a1b2" +down_revision: str | None = "b3c4d5e6f7a8" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def _run_script(sql: str) -> None: + """Execute a multi-statement script on the migration's own connection. + + SQLAlchemy's asyncpg adapter routes every execute through a prepared statement, and + Postgres refuses to prepare more than one command at a time — so ``exec_driver_sql`` + cannot run this. asyncpg's own ``execute`` uses the simple query protocol, which + can; reaching it directly also keeps the DDL inside the migration's transaction, so + a failure rolls back the schema and the version stamp together. + """ + connection = op.get_bind() + raw = connection.connection.driver_connection + if inspect.iscoroutinefunction(getattr(raw, "execute", None)): + await_only(raw.execute(sql)) + else: # a synchronous driver (psycopg) needs none of the above + connection.exec_driver_sql(sql) + + +def upgrade() -> None: + from procrastinate.schema import SchemaManager + + _run_script(SchemaManager.get_schema()) + + +def downgrade() -> None: + _run_script( + """ + DROP TABLE IF EXISTS procrastinate_events CASCADE; + DROP TABLE IF EXISTS procrastinate_periodic_defers CASCADE; + DROP TABLE IF EXISTS procrastinate_jobs CASCADE; + DROP TABLE IF EXISTS procrastinate_workers CASCADE; + DROP TYPE IF EXISTS procrastinate_job_status CASCADE; + DROP TYPE IF EXISTS procrastinate_job_event_type CASCADE; + DROP TYPE IF EXISTS procrastinate_job_to_defer_v1 CASCADE; + """ + ) diff --git a/alembic/versions/d8e9f0a1b2c3_add_rate_limit_hits.py b/alembic/versions/d8e9f0a1b2c3_add_rate_limit_hits.py new file mode 100644 index 0000000..e522e6e --- /dev/null +++ b/alembic/versions/d8e9f0a1b2c3_add_rate_limit_hits.py @@ -0,0 +1,44 @@ +"""Move rate-limit counters into an unlogged table (#213). + +UNLOGGED is the point of writing this by hand rather than autogenerating it: these rows +are expendable throttle state, so keeping them out of the WAL is what makes a write per +login attempt acceptable. Postgres truncates the table after an unclean shutdown, which +at worst forgives some in-progress lockouts. + +Revision ID: d8e9f0a1b2c3 +Revises: c7d8e9f0a1b2 +""" + +from collections.abc import Sequence + +import sqlalchemy as sa + +from alembic import op + +revision: str = "d8e9f0a1b2c3" +down_revision: str | None = "c7d8e9f0a1b2" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + op.execute( + sa.text( + """ + CREATE UNLOGGED TABLE rate_limit_hits ( + id serial PRIMARY KEY, + scope varchar(32) NOT NULL, + key varchar(320) NOT NULL, + at timestamptz NOT NULL + ) + """ + ) + ) + op.create_index( + "ix_rate_limit_hits_scope_key_at", "rate_limit_hits", ["scope", "key", "at"] + ) + + +def downgrade() -> None: + op.drop_index("ix_rate_limit_hits_scope_key_at", table_name="rate_limit_hits") + op.drop_table("rate_limit_hits") diff --git a/app/database.py b/app/database.py index 6cbca50..dd54116 100644 --- a/app/database.py +++ b/app/database.py @@ -29,6 +29,20 @@ def make_async_url(url: str) -> str: return url +def make_asyncpg_dsn(url: str) -> str: + """Strip the SQLAlchemy driver marker so raw asyncpg can dial the same database. + + ``asyncpg.connect`` speaks libpq DSNs and rejects SQLAlchemy's ``+driver`` suffix. + The LISTEN connection (#213) is raw asyncpg rather than a pooled checkout, so it + needs the plain form of whatever ``DATABASE_URL`` was configured with. + """ + async_url = make_async_url(url) + for prefix in ("postgresql+asyncpg://", "postgres+asyncpg://"): + if async_url.startswith(prefix): + return "postgresql://" + async_url[len(prefix) :] + return async_url + + engine = create_async_engine(make_async_url(settings.database_url), pool_pre_ping=True) @@ -59,8 +73,12 @@ async def run_migrations() -> None: Run in a worker thread because Alembic's async ``env.py`` calls ``asyncio.run``, which cannot be invoked from the already-running lifespan - event loop. Safe for the single-replica deployment; multi-replica rollouts - should instead run ``alembic upgrade head`` as a dedicated deploy step. + event loop. + + Safe with several replicas starting at once: ``env.py`` takes a Postgres advisory + lock first, so they migrate one at a time and the losers find the schema already at + head (#213). Migrations must still be forward-compatible across one release — during + a rolling update the previous version is serving against the new schema. """ await asyncio.to_thread(_upgrade_to_head) diff --git a/app/main.py b/app/main.py index 5d2a956..9414076 100644 --- a/app/main.py +++ b/app/main.py @@ -29,6 +29,7 @@ inject_comment, llm_settings, proxy_settings, + rate_limit, report_summary, response, scenario, @@ -70,7 +71,6 @@ ) from app.routers.ui import UIRedirect from app.services import audit_service, background -from app.services.retention_service import retention_task from app.services.ws_manager import heartbeat_task logger = logging.getLogger("iceberg_ttx") @@ -184,47 +184,55 @@ async def lifespan(app: FastAPI): from app.services.oidc import service as oidc_service oidc_service.register_providers() - # Re-arm persisted inject and communication schedules for exercises that were active - # before a restart (#116, #194). Timers remain single-process only. - from app.services.schedule_service import rehydrate_schedules - - await rehydrate_schedules() + # Subscribe to the cross-replica bus, so a config saved on another replica lands in + # this one's caches (#213). Started after the loaders above: the first connect has + # nothing to recover, and connecting is deliberately not awaited to completion — + # a database blip must not stop a replica from serving what it already can. + from app.services import config_sync, ws_relay + from app.services.pg_listener import listener + + config_sync.install() + ws_relay.install() + await listener.start() + # Run a queue worker in this replica, so a single container is still the whole + # deployment (#213). Schedules are durable jobs now, so nothing has to be rehydrated + # from memory; reconcile_release_jobs is a safety net that also carries over + # exercises which were running under a build that kept its timers in a dict. + from app.services import task_queue + from app.services.schedule_service import reconcile_release_jobs + + await task_queue.start_worker() + await reconcile_release_jobs() audit_service.emit("app.startup", severity="info") task = asyncio.create_task(heartbeat_task()) - # Prune audit history past the configured window and dead auth tokens: once now, - # then daily (#251). Not awaited inline like rehydrate_schedules above — a first - # pass over a legacy table is unbounded and would delay readiness. Single-process, - # like the timers: a second live replica would run a second concurrent sweep. - retention = asyncio.create_task(retention_task()) yield # Graceful teardown (#250), strictly ordered: # 1. Record app.shutdown FIRST, while the loop is still live, so its audit-persist and # SIEM-forward tasks are spawned in time to join the drain below instead of being # abandoned into a dying loop (which reliably lost the shutdown record to stdout). audit_service.emit("app.shutdown", severity="info") - # 2. Stop the recurring loops and await their cancellation. Both are bare - # create_task, so background.drain below never sees them — dropping this leaves a - # task holding a pooled connection when engine.dispose() runs. It must also come - # *before* the drain: a sweep's own audit event spawns persist/forward children - # that are drainable, so cancelling first means they are drained, not abandoned - # (same reasoning as step 1). A sweep cancelled mid-batch rolls back and redoes - # that batch next boot. - for loop_task in (task, retention): - loop_task.cancel() - for loop_task in (task, retention): - with suppress(asyncio.CancelledError): - await loop_task - # 3. Drain in-flight background work (mail, audit persist, SIEM forward, LLM runs) and - # armed timers in one bounded, converging pass. cancel_all_schedules, re-invoked each - # round, cancels still-sleeping timers (rehydration re-arms them next boot) and hands - # back any worker already mid-release so it finishes its commit and dispatch atomically - # (#218); the drain re-collects the task sets after each wait so audit/SIEM writes and - # triggered-comm timers spawned *by* those releases are drained too, not disposed out - # from under (see background.drain). - from app.services.schedule_service import cancel_all_schedules - - await background.drain(collect_extra=cancel_all_schedules) - # 4. Close the asyncpg pool cleanly so Postgres doesn't log unexpected EOFs on deploy. + # 2. Stop the socket heartbeat and await its cancellation. It is a bare create_task, + # so background.drain below never sees it — dropping this leaves a task holding a + # pooled connection when engine.dispose() runs. It must also come *before* the + # drain: its own audit events spawn persist/forward children that are drainable, + # so cancelling first means they are drained, not abandoned (as in step 1). + task.cancel() + with suppress(asyncio.CancelledError): + await task + # 3. Stop the queue worker, letting a running job finish within a bounded grace. + # A job killed here is not lost: it stays *doing* until retry_stalled_jobs + # returns it to the queue, and every task is safe to run twice (#213). + await task_queue.stop_worker() + # 4. Unsubscribe from the bus (#213). Same reasoning: a handler mid-refresh is + # holding a pooled connection, and nothing arriving now can be acted on by a + # replica that is going away. + await listener.stop() + # 5. Drain in-flight background work (mail, audit persist, SIEM forward) in one + # bounded, converging pass — the drain re-collects the task set after each wait, + # so audit and SIEM writes spawned *by* the work above are drained too rather + # than disposed out from under (see background.drain). + await background.drain() + # 6. Close the asyncpg pool cleanly so Postgres doesn't log unexpected EOFs on deploy. from app.database import engine await engine.dispose() diff --git a/app/models/rate_limit.py b/app/models/rate_limit.py new file mode 100644 index 0000000..e6f7c76 --- /dev/null +++ b/app/models/rate_limit.py @@ -0,0 +1,32 @@ +"""The rate limiter's storage: one row per counted attempt (#11, #67, #117, #213). + +Declared as a plain Core table rather than a SQLModel entity because nothing ever loads +it as an object — every access is an aggregate or a bulk delete, and a throttle counter +has no identity worth mapping. It is attached to ``SQLModel.metadata`` regardless, so the +test harness (which builds its schema from the models, never from Alembic) gets it for +free. + +**UNLOGGED** is deliberate. These rows are expendable: they are not written to the WAL, +they are not replicated, and Postgres truncates the table after an unclean shutdown. The +worst case is that a crash forgives some in-progress lockouts, which is a fair price for +keeping a per-login-attempt write off the WAL. The corresponding Alembic revision spells +the keyword out; ``create_all`` honours the ``prefixes`` below. +""" + +from sqlalchemy import Column, DateTime, Index, Integer, String, Table +from sqlmodel import SQLModel + +rate_limit_hits = Table( + "rate_limit_hits", + SQLModel.metadata, + Column("id", Integer, primary_key=True), + # The limiter this row belongs to ("login", "registration", "password_reset"), so + # the three share a table without being able to count each other's attempts. + Column("scope", String(32), nullable=False), + # Opaque and caller-defined: an IP, or "ip:email" for login. Attacker-controlled, so + # it is only ever a bind parameter and never interpolated. + Column("key", String(320), nullable=False), + Column("at", DateTime(timezone=True), nullable=False), + Index("ix_rate_limit_hits_scope_key_at", "scope", "key", "at"), + prefixes=["UNLOGGED"], +) diff --git a/app/routers/auth.py b/app/routers/auth.py index 1b0e0de..8ac273a 100644 --- a/app/routers/auth.py +++ b/app/routers/auth.py @@ -132,8 +132,8 @@ async def register( # cannot mass-create accounts. Keyed by IP alone — the email is the thing being # created, so it can't be part of the key. ip = client_ip(request) or "unknown" - if registration_rate_limiter.is_limited(ip): - retry_after = registration_rate_limiter.retry_after(ip) + limited, retry_after = await registration_rate_limiter.check(ip) + if limited: audit_service.emit( "auth.register", result="deny", @@ -146,7 +146,7 @@ async def register( detail="Too many registration attempts. Try again later.", headers={"Retry-After": str(retry_after)}, ) - registration_rate_limiter.record_failure(ip) + await registration_rate_limiter.record_failure(ip) if await user_service.email_exists(session, body.email): raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="Email already registered") @@ -184,8 +184,8 @@ async def login( _require_local_auth() ip = client_ip(request) or "unknown" rate_key = f"{ip}:{body.email}" - if login_rate_limiter.is_limited(rate_key): - retry_after = login_rate_limiter.retry_after(rate_key) + limited, retry_after = await login_rate_limiter.check(rate_key) + if limited: audit_service.emit( "auth.login", result="deny", @@ -205,7 +205,7 @@ async def login( if not user or user.hashed_password is None or not verify_password( body.password, user.hashed_password ): - login_rate_limiter.record_failure(rate_key) + await login_rate_limiter.record_failure(rate_key) audit_service.emit( "auth.login", result="fail", @@ -226,7 +226,7 @@ async def login( ) raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Account disabled") - login_rate_limiter.reset(rate_key) + await login_rate_limiter.reset(rate_key) token = create_access_token(subject=user.email, role=user.role.value, is_admin=user.is_admin) _set_session_cookie(response, token) audit_service.emit( @@ -254,13 +254,14 @@ async def password_reset_request( _require_smtp() _require_link_base() ip = client_ip(request) or "unknown" - if password_reset_rate_limiter.is_limited(ip): + limited, retry_after = await password_reset_rate_limiter.check(ip) + if limited: raise HTTPException( status_code=status.HTTP_429_TOO_MANY_REQUESTS, detail="Too many reset requests. Try again later.", - headers={"Retry-After": str(password_reset_rate_limiter.retry_after(ip))}, + headers={"Retry-After": str(retry_after)}, ) - password_reset_rate_limiter.record_failure(ip) + await password_reset_rate_limiter.record_failure(ip) user = await user_service.get_by_email(session, body.email) if user is not None and user.auth_provider == LOCAL_AUTH_PROVIDER: diff --git a/app/routers/exercises.py b/app/routers/exercises.py index 838d9f5..91be460 100644 --- a/app/routers/exercises.py +++ b/app/routers/exercises.py @@ -57,7 +57,6 @@ ) from app.services.scenario_service import get_scenario_definition, titles_for from app.services.schedule_service import ( - cancel_exercise_schedules, schedule_exercise_injects, ) from app.services.timeline_service import ( @@ -262,12 +261,12 @@ async def _transition( # transaction and dispatched by transition_state_with_history itself (#212), so it has # already gone out by the time we get here. A rolled-back transition raises above and # emits neither projection. - # Start/resume arm pending timers; pause/complete cancel them. This runs only - # after the atomic state/history transaction and canonical WS projection. + # Start/resume enqueue pending schedules. Pause and complete enqueue nothing and + # cancel nothing: a job written before the pause fires, re-reads the exercise, and + # either no-ops or re-defers itself against the resumed clock (#213). if target == ExerciseState.active: await schedule_exercise_injects(session, result.exercise) - else: - cancel_exercise_schedules(exercise_id) + await session.commit() return _exercise_out(result.exercise) @@ -542,7 +541,8 @@ async def draft_report_summary(exercise_id: int, current_user: FacilitatorDep, s status_code=status.HTTP_409_CONFLICT, detail="AI summary unavailable: no provider configured or exercise AI disabled", ) - queued = queue_summary_pipeline(exercise_id) + queued = await queue_summary_pipeline(session, exercise_id) + await session.commit() return {"status": "accepted" if queued else "already-running"} diff --git a/app/routers/injects.py b/app/routers/injects.py index 71a6108..34c37a8 100644 --- a/app/routers/injects.py +++ b/app/routers/injects.py @@ -36,7 +36,7 @@ inject_payload, release_inject, ) -from app.services.schedule_service import arm_inject_schedule, cancel_inject_schedule +from app.services.schedule_service import arm_inject_schedule router = APIRouter(prefix="/exercises/{exercise_id}/injects", tags=["injects"]) @@ -376,15 +376,15 @@ async def update_schedule( ) inject.release_offset_minutes = body.release_offset_minutes session.add(inject) - record(session, InjectUpdated(exercise_id=exercise_id, inject=inject)) + record(session, InjectUpdated(exercise_id=exercise_id, inject_id=inject_id)) + # Enqueued in the same transaction as the new offset (only affects a running + # exercise; start/resume enqueue the rest). Any job written against the old offset + # is left alone: it fires, finds itself early or late against the value it re-reads, + # and re-defers or loses the release CAS (#213). + await arm_inject_schedule(session, exercise, inject) await session.commit() await session.refresh(inject) - # Re-arm the in-memory timer to match the new value (only affects a running exercise; - # start/resume arm the rest). Cancel first so a cleared/edited offset can't double-fire. - cancel_inject_schedule(exercise_id, inject_id) - arm_inject_schedule(exercise, inject) - audit_service.emit( "inject.schedule", actor=current_user, diff --git a/app/routers/responses.py b/app/routers/responses.py index d531f93..841b9b8 100644 --- a/app/routers/responses.py +++ b/app/routers/responses.py @@ -129,7 +129,8 @@ async def submit( if exercise.llm_enabled: assert response.id is not None - queue_llm_pipeline(response.id, body.inject_id, exercise_id) + await queue_llm_pipeline(session, response.id, body.inject_id, exercise_id) + await session.commit() participant_progression = await progression_snapshot(session, exercise_id, group_id=group_id) return response_payload(response, progression=participant_progression) @@ -178,7 +179,8 @@ async def trigger_assess( session.add(r) await session.commit() return {"detail": "Assessment already exists"} - queued = queue_llm_pipeline(response_id, r.inject_id, exercise_id) + queued = await queue_llm_pipeline(session, response_id, r.inject_id, exercise_id) + await session.commit() return {"detail": "Assessment queued" if queued else "Assessment already queued"} diff --git a/app/services/audit_settings_service.py b/app/services/audit_settings_service.py index 36361cb..cffa36d 100644 --- a/app/services/audit_settings_service.py +++ b/app/services/audit_settings_service.py @@ -13,7 +13,7 @@ from app.config import settings from app.models.audit_settings import AuditSettings -from app.services import siem_service, sink_pinning +from app.services import pg_bus, siem_service, sink_pinning _SINGLETON_ID = 1 @@ -96,6 +96,7 @@ async def update_settings(session: AsyncSession, changes: dict[str, Any]) -> Aud if key in changes and changes[key] is not None: setattr(row, key, changes[key]) session.add(row) + await pg_bus.publish_config_changed(session, "siem") await session.commit() await session.refresh(row) siem_service.set_config(_to_config(row)) diff --git a/app/services/background.py b/app/services/background.py index f57a82f..b8a6320 100644 --- a/app/services/background.py +++ b/app/services/background.py @@ -5,14 +5,13 @@ a strong reference until the task completes, which is the pattern every background call site (LLM pipeline, mail delivery, audit persistence) needs. -Delayed exercise work is *not* one of them any more: an inject release or a triggered -communication also has to be cancellable and rehydratable, so ``schedule_service`` keeps -its own keyed registry, which holds the strong reference itself (#211). - -This does not provide durability across process restarts — a delayed task is -still lost if the single process dies (see the task-queue note in CLAUDE.md). It -only guarantees the task is not dropped by the garbage collector while the -process is alive. +Delayed exercise work is *not* one of them: an inject release or a triggered +communication has to survive a restart, so those are durable jobs in Postgres +(``task_queue``) rather than tasks in this process (#211, #213). + +Nothing here is durable. A spawned task is lost if the process dies, which is why +anything that must not be lost belongs on the queue instead. This only guarantees the +task is not dropped by the garbage collector while the process is alive. """ import asyncio @@ -58,16 +57,13 @@ async def drain( """Wait for in-flight spawned work — and anything it spawns while finishing — to settle, then cancel whatever remains, all within a bounded grace period (#250). - Draining is not a single snapshot. A finishing task can spawn *more* background work: a - schedule worker's release fires audit persistence and SIEM forwarding, and its dispatch - can arm fresh triggered-communication timers. A one-shot snapshot would let those - children outlive ``engine.dispose``, so the task sets are re-collected after every wait - until they empty or the deadline passes. ``collect_extra`` is re-invoked each round to - fold in tasks tracked elsewhere — the lifespan passes ``cancel_all_schedules``, which - cancels still-sleeping timers (rehydration re-arms them next boot) and hands back any - worker already mid-release so it can finish its commit and dispatch atomically (#218). - Every task ``collect_extra`` has ever returned stays tracked, so a worker that spans - rounds is never dropped even after its registry entry is cleared. + Draining is not a single snapshot. A finishing task can spawn *more* background work — + a mail send or an LLM run fires audit persistence, which in turn fires SIEM forwarding. + A one-shot snapshot would let those children outlive ``engine.dispose``, so the task + sets are re-collected after every wait until they empty or the deadline passes. + ``collect_extra`` is re-invoked each round to fold in tasks tracked elsewhere, and + every task it has ever returned stays tracked, so one that spans rounds is never + dropped. The whole thing is bounded: the initial waits share one ``timeout`` deadline, and past it stragglers are cancelled and then waited on for a short fixed window — so a task that diff --git a/app/services/communication_service.py b/app/services/communication_service.py index 4f02072..b83cc87 100644 --- a/app/services/communication_service.py +++ b/app/services/communication_service.py @@ -10,7 +10,6 @@ from app.models.communication import CommDirection, Communication, CommunicationRead from app.models.exercise import ExerciseMember -from app.models.inject import Inject from app.models.user import User from app.schemas.api import CommunicationPublic from app.services.domain_events import CommunicationCreated, dispatch, record @@ -300,28 +299,6 @@ async def comm_payload( -def schedule_triggered_comms( - inject: Inject, - trigger_comms: list, # list[TriggerComm] from scenario definition - logical_node_id: str, -) -> None: - """Schedule node-level all-team communications once, across group-specific injects.""" - assert inject.id is not None - from app.services.schedule_service import arm_triggered_communication - - for index, tc in enumerate(trigger_comms): - arm_triggered_communication( - exercise_id=inject.exercise_id, - inject_id=inject.id, - direction=tc.direction, - external_entity=tc.external_entity, - subject=tc.subject, - body=tc.body, - delay=tc.delay_after_release_seconds, - trigger_key=f"{logical_node_id}:{index}", - ) - - async def deliver_triggered_communication( session: AsyncSession, *, diff --git a/app/services/config_sync.py b/app/services/config_sync.py new file mode 100644 index 0000000..b69e41c --- /dev/null +++ b/app/services/config_sync.py @@ -0,0 +1,162 @@ +"""Keep every replica's runtime-config caches honest (#213). + +Six settings singletons (proxy, SIEM, email, general policy, LLM routing, OIDC) are +cached in process memory so hot paths — ``audit_service.emit`` fanning out to SIEM, an +LLM call resolving its provider — never touch the database. Before multi-replica that +cache was invalidated by the very request that wrote the row, which was sufficient +because there was only ever one process. With more than one, an admin's save leaves +every *other* replica serving the old config indefinitely. + +So a save publishes its scope on the bus and each replica re-reads the row. Scopes name +the settings service rather than the changed fields deliberately: shipping a diff would +put config values (some sensitive) on the wire and let a lost message leave a replica +permanently skewed. Re-reading is idempotent, so the publishing replica handling its own +notification is harmless — and one less branch than skipping it. + +Missed invalidations, the unavoidable cost of at-most-once delivery, are recovered by +``refresh_all`` on every (re)connect. +""" + +from __future__ import annotations + +import logging +from collections.abc import Awaitable, Callable +from typing import Any + +from sqlmodel.ext.asyncio.session import AsyncSession + +from app.services import pg_bus + +logger = logging.getLogger(__name__) + +Refresher = Callable[[AsyncSession], Awaitable[None]] + + +async def _refresh_proxy(session: AsyncSession) -> None: + from app.services import proxy_settings_service + + await proxy_settings_service.refresh_cache_and_dependents(session) + + +async def _refresh_siem(session: AsyncSession) -> None: + from app.services import audit_settings_service + + await audit_settings_service.refresh_cache(session) + + +async def _refresh_email(session: AsyncSession) -> None: + from app.services import email_settings_service + + await email_settings_service.refresh_cache(session) + + +async def _refresh_general(session: AsyncSession) -> None: + # Flows on into rate_limit.apply_config via set_config, so limiter thresholds + # follow the same path as the policy they belong to. + from app.services import general_settings_service + + await general_settings_service.refresh_cache(session) + + +async def _refresh_llm(session: AsyncSession) -> None: + from app.services import llm_settings_service + + await llm_settings_service.refresh_cache(session) + + +async def _refresh_oidc(session: AsyncSession) -> None: + from app.services import oidc_settings_service + + await oidc_settings_service.refresh_cache(session) + + +# Ordered, and the order is the lifespan's: proxy must be reloaded before OIDC, because +# OIDC registration bakes the resolved proxy into each Authlib client (#97). Python dicts +# preserve insertion order, so refresh_all inherits that guarantee for free. +SCOPES: dict[str, Refresher] = { + "proxy": _refresh_proxy, + "siem": _refresh_siem, + "email": _refresh_email, + "general": _refresh_general, + "llm": _refresh_llm, + "oidc": _refresh_oidc, +} + + +def engine_for_session() -> Any: + """Resolve the engine at call time. + + The test suite rebinds ``app.database.engine`` after import, so capturing it in a + module global here would leave handlers pointed at the un-patched engine. + """ + from app.database import engine + + return engine + + +async def refresh_scope(scope: str) -> None: + """Re-read one settings singleton into its cache.""" + refresher = SCOPES.get(scope) + if refresher is None: + # A newer replica publishing a scope this build does not know about, during a + # rolling deploy. Nothing here can act on it; the peer that owns it will. + logger.debug("ignoring config invalidation for unknown scope %r", scope) + return + async with AsyncSession(engine_for_session()) as session: + await refresher(session) + logger.info("refreshed %s config from the bus", scope) + + +async def refresh_all() -> None: + """Re-read every scope. The reconnect recovery path: notifications published while + this replica was disconnected are gone, so assume all of them were.""" + for scope in SCOPES: + try: + await refresh_scope(scope) + except Exception: + # Best-effort, exactly like the startup loaders: one unreadable singleton + # must not stop the other five from being refreshed. + logger.exception("failed to refresh %s config on bus reconnect", scope) + + +_first_connect = True + + +async def on_bus_connected() -> None: + """Recover the invalidations missed while this replica was disconnected. + + Skipped on the *first* connect: the startup loaders have just read every scope, so + there is nothing to recover, and refreshing OIDC here would drop the Authlib + registration the lifespan performed moments earlier. + """ + global _first_connect + if _first_connect: + _first_connect = False + return + logger.info("bus reconnected; re-reading every config scope") + await refresh_all() + + +async def handle(descriptor: dict[str, Any]) -> None: + """Bus handler for ``ttx_config``.""" + scope = descriptor.get("scope") + if not isinstance(scope, str): + logger.warning("config invalidation without a scope: %r", descriptor) + return + await refresh_scope(scope) + + +_installed = False + + +def install() -> None: + """Wire this module into the bus. Idempotent — the lifespan may run more than once + in a process (the test client mounts the app repeatedly).""" + global _installed + if _installed: + return + _installed = True + from app.services.pg_listener import listener + + listener.register(pg_bus.CHANNEL_CONFIG, handle) + listener.on_reconnect(pg_bus.CHANNEL_CONFIG, on_bus_connected) diff --git a/app/services/domain_events.py b/app/services/domain_events.py index 6f20b53..de5183e 100644 --- a/app/services/domain_events.py +++ b/app/services/domain_events.py @@ -1,4 +1,4 @@ -"""Post-commit domain events (#212). +"""Post-commit domain events (#212), published across replicas (#213). Services announce that something *durably happened*; subscribers project it outward (WebSocket frames, triggered communications). Nothing is emitted inline any more. @@ -9,21 +9,30 @@ sites and enforced only by reading the code carefully. Here it is structural: record(session, ev) -> appends to session.info["domain_events"].pending + -> each pending event is published on the bus, in-transaction -> SQLAlchemy's after_commit promotes pending -> committed -> after_soft_rollback discards pending - await dispatch(...) -> drains *committed* only + await dispatch(...) -> drains *committed* only, on this replica ``dispatch`` cannot send an uncommitted event, because the only thing that can ever move an event into ``committed`` is a callback that Postgres's COMMIT fires. Forgetting to call ``dispatch`` is caught by a safety-net drain in ``get_session`` teardown (and, in tests, an autouse fixture that fails on a non-empty buffer). -Two rules for anyone adding an event: +The bus publication has the same property for free, and that is why it is issued from +``before_commit`` rather than alongside ``dispatch``: ``NOTIFY`` inside a transaction is +delivered by Postgres only if that transaction commits, and discarded if it rolls back. +The remote replicas' copy of the guarantee is therefore enforced by the database rather +than by another convention nobody can see. + +Three rules for anyone adding an event: * **Record inside the transaction, dispatch after it.** ``record`` raises if the session has no transaction open, which is what "you recorded after the commit" looks like. -* **Dispatch while the session is still open.** Handlers may read the database — the - inject payload builders do — so this is not a fire-and-forget queue. +* **Flush before you record.** Events carry ids, so the row has to exist; every call + site already flushes for its own reasons, and ``record`` will not accept a None id. +* **Dispatch while the session is still open.** Handlers read the database — they are + given only ids — so this is not a fire-and-forget queue. A handler that raises is logged and swallowed: the transaction is already committed and authoritative, and one dead socket must never turn a successful request into a 500. That @@ -34,30 +43,31 @@ import logging from collections.abc import Awaitable, Callable -from dataclasses import dataclass, field +from dataclasses import asdict, dataclass, field, fields from typing import TYPE_CHECKING, Any -from sqlalchemy import event +from sqlalchemy import event, text from sqlalchemy.orm import Session +from app.services import pg_bus + if TYPE_CHECKING: from sqlmodel.ext.asyncio.session import AsyncSession - from app.models.exercise import ExerciseStateTransition - from app.models.inject import Inject - from app.models.inject_comment import InjectComment - from app.models.response import Response - from app.models.suggested_inject import SuggestedInject - logger = logging.getLogger(__name__) # ── Events ──────────────────────────────────────────────────────────────────── # -# Events may carry ORM objects: every session that records one is opened with -# expire_on_commit=False, so attributes stay loaded after the commit that promotes it. -# (app/main.py's lifespan sessions and audit_service's persistence session are NOT — -# never record an event on those.) +# Every event is ids and scalars, never an ORM instance. Two reasons, and the second is +# the one that made it a rule: an ORM object cannot cross a process boundary, so a +# replicated event has to name its rows rather than carry them; and a row read back +# after the commit is authoritative, where a carried instance is a snapshot of whatever +# the recording session happened to hold (the lifecycle CAS runs with +# synchronize_session=False, so that snapshot was demonstrably stale — see #212). +# +# Handlers therefore read what they need. On the replica that recorded the event those +# reads are identity-map hits; on every other one they are the point. @dataclass(frozen=True) @@ -67,61 +77,97 @@ class DomainEvent: @dataclass(frozen=True) class ExerciseStateChanged(DomainEvent): - # No Exercise here on purpose. The lifecycle CAS runs with synchronize_session=False, - # so the in-session Exercise still holds its pre-transition attributes at record time - # and is only refreshed afterwards. Carrying it would work today purely because - # dispatch happens later — the handler re-reads it instead (an identity-map hit). - transition: ExerciseStateTransition + transition_id: int action: str @dataclass(frozen=True) class InjectReleased(DomainEvent): - inject: Inject + inject_id: int @dataclass(frozen=True) class InjectUpdated(DomainEvent): - inject: Inject + inject_id: int @dataclass(frozen=True) class InjectCommentCreated(DomainEvent): - comment: InjectComment - payload: dict[str, Any] + comment_id: int @dataclass(frozen=True) class ResponseSubmitted(DomainEvent): - response: Response + response_id: int @dataclass(frozen=True) class CommunicationCreated(DomainEvent): - # An id, not the ORM row: deliver_triggered_communication learns its id from an - # INSERT ... RETURNING and never loads the object. communication_id: int @dataclass(frozen=True) class ResponseAssessed(DomainEvent): response_id: int - payload: dict[str, Any] + assessment_id: int @dataclass(frozen=True) class InjectSuggested(DomainEvent): - suggested: SuggestedInject + suggested_id: int @dataclass(frozen=True) class SummaryGenerated(DomainEvent): - payload: dict[str, Any] + summary_id: int Handler = Callable[["AsyncSession", Any], Awaitable[None]] +# ── The wire format ─────────────────────────────────────────────────────────── +# +# Wire names are declared rather than derived from the class name, so renaming a class +# cannot silently desynchronise two replicas mid-rolling-deploy. + +EVENT_NAMES: dict[type[DomainEvent], str] = { + ExerciseStateChanged: "exercise_state_changed", + InjectReleased: "inject_released", + InjectUpdated: "inject_updated", + InjectCommentCreated: "inject_comment_created", + ResponseSubmitted: "response_submitted", + CommunicationCreated: "communication_created", + ResponseAssessed: "response_assessed", + InjectSuggested: "inject_suggested", + SummaryGenerated: "summary_generated", +} + +_EVENTS_BY_NAME: dict[str, type[DomainEvent]] = { + name: cls for cls, name in EVENT_NAMES.items() +} + + +def to_descriptor(ev: DomainEvent) -> dict[str, Any]: + """Render an event as a bus descriptor.""" + return pg_bus.envelope(event=EVENT_NAMES[type(ev)], **asdict(ev)) + + +def from_descriptor(descriptor: dict[str, Any]) -> DomainEvent | None: + """Rebuild an event from a bus descriptor, or None if it is not one we project. + + Unknown names and missing fields are skipped rather than raised: during a rolling + deploy the replica on the other side of the bus may be a different build. + """ + cls = _EVENTS_BY_NAME.get(descriptor.get("event", "")) + if cls is None: + return None + try: + return cls(**{f.name: descriptor[f.name] for f in fields(cls)}) + except KeyError: + logger.warning("bus descriptor for %s is missing fields: %r", cls.__name__, descriptor) + return None + + # ── The session-scoped buffer ───────────────────────────────────────────────── _KEY = "domain_events" @@ -155,16 +201,52 @@ def buffer_for(session: Any) -> _Buffer: def record(session: AsyncSession, ev: DomainEvent) -> None: - """Announce ``ev``, to be dispatched only if the current transaction commits.""" + """Announce ``ev``, to be published and dispatched only if this transaction commits.""" if not session.in_transaction(): raise RuntimeError( f"{type(ev).__name__} was recorded with no transaction open — an event must be " "recorded *inside* the unit of work that persists it, or a rollback cannot " "discard it. Move the record() call above the commit." ) + missing = [f.name for f in fields(ev) if getattr(ev, f.name) is None] + if missing: + raise RuntimeError( + f"{type(ev).__name__} was recorded with {', '.join(missing)} unset — the row " + "has not been flushed yet, so no replica (including this one) could read it " + "back. Flush before recording." + ) _buffer(session).pending.append(ev) +@event.listens_for(Session, "before_commit") +def _publish_on_commit(session: Session) -> None: + """Put every pending event on the bus, inside the transaction that is committing. + + Deliberately not "after the commit, alongside dispatch": Postgres holds a NOTIFY + until COMMIT and drops it on rollback, so issuing it here is what makes a peer + replica's copy of the frame exactly as trustworthy as this one's. Publishing after + the commit would reopen the window where the state changed and the announcement was + lost, which is the bug class this whole seam exists to close. + """ + buf = session.info.get(_KEY) + if buf is None or not buf.pending: + return + connection = session.connection() + for ev in buf.pending: + try: + payload = pg_bus.encode(to_descriptor(ev)) + except (ValueError, KeyError): + # A descriptor this replica cannot render is a bug in the event, not in the + # user's request: local projection still works, so log it rather than + # failing a commit that has already done everything it was asked to. + logger.exception("could not render %s for the bus", type(ev).__name__) + continue + connection.execute( + text("SELECT pg_notify(:channel, :payload)"), + {"channel": pg_bus.CHANNEL_EVENTS, "payload": payload}, + ) + + @event.listens_for(Session, "after_commit") def _promote_on_commit(session: Session) -> None: buf = session.info.get(_KEY) @@ -193,10 +275,13 @@ def _discard_on_rollback(session: Session, previous_transaction: Any) -> None: # ── Subscribers ─────────────────────────────────────────────────────────────── _subscribers: dict[type[DomainEvent], list[Handler]] = {} +_local_only: set[Handler] = set() _projectors_loaded = False def subscribe(event_type: type[DomainEvent]) -> Callable[[Handler], Handler]: + """Register a projection: runs on every replica, for local and relayed events alike.""" + def decorate(handler: Handler) -> Handler: _subscribers.setdefault(event_type, []).append(handler) return handler @@ -204,6 +289,23 @@ def decorate(handler: Handler) -> Handler: return decorate +def subscribe_local(event_type: type[DomainEvent]) -> Callable[[Handler], Handler]: + """Register a consequence that must run exactly once, on the replica that recorded it. + + A projection is idempotent by nature — every replica rendering the same frame for + its own sockets is the whole point. Work that *acts* is not: arming a timer on all + three replicas fires it three times. Those handlers stay local until they can be + made durable and transactional (#213 step 3). + """ + + def decorate(handler: Handler) -> Handler: + _subscribers.setdefault(event_type, []).append(handler) + _local_only.add(handler) + return handler + + return decorate + + def _load_projectors() -> None: """Import the subscriber modules on first dispatch. @@ -220,6 +322,29 @@ def _load_projectors() -> None: from app.services import ws_projector # noqa: F401 +async def project(session: AsyncSession, ev: DomainEvent, *, local: bool = True) -> None: + """Run the subscribers for one event. + + ``local=False`` is the relay's path: an event another replica recorded, so only the + projections run — the acting handlers already ran where the work was done. + """ + _load_projectors() + for handler in _subscribers.get(type(ev), []): + if not local and handler in _local_only: + continue + try: + await handler(session, ev) + except Exception: + # The transaction is committed and authoritative. A failed projection is + # logged, never raised: it must not turn a successful request into a 500, + # and it must not stop the other subscribers for this event. + logger.exception( + "domain event subscriber %s failed for %s", + getattr(handler, "__qualname__", handler), + type(ev).__name__, + ) + + async def dispatch(session: AsyncSession) -> None: """Fan out every event whose transaction committed. Call after each unit of work.""" _load_projectors() @@ -237,15 +362,4 @@ async def dispatch(session: AsyncSession) -> None: events, buf.committed = buf.committed, [] for ev in events: - for handler in _subscribers.get(type(ev), []): - try: - await handler(session, ev) - except Exception: - # The transaction is committed and authoritative. A failed projection is - # logged, never raised: it must not turn a successful request into a 500, - # and it must not stop the other subscribers for this event. - logger.exception( - "domain event subscriber %s failed for %s", - getattr(handler, "__qualname__", handler), - type(ev).__name__, - ) + await project(session, ev, local=True) diff --git a/app/services/email_settings_service.py b/app/services/email_settings_service.py index f7beda3..56f551c 100644 --- a/app/services/email_settings_service.py +++ b/app/services/email_settings_service.py @@ -7,7 +7,7 @@ from app.config import settings from app.models.email_settings import EmailSettings -from app.services import mail_service, sink_pinning +from app.services import mail_service, pg_bus, sink_pinning _SINGLETON_ID = 1 EDITABLE_FIELDS = ( @@ -76,6 +76,7 @@ async def update_settings(session: AsyncSession, changes: dict[str, Any]) -> Ema setattr(row, key, changes[key]) row.updated_at = datetime.now(UTC) session.add(row) + await pg_bus.publish_config_changed(session, "email") await session.commit() await session.refresh(row) mail_service.set_config(_to_config(row)) diff --git a/app/services/exercise_service.py b/app/services/exercise_service.py index 7fbccd3..ba14977 100644 --- a/app/services/exercise_service.py +++ b/app/services/exercise_service.py @@ -163,11 +163,12 @@ async def transition_state_with_history( # must be recorded inside this transaction so a failed commit discards it. await session.flush() assert exercise.id is not None + assert transition.id is not None record( session, ExerciseStateChanged( exercise_id=exercise.id, - transition=transition, + transition_id=transition.id, action=transition_action(previous_state, new_state), ), ) diff --git a/app/services/general_settings_service.py b/app/services/general_settings_service.py index e0c0980..fecb35c 100644 --- a/app/services/general_settings_service.py +++ b/app/services/general_settings_service.py @@ -8,6 +8,7 @@ from app.config import settings from app.models.general_settings import GeneralSettings +from app.services import pg_bus _SINGLETON_ID = 1 EDITABLE_FIELDS = ( @@ -89,6 +90,7 @@ async def update_settings(session: AsyncSession, changes: dict[str, Any]) -> Gen setattr(row, key, changes[key]) row.updated_at = datetime.now(UTC) session.add(row) + await pg_bus.publish_config_changed(session, "general") await session.commit() await session.refresh(row) set_config(_to_config(row)) diff --git a/app/services/inject_comment_service.py b/app/services/inject_comment_service.py index 107b594..6479a25 100644 --- a/app/services/inject_comment_service.py +++ b/app/services/inject_comment_service.py @@ -28,8 +28,9 @@ async def comment_group_for_user(session: AsyncSession, inject: Inject, user: Us async def comment_payload(session: AsyncSession, comment: InjectComment) -> dict: """Canonical comment serialization, shared by the HTTP reply and the WS frame. - Lives here rather than in the router because the event that carries it is recorded - *inside* the transaction, so the payload has to be built before the commit. + Called twice per comment on purpose: once pre-commit for the HTTP reply, and once + per replica when the projector rebuilds the frame from the committed row (#213). + Both reads resolve the same fields, so the two representations cannot drift. """ author = await session.get(User, comment.user_id) return _comment_payload( @@ -89,9 +90,10 @@ async def create_inject_comment( session.add(comment) await session.flush() # id + created_at, so the payload is complete pre-commit payload = await comment_payload(session, comment) + assert comment.id is not None record( session, - InjectCommentCreated(exercise_id=exercise_id, comment=comment, payload=payload), + InjectCommentCreated(exercise_id=exercise_id, comment_id=comment.id), ) await session.commit() await session.refresh(comment) diff --git a/app/services/inject_service.py b/app/services/inject_service.py index 6e95479..f402950 100644 --- a/app/services/inject_service.py +++ b/app/services/inject_service.py @@ -154,23 +154,23 @@ async def release_inject( detail="Inject is no longer pending and cannot be released", ) # Recorded inside the transaction, so the compare-and-swap loser above — which - # rolled back — cannot emit a frame or fire triggered comms for a release that - # never happened. Both are subscribers to this one event (see ws_projector). + # rolled back — cannot emit a frame for a release that never happened. assert inject.id is not None - record(session, InjectReleased(exercise_id=inject.exercise_id, inject=inject)) + record(session, InjectReleased(exercise_id=inject.exercise_id, inject_id=inject.id)) + # Triggered communications are enqueued in this same transaction rather than from a + # post-commit subscriber. That is #211: a release that committed and then lost its + # process left every triggered comm silently unarmed. Now the jobs exist if and only + # if the release does, and the CAS loser's rollback discards them with everything + # else. + from app.services.schedule_service import schedule_release_triggered_comms + + await schedule_release_triggered_comms(session, inject.exercise_id, inject) await session.commit() # ``inject`` may already be in this session's identity map, so a returned ORM # row would retain its old pending attributes with synchronize_session=False. # Refresh the authoritative row after commit before constructing side effects. await session.refresh(inject) - # Releasing (manually or on schedule) settles any pending scheduled-release timer only - # after the compare-and-swap has committed. The losing racer must not cancel the winner's - # timer or emit any side effects. - from app.services.schedule_service import cancel_inject_schedule - - cancel_inject_schedule(inject.exercise_id, inject.id) - await dispatch(session) return inject diff --git a/app/services/llm_service.py b/app/services/llm_service.py index e6fcddf..b05f81a 100644 --- a/app/services/llm_service.py +++ b/app/services/llm_service.py @@ -6,11 +6,11 @@ from pydantic import BaseModel, ConfigDict, Field, ValidationError from sqlalchemy.exc import IntegrityError from sqlmodel import select +from sqlmodel.ext.asyncio.session import AsyncSession from app.database import engine from app.schemas.api import AssessmentPublic, ExecutiveSummaryPublic, SuggestedInjectPublic from app.services import llm_settings_service -from app.services.background import spawn from app.services.domain_events import ( InjectSuggested, ResponseAssessed, @@ -46,12 +46,6 @@ class SuggestedInjectOutput(BaseModel): target_teams: list[str] | None = Field(default=None, max_length=_MAX_LLM_TEAMS) -# The application is deliberately single-replica while its real-time and rate-limit -# state is in memory (CLAUDE.md). Keep one assessment task per response in flight so -# automatic and manual triggers cannot duplicate provider calls within that supported -# deployment model. The worker also checks persisted state before calling a provider. -_assessment_inflight: set[int] = set() - _ASSESSMENT_SYSTEM = ( "You are an expert tabletop exercise facilitator. " "Assess participant responses concisely and constructively. " @@ -199,7 +193,7 @@ async def _assess_response_result(session, response, inject, definition): ResponseAssessed( exercise_id=inject.exercise_id, response_id=response_id, - payload=assessment_payload(assessment), + assessment_id=assessment.id, ), ) await session.commit() @@ -282,9 +276,10 @@ async def _suggest_inject_result(session, response, inject, exercise, definition session.add(suggested) try: await session.flush() + assert suggested.id is not None record( session, - InjectSuggested(exercise_id=exercise.id, suggested=suggested), + InjectSuggested(exercise_id=exercise.id, suggested_id=suggested.id), ) await session.commit() except IntegrityError: @@ -323,20 +318,28 @@ async def run_llm_pipeline(response_id: int, inject_id: int, exercise_id: int) - logger.exception("LLM pipeline failed for response %d", response_id) -def queue_llm_pipeline(response_id: int, inject_id: int, exercise_id: int) -> bool: - """Queue one assessment per response; return whether a task was created.""" - if response_id in _assessment_inflight: - return False - _assessment_inflight.add(response_id) - coroutine = run_llm_pipeline(response_id, inject_id, exercise_id) - try: - task = spawn(coroutine) - except Exception: - coroutine.close() - _assessment_inflight.discard(response_id) - raise - task.add_done_callback(lambda _: _assessment_inflight.discard(response_id)) - return True +async def queue_llm_pipeline( + session: AsyncSession, response_id: int, inject_id: int, exercise_id: int +) -> bool: + """Queue one assessment per response; return whether a job was created. + + De-duplication is a queueing lock rather than a process-local set (#213): an + automatic trigger on one replica and a facilitator's manual re-trigger on another + would otherwise each buy a provider call. The persisted-assessment check inside the + pipeline remains the backstop for a job that runs after one already succeeded. + """ + from app.services import task_queue + + return await task_queue.defer_job_once( + session, + task_queue.TASK_ASSESS_RESPONSE, + queueing_lock=f"assess:{response_id}", + args={ + "response_id": response_id, + "inject_id": inject_id, + "exercise_id": exercise_id, + }, + ) async def _run_llm_pipeline(response_id: int, inject_id: int, exercise_id: int) -> None: @@ -466,12 +469,13 @@ async def generate_executive_summary(session, exercise_id: int): session.add(summary) try: await session.flush() + assert summary.id is not None # Inside the transaction: the IntegrityError branch below rolls back, which # discards the buffered event — the loser must not broadcast a frame for a # row it never created (same discipline as the assess path). record( session, - SummaryGenerated(exercise_id=exercise_id, payload=summary_payload(summary)), + SummaryGenerated(exercise_id=exercise_id, summary_id=summary.id), ) await session.commit() except IntegrityError: @@ -493,28 +497,22 @@ async def generate_executive_summary(session, exercise_id: int): return summary -_summary_inflight: set[int] = set() - - -def queue_summary_pipeline(exercise_id: int) -> bool: - """Queue one summary draft per exercise; return whether a task was created. +async def queue_summary_pipeline(session: AsyncSession, exercise_id: int) -> bool: + """Queue one summary draft per exercise; return whether a job was created. Without this, a double-click on "draft summary" fired two paid provider calls racing each other into the unique constraint (#269) — same guard shape as - ``queue_llm_pipeline``. + ``queue_llm_pipeline``, and since #213 it holds across replicas rather than + within one process. """ - if exercise_id in _summary_inflight: - return False - _summary_inflight.add(exercise_id) - coroutine = run_summary_pipeline(exercise_id) - try: - task = spawn(coroutine) - except Exception: - coroutine.close() - _summary_inflight.discard(exercise_id) - raise - task.add_done_callback(lambda _: _summary_inflight.discard(exercise_id)) - return True + from app.services import task_queue + + return await task_queue.defer_job_once( + session, + task_queue.TASK_GENERATE_SUMMARY, + queueing_lock=f"summary:{exercise_id}", + args={"exercise_id": exercise_id}, + ) async def run_summary_pipeline(exercise_id: int) -> None: diff --git a/app/services/llm_settings_service.py b/app/services/llm_settings_service.py index e3efec3..4f29b87 100644 --- a/app/services/llm_settings_service.py +++ b/app/services/llm_settings_service.py @@ -15,6 +15,7 @@ settings, ) from app.models.llm_settings import LLMSettings +from app.services import pg_bus _SINGLETON_ID = 1 EDITABLE_FIELDS = ( @@ -201,6 +202,7 @@ async def update_settings(session: AsyncSession, changes: dict[str, Any]) -> LLM setattr(row, field, getattr(candidate, field)) row.updated_at = datetime.now(UTC) session.add(row) + await pg_bus.publish_config_changed(session, "llm") await session.commit() await session.refresh(row) set_config(_to_config(row)) diff --git a/app/services/oidc/service.py b/app/services/oidc/service.py index 3405d92..ecb9c2c 100644 --- a/app/services/oidc/service.py +++ b/app/services/oidc/service.py @@ -153,6 +153,13 @@ async def _sync_returning_user( if dirty: session.add(user) + if user.role != previous_role and user.id is not None: + # Announced inside the transaction, so peers drop this user's sockets only + # if the downgrade actually commits (#213). This replica's own sockets are + # closed below, once the row is durable. + from app.services.ws_relay import publish_user_connections_close + + await publish_user_connections_close(session, user.id) await session.commit() await session.refresh(user) diff --git a/app/services/oidc_settings_service.py b/app/services/oidc_settings_service.py index c3aeaeb..88be7c2 100644 --- a/app/services/oidc_settings_service.py +++ b/app/services/oidc_settings_service.py @@ -16,6 +16,7 @@ settings, ) from app.models.oidc_settings import OIDCSettings +from app.services import pg_bus _SINGLETON_ID = 1 AUTH_MODES = (AUTH_MODE_LOCAL, AUTH_MODE_OIDC, AUTH_MODE_BOTH) @@ -293,6 +294,7 @@ async def update_settings(session: AsyncSession, changes: dict[str, Any]) -> OID setattr(row, field, getattr(candidate, field)) row.updated_at = datetime.now(UTC) session.add(row) + await pg_bus.publish_config_changed(session, "oidc") await session.commit() await session.refresh(row) set_config(_to_config(row)) diff --git a/app/services/pg_bus.py b/app/services/pg_bus.py new file mode 100644 index 0000000..3c28947 --- /dev/null +++ b/app/services/pg_bus.py @@ -0,0 +1,139 @@ +"""The cross-replica message bus, carried by Postgres LISTEN/NOTIFY (#213). + +Everything this app has to tell its *other* replicas goes through here. There is no +Redis and no broker: the database we already run is the bus, which keeps the hardened +single-container deployment story intact. + +The reason for choosing NOTIFY over a broker is the transactional boundary. ``NOTIFY`` +issued inside a transaction is delivered only if that transaction COMMITs, and is +discarded if it rolls back — natively, by Postgres. That is precisely the guarantee +``domain_events`` hand-builds for the local process, so publishing from *inside* the +unit of work extends it across replicas for free. Publish after the commit instead and +you reintroduce the window where the state changed but the announcement was lost. + +So the rule for every publisher here is the same as ``record``'s: + + **Publish inside the transaction, never after it.** + +The payload is a *descriptor*, not a rendered message: NOTIFY caps a payload at +8000 bytes, and inject/communication content would blow past that. A descriptor names +what happened and the ids to read it back with; each replica re-reads the committed +rows and builds its own frames. Handlers already read the database by design, so this +costs nothing architecturally — and it sidesteps the fact that domain events carry ORM +objects, which cannot cross a process boundary at all. + +Delivery is at-most-once and reaches only currently-connected subscribers. That is the +same guarantee Redis pub/sub offers, and it is the one the frontend already copes with: +clients refetch on WebSocket reconnect. +""" + +from __future__ import annotations + +import json +import logging +from typing import TYPE_CHECKING, Any +from uuid import uuid4 + +from sqlalchemy import text + +if TYPE_CHECKING: + from sqlmodel.ext.asyncio.session import AsyncSession + +logger = logging.getLogger(__name__) + + +# ── Channels ────────────────────────────────────────────────────────────────── + +CHANNEL_EVENTS = "ttx_events" +"""Domain-event descriptors and WebSocket control messages.""" + +CHANNEL_CONFIG = "ttx_config" +"""Runtime-config invalidation, by scope.""" + + +# ── This process ────────────────────────────────────────────────────────────── + +ORIGIN = uuid4().hex +"""Identifies this replica for the lifetime of the process. + +Stamped on every descriptor so a listener can tell its own publications apart from a +peer's. The WebSocket relay uses it to skip descriptors this process already projected +in-band (see ``ws_relay``); config invalidation deliberately does not skip, because a +re-read of a row it just wrote is idempotent and one less branch to get wrong. +""" + +# Postgres rejects a NOTIFY payload over 8000 bytes with an error that would abort the +# publishing transaction — i.e. a payload bug would roll back the state change that +# occasioned it. Descriptors are ids and short strings and land nowhere near this, so +# tripping it means someone started putting content on the bus; fail loudly and locally. +PAYLOAD_LIMIT_BYTES = 7000 + + +def envelope(**fields: Any) -> dict[str, Any]: + """Stamp a descriptor with the bus version and this replica's origin.""" + return {"v": 1, "origin": ORIGIN, **fields} + + +def is_own(descriptor: dict[str, Any]) -> bool: + """True if this process published the descriptor.""" + return descriptor.get("origin") == ORIGIN + + +def encode(descriptor: dict[str, Any]) -> str: + body = json.dumps(descriptor, separators=(",", ":")) + size = len(body.encode("utf-8")) + if size > PAYLOAD_LIMIT_BYTES: + raise ValueError( + f"NOTIFY payload is {size} bytes, over the {PAYLOAD_LIMIT_BYTES}-byte bus " + "limit. Descriptors carry ids, not content — put the content in a row and " + "let each replica read it back." + ) + return body + + +def decode(payload: str) -> dict[str, Any] | None: + """Parse a received payload, or None if it is not a descriptor we understand.""" + try: + descriptor = json.loads(payload) + except ValueError: + logger.warning("discarding malformed bus payload (%d bytes)", len(payload)) + return None + if not isinstance(descriptor, dict): + logger.warning("discarding non-object bus payload of type %s", type(descriptor).__name__) + return None + if descriptor.get("v") != 1: + # A rolling deploy briefly runs two versions against one database. An unknown + # version is a newer replica talking, not corruption: skip it quietly rather + # than crashing the listener that still has to serve this replica's clients. + logger.debug("skipping bus payload with unsupported version %r", descriptor.get("v")) + return None + return descriptor + + +# ── Publishing ──────────────────────────────────────────────────────────────── + + +async def publish(session: AsyncSession, channel: str, descriptor: dict[str, Any]) -> None: + """Queue a NOTIFY on the caller's transaction. + + Nothing is sent here: Postgres holds the notification until the surrounding + transaction commits, and drops it if that transaction rolls back. Call this + *before* the commit that makes the announcement true. + """ + # Issued on the session's own connection, which is what binds the notification to + # the caller's transaction rather than to some other pooled one. + connection = await session.connection() + await connection.execute( + text("SELECT pg_notify(:channel, :payload)"), + {"channel": channel, "payload": encode(descriptor)}, + ) + + +async def publish_config_changed(session: AsyncSession, scope: str) -> None: + """Tell every replica that a runtime-config scope was rewritten. + + Scopes name a settings service (``proxy``, ``siem``, ``email``, ``general``, + ``llm``, ``oidc``) rather than the changed fields: the receiver re-reads the + singleton row, so the diff is never in flight and can never go stale. + """ + await publish(session, CHANNEL_CONFIG, envelope(scope=scope)) diff --git a/app/services/pg_listener.py b/app/services/pg_listener.py new file mode 100644 index 0000000..0e1ae16 --- /dev/null +++ b/app/services/pg_listener.py @@ -0,0 +1,229 @@ +"""This replica's subscription to the Postgres bus (#213). + +One dedicated connection per process, held open for the lifetime of the app, LISTENing +on every channel something has registered a handler for. It is deliberately *not* a +connection from the SQLAlchemy pool: LISTEN is connection-state, so a pooled checkout +would either be pinned out of the pool forever or be recycled out from under the +subscription — a failure mode that presents as "frames stopped arriving hours later". + +Two tasks, not one: + +* ``_maintain_connection`` owns the socket — connect, subscribe, keep alive, and on any + failure reconnect with capped exponential backoff. +* ``_consume`` owns the work — drain the queue and run handlers. + +Splitting them is what keeps a slow handler from blocking asyncpg's protocol callbacks +(the callback itself only does a ``put_nowait``), and what lets a handler crash without +taking the subscription down with it. + +Delivery is at-most-once. Notifications published while this replica was disconnected +are gone — nothing replays them. Anything that cannot tolerate that registers an +``on_reconnect`` hook and re-reads authoritative state instead, which is what config +invalidation does; WebSocket frames simply rely on clients refetching after their own +reconnect. +""" + +from __future__ import annotations + +import asyncio +import contextlib +import logging +import random +from collections.abc import Awaitable, Callable +from typing import Any + +import asyncpg + +from app.services import pg_bus + +logger = logging.getLogger(__name__) + +Handler = Callable[[dict[str, Any]], Awaitable[None]] +ReconnectHook = Callable[[], Awaitable[None]] + +# Backoff bounds for a database that has gone away (failover, restart, network blip). +_BACKOFF_INITIAL_SECONDS = 1.0 +_BACKOFF_MAX_SECONDS = 30.0 +# A silent LISTEN connection is indistinguishable from a wedged one, and a NAT or proxy +# will happily drop an idle socket without telling either end. Probe periodically so a +# dead connection is discovered in bounded time rather than at the next publication. +_KEEPALIVE_SECONDS = 30.0 +# Far above the real rate (a busy exercise produces tens of messages a minute). Reaching +# this means the consumer is wedged, so say so rather than growing without bound. +_QUEUE_MAXSIZE = 1000 + + +class Listener: + """Owns the LISTEN connection and dispatches what arrives on it.""" + + def __init__(self) -> None: + self._handlers: dict[str, list[Handler]] = {} + self._hooks: dict[str, list[ReconnectHook]] = {} + self._queue: asyncio.Queue[tuple[str, str]] | None = None + self._connection: Any = None + self._connection_task: asyncio.Task | None = None + self._consumer_task: asyncio.Task | None = None + self._connected = False + + # ── Registration (before start) ─────────────────────────────────────────── + + def register(self, channel: str, handler: Handler) -> None: + """Subscribe ``handler`` to ``channel``. Registering a channel makes the + connection LISTEN on it at its next (re)connect.""" + self._handlers.setdefault(channel, []).append(handler) + + def on_reconnect(self, channel: str, hook: ReconnectHook) -> None: + """Run ``hook`` whenever the subscription to ``channel`` is (re)established. + + The recovery seam for at-most-once delivery: anything missed while + disconnected has to be recovered by re-reading state, not by replay. + """ + self._hooks.setdefault(channel, []).append(hook) + + @property + def is_connected(self) -> bool: + return self._connected + + # ── Lifecycle ───────────────────────────────────────────────────────────── + + async def start(self) -> None: + """Begin listening. Returns immediately; the first connect happens in the task. + + Startup never blocks on the bus: a database that is briefly unreachable must not + stop this replica from serving requests it can already serve. + """ + if self._connection_task is not None: + return + if not self._handlers: + logger.debug("no bus handlers registered; listener not started") + return + self._queue = asyncio.Queue(maxsize=_QUEUE_MAXSIZE) + self._consumer_task = asyncio.create_task(self._consume()) + self._connection_task = asyncio.create_task(self._maintain_connection()) + + async def stop(self) -> None: + """Stop listening and close the connection. + + Must run before ``engine.dispose()`` in teardown — not because they share a + connection (they do not), but because a handler this cancels mid-flight is + holding a pooled one. + """ + for task in (self._connection_task, self._consumer_task): + if task is not None: + task.cancel() + for task in (self._connection_task, self._consumer_task): + if task is not None: + with contextlib.suppress(asyncio.CancelledError): + await task + self._connection_task = None + self._consumer_task = None + await self._close_connection() + self._queue = None + + # ── The connection ──────────────────────────────────────────────────────── + + async def _maintain_connection(self) -> None: + delay = _BACKOFF_INITIAL_SECONDS + while True: + try: + await self._connect_and_subscribe() + delay = _BACKOFF_INITIAL_SECONDS + await self._hold_open() + except asyncio.CancelledError: + raise + except Exception: + self._connected = False + logger.exception("bus listener connection failed; reconnecting in %.1fs", delay) + await self._close_connection() + await self._sleep_before_retry(delay) + delay = min(delay * 2, _BACKOFF_MAX_SECONDS) + + async def _sleep_before_retry(self, delay: float) -> None: + # Jitter so a fleet of replicas that lost the database together does not + # reconnect in lockstep and stampede it as it comes back. + await asyncio.sleep(delay * (0.5 + random.random())) # nosec B311 - spreads retries, not a secret + + async def _connect_and_subscribe(self) -> None: + from app.config import settings + from app.database import make_asyncpg_dsn + + self._connection = await asyncpg.connect(make_asyncpg_dsn(settings.database_url)) + for channel in self._handlers: + await self._connection.add_listener(channel, self._on_notify) + self._connected = True + logger.info("bus listener subscribed to %s", ", ".join(sorted(self._handlers))) + await self._run_reconnect_hooks() + + async def _run_reconnect_hooks(self) -> None: + for channel, hooks in self._hooks.items(): + for hook in hooks: + try: + await hook() + except Exception: + logger.exception("bus reconnect hook for %s failed", channel) + + async def _hold_open(self) -> None: + """Keep the subscription alive until the connection dies.""" + while True: + await asyncio.sleep(_KEEPALIVE_SECONDS) + # Raises if the connection has gone away, which returns control to the + # reconnect loop. asyncpg notices some drops on its own; this notices the + # rest (silently dropped sockets) without waiting for a publication. + await self._connection.execute("SELECT 1") + + async def _close_connection(self) -> None: + connection, self._connection = self._connection, None + self._connected = False + if connection is None: + return + with contextlib.suppress(Exception): + await connection.close(timeout=5) + + # ── Delivery ────────────────────────────────────────────────────────────── + + def _on_notify(self, connection: Any, pid: int, channel: str, payload: str) -> None: + """asyncpg's callback. Sync and non-blocking on purpose — it runs inside the + protocol's read loop, so the only thing it may do is hand work off.""" + if self._queue is None: + return + try: + self._queue.put_nowait((channel, payload)) + except asyncio.QueueFull: + logger.error( + "bus queue full (%d messages); dropping a %s notification. The consumer " + "is not keeping up or is wedged.", + _QUEUE_MAXSIZE, + channel, + ) + + async def _consume(self) -> None: + assert self._queue is not None + while True: + channel, payload = await self._queue.get() + try: + await self._dispatch(channel, payload) + except asyncio.CancelledError: + raise + except Exception: + # Never let one bad message end the consumer: this task is the only + # thing delivering cross-replica work, and it has to outlive any + # individual handler's bad day. + logger.exception("bus consumer failed on a %s notification", channel) + + async def _dispatch(self, channel: str, payload: str) -> None: + descriptor = pg_bus.decode(payload) + if descriptor is None: + return + for handler in self._handlers.get(channel, []): + try: + await handler(descriptor) + except Exception: + logger.exception( + "bus handler %s failed for a %s notification", + getattr(handler, "__qualname__", handler), + channel, + ) + + +listener = Listener() +"""The process-wide listener. Register handlers at import; start it in the lifespan.""" diff --git a/app/services/proxy.py b/app/services/proxy.py index a026ac5..e7a17ce 100644 --- a/app/services/proxy.py +++ b/app/services/proxy.py @@ -20,7 +20,8 @@ Routing is read from an in-memory snapshot (``get_config``/``set_config``) refreshed at startup and whenever an admin saves — the sync ``audit_service.emit`` path has no -DB session, exactly as with ``siem_service`` (single-process, like ``ws_manager``). +DB session, exactly as with ``siem_service``. The snapshot is per-process, so a save on +another replica reaches this one over the config bus (``config_sync``, #213). Callers must treat a ``None`` config as "feature not loaded" and pass no kwargs at all, so behaviour is byte-for-byte what it was before this feature: diff --git a/app/services/proxy_settings_service.py b/app/services/proxy_settings_service.py index 81b0c8e..bfdc4d8 100644 --- a/app/services/proxy_settings_service.py +++ b/app/services/proxy_settings_service.py @@ -23,7 +23,7 @@ from app.config import settings from app.models.proxy_settings import ProxySettings -from app.services import proxy, sink_pinning +from app.services import pg_bus, proxy, sink_pinning _SINGLETON_ID = 1 @@ -96,6 +96,9 @@ async def update_settings(session: AsyncSession, changes: dict[str, Any]) -> Pro setattr(row, key, changes[key]) row.updated_at = datetime.now(UTC) session.add(row) + # Announced from inside the transaction, so a rollback takes the announcement with + # it and every replica's cache reflects exactly what committed (#213). + await pg_bus.publish_config_changed(session, "proxy") await session.commit() await session.refresh(row) proxy.set_config(_to_config(row)) @@ -107,3 +110,14 @@ async def refresh_cache(session: AsyncSession) -> None: """Load the singleton row into the proxy in-memory cache (startup).""" row = await get_settings(session) proxy.set_config(_to_config(row)) + + +async def refresh_cache_and_dependents(session: AsyncSession) -> None: + """Reload the cache *and* drop the clients that captured the previous proxy. + + What ``update_settings`` does locally after a save, for a replica learning about + that save over the bus (#213). Startup uses the plain ``refresh_cache``: nothing has + been constructed yet at that point, so there is nothing to invalidate. + """ + await refresh_cache(session) + _invalidate_dependent_caches() diff --git a/app/services/rate_limit.py b/app/services/rate_limit.py index d10c2ac..5bec51b 100644 --- a/app/services/rate_limit.py +++ b/app/services/rate_limit.py @@ -1,114 +1,151 @@ -"""In-memory sliding-window rate limiter for login brute-force protection (#11). - -In-memory only: like ws_manager this assumes a single app process. A multi- -replica deployment would need a shared store (e.g. Redis) — see the replica -constraint in CLAUDE.md. - -Memory is bounded (#49): read paths never materialise keys, a key is dropped as -soon as its window empties, and an opportunistic sweep purges expired keys once -per window so rotating attacker-controlled keys (spoofed X-Forwarded-For / email) -cannot accumulate without bound. +"""Sliding-window rate limiter for brute-force protection (#11, #67, #117). + +Counters live in Postgres, in an unlogged table, one row per counted attempt. They used +to be deques in process memory, which meant every replica enforced its own private +allowance: three replicas behind a load balancer multiplied the effective login limit by +three, and an attacker who simply kept reconnecting got the multiplier for free (#213). + +Still a *sliding* window, not a fixed bucket, and for the same reason as before: a fixed +bucket lets an attacker spend the whole allowance at the end of one window and again at +the start of the next, i.e. double the limit back to back. + +Three properties are worth stating because they are easy to break: + +* **Attempts are counted outside the request's transaction.** Recording a failed login + on the request session would let the 401's rollback erase the strike. Each limiter + call takes its own short transaction and commits on its own. +* **Reads and writes are one round trip each.** ``check`` returns the count and the + oldest hit together, so a limited caller learns its ``Retry-After`` without a second + query racing the first. +* **Growth is bounded by a sweep, not by luck.** Keys are attacker-controlled (a spoofed + ``X-Forwarded-For``, an arbitrary email), so expired rows are deleted on a schedule by + ``task_queue.rate_limit_sweep`` rather than opportunistically by whoever calls next (#49). """ -import time -from collections import deque +from datetime import UTC, datetime, timedelta + +from sqlalchemy import and_, delete, func, insert, select from app.config import settings +from app.models.rate_limit import rate_limit_hits + + +def now() -> datetime: + """Indirection so tests can drive the window without sleeping through it.""" + return datetime.now(UTC) + + +def _engine(): + """Resolve the engine at call time — the test suite rebinds it after import.""" + from app.database import engine + + return engine class RateLimiter: - def __init__(self, max_attempts: int, window_seconds: int): + """One named limiter. Instances are cheap: the state is entirely in the database.""" + + def __init__(self, scope: str, max_attempts: int, window_seconds: int): + self.scope = scope self.max_attempts = max_attempts self.window_seconds = window_seconds - self._hits: dict[str, deque[float]] = {} - self._last_sweep = time.monotonic() - - def _trim(self, dq: deque[float], now: float) -> None: - while dq and now - dq[0] > self.window_seconds: - dq.popleft() - def _sweep(self, now: float) -> None: - """Drop every key whose window has fully expired. + async def check(self, key: str) -> tuple[bool, int]: + """Return ``(is_limited, retry_after_seconds)`` for ``key``. - Called opportunistically at most once per window. Without this, a key - that receives a single failed attempt and is never touched again would - persist forever, so an attacker rotating keys grows the dict without - bound (#49). The per-window sweep caps residency to the distinct keys - seen within roughly one window. + One query, because a limited caller immediately needs the retry hint and asking + twice would let the window move between the two answers. """ - for key in list(self._hits): - dq = self._hits[key] - self._trim(dq, now) - if not dq: - del self._hits[key] - self._last_sweep = now - - def _active_hits(self, key: str, now: float) -> deque[float]: - """Return the pruned deque for a key **without** materialising it. - - A missing key yields a throwaway empty deque (never stored), and a key - that prunes down to empty is evicted, so merely checking a novel key can - never leave a permanent entry behind. + moment = now() + cutoff = moment - timedelta(seconds=self.window_seconds) + statement = select( + func.count(rate_limit_hits.c.id), func.min(rate_limit_hits.c.at) + ).where( + and_( + rate_limit_hits.c.scope == self.scope, + rate_limit_hits.c.key == key, + rate_limit_hits.c.at > cutoff, + ) + ) + async with _engine().connect() as connection: + count, oldest = (await connection.execute(statement)).one() + if count < self.max_attempts or oldest is None: + return False, 0 + remaining = self.window_seconds - (moment - oldest).total_seconds() + # At least a second: a caller told to retry after 0 retries immediately, which is + # indistinguishable from not being limited at all. + return True, max(1, int(remaining)) + + async def is_limited(self, key: str) -> bool: + limited, _ = await self.check(key) + return limited + + async def retry_after(self, key: str) -> int: + _, retry = await self.check(key) + return retry + + async def record_failure(self, key: str) -> None: + """Count one attempt against ``key``. + + Committed on its own connection, deliberately: the caller is usually about to + raise, and a strike a rollback could erase is not a strike. """ - if now - self._last_sweep > self.window_seconds: - self._sweep(now) - dq = self._hits.get(key) - if dq is None: - return deque() - self._trim(dq, now) - if not dq: - del self._hits[key] - return dq - - def is_limited(self, key: str) -> bool: - return len(self._active_hits(key, time.monotonic())) >= self.max_attempts - - def retry_after(self, key: str) -> int: - dq = self._active_hits(key, time.monotonic()) - if not dq: - return 0 - remaining = self.window_seconds - (time.monotonic() - dq[0]) - return max(1, int(remaining)) - - def record_failure(self, key: str) -> None: - now = time.monotonic() - dq = self._hits.get(key) - if dq is None: - dq = deque() - self._hits[key] = dq - else: - self._trim(dq, now) - dq.append(now) - - def reset(self, key: str) -> None: - self._hits.pop(key, None) - - def clear(self) -> None: - self._hits.clear() + async with _engine().begin() as connection: + await connection.execute( + insert(rate_limit_hits).values(scope=self.scope, key=key, at=now()) + ) + + async def reset(self, key: str) -> None: + """Forget every attempt for ``key`` — a successful login clearing its own slate.""" + async with _engine().begin() as connection: + await connection.execute( + delete(rate_limit_hits).where( + and_( + rate_limit_hits.c.scope == self.scope, + rate_limit_hits.c.key == key, + ) + ) + ) + + async def clear(self) -> None: + """Drop this limiter's whole history (tests, and an admin unlocking everyone).""" + async with _engine().begin() as connection: + await connection.execute( + delete(rate_limit_hits).where(rate_limit_hits.c.scope == self.scope) + ) def reconfigure(self, max_attempts: int, window_seconds: int) -> None: - """Apply new limits without erasing active histories.""" + """Apply new limits without erasing active histories. + + Thresholds are configuration, not state, so they stay in memory and reach the + other replicas over the config bus with the settings they belong to (#213). + """ self.max_attempts = max_attempts self.window_seconds = window_seconds - self._last_sweep = time.monotonic() login_rate_limiter = RateLimiter( - settings.login_max_attempts, settings.login_lockout_seconds + "login", settings.login_max_attempts, settings.login_lockout_seconds ) # Registration flood protection (#67), keyed per source IP (every attempt counts, # not just failures) — caps mass account creation from one host. registration_rate_limiter = RateLimiter( - settings.registration_max_attempts, settings.registration_lockout_seconds + "registration", + settings.registration_max_attempts, + settings.registration_lockout_seconds, ) # Password-reset request throttle (#117), keyed per source IP — caps outbound reset # emails / token minting from one host (independent of whether the account exists). password_reset_rate_limiter = RateLimiter( - settings.password_reset_max_attempts, settings.password_reset_lockout_seconds + "password_reset", + settings.password_reset_max_attempts, + settings.password_reset_lockout_seconds, ) +ALL_LIMITERS = (login_rate_limiter, registration_rate_limiter, password_reset_rate_limiter) + def apply_config( *, @@ -127,3 +164,19 @@ def apply_config( password_reset_rate_limiter.reconfigure( password_reset_max_attempts, password_reset_lockout_seconds ) + + +async def sweep() -> int: + """Delete hits older than the widest configured window. Returns rows removed. + + Scheduled rather than opportunistic: the previous in-process version swept on a read + path, so a limiter nobody consulted never shed its rows — and the keys are exactly + the ones an attacker can mint at will. + """ + widest = max(limiter.window_seconds for limiter in ALL_LIMITERS) + cutoff = now() - timedelta(seconds=widest) + async with _engine().begin() as connection: + result = await connection.execute( + delete(rate_limit_hits).where(rate_limit_hits.c.at < cutoff) + ) + return result.rowcount or 0 diff --git a/app/services/response_service.py b/app/services/response_service.py index ca3bb2e..834d1a5 100644 --- a/app/services/response_service.py +++ b/app/services/response_service.py @@ -115,7 +115,19 @@ async def submit_response( # Inside the transaction: the IntegrityError branch below rolls back, which # discards this — so a duplicate submission cannot broadcast. That used to be # guaranteed only by the broadcast sitting after the try block. - record(session, ResponseSubmitted(exercise_id=exercise_id, response=response)) + assert response.id is not None + record(session, ResponseSubmitted(exercise_id=exercise_id, response_id=response.id)) + # A response is the only thing that advances a progression cursor, and a cursor + # advance is the only thing that unlocks a scheduled inject the team had not + # reached (#218). Enqueued here, in the response's own transaction: the release + # job exists if and only if the cursor advance it depends on committed, which is + # the race the previous post-commit version had to coordinate around by hand. + from app.models.exercise import Exercise + from app.services.schedule_service import arm_cursor_reached_injects + + exercise = await session.get(Exercise, exercise_id) + if exercise is not None: + await arm_cursor_reached_injects(session, exercise) await session.commit() except IntegrityError as exc: await session.rollback() diff --git a/app/services/retention_service.py b/app/services/retention_service.py index 06efdb1..e87bbf5 100644 --- a/app/services/retention_service.py +++ b/app/services/retention_service.py @@ -17,11 +17,10 @@ with no purpose, so those rows are purged **unconditionally** — the audit trail separately records issuance and acceptance, so nothing is lost. -Single-process, like every other timer in the app: two replicas would run two -concurrent sweeps. See the single-replica note in CLAUDE.md. +The sweep runs as a periodic job on the task queue, so exactly one replica performs each +night's purge however many are running (#213). """ -import asyncio import logging from datetime import UTC, datetime, timedelta @@ -35,8 +34,6 @@ logger = logging.getLogger(__name__) -_SWEEP_INTERVAL_SECONDS = 86_400 # daily - # Rows deleted per transaction. One unbounded DELETE across a year of history would be # a single giant transaction, a long row-lock window and a WAL spike; per-batch commits # bound each one. _MAX_BATCHES bounds the whole sweep so the first-ever pass over a @@ -140,19 +137,7 @@ async def sweep_once() -> tuple[int, int]: return events, tokens -async def retention_task() -> None: - """Background task: sweep on startup, then once a day. - - Sweeping *before* the first sleep is what collapses the issue's "on-startup pass - plus daily timer" into one construct. It deliberately runs inside the task rather - than being awaited inline in the lifespan like ``rehydrate_schedules``: a first - purge over a legacy table is unbounded and would delay readiness. - """ - while True: - try: - await sweep_once() - except Exception: - # A failed sweep must never kill the loop. CancelledError is BaseException, - # so the lifespan's task.cancel() still stops shutdown (cf. heartbeat_task). - logger.exception("retention sweep failed; continuing") - await asyncio.sleep(_SWEEP_INTERVAL_SECONDS) +# The daily sweep is a periodic job on the task queue (``task_queue.retention_sweep``), +# not a loop in every process. procrastinate's periodic defer is unique per (task, +# timestamp), so exactly one replica runs each night's purge — where a per-process loop +# would have had every replica purging the same rows concurrently (#213). diff --git a/app/services/sample_service.py b/app/services/sample_service.py index 06402e7..b78e54e 100644 --- a/app/services/sample_service.py +++ b/app/services/sample_service.py @@ -98,9 +98,12 @@ async def create_sample_demo_exercise( session.add(member) await session.commit() exercise = await transition_state(session, exercise, ExerciseState.active) - # Arm any release_at_minutes timers the sample defines (#269): activating via - # transition_state alone left them dormant until a restart or pause/resume. + # Enqueue any release_at_minutes schedules the sample defines (#269): activating via + # transition_state alone leaves them dormant until a resume or restart. Committed + # here and not left to a later caller: the release below is conditional, so without + # this the no-start-inject path would silently roll the jobs back at teardown. await schedule_exercise_injects(session, exercise) + await session.commit() # A multi-team start node seeds one physical inject per team; unordered .first() # could pick another team's copy, whose audience (only the demo member's group # is enrolled) is empty — and release_inject then 409s the whole demo load diff --git a/app/services/schedule_service.py b/app/services/schedule_service.py index 6e179f5..fd2c665 100644 --- a/app/services/schedule_service.py +++ b/app/services/schedule_service.py @@ -1,34 +1,41 @@ -"""Pause-aware, restart-safe exercise scheduling (#116, #194, #211, #218). +"""Pause-aware, durable exercise scheduling (#116, #194, #211, #218, #213). An inject may carry a ``release_offset_minutes`` — minutes after the exercise's effective -start at which it auto-releases. The registry also covers scenario -``triggers_communications`` (#211), so every pending timer is deferred on pause, cancelled -on completion, and rehydrated after restart. +start at which it auto-releases. Scenario ``triggers_communications`` work the same way, +measured from their inject's release. -A timer says *when* an inject may release; the progression cursor still says *whether* it -may. Three mechanisms compose to keep a schedule from silently evaporating when the room +Both are **jobs in Postgres**, enqueued inside the transaction that makes them due (see +``task_queue``). That is what makes them survive a restart, and what lets more than one +replica run: before this, they were ``asyncio`` timers in a dict, so a deploy mid-exercise +silently dropped every pending communication (#211). + +A schedule says *when* an inject may release; the progression cursor still says *whether* +it may. Two mechanisms compose to keep a schedule from silently evaporating when the room runs slow (#218): -* **the clock** — ``schedule_exercise_injects`` arms *every* scheduled inject on start, - resume, and restart, whether or not a cursor has reached it yet. -* **not yet** — a timer that comes due on a node no cursor points at is *deferred*: - ``_release_when_due`` checks the gate itself and returns without releasing. It is a - one-shot task, so it ends there. -* **the gate opened** — ``arm_cursor_reached_injects`` brings that timer back the moment a - response advances a cursor onto its node, at delay 0 if the offset has already elapsed, - so an overdue inject releases at once rather than never. - -Single-process only: the registry is in-memory, so a multi-process deployment would need a -task queue (Celery/ARQ) — see the single-replica note in CLAUDE.md. Startup rehydration -(``app/main.py``) re-arms schedules for active exercises after a single-process restart; it -does not survive across replicas. +* **the clock** — ``schedule_exercise_injects`` enqueues *every* scheduled inject on start + and resume, whether or not a cursor has reached it yet. +* **the gate opened** — ``arm_cursor_reached_injects`` enqueues a fresh job the moment a + response advances a cursor onto an inject's node, in that response's own transaction. A + job that ran too early simply found the gate shut and stopped; this is what brings it + back, at delay 0 if the offset has already elapsed, so an overdue inject releases at + once rather than never. + +There is deliberately no cancellation. Pausing, completing, or releasing early leaves the +job in place; it fires, re-reads durable state, and no-ops (or re-defers itself if a pause +moved its deadline). Duplicate and stale jobs are therefore expected and cheap — the CAS +release and the ``(exercise_id, trigger_key)`` unique insert already make them no-ops. +Losing a job is the only outcome that would actually hurt, and the transactional enqueue +is what rules that out. """ -import asyncio +from __future__ import annotations + +import json import logging -from dataclasses import dataclass -from datetime import UTC, datetime +from datetime import datetime, timedelta +from sqlalchemy import text from sqlmodel import col, select from sqlmodel.ext.asyncio.session import AsyncSession @@ -40,36 +47,12 @@ ExerciseStateTransition, ) from app.models.inject import Inject, InjectState -from app.services import audit_service +from app.services import task_queue logger = logging.getLogger(__name__) -@dataclass -class _Timer: - """A pending release, plus the one bit of coordination re-arming needs. - - ``rearm_requested`` is set when a cursor advances onto this inject while its worker is - *already running*. The worker cannot trust its own gate query in that case — the query - may have read the cursor from before the response committed — so it re-arms instead of - dropping the only timer there is. See ``_consume_rearm_or_deregister``. - """ - - task: asyncio.Task - rearm_requested: bool = False - -# exercise_id -> {inject_id -> pending release timer}. Holds a strong reference so the -# task isn't GC'd (cf. background.spawn) *and* lets us cancel a specific timer. -_scheduled: dict[int, dict[int, _Timer]] = {} -_scheduled_comms: dict[int, dict[str, asyncio.Task]] = {} - -# Workers that have passed their sleep and entered the release/deliver critical section. -# A graceful shutdown (cancel_all_schedules) must leave these running so they finish their -# commit *and* dispatch atomically (#218) instead of being killed mid-release; a worker -# still sleeping is safe to cancel outright — rehydrate_schedules re-arms it next boot. -# The worker adds itself with no intervening await, and cancel_all_schedules reads the set -# without yielding, so the two never interleave and the check is race-free (#250). -_releasing: set[asyncio.Task] = set() +# ── The pause-aware clock ───────────────────────────────────────────────────── def _effective_elapsed_seconds(exercise: Exercise) -> float: @@ -80,150 +63,15 @@ def _effective_elapsed_seconds(exercise: Exercise) -> float: """ if exercise.started_at is None: return 0.0 - now = datetime.now(UTC) + now = task_queue.now() return (now - exercise.started_at).total_seconds() - exercise.accumulated_pause_seconds -def _forget(exercise_id: int, inject_id: int, task: asyncio.Task) -> None: - ex_tasks = _scheduled.get(exercise_id) - timer = ex_tasks.get(inject_id) if ex_tasks else None - # Only drop if this is still the registered task — a re-arm may have replaced it. - if ex_tasks and timer is not None and timer.task is task: - ex_tasks.pop(inject_id, None) - if not ex_tasks: - _scheduled.pop(exercise_id, None) - - -def _arm(exercise_id: int, inject_id: int, delay: float) -> None: - task = asyncio.ensure_future(_release_when_due(exercise_id, inject_id, delay)) - _scheduled.setdefault(exercise_id, {})[inject_id] = _Timer(task=task) - task.add_done_callback(lambda t: _forget(exercise_id, inject_id, t)) - - -def _consume_rearm_or_deregister(exercise_id: int, inject_id: int) -> bool: - """Settle, in one await-free step, whether a deferring worker must re-arm itself. - - The registry entry outlives the worker's decision to defer — it is only dropped by the - done-callback — so between "the gate said no" and "the task finished" there is a window - in which ``arm_cursor_reached_injects`` would see a live-looking timer and skip. If the - worker's gate query had read the cursor from *before* that response committed, both - sides stand down and the inject is stranded pending: the exact failure #218 exists to - fix, just narrower. - - Closing it needs no lock, only the absence of an ``await``. The event loop cannot - interleave the arm handler between the check and the write here, so exactly one of two - things is true: - - * the flag is already set — the arm handler got here first; take it and re-arm. - * it is not — deregister *now*, so an arm handler arriving later finds no timer to skip - and arms a fresh one itself. - """ - ex_tasks = _scheduled.get(exercise_id) - timer = ex_tasks.get(inject_id) if ex_tasks else None - if timer is None or timer.task is not asyncio.current_task(): - return False - if timer.rearm_requested: - timer.rearm_requested = False - return True - ex_tasks.pop(inject_id, None) # type: ignore[union-attr] - if not ex_tasks: - _scheduled.pop(exercise_id, None) - return False - - -def _forget_comm(exercise_id: int, trigger_key: str, task: asyncio.Task) -> None: - exercise_tasks = _scheduled_comms.get(exercise_id) - if exercise_tasks and exercise_tasks.get(trigger_key) is task: - exercise_tasks.pop(trigger_key, None) - if not exercise_tasks: - _scheduled_comms.pop(exercise_id, None) - - -def arm_triggered_communication( - *, - exercise_id: int, - inject_id: int, - direction: str, - external_entity: str, - subject: str, - body: str, - delay: float, - trigger_key: str, -) -> None: - """Arm one logical communication unless that key is already pending.""" - if trigger_key in _scheduled_comms.get(exercise_id, {}): - return - task = asyncio.ensure_future( - _communication_when_due( - exercise_id=exercise_id, - inject_id=inject_id, - direction=direction, - external_entity=external_entity, - subject=subject, - body=body, - delay=max(0.0, delay), - trigger_key=trigger_key, - ) - ) - _scheduled_comms.setdefault(exercise_id, {})[trigger_key] = task - task.add_done_callback(lambda done: _forget_comm(exercise_id, trigger_key, done)) - - -def cancel_inject_schedule(exercise_id: int, inject_id: int | None) -> None: - """Cancel one inject's pending release timer (release-early, cancel, or re-arm).""" - if inject_id is None: - return - ex_tasks = _scheduled.get(exercise_id) - if not ex_tasks: - return - timer = ex_tasks.pop(inject_id, None) - if not ex_tasks: - _scheduled.pop(exercise_id, None) - # Never cancel the worker that is itself calling this (via release_inject on fire). - if timer is not None and timer.task is not asyncio.current_task(): - timer.task.cancel() - - -def cancel_exercise_schedules(exercise_id: int) -> None: - """Cancel every pending exercise timer (pause defers, completion drops).""" - ex_tasks = _scheduled.pop(exercise_id, None) or {} - comm_tasks = _scheduled_comms.pop(exercise_id, None) or {} - current = asyncio.current_task() - tasks = [timer.task for timer in ex_tasks.values()] + list(comm_tasks.values()) - for task in tasks: - if task is not current: - task.cancel() - - -def cancel_all_schedules() -> list[asyncio.Task]: - """Tear down every armed timer for a graceful shutdown (#250). - - Sleeping (pre-release) workers are cancelled outright — they are restart-safe, since - rehydrate_schedules re-arms them on the next boot. A worker already past its sleep is - left running so it can finish its commit and dispatch atomically rather than be killed - mid-release (#218); the caller drains those with a bounded grace period. Returns every - worker task — the cancelled sleepers included — so the caller can await their settling - inside that same drain and no task is destroyed while still pending. - """ - current = asyncio.current_task() - tasks: list[asyncio.Task] = [] - for ex_tasks in list(_scheduled.values()): - for timer in list(ex_tasks.values()): - if timer.task is current: - continue - tasks.append(timer.task) - if timer.task not in _releasing: - timer.task.cancel() - for comm_tasks in list(_scheduled_comms.values()): - for task in list(comm_tasks.values()): - if task is current: - continue - tasks.append(task) - if task not in _releasing: - task.cancel() - _scheduled.clear() - _scheduled_comms.clear() - return tasks +def seconds_until_due(exercise: Exercise, inject: Inject) -> float: + """How much running time is left before this inject's offset comes up.""" + if inject.release_offset_minutes is None: + return 0.0 + return inject.release_offset_minutes * 60 - _effective_elapsed_seconds(exercise) def _active_elapsed_since( @@ -232,7 +80,7 @@ def _active_elapsed_since( transitions: list[ExerciseStateTransition], ) -> float: """Running seconds since release, excluding every persisted pause span.""" - now = datetime.now(UTC) + now = task_queue.now() end = min(exercise.ended_at or now, now) paused_seconds = 0.0 pause_started: datetime | None = None @@ -249,10 +97,147 @@ def _active_elapsed_since( return max(0.0, (end - released_at).total_seconds() - paused_seconds) -async def _schedule_exercise_communications( - session: AsyncSession, exercise: Exercise +async def active_seconds_since( + session: AsyncSession, exercise: Exercise, released_at: datetime +) -> float: + """``_active_elapsed_since`` with the transitions read for you.""" + transitions = list( + ( + await session.exec( + select(ExerciseStateTransition).where( + ExerciseStateTransition.exercise_id == exercise.id + ) + ) + ).all() + ) + return _active_elapsed_since(exercise, released_at, transitions) + + +# ── Enqueueing ──────────────────────────────────────────────────────────────── + + +async def _live_job_args(session: AsyncSession, task_name: str) -> list[dict]: + """The args of every job for ``task_name`` still waiting to run. + + Lets the bulk re-enqueue paths (resume, startup reconciliation) skip work that is + already queued, so restarts and pause cycles do not stack a duplicate job set each + time. ``todo`` only, deliberately: a ``doing`` job may have read its state *before* + the change that prompted this re-enqueue (a resume racing a job that fired during + the pause), so counting it as live could skip the only enqueue that was going to + happen. A duplicate against a mid-flight job is a cheap no-op; a skipped enqueue + against a job about to stand down is a stranded schedule. + + Skipping a stale ``todo`` job is safe because a stale deadline is only ever *early* + — pauses extend deadlines, never shorten them — and a job that fires early re-defers + itself with the recomputed remaining time. + """ + connection = await session.connection() + rows = ( + ( + await connection.execute( + text( + "SELECT args FROM procrastinate_jobs " + "WHERE task_name = :task AND status = 'todo'" + ), + {"task": task_name}, + ) + ) + .scalars() + .all() + ) + return [json.loads(args) if isinstance(args, str) else args for args in rows] + + +async def defer_inject_release( + session: AsyncSession, exercise: Exercise, inject: Inject, *, delay: float | None = None ) -> None: - """Reconstruct undelivered logical communications from durable scenario state.""" + """Enqueue one inject's release, on the caller's transaction.""" + if exercise.id is None or inject.id is None: + return + wait = seconds_until_due(exercise, inject) if delay is None else delay + await task_queue.defer_job( + session, + task_queue.TASK_RELEASE_INJECT, + args={"exercise_id": exercise.id, "inject_id": inject.id}, + scheduled_at=task_queue.now() + timedelta(seconds=max(0.0, wait)), + ) + + +async def defer_triggered_comm( + session: AsyncSession, + *, + exercise_id: int, + inject_id: int, + node_id: str, + trigger_index: int, + delay: float, +) -> None: + """Enqueue one scenario-triggered communication, on the caller's transaction.""" + await task_queue.defer_job( + session, + task_queue.TASK_DELIVER_COMM, + args={ + "exercise_id": exercise_id, + "inject_id": inject_id, + "node_id": node_id, + "trigger_index": trigger_index, + }, + scheduled_at=task_queue.now() + timedelta(seconds=max(0.0, delay)), + ) + + +async def arm_inject_schedule( + session: AsyncSession, exercise: Exercise, inject: Inject +) -> None: + """(Re)enqueue a single inject's release — used by runtime schedule edits.""" + if ( + exercise.state != ExerciseState.active + or exercise.started_at is None + or inject.state != InjectState.pending + or inject.release_offset_minutes is None + or inject.id is None + ): + return + await defer_inject_release(session, exercise, inject) + + +async def schedule_exercise_injects(session: AsyncSession, exercise: Exercise) -> None: + """Enqueue every pending scheduled inject and undelivered comm — start and resume. + + An inject that already has a live job is skipped, so pause cycles and restarts do + not stack duplicates. The live job's deadline may predate a pause, but stale is + only ever *early*, and an early-firing job re-defers itself with the recomputed + remaining time (see ``_live_job_args``). + """ + if exercise.state != ExerciseState.active or exercise.started_at is None or exercise.id is None: + return + live = { + (args.get("exercise_id"), args.get("inject_id")) + for args in await _live_job_args(session, task_queue.TASK_RELEASE_INJECT) + } + injects = ( + await session.exec( + select(Inject).where( + Inject.exercise_id == exercise.id, + Inject.state == InjectState.pending, + col(Inject.release_offset_minutes).is_not(None), + ) + ) + ).all() + for inject in injects: + if (exercise.id, inject.id) in live: + continue + await defer_inject_release(session, exercise, inject) + await schedule_exercise_communications(session, exercise) + + +async def schedule_exercise_communications(session: AsyncSession, exercise: Exercise) -> None: + """Enqueue the triggered comms that durable state says are still owed. + + Derived entirely from rows — released injects, the scenario definition, and the + ``trigger_key``s already persisted — so it is correct whether it runs on resume, on + reconciliation, or after a restart that lost nothing but memory. + """ if exercise.state != ExerciseState.active or exercise.id is None: return from app.services.scenario_service import definition_for_exercise @@ -284,6 +269,12 @@ async def _schedule_exercise_communications( ).all() if key is not None } + # Keyed on the logical trigger, not the physical inject: a multi-team node seeds one + # inject per team, and any of them satisfies the same delivery. + live = { + (args.get("exercise_id"), args.get("node_id"), args.get("trigger_index")) + for args in await _live_job_args(session, task_queue.TASK_DELIVER_COMM) + } transitions = list( ( await session.exec( @@ -305,64 +296,57 @@ async def _schedule_exercise_communications( if trigger_key in considered: continue considered.add(trigger_key) - arm_triggered_communication( + if (exercise.id, node.id, index) in live: + continue + await defer_triggered_comm( + session, exercise_id=exercise.id, inject_id=inject.id, - direction=trigger.direction, - external_entity=trigger.external_entity, - subject=trigger.subject, - body=trigger.body, + node_id=node.id, + trigger_index=index, delay=trigger.delay_after_release_seconds - elapsed, - trigger_key=trigger_key, ) -def arm_inject_schedule(exercise: Exercise, inject: Inject) -> None: - """(Re)arm a single inject's timer against a running exercise — used by runtime edits.""" - if ( - exercise.state != ExerciseState.active - or exercise.started_at is None - or inject.state != InjectState.pending - or inject.release_offset_minutes is None - or inject.id is None - ): - return - assert exercise.id is not None - cancel_inject_schedule(exercise.id, inject.id) - delay = inject.release_offset_minutes * 60 - _effective_elapsed_seconds(exercise) - _arm(exercise.id, inject.id, max(0.0, delay)) +async def schedule_release_triggered_comms( + session: AsyncSession, exercise_id: int, inject: Inject +) -> None: + """Enqueue the comms a just-released inject triggers, in the release's transaction. + This used to be a post-commit subscriber, which meant a release could commit and the + process die before its comms were armed — #211 exactly. Enqueued here it is part of + the same unit of work as the release itself. + """ + from app.services.scenario_service import definition_for_exercise, get_inject_node -async def schedule_exercise_injects(session: AsyncSession, exercise: Exercise) -> None: - """Arm persisted inject and communication timers on start, resume, or restart.""" - if exercise.state != ExerciseState.active or exercise.started_at is None or exercise.id is None: + if not inject.scenario_node_id or inject.id is None: return - elapsed = _effective_elapsed_seconds(exercise) - injects = ( - await session.exec( - select(Inject).where( - Inject.exercise_id == exercise.id, - Inject.state == InjectState.pending, - col(Inject.release_offset_minutes).is_not(None), - ) + definition = await definition_for_exercise(session, exercise_id) + if definition is None: + return + node = get_inject_node(definition, inject.scenario_node_id) + if node is None: + return + for index, trigger in enumerate(node.triggers_communications): + await defer_triggered_comm( + session, + exercise_id=exercise_id, + inject_id=inject.id, + node_id=node.id, + trigger_index=index, + delay=trigger.delay_after_release_seconds, ) - ).all() - for inject in injects: - assert inject.id is not None and inject.release_offset_minutes is not None - cancel_inject_schedule(exercise.id, inject.id) # idempotent re-arm - delay = inject.release_offset_minutes * 60 - elapsed - _arm(exercise.id, inject.id, max(0.0, delay)) - await _schedule_exercise_communications(session, exercise) async def arm_cursor_reached_injects(session: AsyncSession, exercise: Exercise) -> None: - """Arm scheduled injects a progression cursor now points at (#218). + """Enqueue scheduled injects a progression cursor now points at (#218). - A timer is one-shot, so one that came due before the team reached its node was - deferred and dropped (``_release_when_due``). This brings it back the moment a - response advances a cursor onto that node — ``arm_inject_schedule`` computes the delay - from the offset's absolute basis, so an inject whose offset has already elapsed arms at - 0 and releases immediately rather than never. + A job that came due before the team reached its node found the gate shut and + stopped. This brings it back the moment a response advances a cursor onto that node + — and because the enqueue rides the response's own transaction, the window the old + in-memory version had to coordinate around (worker deciding to stand down while a + response commits) cannot exist: either the response committed and the job is there, + or neither happened. Matches on ``current_node_id`` because that is what ``release_is_allowed`` matches on; the two are the same predicate seen from either end and have to stay in lockstep. @@ -391,153 +375,29 @@ async def arm_cursor_reached_injects(session: AsyncSession, exercise: Exercise) ) ) ).all() - armed = _scheduled.get(exercise.id, {}) for inject in injects: - timer = armed.get(inject.id) if inject.id is not None else None - if timer is not None: - # Never cancel a live timer to replace it. arm_inject_schedule cancels before it - # arms, so a response landing while this inject's worker is mid-release would - # kill it between its commit and its dispatch — an inject released with no frame - # and no triggered comms. Its deadline cannot have moved, so there is nothing to - # replace anyway; the worker only needs to know a cursor reached it, in case its - # own gate query answered from before this response committed. - timer.rearm_requested = True - continue - arm_inject_schedule(exercise, inject) + await defer_inject_release(session, exercise, inject) -async def rehydrate_schedules() -> None: - """Re-arm pending exercise timers for every active exercise on startup. +async def reconcile_release_jobs() -> None: + """Re-enqueue schedules for active exercises on startup. - In-memory timers don't survive a process restart (cf. background.py). This re-derives - them from persisted state so a single-process restart mid-exercise doesn't silently - drop pending releases or communications. Multi-process deployments still need a - task queue. + Jobs are durable, so unlike the rehydration this replaces, it is not load-bearing — + it is a safety net, and the cutover path for exercises that were already running + when this version was deployed and whose schedules only ever lived in the previous + process's memory. """ - from app.database import engine - - async with AsyncSession(engine, expire_on_commit=False) as session: + async with AsyncSession(engine_for_session(), expire_on_commit=False) as session: exercises = ( await session.exec(select(Exercise).where(Exercise.state == ExerciseState.active)) ).all() for exercise in exercises: await schedule_exercise_injects(session, exercise) + await session.commit() -async def _release_when_due(exercise_id: int, inject_id: int, delay: float) -> None: - """Sleep, then release the inject through the normal path (WS + triggered comms).""" - try: - if delay > 0: - await asyncio.sleep(delay) - - # Past the sleep: mark this worker in-release so a shutdown cancel-all lets it - # finish its commit and dispatch rather than kill it mid-release (#218, #250). Set - # with no await before it so the read in cancel_all_schedules cannot interleave. - current = asyncio.current_task() - if current is not None: - _releasing.add(current) - try: - from app.database import engine - from app.services.inject_service import release_inject - from app.services.progression_service import release_is_allowed - - async with AsyncSession(engine, expire_on_commit=False) as session: - inject = await session.get(Inject, inject_id) - exercise = await session.get(Exercise, exercise_id) - # Guard the pause/cancel/manual-release race: only fire if still pending, - # still active, and still scheduled. - if ( - inject is None - or exercise is None - or inject.state != InjectState.pending - or exercise.state != ExerciseState.active - or inject.release_offset_minutes is None - ): - return - # Not a failure, so not an exception: the team simply has not reached this - # node yet. Checking the gate here rather than letting release_inject raise - # keeps the case distinguishable from a real one, and - # arm_cursor_reached_injects brings the timer back when a response advances - # a cursor onto the node — at delay 0 if the offset has already passed (#218). - if not await release_is_allowed(session, inject, scheduled=True): - # No await between the gate's answer and this: a response committing in - # that gap would otherwise find this timer still registered, skip it, and - # leave the inject with no timer at all once we drop out (see docstring). - if _consume_rearm_or_deregister(exercise_id, inject_id): - arm_inject_schedule(exercise, inject) - return - logger.info( - "Scheduled release deferred for inject %d: no cursor has reached it", - inject_id, - ) - audit_service.emit( - "inject.release_deferred", - actor=None, - target_type="inject", - target_id=inject_id, - reason="cursor_not_reached", - ) - return - await release_inject(session, inject, released_by=None, scheduled=True) - audit_service.emit( - "inject.release", - actor=None, - target_type="inject", - target_id=inject_id, - reason="scheduled", - ) - finally: - if current is not None: - _releasing.discard(current) - except asyncio.CancelledError: - raise - except Exception: - logger.exception("Scheduled release failed for inject %d", inject_id) - +def engine_for_session(): + """Resolve the engine at call time — the test suite rebinds it after import.""" + from app.database import engine -async def _communication_when_due( - *, - exercise_id: int, - inject_id: int, - direction: str, - external_entity: str, - subject: str, - body: str, - delay: float, - trigger_key: str, -) -> None: - """Deliver through the durable idempotent insert only while exercise is active.""" - try: - if delay > 0: - await asyncio.sleep(delay) - # Past the sleep: mark in-release so a shutdown cancel-all lets the idempotent - # delivery commit rather than kill it mid-write (#218, #250). Set with no await - # before it so the read in cancel_all_schedules cannot interleave. - current = asyncio.current_task() - if current is not None: - _releasing.add(current) - try: - from app.database import engine - from app.services.communication_service import deliver_triggered_communication - - async with AsyncSession(engine, expire_on_commit=False) as session: - exercise = await session.get(Exercise, exercise_id) - if exercise is None or exercise.state != ExerciseState.active: - return - await deliver_triggered_communication( - session, - exercise_id=exercise_id, - inject_id=inject_id, - direction=direction, - external_entity=external_entity, - subject=subject, - body=body, - trigger_key=trigger_key, - ) - finally: - if current is not None: - _releasing.discard(current) - except asyncio.CancelledError: - raise - except Exception: - logger.exception("Scheduled communication failed for trigger %s", trigger_key) + return engine diff --git a/app/services/siem_service.py b/app/services/siem_service.py index 7e34eee..feea360 100644 --- a/app/services/siem_service.py +++ b/app/services/siem_service.py @@ -15,8 +15,9 @@ Every sink is wrapped so a failing/unreachable SIEM is logged locally but **never** raises — auditing must not break the request that triggered it. Routing is read from an in-memory snapshot (``get_config``/``set_config``) refreshed at startup and -whenever an admin saves, so the sync ``emit`` path never does a per-event DB read -(single-process, like ``ws_manager``/``rate_limit``). +whenever an admin saves, so the sync ``emit`` path never does a per-event DB read. The +snapshot is per-process; a save on another replica reaches this one over the config bus +(``config_sync``, #213). """ import asyncio diff --git a/app/services/task_queue.py b/app/services/task_queue.py new file mode 100644 index 0000000..5883e73 --- /dev/null +++ b/app/services/task_queue.py @@ -0,0 +1,424 @@ +"""The durable task queue (#213). + +Everything this app schedules for later — an inject's timed release, a scenario's +triggered communications, an LLM pipeline, the nightly audit purge — used to be an +``asyncio`` task holding its state in a dict. That is why a restart mid-exercise lost +pending work (#211) and why the app could not run more than one replica. + +Jobs now live in Postgres, via procrastinate. The reason it is procrastinate and not +Celery or ARQ is **transactional enqueue**: the job row is INSERTed by +``procrastinate_defer_jobs_v1`` on *the caller's own session*, inside the caller's +transaction, and procrastinate's insert trigger raises its ``pg_notify`` — which +Postgres holds until COMMIT. So the job exists if and only if the state change that +warranted it committed. With a Redis-backed queue that is two stores and no shared +transaction, and "committed but never enqueued" comes back as permanent architecture. + +Two consequences shape every task below: + +* **Guards, not cancellation.** Nothing cancels a job when an exercise pauses or an + inject is released early. Each task re-derives from durable state whether its work + still applies and no-ops if not, which the existing CAS release and the + ``(exercise_id, trigger_key)`` unique insert already make safe. A duplicate or stale + job is therefore cheap and correct, and losing one is the only real failure — which + is the trade the transactional enqueue is designed around. + +* **Re-defer, don't sleep.** A task that finds itself early (a pause moved its deadline + out) re-defers itself for the remaining time rather than sleeping. A worker holding a + connection for two hours is exactly the model this replaces. + +The worker runs in-process on every replica, so a deployment is still one container. +""" + +from __future__ import annotations + +import asyncio +import contextlib +import json +import logging +from datetime import UTC, datetime +from typing import Any + +from procrastinate import App, PsycopgConnector +from sqlalchemy import text +from sqlalchemy.exc import IntegrityError +from sqlmodel.ext.asyncio.session import AsyncSession + +logger = logging.getLogger(__name__) + + +def _dsn() -> str: + from app.config import settings + from app.database import make_asyncpg_dsn + + return make_asyncpg_dsn(settings.database_url) + + +queue_app = App(connector=PsycopgConnector(conninfo=_dsn())) +"""Task registry and worker host. Constructing it opens nothing — the pool is lazy.""" + + +# Task names are declared, never derived, so a rename cannot orphan the jobs a +# previous release already wrote into the table. +TASK_RELEASE_INJECT = "inject.release_scheduled" +TASK_DELIVER_COMM = "comm.deliver_triggered" +TASK_ASSESS_RESPONSE = "llm.assess_response" +TASK_GENERATE_SUMMARY = "llm.generate_summary" + +QUEUE_DEFAULT = "default" + +# How early a task may run and still count as due. Postgres timestamps and the worker's +# poll interval do not align perfectly, and re-deferring by a fraction of a second would +# just burn a round trip. +DUE_EPSILON_SECONDS = 1.0 + + +# ── Enqueueing ──────────────────────────────────────────────────────────────── + +_DEFER_SQL = text( + """ + SELECT procrastinate_defer_jobs_v1( + ARRAY[ + ROW( + CAST(:queue_name AS character varying), + CAST(:task_name AS character varying), + CAST(:priority AS integer), + CAST(:lock AS text), + CAST(:queueing_lock AS text), + CAST(:args AS jsonb), + CAST(:scheduled_at AS timestamptz) + ) + ]::procrastinate_job_to_defer_v1[] + ) + """ +) + + +async def defer_job( + session: AsyncSession, + task_name: str, + *, + args: dict[str, Any] | None = None, + scheduled_at: datetime | None = None, + queue: str = QUEUE_DEFAULT, + priority: int = 0, + lock: str | None = None, + queueing_lock: str | None = None, +) -> None: + """Enqueue a job on the caller's transaction. + + Nothing is dispatched here. The row commits with everything else in the unit of + work, and the worker is woken by a NOTIFY that Postgres releases at the same moment. + Call this *before* the commit that makes the job's premise true. + """ + connection = await session.connection() + await connection.execute( + _DEFER_SQL, + { + "queue_name": queue, + "task_name": task_name, + "priority": priority, + "lock": lock, + "queueing_lock": queueing_lock, + "args": json.dumps(args or {}), + # A datetime, not an ISO string: the CAST tells asyncpg to expect timestamptz + # and it refuses to coerce text into one. + "scheduled_at": scheduled_at, + }, + ) + + +async def defer_job_once( + session: AsyncSession, task_name: str, *, queueing_lock: str, **kwargs: Any +) -> bool: + """Enqueue unless an identical job is already waiting. True if this call created it. + + The cross-replica replacement for an in-process "is it already running?" set: the + queueing lock is a partial unique index over jobs still to do, so the database + arbitrates rather than one process's memory. The savepoint is what keeps a losing + race from poisoning the caller's transaction — a raw unique violation would abort + everything the request had done so far. + """ + try: + async with session.begin_nested(): + await defer_job(session, task_name, queueing_lock=queueing_lock, **kwargs) + except IntegrityError: + return False + return True + + +# ── Tasks ───────────────────────────────────────────────────────────────────── + + +@queue_app.task(name=TASK_RELEASE_INJECT) +async def release_scheduled_inject(*, exercise_id: int, inject_id: int) -> None: + """Release an inject whose offset has come up. + + Every precondition is re-read here rather than trusted from enqueue time, because + the job may be a duplicate, may predate a pause, or may have been written by a + replica that has since died. + """ + from app.models.exercise import Exercise, ExerciseState + from app.models.inject import Inject, InjectState + from app.services import audit_service, schedule_service + from app.services.inject_service import release_inject + from app.services.progression_service import release_is_allowed + + async with AsyncSession(engine_for_session(), expire_on_commit=False) as session: + inject = await session.get(Inject, inject_id) + exercise = await session.get(Exercise, exercise_id) + if inject is None or exercise is None: + return + if ( + inject.state != InjectState.pending + or exercise.state != ExerciseState.active + or inject.release_offset_minutes is None + ): + # Released early, cancelled, paused, or finished. A paused exercise re-defers + # everything on resume, so there is nothing to reschedule here. + return + + remaining = schedule_service.seconds_until_due(exercise, inject) + if remaining > DUE_EPSILON_SECONDS: + # A pause pushed the deadline out after this job was written. Come back then. + await schedule_service.defer_inject_release(session, exercise, inject) + await session.commit() + return + + if not await release_is_allowed(session, inject, scheduled=True): + # Not a failure: the team simply has not reached this node. The response that + # advances a cursor onto it enqueues a fresh job in its own transaction, so + # unlike the old in-memory timer there is no race to lose here (#218). + logger.info( + "Scheduled release deferred for inject %d: no cursor has reached it", inject_id + ) + audit_service.emit( + "inject.release_deferred", + actor=None, + target_type="inject", + target_id=inject_id, + reason="cursor_not_reached", + ) + return + + await release_inject(session, inject, released_by=None, scheduled=True) + audit_service.emit( + "inject.release", + actor=None, + target_type="inject", + target_id=inject_id, + reason="scheduled", + ) + + +@queue_app.task(name=TASK_DELIVER_COMM) +async def deliver_triggered_comm( + *, exercise_id: int, inject_id: int, node_id: str, trigger_index: int +) -> None: + """Deliver one scenario-triggered communication. + + The message content is re-read from the scenario rather than carried in the job + args: a job written before a scenario edit would otherwise deliver text the + facilitator has since changed. + """ + from app.models.exercise import Exercise, ExerciseState + from app.models.inject import Inject + from app.services import schedule_service + from app.services.communication_service import deliver_triggered_communication + from app.services.scenario_service import definition_for_exercise, get_inject_node + + async with AsyncSession(engine_for_session(), expire_on_commit=False) as session: + exercise = await session.get(Exercise, exercise_id) + inject = await session.get(Inject, inject_id) + if exercise is None or inject is None or exercise.state != ExerciseState.active: + return + if inject.released_at is None: + return + definition = await definition_for_exercise(session, exercise_id) + if definition is None: + return + node = get_inject_node(definition, node_id) + if node is None or trigger_index >= len(node.triggers_communications): + return + trigger = node.triggers_communications[trigger_index] + + elapsed = await schedule_service.active_seconds_since( + session, exercise, inject.released_at + ) + remaining = trigger.delay_after_release_seconds - elapsed + if remaining > DUE_EPSILON_SECONDS: + await schedule_service.defer_triggered_comm( + session, + exercise_id=exercise_id, + inject_id=inject_id, + node_id=node_id, + trigger_index=trigger_index, + delay=remaining, + ) + await session.commit() + return + + await deliver_triggered_communication( + session, + exercise_id=exercise_id, + inject_id=inject_id, + direction=trigger.direction, + external_entity=trigger.external_entity, + subject=trigger.subject, + body=trigger.body, + trigger_key=f"{node_id}:{trigger_index}", + ) + + +@queue_app.task(name=TASK_ASSESS_RESPONSE) +async def assess_response(*, response_id: int, inject_id: int, exercise_id: int) -> None: + from app.services.llm_service import run_llm_pipeline + + await run_llm_pipeline(response_id, inject_id, exercise_id) + + +@queue_app.task(name=TASK_GENERATE_SUMMARY) +async def generate_summary(*, exercise_id: int) -> None: + from app.services.llm_service import run_summary_pipeline + + await run_summary_pipeline(exercise_id) + + +# ── Periodic maintenance ────────────────────────────────────────────────────── +# +# Cross-replica single-fire is procrastinate's, not ours: a periodic defer is unique on +# (task, periodic_id, timestamp), so whichever replica gets there first wins and the +# rest lose the insert. That is what makes it safe to run a worker in every replica — +# before this, a second replica meant a second concurrent audit purge. + + +@queue_app.periodic(cron="17 3 * * *") +@queue_app.task(name="maintenance.retention_sweep") +async def retention_sweep(timestamp: int) -> None: + from app.services.retention_service import sweep_once + + await sweep_once() + + +@queue_app.periodic(cron="43 4 * * *") +@queue_app.task(name="maintenance.remove_old_jobs") +async def remove_old_jobs(timestamp: int) -> None: + """Keep the job table from growing without bound. + + Failed jobs are kept: they are the record of work this app promised and did not do, + which is worth more than the rows cost. + """ + await queue_app.job_manager.delete_old_jobs(nb_hours=24 * 7) + + +@queue_app.periodic(cron="29 * * * *") +@queue_app.task(name="maintenance.rate_limit_sweep") +async def rate_limit_sweep(timestamp: int) -> None: + """Delete throttle counters past their window. + + Hourly and centralised, because the keys are attacker-controlled: the previous + in-process version swept on a read path, so rows for a key nobody looked at again + stayed forever. + """ + from app.services.rate_limit import sweep + + removed = await sweep() + if removed: + logger.info("swept %d expired rate-limit hits", removed) + + +@queue_app.periodic(cron="*/10 * * * *") +@queue_app.task(name="maintenance.retry_stalled_jobs") +async def retry_stalled_jobs(timestamp: int) -> None: + """Return jobs orphaned by a killed worker to the queue. + + A replica SIGKILLed mid-job (an OOM, a node eviction) leaves its job marked *doing* + with nobody working it. Retrying is safe precisely because every task above is a + guarded, idempotent no-op when its work has already happened. + """ + stalled = await queue_app.job_manager.get_stalled_jobs(seconds_since_heartbeat=120) + for job in stalled: + logger.warning("retrying stalled job %s (%s)", job.id, job.task_name) + await queue_app.job_manager.retry_job(job) + + +# ── Wiring ──────────────────────────────────────────────────────────────────── + + +def engine_for_session() -> Any: + """Resolve the engine at call time — the test suite rebinds it after import.""" + from app.database import engine + + return engine + + +def schema_sql() -> str: + """procrastinate's own DDL, for the Alembic revision that installs it.""" + from procrastinate.schema import SchemaManager + + return SchemaManager.get_schema() + + +def apply_schema(dsn: str | None = None) -> None: + """Install the schema over a synchronous connection, if it is not already there. + + Used by the test harness, which builds its schema from the models and so never runs + Alembic. Idempotent because the two installers can meet: the UI suite runs against a + server whose startup migrations already installed the schema, and procrastinate's + DDL — unlike ``create_all`` — does not skip objects that exist. Synchronous because + that DDL is a multi-statement script, which asyncpg's prepared-statement path cannot + execute. + """ + from procrastinate import SyncPsycopgConnector + from procrastinate.schema import SchemaManager + + connector = SyncPsycopgConnector(conninfo=dsn or _dsn()) + connector.open() + try: + rows = connector.execute_query_all( + query="SELECT to_regclass('procrastinate_jobs') IS NOT NULL AS present" + ) + if rows and rows[0]["present"]: + return + SchemaManager(connector=connector).apply_schema() + finally: + connector.close() + + +_worker_task: asyncio.Task | None = None + + +async def start_worker() -> None: + """Open the pool and run a worker for this replica.""" + global _worker_task + if _worker_task is not None: + return + await queue_app.open_async() + _worker_task = asyncio.create_task( + queue_app.run_worker_async( + install_signal_handlers=False, # the lifespan owns shutdown + listen_notify=True, + concurrency=4, + ) + ) + logger.info("task queue worker started") + + +async def stop_worker(timeout: float = 10.0) -> None: + """Stop the worker, giving running jobs a bounded chance to finish. + + A job killed here is not lost — it stays *doing* until ``retry_stalled_jobs`` picks + it up, and every task is safe to run twice. + """ + global _worker_task + task, _worker_task = _worker_task, None + if task is not None: + task.cancel() + with contextlib.suppress(asyncio.CancelledError, TimeoutError): + await asyncio.wait_for(task, timeout=timeout) + with contextlib.suppress(Exception): + await queue_app.close_async() + + +def now() -> datetime: + """Indirection so tests can freeze the clock the schedulers compute against.""" + return datetime.now(UTC) diff --git a/app/services/ws_projector.py b/app/services/ws_projector.py index 2124028..4688d2e 100644 --- a/app/services/ws_projector.py +++ b/app/services/ws_projector.py @@ -6,16 +6,24 @@ That single choke point is the point. Before this, nine call sites across five services each reached into ``ws_manager`` directly, so "who is this frame for?" was a question you -answered by grepping. It also means multi-replica (#213) has exactly one seam to replace: -a Redis-backed fan-out swaps ``manager`` here, and no service changes. +answered by grepping. It is also what made multi-replica (#213) a change of one seam: +these handlers run identically whether the event was recorded by this replica or relayed +from another over the Postgres bus, because they are given ids and read the rows +themselves. ``ws_manager`` stays per-replica — it holds this process's actual sockets. The frame envelope is uniform — ``{type, exercise_id, timestamp, payload}`` — and the payload dicts are built through the ``schemas/api`` models wherever one exists, so the HTTP and WebSocket representations of the same thing cannot drift (#21, #31). + +Every handler tolerates a missing row. Between the commit and the projection — and +especially across the bus, where the read happens on another machine — the row can have +been deleted, and a retention sweep or a cascading delete must not fill the log with +tracebacks. """ from __future__ import annotations +import logging from datetime import UTC, datetime from typing import Any @@ -36,6 +44,8 @@ ) from app.services.ws_manager import manager +logger = logging.getLogger(__name__) + def _frame(kind: str, exercise_id: int, payload: Any, timestamp: str | None = None) -> dict: return { @@ -46,6 +56,10 @@ def _frame(kind: str, exercise_id: int, payload: Any, timestamp: str | None = No } +def _gone(what: str, ident: int) -> None: + logger.debug("skipping projection: %s %s no longer exists", what, ident) + + @subscribe(ExerciseStateChanged) async def on_exercise_state_changed(session: AsyncSession, ev: ExerciseStateChanged) -> None: """The canonical lifecycle frame (#129). @@ -54,14 +68,16 @@ async def on_exercise_state_changed(session: AsyncSession, ev: ExerciseStateChan per-second traffic — the clock ticks locally. The timestamp is the *durable* event's, not the moment we happened to broadcast. """ - from app.models.exercise import Exercise + from app.models.exercise import Exercise, ExerciseStateTransition - transition = ev.transition - assert transition.id is not None + transition = await session.get(ExerciseStateTransition, ev.transition_id) + if transition is None: + return _gone("transition", ev.transition_id) exercise = await session.get(Exercise, ev.exercise_id) - assert exercise is not None + if exercise is None: + return _gone("exercise", ev.exercise_id) payload = ExerciseStateChange( - transition_id=transition.id, + transition_id=ev.transition_id, exercise_id=ev.exercise_id, previous_state=transition.from_state, new_state=transition.to_state, @@ -86,17 +102,21 @@ async def on_exercise_state_changed(session: AsyncSession, ev: ExerciseStateChan @subscribe(InjectReleased) async def on_inject_released(session: AsyncSession, ev: InjectReleased) -> None: + from app.models.inject import Inject from app.services.inject_service import inject_payload, inject_target_groups + inject = await session.get(Inject, ev.inject_id) + if inject is None: + return _gone("inject", ev.inject_id) # Participants/observers get the redacted payload; facilitators get the full one # with branch topology (#266). The manager routes the two frames by role in one pass. - base = _frame("inject_released", ev.exercise_id, await inject_payload(session, ev.inject)) + base = _frame("inject_released", ev.exercise_id, await inject_payload(session, inject)) full = _frame( "inject_released", ev.exercise_id, - await inject_payload(session, ev.inject, include_progression=True), + await inject_payload(session, inject, include_progression=True), ) - groups = inject_target_groups(ev.inject) + groups = inject_target_groups(inject) if groups: await manager.broadcast_to_groups( ev.exercise_id, groups, base, facilitator_message=full @@ -109,23 +129,34 @@ async def on_inject_released(session: AsyncSession, ev: InjectReleased) -> None: async def on_inject_updated(session: AsyncSession, ev: InjectUpdated) -> None: """Facilitator-only: participants see *released* injects, so a pending inject's schedule edit is irrelevant to them and stays off their socket.""" + from app.models.inject import Inject from app.services.inject_service import inject_payload + inject = await session.get(Inject, ev.inject_id) + if inject is None: + return _gone("inject", ev.inject_id) await manager.send_to_facilitators( ev.exercise_id, _frame( "inject_updated", ev.exercise_id, - await inject_payload(session, ev.inject, include_progression=True), + await inject_payload(session, inject, include_progression=True), ), ) @subscribe(InjectCommentCreated) async def on_inject_comment_created(session: AsyncSession, ev: InjectCommentCreated) -> None: - frame = _frame("inject_comment_created", ev.exercise_id, ev.payload) - if ev.comment.group_id: - await manager.broadcast_to_groups(ev.exercise_id, [ev.comment.group_id], frame) + from app.models.inject_comment import InjectComment + from app.services.inject_comment_service import comment_payload + + comment = await session.get(InjectComment, ev.comment_id) + if comment is None: + return _gone("comment", ev.comment_id) + payload = await comment_payload(session, comment) + frame = _frame("inject_comment_created", ev.exercise_id, payload) + if comment.group_id: + await manager.broadcast_to_groups(ev.exercise_id, [comment.group_id], frame) else: await manager.broadcast_to_exercise(ev.exercise_id, frame) @@ -139,6 +170,7 @@ async def on_response_submitted(session: AsyncSession, ev: ResponseSubmitted) -> cursor can only be read once the response is durable. They were computed post-commit before this seam too, so the payload is unchanged. """ + from app.models.response import Response from app.services.progression_service import progression_snapshot from app.services.response_service import ( compute_next_inject_ids, @@ -146,7 +178,9 @@ async def on_response_submitted(session: AsyncSession, ev: ResponseSubmitted) -> response_payload, ) - r = ev.response + r = await session.get(Response, ev.response_id) + if r is None: + return _gone("response", ev.response_id) next_ids = await compute_next_inject_ids( session, ev.exercise_id, r.inject_id, r.selected_option ) @@ -180,8 +214,8 @@ async def on_communication_created(session: AsyncSession, ev: CommunicationCreat from app.services.communication_service import comm_payload comm = await session.get(Communication, ev.communication_id) - if comm is None: # deleted between commit and dispatch - return + if comm is None: + return _gone("communication", ev.communication_id) # comm_payload is called WITHOUT a session, and the fan-out reads the raw # visible_to_teams column rather than the inbound-expanding helper — both exactly as # the inline broadcast did. Handing it the live session here would silently start @@ -203,71 +237,52 @@ async def on_communication_created(session: AsyncSession, ev: CommunicationCreat @subscribe(ResponseAssessed) async def on_response_assessed(session: AsyncSession, ev: ResponseAssessed) -> None: + from app.models.assessment import ResponseAssessment + from app.services.llm_service import assessment_payload + + assessment = await session.get(ResponseAssessment, ev.assessment_id) + if assessment is None: + return _gone("assessment", ev.assessment_id) await manager.send_to_facilitators( ev.exercise_id, _frame( "assessment_ready", ev.exercise_id, - {"response_id": ev.response_id, "assessment": ev.payload}, + {"response_id": ev.response_id, "assessment": assessment_payload(assessment)}, ), ) @subscribe(InjectSuggested) async def on_inject_suggested(session: AsyncSession, ev: InjectSuggested) -> None: + from app.models.suggested_inject import SuggestedInject from app.services.llm_service import suggested_payload + suggested = await session.get(SuggestedInject, ev.suggested_id) + if suggested is None: + return _gone("suggested inject", ev.suggested_id) await manager.send_to_facilitators( ev.exercise_id, - _frame("inject_suggested", ev.exercise_id, suggested_payload(ev.suggested)), + _frame("inject_suggested", ev.exercise_id, suggested_payload(suggested)), ) @subscribe(SummaryGenerated) async def on_summary_generated(session: AsyncSession, ev: SummaryGenerated) -> None: + from app.models.report_summary import ExecutiveSummary + from app.services.llm_service import summary_payload + + summary = await session.get(ExecutiveSummary, ev.summary_id) + if summary is None: + return _gone("summary", ev.summary_id) await manager.send_to_facilitators( - ev.exercise_id, _frame("summary_ready", ev.exercise_id, ev.payload) + ev.exercise_id, _frame("summary_ready", ev.exercise_id, summary_payload(summary)) ) -# ── Non-WebSocket subscribers ───────────────────────────────────────────────── - - -@subscribe(InjectReleased) -async def schedule_triggered_communications(session: AsyncSession, ev: InjectReleased) -> None: - """A second subscriber to the same event, and the reason this is a bus rather than a - broadcast helper: triggered comms are a post-commit consequence of a release in - exactly the way the frame is, and they used to be a hand-sequenced call inside - ``release_inject``. Now the release just announces itself.""" - from app.services.communication_service import schedule_triggered_comms - from app.services.scenario_service import definition_for_exercise, get_inject_node - - inject = ev.inject - if not inject.scenario_node_id: - return - definition = await definition_for_exercise(session, ev.exercise_id) - if not definition: - return - node = get_inject_node(definition, inject.scenario_node_id) - if node and node.triggers_communications: - schedule_triggered_comms(inject, node.triggers_communications, node.id) - - -@subscribe(ResponseSubmitted) -async def arm_schedules_the_response_unlocked(session: AsyncSession, ev: ResponseSubmitted) -> None: - """A response is the only thing that advances a progression cursor, and a cursor - advance is the only thing that unlocks a scheduled inject the team had not reached - (#218). So the re-arm belongs here, on the same post-commit seam as the frame: arming a - timer against a cursor advance that then rolled back would release an inject the - participants never chose. - - Registered after ``on_response_submitted`` — subscriber order is registration order, so - the ``response_submitted`` frame still lists the newly-armed inject as pending, and any - immediate ``inject_released`` follows it rather than racing it. - """ - from app.models.exercise import Exercise - from app.services.schedule_service import arm_cursor_reached_injects - - exercise = await session.get(Exercise, ev.exercise_id) - if exercise is not None: - await arm_cursor_reached_injects(session, exercise) +# Every subscriber here is a pure projection, and that is now a property worth keeping: +# each one renders a frame for whichever sockets its own replica holds, so running it +# everywhere is exactly right. Work that *acts* — arming a schedule, delivering a comm — +# moved into the transaction that occasions it (see ``schedule_service``), because +# running it once per replica would do it once per replica. If something ever has to act +# from here, register it with ``subscribe_local``. diff --git a/app/services/ws_relay.py b/app/services/ws_relay.py new file mode 100644 index 0000000..911b61d --- /dev/null +++ b/app/services/ws_relay.py @@ -0,0 +1,94 @@ +"""Project the domain events *other* replicas recorded (#213). + +The listener hands this module descriptors off the ``ttx_events`` channel; it rebuilds +the event and runs the same ``ws_projector`` handlers the recording replica ran, against +its own sockets. Nothing here knows what a frame looks like — that stays in one place. + +Descriptors this replica published are skipped. The recording replica has already +projected them in-band, at the end of the unit of work that produced them, through +``dispatch``. Relaying them back would double every local frame; more importantly, +keeping the local path in-band is what lets the savepoint-isolated test suite exercise +the real projection end to end, since a transaction that never truly commits can never +deliver a NOTIFY. + +So the two paths differ only in who invokes them: + + origin replica: commit -> dispatch(session) -> project(local=True) + peer replica: NOTIFY -> handle(descriptor) -> project(local=False) + +``local=False`` is what keeps handlers that *act* — arming a timer — from running once +per replica. See ``subscribe_local`` in ``domain_events``. +""" + +from __future__ import annotations + +import logging +from typing import Any + +from sqlmodel.ext.asyncio.session import AsyncSession + +from app.services import domain_events, pg_bus + +logger = logging.getLogger(__name__) + +EVENT_USER_CONNECTIONS_CLOSE = "user_connections_close" +"""Not a domain event: a direct instruction to drop one user's sockets. + +Authorization is per-connection state held by whichever replica terminates the socket, +so a role downgrade has to reach all of them (#25). The originating replica closes its +own connections inline, which is why this is skipped by origin like everything else. +""" + + +async def handle(descriptor: dict[str, Any]) -> None: + """Bus handler for ``ttx_events``.""" + if pg_bus.is_own(descriptor): + return + if descriptor.get("event") == EVENT_USER_CONNECTIONS_CLOSE: + await _close_user_connections(descriptor) + return + ev = domain_events.from_descriptor(descriptor) + if ev is None: + return + async with AsyncSession(engine_for_session()) as session: + await domain_events.project(session, ev, local=False) + + +async def _close_user_connections(descriptor: dict[str, Any]) -> None: + from app.services.ws_manager import manager + + user_id = descriptor.get("user_id") + if not isinstance(user_id, int): + logger.warning("user_connections_close without a user id: %r", descriptor) + return + await manager.close_user_connections(user_id) + + +async def publish_user_connections_close(session: AsyncSession, user_id: int) -> None: + """Tell every other replica to drop this user's sockets, if this commit lands.""" + await pg_bus.publish( + session, + pg_bus.CHANNEL_EVENTS, + pg_bus.envelope(event=EVENT_USER_CONNECTIONS_CLOSE, user_id=user_id), + ) + + +def engine_for_session() -> Any: + """Resolve the engine at call time — the test suite rebinds it after import.""" + from app.database import engine + + return engine + + +_installed = False + + +def install() -> None: + """Wire this module into the bus. Idempotent.""" + global _installed + if _installed: + return + _installed = True + from app.services.pg_listener import listener + + listener.register(pg_bus.CHANNEL_EVENTS, handle) diff --git a/docs/AGENT_ARCHITECTURE.md b/docs/AGENT_ARCHITECTURE.md index c469ab8..4fd6f58 100644 --- a/docs/AGENT_ARCHITECTURE.md +++ b/docs/AGENT_ARCHITECTURE.md @@ -27,7 +27,7 @@ API-first architecture. **Database (async, Postgres-only)**: PostgreSQL via the async `asyncpg` driver everywhere — dev, tests, and containers. `database.py` builds a single `create_async_engine` and exposes an `async def get_session()` yielding a SQLModel `AsyncSession` (`expire_on_commit=False`, so attributes stay populated after commit for response serialization). `make_async_url()` rewrites a plain `postgresql://` URL to `postgresql+asyncpg://`, so existing `DATABASE_URL` secrets in `docker-compose.yml`/`k8s` keep working unchanged. -**Schema migrations (Alembic, #19)**: Schema is managed by Alembic. `app/database.py` exposes `run_migrations()`, called from the async lifespan, which runs `alembic upgrade head` in a worker thread (`asyncio.to_thread` — Alembic's async `env.py` calls `asyncio.run`, which can't run inside the already-running lifespan loop). This self-migrates on startup, which suits the single-replica constraint; multi-replica rollouts should instead run `alembic upgrade head` as a dedicated deploy step. `alembic/env.py` derives the URL from `settings` via `make_async_url()` and uses `SQLModel.metadata` as the autogenerate target (importing every `app/models/*` module, same list as `app/main.py`). Create new migrations with `alembic revision --autogenerate -m "..."`; generated files under `alembic/versions/` are excluded from ruff. When hand-writing a migration instead (autogenerate needs a running Postgres), its `revision` id must be unique — the existing ids are similar rotated hex and a collision surfaces as a misleading `Cycle is detected in revisions` error, not a duplicate-id message; grep `alembic/versions/` for the id first. The test suite does **not** use Alembic — `tests/conftest.py` builds a throwaway schema with `metadata.create_all`, and `create_db_and_tables()` remains for that path. The `alembic.ini` + `alembic/` dir are copied into the Docker image so startup migration works in containers. +**Schema migrations (Alembic, #19)**: Schema is managed by Alembic. `app/database.py` exposes `run_migrations()`, called from the async lifespan, which runs `alembic upgrade head` in a worker thread (`asyncio.to_thread` — Alembic's async `env.py` calls `asyncio.run`, which can't run inside the already-running lifespan loop). This self-migrates on startup behind a Postgres **advisory lock** (`env.py`), so several replicas booting at once migrate one at a time and the losers find the schema already at head (#213); a migration must therefore be forward-compatible across one release, since the previous version serves against the new schema for the length of a rolling update. procrastinate's tables are owned by the library and installed verbatim by revision `c7d8e9f0a1b2`, so they are deliberately absent from `SQLModel.metadata` and filtered out of autogenerate by `include_name` — without that filter every autogenerate would propose dropping them and the `alembic check` that gates CI would fail on a correct database. `alembic/env.py` derives the URL from `settings` via `make_async_url()` and uses `SQLModel.metadata` as the autogenerate target (importing every `app/models/*` module, same list as `app/main.py`). Create new migrations with `alembic revision --autogenerate -m "..."`; generated files under `alembic/versions/` are excluded from ruff. When hand-writing a migration instead (autogenerate needs a running Postgres), its `revision` id must be unique — the existing ids are similar rotated hex and a collision surfaces as a misleading `Cycle is detected in revisions` error, not a duplicate-id message; grep `alembic/versions/` for the id first. The test suite does **not** use Alembic — `tests/conftest.py` builds a throwaway schema with `metadata.create_all`, and `create_db_and_tables()` remains for that path. The `alembic.ini` + `alembic/` dir are copied into the Docker image so startup migration works in containers. **Everything that touches the DB is async**: services and router handlers are `async def` and `await session.exec(...)/get/commit/refresh/delete` (`session.add` stays sync). Background workers that open their own session — `run_llm_pipeline` (`llm_service.py`), the inject/communication workers in `schedule_service.py`, and the retention sweep in `retention_service.py` — use `async with AsyncSession(engine)`. The `anthropic` client was already `AsyncAnthropic`. @@ -49,15 +49,15 @@ API-first architecture. **Layering: services own the queries, `schemas/` owns what crosses (#214)**: routers authorize, call, serialize — the data access lives in the service that owns the model (`user_service` owns `User`; nothing did, so "get the user with this email" was written out longhand eleven times). The exception, deliberately kept: the **list-exercises visibility** query in `exercises.py` is an authz projection, genuinely router-shaped, and forcing it into a service buys nothing. **Schema placement**: `app/schemas/` holds every model that *crosses a module boundary* — all response models (services build payloads through them, so the HTTP and WS shapes cannot drift, #21/#31) and any request body used by more than one module (which is why `schemas/auth.py` holds request bodies: `users.py` imports them too). A request body used by exactly **one** router stays with it. Do **not** "tidy" the settings-update models into `schemas/`: their validators bind to service constants (`proxy.ProxyMode`, `_ALLOWED_METHODS`), and ten service modules already import `app.schemas` — that edge is a cycle. -**Post-commit domain events (#212)**: services never broadcast inline. They `record(session, Event(...))` **inside** the transaction; a SQLAlchemy `after_commit` listener promotes buffered events to dispatchable and `after_soft_rollback` **discards** them (load-bearing — a request session is reused across units of work, so a stranded event would otherwise be swept up by the *next* commit and broadcast something that never happened). `await dispatch(session)` then fans out to `ws_projector`, the one module that builds every WS frame and the single choke point #213's Redis work will swap. So "emit only after commit" is a precondition gated by a callback only a real COMMIT fires, not a convention re-typed at nine call sites. **The rule: whoever commits the unit of work dispatches it** — not the caller, or every *other* caller (a fixture, the sample loader) strands a committed event. Dispatch while the session is still open (handlers read the DB), and record only on sessions with `expire_on_commit=False` (not `main.py`'s lifespan sessions). Handler failures are logged, never raised: the transaction is already authoritative and a dead socket must not 500 a committed request. An autouse conftest fixture fails any test that leaves an undispatched event. +**Post-commit domain events (#212)**: services never broadcast inline. They `record(session, Event(...))` **inside** the transaction; a SQLAlchemy `after_commit` listener promotes buffered events to dispatchable and `after_soft_rollback` **discards** them (load-bearing — a request session is reused across units of work, so a stranded event would otherwise be swept up by the *next* commit and broadcast something that never happened). `await dispatch(session)` then fans out to `ws_projector`, the one module that builds every WS frame — and, because it was a single choke point, the only seam #213 had to change to reach every replica. So "emit only after commit" is a precondition gated by a callback only a real COMMIT fires, not a convention re-typed at nine call sites. **The rule: whoever commits the unit of work dispatches it** — not the caller, or every *other* caller (a fixture, the sample loader) strands a committed event. Dispatch while the session is still open (handlers read the DB), and record only ids and scalars, never ORM instances — an ORM object cannot cross a process boundary, so a replicated event names its rows and each replica reads them back (`record` refuses an unset id, since a row nobody can read back is not announceable). Handler failures are logged, never raised: the transaction is already authoritative and a dead socket must not 500 a committed request. An autouse conftest fixture fails any test that leaves an undispatched event. **Publication is issued from `before_commit`** (#213): a `NOTIFY` inside the transaction is delivered by Postgres only on COMMIT and discarded on rollback, so a peer replica's copy of a frame is exactly as trustworthy as the local one. The origin replica still projects in-band through `dispatch` and skips its own descriptors, which keeps frame-building identical on both paths and lets the savepoint-isolated test suite exercise it end to end. `ws_relay` runs the same subscribers for a peer's events; `subscribe_local` marks any handler that *acts* rather than projects, so it cannot run once per replica. **Linear inject flows**: In addition to per-option `next_inject_id` branching, an `InjectNode` may set a node-level `next_inject_id` (in `scenario_json.py`) to chain to the next inject without requiring a participant decision — i.e. a straight-line sequence. The `ScenarioDefinition` validator checks the referenced ID exists, and `_check_no_cycles()` includes node-level `next_inject_id` edges in its adjacency graph so linear chains can't form a cycle. **JWT auth**: The browser holds the JWT **only** in an `httpOnly` cookie — never JS-readable storage (#264) — and authenticates page navigation, `apiFetch` calls, and the WebSocket upgrade with it. The `get_current_user` FastAPI dependency still also accepts an `Authorization: Bearer` header (which takes precedence) for non-browser API clients; browser code never sets one. Because the app's own fetch calls are now cookie-authenticated, their state-changing requests go through the `CSRFOriginMiddleware` `Origin`/`Referer` check (same-origin, so they pass) instead of the old Bearer exemption. -**Email / SMTP (#117, #186)**: Feature-flagged on cached `MailConfig.smtp_enabled` (`enabled` plus host and From address) — dependent endpoints 404 and their UI entry points hide when off. Non-secret values live in the `EmailSettings` singleton, seed once from `SMTP_*`, and are editable at `/admin/email`; `SMTP_PASSWORD` stays env-only and is read live per delivery. `mail_service.send` is a best-effort async `aiosmtplib` call fired via `background.spawn`; SMTP is a raw socket and goes **direct**, NOT through the httpx proxy (#97). Self-service password reset and participant invites use single-use, expiring, SHA-256-hashed `AuthToken` rows. `POST /api/email/test` can only send to the requesting admin and reports only an exception class. The cache is per-process, so the single-replica constraint applies. +**Email / SMTP (#117, #186)**: Feature-flagged on cached `MailConfig.smtp_enabled` (`enabled` plus host and From address) — dependent endpoints 404 and their UI entry points hide when off. Non-secret values live in the `EmailSettings` singleton, seed once from `SMTP_*`, and are editable at `/admin/email`; `SMTP_PASSWORD` stays env-only and is read live per delivery. `mail_service.send` is a best-effort async `aiosmtplib` call fired via `background.spawn`; SMTP is a raw socket and goes **direct**, NOT through the httpx proxy (#97). Self-service password reset and participant invites use single-use, expiring, SHA-256-hashed `AuthToken` rows. `POST /api/email/test` can only send to the requesting admin and reports only an exception class. The cache is per-process and kept honest across replicas by `config_sync` on the `email` scope (#213). -**Single-replica constraint**: every piece of cross-request state lives in the **process**, so the app must run as one replica (k8s enforces `replicas: 1` + `strategy: Recreate`). **Redis pub/sub alone does not lift this** — it only fixes `ws_manager` fan-out. Also in-memory, each needing its own answer: exercise timers → need a task queue (Celery/ARQ); `rate_limit`, whose per-process counters multiply the effective login/registration/reset limits by the replica count → needs a shared store; the `SiemConfig`/`ProxyConfig` caches → need cross-replica invalidation; and the daily `retention_service` sweep → every replica would run its own purge, competing over the same batched deletes. Full table in [deployment.md](../website/docs/deployment.md). `schedule_service` keeps keyed registries for inject releases and delayed triggered communications. Pause cancels both, resume reconstructs their remaining active-time delay, completion drops them, and `rehydrate_schedules()` derives undelivered timers from persisted state after a **single-process** restart. Multiple live replicas remain unsupported. +**Multi-replica (#213)**: all cross-request state is shared through PostgreSQL — no Redis, no broker, nothing extra to operate. Two mechanisms carry it. `pg_bus` publishes compact **descriptors** (ids, never content — `NOTIFY` caps at 8000 bytes) on `LISTEN`/`NOTIFY` channels, issued **inside** the publishing transaction so Postgres delivers them only on COMMIT and discards them on rollback; `pg_listener` holds one dedicated asyncpg connection per replica (never a pooled checkout — LISTEN is connection state) with a jittered backoff reconnect, and `on_reconnect` hooks recover what at-most-once delivery lost. `task_queue` is procrastinate: jobs are INSERTed by `procrastinate_defer_jobs_v1` **on the caller's own session**, so a job exists if and only if the state change that warranted it committed — the property that makes #211, #212 and #218 unrepresentable rather than merely fixed. Per subsystem: WS fan-out → `ws_relay` (above); config caches → `config_sync` re-reads the named scope, and every scope on reconnect; inject releases and triggered comms → durable jobs with **guards instead of cancellation** (nothing cancels on pause or early release; a stale job re-reads durable state and no-ops or re-defers, which the CAS release and the `(exercise_id, trigger_key)` unique insert already made safe); LLM pipelines → jobs with a queueing lock; the retention sweep → a periodic job, so exactly one replica purges each night; `rate_limit` → rows in an UNLOGGED table, so the limit no longer multiplies by the replica count. Startup migrations serialise on a Postgres advisory lock. `ws_manager` stays per-replica and should: it holds this process's actual sockets. The remaining constraint is **storage, not state** — the uploads PVC is ReadWriteOnce and access modes are immutable, so the base manifests keep `replicas: 1` and `k8s/overlays/multi-replica` is the opt-in path for clusters with RWX. Full table in [deployment.md](../website/docs/deployment.md). **Subsystem deep-dives live in [PLAN.md](../PLAN.md) § Subsystem Decisions** — read the relevant entry before touching that subsystem: - *Security & auth*: startup secret validation (#9), self-registration (#8), password policy (#13), token revocation (#14), admin password reset (#66), registration controls (#67), cookie security & CSRF (#10), security headers (#77), login brute-force (#11), trusted-proxy client IP (#36), WebSocket auth (#68), facilitator ownership scoping (#12). @@ -73,7 +73,7 @@ API-first architecture. **Strict CSP + CSP-safe Alpine (#77)**: the app ships a strict same-origin `Content-Security-Policy` with **`script-src 'self'`** — no `'unsafe-inline'`/`'unsafe-eval'`. This required the **`@alpinejs/csp`** build (vendored, version-pinned at `static/js/vendor/alpine-csp-.min.js`) and moving **all** inline JS out of the templates. There are **no inline `