Stale autosave can overwrite a logout/admin save (cross-channel write race) #44

New issue

Closed

opened 2026-06-11 14:06:13 +00:00 by sickday · 0 comments

sickday commented

2026-06-11 14:06:13 +00:00

Owner

Problem

Two uncoordinated channels persist the same profile row:

Async pipeline — Persistence.enqueue_save_all/1 (5-min :autosave) snapshots state and enqueues on Database.Pipeline (per-profile_id FIFO + dedupe).
Synchronous — Persistence.save_all/1 from Game.Entity.Player.terminate/2 (logout) and save_skills/1 from admin commands, written directly (not through the pipeline).

The pipeline's dedupe/FIFO only orders writes within the queue, so a stale autosave job can run after a newer synchronous write:

T: autosave snapshots (Tsnap) and enqueues.
T+1: player logs out; terminate/2 writes save_all synchronously (newer data).
T+2: the queued Tsnap job runs and overwrites with stale data → silent rollback / data loss.

Fix options

Route logout/admin saves through the same pipeline (FIFO + dedupe order them), or
Flush/cancel pending pipeline jobs for the profile_id before the synchronous write, or
Stamp rows with a monotonic version and drop out-of-order writes.

Acceptance

A logout immediately following an autosave never loses the post-snapshot delta.
Regression test simulating enqueue → logout → delayed-job ordering.

Refs: lib/black_lobster/game/entity/player/persistence.ex (save_all, enqueue_save_all), lib/black_lobster/game/entity/player.ex (terminate/2), lib/black_lobster/database/pipeline/*.

Part of the Security & Data-Integrity Hardening epic.

## Problem Two uncoordinated channels persist the same profile row: - **Async pipeline** — `Persistence.enqueue_save_all/1` (5-min `:autosave`) snapshots state and enqueues on `Database.Pipeline` (per-`profile_id` FIFO + dedupe). - **Synchronous** — `Persistence.save_all/1` from `Game.Entity.Player.terminate/2` (logout) and `save_skills/1` from admin commands, written **directly** (not through the pipeline). The pipeline's dedupe/FIFO only orders writes *within* the queue, so a stale autosave job can run **after** a newer synchronous write: 1. T: autosave snapshots (Tsnap) and enqueues. 2. T+1: player logs out; `terminate/2` writes `save_all` synchronously (newer data). 3. T+2: the queued Tsnap job runs and overwrites with stale data → **silent rollback / data loss**. ## Fix options - Route logout/admin saves through the same pipeline (FIFO + dedupe order them), or - Flush/cancel pending pipeline jobs for the `profile_id` before the synchronous write, or - Stamp rows with a monotonic version and drop out-of-order writes. ## Acceptance - A logout immediately following an autosave never loses the post-snapshot delta. - Regression test simulating enqueue → logout → delayed-job ordering. Refs: `lib/black_lobster/game/entity/player/persistence.ex` (`save_all`, `enqueue_save_all`), `lib/black_lobster/game/entity/player.ex` (`terminate/2`), `lib/black_lobster/database/pipeline/*`. Part of the Security & Data-Integrity Hardening epic.

sickday referenced this issue

2026-06-11 14:06:43 +00:00

Epic: Security & Data-Integrity Hardening #50

sickday added the

bug

epic:security-hardening

priority:high

server

labels

2026-06-11 14:07:08 +00:00

sickday added this to the v1 milestone

2026-06-11 14:11:16 +00:00