fix: unbounded essence farming, WS crash, and two economy races
Unbounded essence/item farming: every other reward trigger (summon,
question, fragment) had both a per-user and per-IP limiter, but
ritual_start and judgment had none at all — and judgment has no
"already resolved" state either. A scripted client could replay
{"type":"judgment","verdict":"cross_over"} in a tight loop and mint
CROSS_OVER_ESSENCE (25) plus a 20% item roll every iteration, forever.
Same for ritual_start -> 4x ritual_step. Added ritual/judgment limiters
in both flavors, matching the existing pattern.
WS session crash: _handle_question did `if state.entity is None:
await _handle_summon(state)` then `assert state.entity is not None`.
_handle_summon returns early *without* setting state.entity when the
seeker is rate-limited, so the assert fired unhandled — and the message
loop only catches WebSocketDisconnect, so it killed the whole connection.
Reachable with no malice: click summon a few times impatiently, then ask a
question. Now returns cleanly (the rate_limited frame was already sent).
Essence double-spend: purchase_unlock() deliberately uses SELECT ... FOR
UPDATE to serialize concurrent purchases, but the three credit_essence
call sites in ws.py did an unlocked db.get() read-modify-write. An
unlocked read doesn't block on a row lock, so a reward computed from a
pre-purchase balance could be written after the purchase committed,
silently reverting the deduction — user keeps the unlock and the essence.
All three now lock the row the same way.
Entity mint collision: _summon does a racy check-then-insert against
Entity.signature and Entity.name, both DB-unique, with no IntegrityError
handling — a concurrent mint of the same signature crashed the session.
Forceable by a user with two accounts (anomaly frequency/magnitude are
client-controlled), and plausible without malice in wire mode, where
sample_network() reads host-wide /proc/net/dev counters so two idle
sessions genuinely measure the same traffic. Now retries once, which
re-runs the match against whatever the winner committed.
Also added a unique constraint on unlocks(user_id, unlock_key) as
defense-in-depth, with an idempotent catalog-guarded migration.
221 backend tests pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -69,6 +69,20 @@ async def lifespan(app: FastAPI):
|
|||||||
await conn.execute(text(
|
await conn.execute(text(
|
||||||
"ALTER TABLE entities ADD COLUMN IF NOT EXISTS at_peace BOOLEAN NOT NULL DEFAULT false"
|
"ALTER TABLE entities ADD COLUMN IF NOT EXISTS at_peace BOOLEAN NOT NULL DEFAULT false"
|
||||||
))
|
))
|
||||||
|
# Defense-in-depth: purchase_unlock() already enforces one row per
|
||||||
|
# (user, unlock_key) via a row-locked check-then-insert, so this
|
||||||
|
# constraint should never actually find a conflict on a live DB.
|
||||||
|
# `ADD CONSTRAINT` has no IF NOT EXISTS form, so the guard is a
|
||||||
|
# catalog check instead — safe to run on every startup.
|
||||||
|
await conn.execute(text(
|
||||||
|
"DO $$ BEGIN "
|
||||||
|
"IF NOT EXISTS ("
|
||||||
|
" SELECT 1 FROM pg_constraint WHERE conname = 'uq_unlocks_user_key'"
|
||||||
|
") THEN "
|
||||||
|
" ALTER TABLE unlocks ADD CONSTRAINT uq_unlocks_user_key UNIQUE (user_id, unlock_key); "
|
||||||
|
"END IF; "
|
||||||
|
"END $$;"
|
||||||
|
))
|
||||||
cleanup_task = asyncio.create_task(_session_cleanup_loop())
|
cleanup_task = asyncio.create_task(_session_cleanup_loop())
|
||||||
try:
|
try:
|
||||||
yield
|
yield
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
import uuid
|
import uuid
|
||||||
from datetime import datetime, timezone
|
from datetime import datetime, timezone
|
||||||
|
|
||||||
from sqlalchemy import DateTime, ForeignKey, String
|
from sqlalchemy import DateTime, ForeignKey, String, UniqueConstraint
|
||||||
from sqlalchemy.orm import Mapped, mapped_column
|
from sqlalchemy.orm import Mapped, mapped_column
|
||||||
|
|
||||||
from app.db import Base
|
from app.db import Base
|
||||||
@@ -9,9 +9,18 @@ from app.db import Base
|
|||||||
|
|
||||||
class UnlockRecord(Base):
|
class UnlockRecord(Base):
|
||||||
"""A permanent unlock a seeker has purchased with essence (e.g. the
|
"""A permanent unlock a seeker has purchased with essence (e.g. the
|
||||||
listening tool). One row per (user, unlock_key)."""
|
listening tool). One row per (user, unlock_key).
|
||||||
|
|
||||||
|
The unique constraint is defense-in-depth: today, idempotency (no double
|
||||||
|
charge for an already-owned unlock) is enforced entirely by
|
||||||
|
`app.inventory.purchase_unlock`'s row-locked check-then-insert. This
|
||||||
|
constraint means any *other* code path that ever inserts an
|
||||||
|
UnlockRecord without going through that lock still can't create a
|
||||||
|
duplicate row for the same (user, unlock_key).
|
||||||
|
"""
|
||||||
|
|
||||||
__tablename__ = "unlocks"
|
__tablename__ = "unlocks"
|
||||||
|
__table_args__ = (UniqueConstraint("user_id", "unlock_key", name="uq_unlocks_user_key"),)
|
||||||
|
|
||||||
id: Mapped[uuid.UUID] = mapped_column(primary_key=True, default=uuid.uuid4)
|
id: Mapped[uuid.UUID] = mapped_column(primary_key=True, default=uuid.uuid4)
|
||||||
user_id: Mapped[uuid.UUID] = mapped_column(ForeignKey("users.id"), index=True)
|
user_id: Mapped[uuid.UUID] = mapped_column(ForeignKey("users.id"), index=True)
|
||||||
|
|||||||
@@ -29,6 +29,7 @@ from pathlib import Path
|
|||||||
|
|
||||||
from fastapi import APIRouter, WebSocket, WebSocketDisconnect
|
from fastapi import APIRouter, WebSocket, WebSocketDisconnect
|
||||||
from sqlalchemy import select
|
from sqlalchemy import select
|
||||||
|
from sqlalchemy.exc import IntegrityError
|
||||||
|
|
||||||
from app import judgment
|
from app import judgment
|
||||||
from app.config import settings
|
from app.config import settings
|
||||||
@@ -68,6 +69,15 @@ fragment_limiter = RateLimiter(max_requests=30, window_seconds=60)
|
|||||||
question_limiter = RateLimiter(max_requests=6, window_seconds=60)
|
question_limiter = RateLimiter(max_requests=6, window_seconds=60)
|
||||||
summon_limiter = RateLimiter(max_requests=4, window_seconds=60)
|
summon_limiter = RateLimiter(max_requests=4, window_seconds=60)
|
||||||
|
|
||||||
|
# ritual_start/judgment don't call the LLM, but each one is an essence/favor/
|
||||||
|
# item-drop reward trigger point same as summon/question — unlike those,
|
||||||
|
# they previously had no limiter at all, which meant a scripted client could
|
||||||
|
# credit itself unbounded essence by simply replaying "judgment" (or
|
||||||
|
# ritual_start -> 4x ritual_step) in a tight loop. These bound that to the
|
||||||
|
# same modest, human-plausible cadence as the other reward triggers.
|
||||||
|
ritual_limiter = RateLimiter(max_requests=6, window_seconds=60)
|
||||||
|
judgment_limiter = RateLimiter(max_requests=10, window_seconds=60)
|
||||||
|
|
||||||
# Per-IP limiters for the same trigger points (spec §5). Ollama is a shared,
|
# Per-IP limiters for the same trigger points (spec §5). Ollama is a shared,
|
||||||
# single-instance, CPU-only resource — per-account limits alone don't stop
|
# single-instance, CPU-only resource — per-account limits alone don't stop
|
||||||
# one compromised/scripted account from hammering it across many source IPs,
|
# one compromised/scripted account from hammering it across many source IPs,
|
||||||
@@ -77,6 +87,8 @@ summon_limiter = RateLimiter(max_requests=4, window_seconds=60)
|
|||||||
fragment_ip_limiter = RateLimiter(max_requests=60, window_seconds=60)
|
fragment_ip_limiter = RateLimiter(max_requests=60, window_seconds=60)
|
||||||
question_ip_limiter = RateLimiter(max_requests=12, window_seconds=60)
|
question_ip_limiter = RateLimiter(max_requests=12, window_seconds=60)
|
||||||
summon_ip_limiter = RateLimiter(max_requests=8, window_seconds=60)
|
summon_ip_limiter = RateLimiter(max_requests=8, window_seconds=60)
|
||||||
|
ritual_ip_limiter = RateLimiter(max_requests=12, window_seconds=60)
|
||||||
|
judgment_ip_limiter = RateLimiter(max_requests=20, window_seconds=60)
|
||||||
|
|
||||||
AUDIO_DIR = Path(settings.data_dir) / "audio"
|
AUDIO_DIR = Path(settings.data_dir) / "audio"
|
||||||
|
|
||||||
@@ -287,7 +299,18 @@ async def _summon(state: SeanceState, channel: str) -> tuple[Entity, bool]:
|
|||||||
str(state.session_id)
|
str(state.session_id)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Two concurrent sessions can race to mint the same signature (this is
|
||||||
|
# the whole point of the match-or-mint check above being racy across
|
||||||
|
# connections), or two mints can independently decide on the same
|
||||||
|
# "next available" name via _unique_entity_name's read-then-decide
|
||||||
|
# check — either raises IntegrityError on commit against Entity's
|
||||||
|
# unique(signature)/unique(name) constraints. Retrying re-runs the
|
||||||
|
# match against what the winning transaction just committed, so the
|
||||||
|
# loser finds and reuses that row instead of crashing the session.
|
||||||
|
last_error: IntegrityError | None = None
|
||||||
|
for _attempt in range(2):
|
||||||
async with session_maker() as db:
|
async with session_maker() as db:
|
||||||
|
try:
|
||||||
entity = await db.scalar(
|
entity = await db.scalar(
|
||||||
select(Entity).where(Entity.signature == signature, Entity.at_peace.is_(False))
|
select(Entity).where(Entity.signature == signature, Entity.at_peace.is_(False))
|
||||||
)
|
)
|
||||||
@@ -336,6 +359,13 @@ async def _summon(state: SeanceState, channel: str) -> tuple[Entity, bool]:
|
|||||||
await db.commit()
|
await db.commit()
|
||||||
await db.refresh(entity)
|
await db.refresh(entity)
|
||||||
return entity, is_new
|
return entity, is_new
|
||||||
|
except IntegrityError as exc:
|
||||||
|
await db.rollback()
|
||||||
|
last_error = exc
|
||||||
|
continue
|
||||||
|
|
||||||
|
assert last_error is not None
|
||||||
|
raise last_error
|
||||||
|
|
||||||
|
|
||||||
async def _reward_summon(state: SeanceState) -> None:
|
async def _reward_summon(state: SeanceState) -> None:
|
||||||
@@ -353,7 +383,10 @@ async def _reward_summon(state: SeanceState) -> None:
|
|||||||
rarity = state.entity.get("rarity", "common")
|
rarity = state.entity.get("rarity", "common")
|
||||||
|
|
||||||
async with session_maker() as db:
|
async with session_maker() as db:
|
||||||
user = await db.get(User, state.user_id)
|
# Locked (see purchase_unlock's docstring in app/inventory.py) so
|
||||||
|
# this read-modify-write on essence can't race a concurrent
|
||||||
|
# purchase's own locked deduction and silently clobber it.
|
||||||
|
user = await db.scalar(select(User).where(User.id == state.user_id).with_for_update())
|
||||||
if user is None:
|
if user is None:
|
||||||
return
|
return
|
||||||
credit_essence(user, SUMMON_ESSENCE_TRICKLE)
|
credit_essence(user, SUMMON_ESSENCE_TRICKLE)
|
||||||
@@ -480,7 +513,14 @@ async def _handle_question(state: SeanceState, text: str) -> None:
|
|||||||
|
|
||||||
if state.entity is None:
|
if state.entity is None:
|
||||||
await _handle_summon(state)
|
await _handle_summon(state)
|
||||||
assert state.entity is not None
|
if state.entity is None:
|
||||||
|
# _handle_summon() returns without setting state.entity when
|
||||||
|
# the seeker is rate-limited (it already sent its own
|
||||||
|
# "rate_limited" error frame in that case) — bail out here
|
||||||
|
# instead of asserting, which would raise uncaught and crash
|
||||||
|
# this session's whole WS message loop (only WebSocketDisconnect
|
||||||
|
# is caught around it in session_socket()).
|
||||||
|
return
|
||||||
|
|
||||||
text = text.strip()[:500]
|
text = text.strip()[:500]
|
||||||
await _record_event(state.session_id, "question", text=text)
|
await _record_event(state.session_id, "question", text=text)
|
||||||
@@ -582,6 +622,18 @@ RITUAL_STEPS_REQUIRED = 4
|
|||||||
async def _handle_ritual_start(state: SeanceState) -> None:
|
async def _handle_ritual_start(state: SeanceState) -> None:
|
||||||
if state.entity is None:
|
if state.entity is None:
|
||||||
return # no presence to focus on — frontend already gates the button
|
return # no presence to focus on — frontend already gates the button
|
||||||
|
if not (
|
||||||
|
ritual_limiter.allow(str(state.user_id))
|
||||||
|
and ritual_ip_limiter.allow(state.client_ip)
|
||||||
|
):
|
||||||
|
await state.send_queue.put(
|
||||||
|
{
|
||||||
|
"type": "error",
|
||||||
|
"code": "rate_limited",
|
||||||
|
"message": "The channel needs a moment to settle before it can be focused again.",
|
||||||
|
}
|
||||||
|
)
|
||||||
|
return
|
||||||
state.ritual_steps = 0
|
state.ritual_steps = 0
|
||||||
state.ritual_completed = False
|
state.ritual_completed = False
|
||||||
state.ritual_success = False
|
state.ritual_success = False
|
||||||
@@ -592,7 +644,8 @@ async def _reward_ritual_success(state: SeanceState) -> None:
|
|||||||
ritual milestone trigger point."""
|
ritual milestone trigger point."""
|
||||||
item = None
|
item = None
|
||||||
async with session_maker() as db:
|
async with session_maker() as db:
|
||||||
user = await db.get(User, state.user_id)
|
# Locked — see the same comment in _reward_summon above.
|
||||||
|
user = await db.scalar(select(User).where(User.id == state.user_id).with_for_update())
|
||||||
if user is None:
|
if user is None:
|
||||||
return
|
return
|
||||||
credit_essence(user, RITUAL_SUCCESS_ESSENCE)
|
credit_essence(user, RITUAL_SUCCESS_ESSENCE)
|
||||||
@@ -640,6 +693,18 @@ async def _handle_judgment(state: SeanceState, message: dict) -> None:
|
|||||||
verdict = message.get("verdict")
|
verdict = message.get("verdict")
|
||||||
if verdict not in judgment.VERDICTS:
|
if verdict not in judgment.VERDICTS:
|
||||||
return
|
return
|
||||||
|
if not (
|
||||||
|
judgment_limiter.allow(str(state.user_id))
|
||||||
|
and judgment_ip_limiter.allow(state.client_ip)
|
||||||
|
):
|
||||||
|
await state.send_queue.put(
|
||||||
|
{
|
||||||
|
"type": "error",
|
||||||
|
"code": "rate_limited",
|
||||||
|
"message": "The veil needs a moment before it can render another verdict.",
|
||||||
|
}
|
||||||
|
)
|
||||||
|
return
|
||||||
|
|
||||||
traits = state.entity.get("traits", {})
|
traits = state.entity.get("traits", {})
|
||||||
outcome = judgment.judge_verdict(
|
outcome = judgment.judge_verdict(
|
||||||
@@ -658,7 +723,9 @@ async def _handle_judgment(state: SeanceState, message: dict) -> None:
|
|||||||
"crossed_over",
|
"crossed_over",
|
||||||
):
|
):
|
||||||
async with session_maker() as db:
|
async with session_maker() as db:
|
||||||
user = await db.get(User, state.user_id)
|
# Locked — see the same comment in _reward_summon above; this
|
||||||
|
# path writes essence too, so it's exposed to the same race.
|
||||||
|
user = await db.scalar(select(User).where(User.id == state.user_id).with_for_update())
|
||||||
if user is not None:
|
if user is not None:
|
||||||
if outcome.favor_delta:
|
if outcome.favor_delta:
|
||||||
user.favor = judgment.clamp_favor(user.favor + outcome.favor_delta)
|
user.favor = judgment.clamp_favor(user.favor + outcome.favor_delta)
|
||||||
|
|||||||
@@ -59,6 +59,10 @@ def _fake_spirits(monkeypatch):
|
|||||||
monkeypatch.setattr(app.ws, "question_ip_limiter", RateLimiter(max_requests=1000, window_seconds=60))
|
monkeypatch.setattr(app.ws, "question_ip_limiter", RateLimiter(max_requests=1000, window_seconds=60))
|
||||||
monkeypatch.setattr(app.ws, "fragment_limiter", RateLimiter(max_requests=1000, window_seconds=60))
|
monkeypatch.setattr(app.ws, "fragment_limiter", RateLimiter(max_requests=1000, window_seconds=60))
|
||||||
monkeypatch.setattr(app.ws, "fragment_ip_limiter", RateLimiter(max_requests=1000, window_seconds=60))
|
monkeypatch.setattr(app.ws, "fragment_ip_limiter", RateLimiter(max_requests=1000, window_seconds=60))
|
||||||
|
monkeypatch.setattr(app.ws, "ritual_limiter", RateLimiter(max_requests=1000, window_seconds=60))
|
||||||
|
monkeypatch.setattr(app.ws, "ritual_ip_limiter", RateLimiter(max_requests=1000, window_seconds=60))
|
||||||
|
monkeypatch.setattr(app.ws, "judgment_limiter", RateLimiter(max_requests=1000, window_seconds=60))
|
||||||
|
monkeypatch.setattr(app.ws, "judgment_ip_limiter", RateLimiter(max_requests=1000, window_seconds=60))
|
||||||
|
|
||||||
|
|
||||||
def _read_until(ws, msg_type, max_frames=60, **match):
|
def _read_until(ws, msg_type, max_frames=60, **match):
|
||||||
|
|||||||
Reference in New Issue
Block a user