Merge pull request 'fix(server): transactional sync push; single-use refresh rotation' (#136) from fix/sync-auth-races into master

Two concurrency fixes from the 2026-07-06 review (findings M1, M2):

sync push (M1): the load→merge→store cycle ran as three separate DB
operations. Two devices pushing concurrently both read the same stored
payload, merged independently, and the second store overwrote the first
merge — the server visibly regressed until the losing device pushed
again. The whole cycle (including the leaderboard update) now runs in
one transaction; SQLite serialises the writers. The leaderboard
helper's stale docstring (claiming a single conditional UPDATE that was
actually two statements) is corrected — the transaction now provides
the atomicity it described.

refresh rotation (M2): SELECT-then-DELETE let two concurrent refreshes
with the same token both pass the liveness check and both mint fresh
token pairs. The SELECT is gone; rotation now gates on the DELETE's
rows_affected — whoever removes the jti row wins, everyone else gets
401. Sequential reuse was already covered by
consumed_refresh_token_is_rejected, which still passes unchanged.

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

solitaire_server/src/auth.rs

@@ -283,23 +283,20 @@ pub async fn refresh(
     // Tokens without jti predate rotation — require re-login.
     let jti = claims.jti.ok_or(AppError::Unauthorized)?;
 
-    // Verify this jti is still live (not yet consumed or from a deleted account).
+    // Consume the old token before issuing new ones, gating on the DELETE
-    // SQLite TEXT columns are always nullable in sqlx; flatten the double-Option.
+    // actually removing a row. rows_affected == 0 covers both "jti never
-    let exists: Option<String> =
+    // existed / account deleted" and "a concurrent refresh already consumed
-        sqlx::query_scalar!("SELECT jti FROM refresh_tokens WHERE jti = ?", jti)
+    // it" — the previous SELECT-then-DELETE let two concurrent refreshes
-            .fetch_optional(&state.pool)
+    // both pass the check and both mint fresh token pairs. The DELETE is
-            .await?
+    // the mutex: whoever removes the row wins; everyone else gets 401.
-            .flatten();
+    // If the insert below fails, the user loses this session (must
-
+    // re-login) — safe by design.
-    if exists.is_none() {
+    let deleted = sqlx::query!("DELETE FROM refresh_tokens WHERE jti = ?", jti)
-        return Err(AppError::Unauthorized);
-    }
-
-    // Consume the old token before issuing new ones. If the insert below
-    // fails, the user loses this session (must re-login) — safe by design.
-    sqlx::query!("DELETE FROM refresh_tokens WHERE jti = ?", jti)
         .execute(&state.pool)
         .await?;
+    if deleted.rows_affected() != 1 {
+        return Err(AppError::Unauthorized);
+    }
 
     let new_access = make_access_token(&claims.sub, &state.jwt_secret)?;
     let (new_refresh, new_jti) = make_refresh_token(&claims.sub, &state.jwt_secret)?;

solitaire_server/src/sync.rs

@@ -5,7 +5,6 @@
 
 use axum::{Json, extract::State};
 use chrono::Utc;
-use sqlx::SqlitePool;
 use uuid::Uuid;
 
 use solitaire_sync::{
@@ -26,13 +25,19 @@ struct SyncRow {
 
 /// Load the stored `SyncPayload` for `user_id` from the database.
 /// Returns `None` if this user has not pushed any data yet.
-async fn load_sync_row(pool: &SqlitePool, user_id: &str) -> Result<Option<SyncRow>, AppError> {
+///
+/// Executor-generic so `push` can run it inside its transaction while
+/// `pull` keeps passing the pool directly.
+async fn load_sync_row(
+    exec: impl sqlx::Executor<'_, Database = sqlx::Sqlite>,
+    user_id: &str,
+) -> Result<Option<SyncRow>, AppError> {
     let row = sqlx::query_as!(
         SyncRow,
         "SELECT stats_json, achievements_json, progress_json FROM sync_state WHERE user_id = ?",
         user_id
     )
-    .fetch_optional(pool)
+    .fetch_optional(exec)
     .await?;
     Ok(row)
 }
@@ -69,7 +74,7 @@ fn row_to_payload(row: &SyncRow, user_id: &str) -> Result<SyncPayload, AppError>
 
 /// Persist a `SyncPayload` for `user_id` using an upsert.
 async fn store_payload(
-    pool: &SqlitePool,
+    exec: impl sqlx::Executor<'_, Database = sqlx::Sqlite>,
     user_id: &str,
     payload: &SyncPayload,
 ) -> Result<(), AppError> {
@@ -92,7 +97,7 @@ async fn store_payload(
         progress_json,
         now
     )
-    .execute(pool)
+    .execute(exec)
     .await?;
 
     Ok(())
@@ -159,12 +164,20 @@ pub async fn push(
         return Err(AppError::BadRequest("user_id mismatch".into()));
     }
 
-    let server_payload = match load_sync_row(&state.pool, &user.user_id).await? {
+    // The whole read-merge-write cycle runs in ONE transaction. Without it,
+    // two devices pushing concurrently both read the same stored payload,
+    // merge independently, and the second store overwrites the first merge —
+    // the server visibly regresses until the losing device pushes again.
+    // SQLite serialises writers, so the second transaction simply waits.
+    let mut tx = state.pool.begin().await?;
+
+    let server_payload = match load_sync_row(&mut *tx, &user.user_id).await? {
         Some(row) => row_to_payload(&row, &user.user_id)?,
         None => {
             // First push — nothing to merge against; store directly.
-            store_payload(&state.pool, &user.user_id, &client_payload).await?;
+            store_payload(&mut *tx, &user.user_id, &client_payload).await?;
-            update_leaderboard_if_opted_in(&state.pool, &user.user_id, &client_payload).await?;
+            update_leaderboard_if_opted_in(&mut tx, &user.user_id, &client_payload).await?;
+            tx.commit().await?;
             return Ok(Json(SyncResponse {
                 merged: client_payload,
                 server_time: Utc::now(),
@@ -175,8 +188,9 @@ pub async fn push(
 
     let (merged, conflicts) = merge(&client_payload, &server_payload);
 
-    store_payload(&state.pool, &user.user_id, &merged).await?;
+    store_payload(&mut *tx, &user.user_id, &merged).await?;
-    update_leaderboard_if_opted_in(&state.pool, &user.user_id, &merged).await?;
+    update_leaderboard_if_opted_in(&mut tx, &user.user_id, &merged).await?;
+    tx.commit().await?;
 
     Ok(Json(SyncResponse {
         merged,
@@ -188,16 +202,18 @@ pub async fn push(
 /// If the user is opted in to the leaderboard, update their row with the
 /// better of the stored and incoming `best_single_score` / `fastest_win_seconds`.
 ///
-/// The opt-in check and the update are performed atomically in a single
+/// Runs on the caller's transaction connection, so the opt-in check and the
-/// conditional UPDATE (WHERE EXISTS subquery) to avoid a TOCTOU race where
+/// update are atomic with the surrounding push — an opt-out between the check
-/// the user opts out between the check and the write.
+/// and the write can no longer interleave. (An earlier doc comment claimed
+/// this was a single conditional UPDATE; it has always been two statements —
+/// the enclosing transaction is what actually provides the atomicity.)
 async fn update_leaderboard_if_opted_in(
-    pool: &SqlitePool,
+    conn: &mut sqlx::SqliteConnection,
     user_id: &str,
     payload: &SyncPayload,
 ) -> Result<(), AppError> {
     let opted_in = sqlx::query!("SELECT leaderboard_opt_in FROM users WHERE id = ?", user_id)
-        .fetch_optional(pool)
+        .fetch_optional(&mut *conn)
         .await?
         .map(|r| r.leaderboard_opt_in)
         .unwrap_or(0);
@@ -231,7 +247,7 @@ async fn update_leaderboard_if_opted_in(
         now,
         user_id
     )
-    .execute(pool)
+    .execute(&mut *conn)
     .await?;
 
     Ok(())

solitaire_server/web/pkg/canvas.js

@@ -1649,63 +1649,63 @@ function __wbg_get_imports() {
             return ret;
         },
         __wbindgen_cast_0000000000000001: function(arg0, arg1) {
-            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [Externref], shim_idx: 114880, ret: Result(Unit), inner_ret: Some(Result(Unit)) }, mutable: true }) -> Externref`.
+            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [Externref], shim_idx: 61868, ret: Result(Unit), inner_ret: Some(Result(Unit)) }, mutable: true }) -> Externref`.
-            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__hfb2e9a2f0bbd9ecc);
+            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__hd94d76233321402f);
             return ret;
         },
         __wbindgen_cast_0000000000000002: function(arg0, arg1) {
-            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [Externref], shim_idx: 9853, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
+            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [Externref], shim_idx: 7311, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
-            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a);
+            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h26ff63c654218354);
             return ret;
         },
         __wbindgen_cast_0000000000000003: function(arg0, arg1) {
-            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("Array<any>"), NamedExternref("ResizeObserver")], shim_idx: 9866, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
+            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("Array<any>"), NamedExternref("ResizeObserver")], shim_idx: 7314, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
-            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h876550298b312ff8);
+            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__hfc779804ccb0943e);
             return ret;
         },
         __wbindgen_cast_0000000000000004: function(arg0, arg1) {
-            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("Array<any>")], shim_idx: 9853, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
+            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("Array<any>")], shim_idx: 7311, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
-            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_3);
+            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_3);
             return ret;
         },
         __wbindgen_cast_0000000000000005: function(arg0, arg1) {
-            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("Event")], shim_idx: 9853, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
+            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("Event")], shim_idx: 7311, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
-            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_4);
+            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_4);
             return ret;
         },
         __wbindgen_cast_0000000000000006: function(arg0, arg1) {
-            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("FocusEvent")], shim_idx: 9853, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
+            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("FocusEvent")], shim_idx: 7311, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
-            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_5);
+            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_5);
             return ret;
         },
         __wbindgen_cast_0000000000000007: function(arg0, arg1) {
-            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("KeyboardEvent")], shim_idx: 9853, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
+            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("KeyboardEvent")], shim_idx: 7311, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
-            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_6);
+            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_6);
             return ret;
         },
         __wbindgen_cast_0000000000000008: function(arg0, arg1) {
-            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("PageTransitionEvent")], shim_idx: 9853, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
+            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("PageTransitionEvent")], shim_idx: 7311, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
-            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_7);
+            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_7);
             return ret;
         },
         __wbindgen_cast_0000000000000009: function(arg0, arg1) {
-            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("PointerEvent")], shim_idx: 9853, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
+            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("PointerEvent")], shim_idx: 7311, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
-            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_8);
+            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_8);
             return ret;
         },
         __wbindgen_cast_000000000000000a: function(arg0, arg1) {
-            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("WheelEvent")], shim_idx: 9853, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
+            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [NamedExternref("WheelEvent")], shim_idx: 7311, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
-            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_9);
+            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_9);
             return ret;
         },
         __wbindgen_cast_000000000000000b: function(arg0, arg1) {
-            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [Option(NamedExternref("Blob"))], shim_idx: 9863, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
+            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [Option(NamedExternref("Blob"))], shim_idx: 7312, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
-            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h657f46feffff6fe4);
+            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__hfd77696cd35180b1);
             return ret;
         },
         __wbindgen_cast_000000000000000c: function(arg0, arg1) {
-            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [], shim_idx: 9857, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
+            // Cast intrinsic for `Closure(Closure { owned: true, function: Function { arguments: [], shim_idx: 7313, ret: Unit, inner_ret: Some(Unit) }, mutable: true }) -> Externref`.
-            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__h545edb23183e448a);
+            const ret = makeMutClosure(arg0, arg1, wasm_bindgen__convert__closures_____invoke__hebcd362fbbe0fc8c);
             return ret;
         },
         __wbindgen_cast_000000000000000d: function(arg0) {
@@ -1769,55 +1769,55 @@ function __wbg_get_imports() {
     };
 }
 
-function wasm_bindgen__convert__closures_____invoke__h545edb23183e448a(arg0, arg1) {
+function wasm_bindgen__convert__closures_____invoke__hebcd362fbbe0fc8c(arg0, arg1) {
-    wasm.wasm_bindgen__convert__closures_____invoke__h545edb23183e448a(arg0, arg1);
+    wasm.wasm_bindgen__convert__closures_____invoke__hebcd362fbbe0fc8c(arg0, arg1);
 }
 
-function wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a(arg0, arg1, arg2) {
+function wasm_bindgen__convert__closures_____invoke__h26ff63c654218354(arg0, arg1, arg2) {
-    wasm.wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a(arg0, arg1, arg2);
+    wasm.wasm_bindgen__convert__closures_____invoke__h26ff63c654218354(arg0, arg1, arg2);
 }
 
-function wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_3(arg0, arg1, arg2) {
+function wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_3(arg0, arg1, arg2) {
-    wasm.wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_3(arg0, arg1, arg2);
+    wasm.wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_3(arg0, arg1, arg2);
 }
 
-function wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_4(arg0, arg1, arg2) {
+function wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_4(arg0, arg1, arg2) {
-    wasm.wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_4(arg0, arg1, arg2);
+    wasm.wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_4(arg0, arg1, arg2);
 }
 
-function wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_5(arg0, arg1, arg2) {
+function wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_5(arg0, arg1, arg2) {
-    wasm.wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_5(arg0, arg1, arg2);
+    wasm.wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_5(arg0, arg1, arg2);
 }
 
-function wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_6(arg0, arg1, arg2) {
+function wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_6(arg0, arg1, arg2) {
-    wasm.wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_6(arg0, arg1, arg2);
+    wasm.wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_6(arg0, arg1, arg2);
 }
 
-function wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_7(arg0, arg1, arg2) {
+function wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_7(arg0, arg1, arg2) {
-    wasm.wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_7(arg0, arg1, arg2);
+    wasm.wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_7(arg0, arg1, arg2);
 }
 
-function wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_8(arg0, arg1, arg2) {
+function wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_8(arg0, arg1, arg2) {
-    wasm.wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_8(arg0, arg1, arg2);
+    wasm.wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_8(arg0, arg1, arg2);
 }
 
-function wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_9(arg0, arg1, arg2) {
+function wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_9(arg0, arg1, arg2) {
-    wasm.wasm_bindgen__convert__closures_____invoke__h15aa57dbc666225a_9(arg0, arg1, arg2);
+    wasm.wasm_bindgen__convert__closures_____invoke__h26ff63c654218354_9(arg0, arg1, arg2);
 }
 
-function wasm_bindgen__convert__closures_____invoke__hfb2e9a2f0bbd9ecc(arg0, arg1, arg2) {
+function wasm_bindgen__convert__closures_____invoke__hd94d76233321402f(arg0, arg1, arg2) {
-    const ret = wasm.wasm_bindgen__convert__closures_____invoke__hfb2e9a2f0bbd9ecc(arg0, arg1, arg2);
+    const ret = wasm.wasm_bindgen__convert__closures_____invoke__hd94d76233321402f(arg0, arg1, arg2);
     if (ret[1]) {
         throw takeFromExternrefTable0(ret[0]);
     }
 }
 
-function wasm_bindgen__convert__closures_____invoke__h876550298b312ff8(arg0, arg1, arg2, arg3) {
+function wasm_bindgen__convert__closures_____invoke__hfc779804ccb0943e(arg0, arg1, arg2, arg3) {
-    wasm.wasm_bindgen__convert__closures_____invoke__h876550298b312ff8(arg0, arg1, arg2, arg3);
+    wasm.wasm_bindgen__convert__closures_____invoke__hfc779804ccb0943e(arg0, arg1, arg2, arg3);
 }
 
-function wasm_bindgen__convert__closures_____invoke__h657f46feffff6fe4(arg0, arg1, arg2) {
+function wasm_bindgen__convert__closures_____invoke__hfd77696cd35180b1(arg0, arg1, arg2) {
-    wasm.wasm_bindgen__convert__closures_____invoke__h657f46feffff6fe4(arg0, arg1, isLikeNone(arg2) ? 0 : addToExternrefTable0(arg2));
+    wasm.wasm_bindgen__convert__closures_____invoke__hfd77696cd35180b1(arg0, arg1, isLikeNone(arg2) ? 0 : addToExternrefTable0(arg2));
 }