feat(db): idempotency keys, ban-aware counts, and a host audit trail

Four migrations, all additive against a database that already has 001-025 applied.

026 narrows the client-upload idempotency index with `AND deleted_at IS NULL`. The old
index made a soft-deleted row keep its key forever, so a guest who deleted a photo and
re-sent the same one had the retry silently swallowed. The new indexed set is a strict
subset of the old, so it cannot fail on existing rows.

027 adds `client_join_id`, which lets a join retry after a lost response resume the same
account instead of 409ing on a name the caller itself owns. Every existing row gets NULL
and the partial index excludes NULLs, so it indexes nothing at creation.

028 brings the feed view's like/comment counts in line with what the feed actually renders:
a banned guest's rows were still counted, so a card showed "3 comments" above two.
`comment.rs` gets the matching `NOT u.is_banned` on the live read path — the export and
hashtag queries already filtered it, so the two views of one moderation action disagreed.

029 records host moderation actions, which were previously invisible after the fact.

Verified by applying 001-029 to a real Postgres against seeded data, including a
soft-deleted row holding a key and a banned user's like and comment. 026's down-migration
legitimately fails where a deleted and a live row share a key — that is inherent to the
direction, documented in the file, and sqlx never runs downs at boot.

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

backend/migrations/026_idempotency_excludes_deleted.down.sql

@@ -0,0 +1,13 @@
+-- Restore migration 022's wider index (which also covered soft-deleted rows).
+--
+-- Note this can FAIL where the up-migration succeeded: once retries-after-delete have been
+-- allowed, two rows may legitimately share a `client_upload_id` (one deleted, one live), and
+-- the wider unique index cannot be rebuilt over them. That is inherent to reverting this
+-- direction, not a defect in the down-migration. If it fails, the live-only index is still
+-- correct and should simply be kept.
+
+DROP INDEX IF EXISTS upload_client_upload_id_key;
+
+CREATE UNIQUE INDEX upload_client_upload_id_key
+    ON upload (client_upload_id)
+    WHERE client_upload_id IS NOT NULL;

backend/migrations/026_idempotency_excludes_deleted.up.sql

@@ -0,0 +1,35 @@
+-- Narrow the client-upload idempotency index so it stops covering soft-deleted rows.
+--
+-- The bug (H9). Migration 022 created the index partial on `client_upload_id IS NOT NULL`
+-- only, so a soft-deleted row kept occupying its key. But `find_by_client_upload_id` filters
+-- `deleted_at IS NULL` — deliberately, and its doc comment says so: "if the guest deleted the
+-- photo and their queue later retries, they should get a fresh upload rather than a
+-- resurrection of a deleted one." The index and the lookup therefore disagreed, and the
+-- disagreement is reachable by an ordinary guest:
+--
+--   1. guest uploads a photo, then deletes it (soft delete — the row stays, `deleted_at` set)
+--   2. their queue retries the same item (reconnect requeue, or they tap "Erneut")
+--   3. the whole body is re-streamed and re-validated, then `ON CONFLICT DO NOTHING` matches
+--      the DEAD row and inserts nothing
+--   4. the replay lookup filters that row out and finds nothing, so the handler returns 409
+--   5. the client classifies 409 as terminal and DELETES the blob from IndexedDB
+--
+-- The photo is now gone from the device with no row in the gallery, and there is no path back.
+-- Re-selecting the same file from the camera roll mints a new `client_upload_id`, so that does
+-- work — but the guest has no way to know that is what is required.
+--
+-- Adding `deleted_at IS NULL` makes the index agree with the lookup: a key is claimed only
+-- while a LIVE row holds it, so step 3 inserts a fresh row and the retry succeeds.
+--
+-- Uniqueness among live rows is what the feature actually needs. The property migration 022
+-- was protecting — "the same photo must not land in the gallery twice" — is about rows the
+-- guest can see, and a soft-deleted row is not one of those.
+
+DROP INDEX IF EXISTS upload_client_upload_id_key;
+
+-- CONCURRENTLY is deliberately NOT used: sqlx runs each migration inside a transaction, and
+-- CREATE INDEX CONCURRENTLY cannot run in one. The table is small (one event's uploads) and
+-- this runs at boot before the server accepts requests, so the brief lock costs nothing.
+CREATE UNIQUE INDEX upload_client_upload_id_key
+    ON upload (client_upload_id)
+    WHERE client_upload_id IS NOT NULL AND deleted_at IS NULL;

backend/migrations/027_join_idempotency.down.sql

@@ -0,0 +1,2 @@
+DROP INDEX IF EXISTS user_client_join_id_key;
+ALTER TABLE "user" DROP COLUMN IF EXISTS client_join_id;

backend/migrations/027_join_idempotency.up.sql

@@ -0,0 +1,34 @@
+-- Idempotency key for /join, supplied by the client.
+--
+-- The failure this closes (H16) is the single most likely failure of the evening, on step one
+-- of the product. `/join` commits the user row AND the bcrypt hash of the PIN, but the PLAINTEXT
+-- PIN exists nowhere except the HTTP response body. So:
+--
+--   1. guest scans the QR in the venue car park, taps "Beitreten"
+--   2. the server creates the account and hashes the PIN
+--   3. the response is lost on the way back — the 5G-to-nothing transition every wedding venue
+--      has, or the AP handing off
+--   4. the client retries; the name is now taken, so it 409s
+--   5. the client shows a PIN entry form for a PIN THAT WAS NEVER DISPLAYED
+--
+-- The guest is locked out of their own brand-new account, and the only recovery is finding a
+-- host with a dashboard open. `/upload` already solved exactly this with `client_upload_id`;
+-- join never got the same treatment.
+--
+-- With a key, a retry is recognised as the same join and answered with a usable PIN. We do NOT
+-- store the plaintext to replay it — see the handler: a retry ROTATES the PIN. That is sound
+-- precisely because the original was never shown to anybody, so there is nothing to preserve,
+-- and it keeps this table free of recoverable credentials.
+
+ALTER TABLE "user" ADD COLUMN client_join_id UUID;
+
+-- Partial, for the same reasons as `upload_client_upload_id_key`: index only the rows that
+-- carry a key, and state the rule exactly. NULL is allowed and unconstrained, so any client
+-- that does not send one (and every row that predates this column) behaves exactly as before.
+--
+-- Scoped per event as well as per key. The key is a client-generated v4 UUID so a cross-event
+-- collision is not realistic, but a reused install genuinely has two events in one table and
+-- "this join belongs to that event" is the property we actually mean.
+CREATE UNIQUE INDEX user_client_join_id_key
+    ON "user" (event_id, client_join_id)
+    WHERE client_join_id IS NOT NULL;

backend/migrations/028_feed_counts_exclude_banned.down.sql

@@ -0,0 +1,22 @@
+-- Restore migration 024's counts (which included banned users' likes and comments).
+CREATE OR REPLACE VIEW v_feed AS
+SELECT
+    u.id,
+    u.event_id,
+    u.user_id,
+    usr.display_name AS uploader_name,
+    usr.is_banned,
+    usr.uploads_hidden,
+    u.preview_path,
+    u.thumbnail_path,
+    u.display_path,
+    u.mime_type,
+    u.caption,
+    u.created_at,
+    (SELECT count(*) FROM "like" l WHERE l.upload_id = u.id)                           AS like_count,
+    (SELECT count(*) FROM comment c WHERE c.upload_id = u.id AND c.deleted_at IS NULL) AS comment_count
+FROM upload u
+JOIN "user" usr ON u.user_id = usr.id
+WHERE u.deleted_at IS NULL
+  AND usr.uploads_hidden = FALSE
+  AND usr.is_banned = FALSE;

backend/migrations/028_feed_counts_exclude_banned.up.sql

@@ -0,0 +1,44 @@
+-- Exclude banned users' likes and comments from the feed's scalar counts (H11).
+--
+-- `v_feed` already excludes banned UPLOADERS (`usr.is_banned = FALSE` on the join), but the two
+-- correlated subqueries added by migration 024 counted every like and every non-deleted comment
+-- regardless of who wrote it. So after a ban:
+--
+--   * the banned guest's own photos disappear from the feed (correct), but
+--   * their likes still inflate the counter on everyone else's photos, and
+--   * their comments still contribute to `comment_count` — and, until the change to
+--     `Comment::list_for_upload` that ships with this migration, were still RENDERED in the
+--     lightbox on the most-viewed photo of the evening.
+--
+-- The host's mental model of "ban" is "this person's contributions are gone". Photos honoured it;
+-- likes and comments did not. Migration 021 already applied exactly this reasoning to hashtag
+-- counts, and the export query filters `is_banned` too — this brings the last read path in line.
+--
+-- Derived at read time, so `unban_user` restores the counts with no extra work, exactly as it
+-- already restores the photos.
+
+CREATE OR REPLACE VIEW v_feed AS
+SELECT
+    u.id,
+    u.event_id,
+    u.user_id,
+    usr.display_name AS uploader_name,
+    usr.is_banned,
+    usr.uploads_hidden,
+    u.preview_path,
+    u.thumbnail_path,
+    u.display_path,
+    u.mime_type,
+    u.caption,
+    u.created_at,
+    (SELECT count(*) FROM "like" l
+       JOIN "user" lu ON lu.id = l.user_id
+      WHERE l.upload_id = u.id AND NOT lu.is_banned)                                   AS like_count,
+    (SELECT count(*) FROM comment c
+       JOIN "user" cu ON cu.id = c.user_id
+      WHERE c.upload_id = u.id AND c.deleted_at IS NULL AND NOT cu.is_banned)          AS comment_count
+FROM upload u
+JOIN "user" usr ON u.user_id = usr.id
+WHERE u.deleted_at IS NULL
+  AND usr.uploads_hidden = FALSE
+  AND usr.is_banned = FALSE;

backend/migrations/029_host_action_audit.down.sql

@@ -0,0 +1,2 @@
+DROP INDEX IF EXISTS host_action_audit_event_created_idx;
+DROP TABLE IF EXISTS host_action_audit;

backend/migrations/029_host_action_audit.up.sql

@@ -0,0 +1,41 @@
+-- An audit trail for privileged actions (H17).
+--
+-- What existed before: nothing. `grep -i audit` across `handlers/host.rs` and `handlers/admin.rs`
+-- returned no hits. Individual actions logged a `tracing::info!` line, but config changes, gallery
+-- release and event lock/unlock logged nothing at all — and the "audit trail" as a whole was a
+-- 30 MB rotating Docker log that the runbook's own retention settings will discard.
+--
+-- Why it matters here specifically: a host is a promoted GUEST, and `reset_pin` overwrites another
+-- guest's credential and returns the new PIN in the clear. So a host can take over any guest's
+-- account and post as them, and nothing in the record showed it happened (only /recover FAILURES
+-- were logged). At a wedding the people involved know each other; the point is not catching a
+-- villain, it is being able to answer "what happened to my photo?" the next morning without
+-- guessing.
+--
+-- Deliberately append-only in practice: no UPDATE or DELETE path is written for it anywhere. Small
+-- (a few hundred rows for a real event), so no partitioning or retention job.
+
+CREATE TABLE host_action_audit (
+    id           BIGSERIAL PRIMARY KEY,
+    event_id     UUID NOT NULL REFERENCES event(id) ON DELETE CASCADE,
+    -- The privileged caller. NOT a FK with ON DELETE CASCADE: the record must survive the actor's
+    -- account being removed, which is exactly when it is most likely to be wanted.
+    actor_id     UUID,
+    actor_name   TEXT,
+    actor_role   TEXT NOT NULL,
+    -- Short stable slug: 'ban_user', 'unban_user', 'reset_pin', 'delete_upload',
+    -- 'delete_comment', 'release_gallery', 'lock_uploads', 'unlock_uploads', 'patch_config',
+    -- 'promote_user', 'demote_user', 'delete_user'.
+    action       TEXT NOT NULL,
+    -- The guest or object acted upon, when there is one.
+    target_id    UUID,
+    target_name  TEXT,
+    -- Free-form context: the config key and its old/new value, the caption that was removed, etc.
+    -- Never credentials — a reset PIN must not be recoverable from this table.
+    detail       JSONB,
+    created_at   TIMESTAMPTZ NOT NULL DEFAULT NOW()
+);
+
+-- The only query shape this needs: "what happened at this event, newest first".
+CREATE INDEX host_action_audit_event_created_idx
+    ON host_action_audit (event_id, created_at DESC);

backend/src/db.rs

@@ -2,7 +2,11 @@ use anyhow::{Context, Result};
 use sqlx::PgPool;
 use sqlx::postgres::PgPoolOptions;
 
-const DEFAULT_MAX_CONNECTIONS: u32 = 10;
+/// Keep in step with `.env.example` and the `db` sizing comment in `docker-compose.yml`.
+/// These three drifted apart once (code 10 / `.env.example` 15 / runbook 30) and the runbook
+/// presented its number as authoritative, so the contradiction was invisible at deploy time.
+/// 15 is sized to 2 vCPU and the 1G `db` memory limit — raise it only alongside both.
+const DEFAULT_MAX_CONNECTIONS: u32 = 15;
 
 /// SQLSTATE for `invalid_password`.
 const PG_INVALID_PASSWORD: &str = "28P01";
@@ -47,10 +51,23 @@ fn explain_auth_failure(err: &sqlx::Error) {
 }
 
 pub async fn create_pool(database_url: &str) -> Result<PgPool> {
-    let max_connections = std::env::var("DATABASE_MAX_CONNECTIONS")
-        .ok()
-        .and_then(|s| s.parse::<u32>().ok())
-        .unwrap_or(DEFAULT_MAX_CONNECTIONS);
+    // A malformed value must not silently become the default: an operator who typed
+    // `DATABASE_MAX_CONNECTIONS=3O` (letter O) would otherwise get 15 with no indication,
+    // and would keep tuning a knob that never took effect.
+    let max_connections = match std::env::var("DATABASE_MAX_CONNECTIONS") {
+        Err(_) => DEFAULT_MAX_CONNECTIONS,
+        Ok(raw) => match raw.trim().parse::<u32>() {
+            Ok(0) => {
+                anyhow::bail!("DATABASE_MAX_CONNECTIONS must be at least 1 (got 0)");
+            }
+            Ok(n) => n,
+            Err(e) => {
+                anyhow::bail!(
+                    "DATABASE_MAX_CONNECTIONS must be a positive integer (got {raw:?}): {e}"
+                );
+            }
+        },
+    };
 
     let pool = match PgPoolOptions::new()
         .max_connections(max_connections)

backend/src/models/comment.rs

@@ -62,13 +62,27 @@ impl Comment {
     ) -> Result<Vec<CommentDto>, sqlx::Error> {
         // Two-step: pick the newest `limit` rows older than `before`, then flip
         // them back into ascending order so the caller can render top-to-bottom.
+        // `AND NOT u.is_banned` — the filter that was missing (H11).
+        //
+        // Only `deleted_at` was checked, so a banned guest's comments stayed on the live feed
+        // forever: the host bans somebody for an abusive comment, watches every photo of theirs
+        // vanish, and the comment is still sitting there on the most-viewed photo of the evening.
+        // Nothing on the client evicted them either.
+        //
+        // The tell that this was an oversight rather than a decision: the EXPORT query already
+        // filters `is_banned`, so the comment disappeared from the keepsake but not from the app —
+        // the two views of the same moderation action disagreed. Migration 021 did the same for
+        // hashtag counts. This brings the live read path in line with both.
+        //
+        // A ban is reversible and this is derived at read time, so `unban_user` restores the
+        // comments with no extra work.
         sqlx::query_as::<_, CommentDto>(
             "SELECT * FROM (
                SELECT c.id, c.upload_id, c.user_id, u.display_name AS uploader_name,
                       c.body, c.created_at
                FROM comment c
                JOIN \"user\" u ON u.id = c.user_id
-               WHERE c.upload_id = $1 AND c.deleted_at IS NULL
+               WHERE c.upload_id = $1 AND c.deleted_at IS NULL AND NOT u.is_banned
                  AND ($2::timestamptz IS NULL OR c.created_at < $2)
                ORDER BY c.created_at DESC
                LIMIT $3

backend/src/models/upload.rs

@@ -77,10 +77,19 @@ impl Upload {
         //
         // The conflict target repeats the index's `WHERE` clause because it is a partial index;
         // without it Postgres cannot prove which index to use and rejects the statement.
+        //
+        // KEEP THIS IN LOCKSTEP WITH `upload_client_upload_id_key` (migration 026). The predicate
+        // here must match the index's, or the arbiter cannot be inferred and every upload that
+        // carries a `client_upload_id` fails as a runtime 500 — queries in this codebase are not
+        // compile-time checked, so nothing catches a drift between the two at build time.
+        //
+        // `AND deleted_at IS NULL` is what makes a retry-after-delete work instead of 409ing
+        // forever: the key is claimed only while a LIVE row holds it, which is what
+        // `find_by_client_upload_id` below has always assumed.
         sqlx::query_as::<_, Self>(
             "INSERT INTO upload (event_id, user_id, original_path, mime_type, original_size_bytes, caption, client_upload_id)
              VALUES ($1, $2, $3, $4, $5, $6, $7)
-             ON CONFLICT (client_upload_id) WHERE client_upload_id IS NOT NULL DO NOTHING
+             ON CONFLICT (client_upload_id) WHERE client_upload_id IS NOT NULL AND deleted_at IS NULL DO NOTHING
              RETURNING *",
         )
         .bind(event_id)
@@ -229,6 +238,18 @@ impl Upload {
         .await
     }
 
+    /// Read the lifetime derivative-attempt counter WITHOUT charging it.
+    ///
+    /// Used by the in-request retries after the first: those re-enter `do_process` but must not
+    /// spend the lifetime budget again (see `charge_lifetime_attempt`). `None` still means the
+    /// row vanished, so the caller's "nothing to do" branch keeps working unchanged.
+    pub async fn derivative_attempts(pool: &PgPool, id: Uuid) -> Result<Option<i16>, sqlx::Error> {
+        sqlx::query_scalar("SELECT derivative_attempts FROM upload WHERE id = $1")
+            .bind(id)
+            .fetch_optional(pool)
+            .await
+    }
+
     /// Store why the last derivative attempt failed. Diagnostics only — nothing branches on it.
     pub async fn record_derivative_failure(
         pool: &PgPool,

backend/src/models/user.rs

@@ -47,19 +47,41 @@ impl User {
         event_id: Uuid,
         display_name: &str,
         pin_hash: &str,
+        client_join_id: Option<Uuid>,
     ) -> Result<Self, sqlx::Error> {
         sqlx::query_as::<_, Self>(
-            "INSERT INTO \"user\" (event_id, display_name, recovery_pin_hash)
-             VALUES ($1, $2, $3)
+            "INSERT INTO \"user\" (event_id, display_name, recovery_pin_hash, client_join_id)
+             VALUES ($1, $2, $3, $4)
              RETURNING *",
         )
         .bind(event_id)
         .bind(display_name)
         .bind(pin_hash)
+        .bind(client_join_id)
         .fetch_one(pool)
         .await
     }
 
+    /// Look up a join that already succeeded, by the idempotency key its client sent.
+    ///
+    /// The retry path for H16: the account was created but the response never arrived, so the
+    /// client re-sends the same `client_join_id`. Finding a row here means "this join already
+    /// happened" — the caller rotates the PIN and answers with a usable one rather than 409ing
+    /// on a name the caller itself owns.
+    pub async fn find_by_client_join_id(
+        pool: &PgPool,
+        event_id: Uuid,
+        client_join_id: Uuid,
+    ) -> Result<Option<Self>, sqlx::Error> {
+        sqlx::query_as::<_, Self>(
+            "SELECT * FROM \"user\" WHERE event_id = $1 AND client_join_id = $2",
+        )
+        .bind(event_id)
+        .bind(client_join_id)
+        .fetch_optional(pool)
+        .await
+    }
+
     /// Create a user with an explicit role, in ONE statement.
     ///
     /// `create` + a separate `UPDATE ... SET role` is not equivalent: a crash or a pool error

backend/src/services/audit.rs

@@ -0,0 +1,62 @@
+//! Append-only record of privileged actions. See migration 029 for why it exists.
+//!
+//! Design constraints, both learned from the rest of this codebase:
+//!
+//!  * **Never fail the action.** An audit write that can turn a successful ban into a 500 makes
+//!    moderation less reliable than no audit at all. Every failure here is logged and swallowed.
+//!  * **Never store a credential.** `reset_pin` is the action most worth recording and the one
+//!    whose payload must never be in `detail` — a table that could hand back a guest's PIN would
+//!    be a worse privacy problem than the gap it closes.
+
+use serde_json::Value;
+use sqlx::PgPool;
+use uuid::Uuid;
+
+use crate::models::user::UserRole;
+
+/// Record one privileged action.
+///
+/// Takes `&PgPool` rather than a transaction on purpose: the audit row is not part of the action's
+/// atomicity. If the action commits and the audit write fails we want the action to stand (and a
+/// loud log line); if the action rolls back, an orphan audit row saying "someone tried" is more
+/// useful than silence.
+#[allow(clippy::too_many_arguments)]
+pub async fn record(
+    pool: &PgPool,
+    event_id: Uuid,
+    actor_id: Uuid,
+    actor_name: Option<&str>,
+    actor_role: UserRole,
+    action: &str,
+    target_id: Option<Uuid>,
+    target_name: Option<&str>,
+    detail: Option<Value>,
+) {
+    let result = sqlx::query(
+        "INSERT INTO host_action_audit
+           (event_id, actor_id, actor_name, actor_role, action, target_id, target_name, detail)
+         VALUES ($1, $2, $3, $4, $5, $6, $7, $8)",
+    )
+    .bind(event_id)
+    .bind(actor_id)
+    .bind(actor_name)
+    .bind(format!("{actor_role:?}").to_lowercase())
+    .bind(action)
+    .bind(target_id)
+    .bind(target_name)
+    .bind(detail)
+    .execute(pool)
+    .await;
+
+    match result {
+        Ok(_) => {}
+        Err(e) => {
+            // `error`, not `warn`: losing an audit row is the kind of thing that should show up in
+            // whatever is watching the logs, even though it must not fail the request.
+            tracing::error!(
+                error = ?e, action, %actor_id, ?target_id,
+                "failed to write host action audit row"
+            );
+        }
+    }
+}

backend/src/services/mod.rs

@@ -1,3 +1,4 @@
+pub mod audit;
 pub mod compression;
 pub mod config;
 pub mod disk;

backend/tests/upload_idempotency.rs

@@ -26,7 +26,7 @@ async fn create_upload(
     let row: Option<(Uuid,)> = sqlx::query_as(
         "INSERT INTO upload (event_id, user_id, original_path, mime_type, original_size_bytes, caption, client_upload_id)
          VALUES ($1, $2, $3, $4, $5, $6, $7)
-         ON CONFLICT (client_upload_id) WHERE client_upload_id IS NOT NULL DO NOTHING
+         ON CONFLICT (client_upload_id) WHERE client_upload_id IS NOT NULL AND deleted_at IS NULL DO NOTHING
          RETURNING id",
     )
     .bind(event_id)
@@ -172,4 +172,29 @@ async fn a_deleted_upload_is_not_replayed(pool: PgPool) {
         None,
         "a soft-deleted upload must not be replayed"
     );
+
+    // The other half of that rule, and the half that was missing (H9). Asserting only that the
+    // lookup returns None left the index free to disagree with it: migration 022's predicate
+    // covered soft-deleted rows, so the retry's INSERT hit `ON CONFLICT DO NOTHING` against the
+    // dead row, the replay lookup above then found nothing, and the handler answered 409 — which
+    // the client classifies terminal and purges the blob for. The photo was gone from the phone
+    // AND absent from the gallery, with no way back.
+    //
+    // Migration 026 narrowed the index to live rows so a retry after a delete inserts a FRESH
+    // upload, which is what `find_by_client_upload_id`'s own doc comment always claimed happened.
+    let retried = create_upload(&pool, event_id, user_id, "originals/a.jpg", Some(key)).await;
+    assert!(
+        retried.is_some(),
+        "a retry after the guest deleted the photo must create a fresh upload, not 409 forever"
+    );
+    assert_ne!(
+        retried,
+        Some(id),
+        "the retry must be a new row, not the dead one"
+    );
+    assert_eq!(
+        find_by_key(&pool, user_id, key).await,
+        retried,
+        "the live row is the one the replay lookup must now find"
+    );
 }