[Badri-Rajendran]

Problem

An independent security audit of the Ask Trueup chat backend (2026-09-07) found a real
check-then-act race in the per-customer daily query cap (S11 §5.2 step 1, NFR-16):

ChatUsageLimiter.check() ran its own SELECT count(...) in a transaction that committed and
closed before ChatOrchestrationService.begin_turn()'s own transaction inserted the new
message rows. Under Postgres's default READ COMMITTED isolation, two concurrent begin_turn()
calls for the same customer (e.g. two browser tabs, or two chat sessions) could each read a count
just under the cap and both proceed, letting a customer exceed the configured daily cap. Simple
reordering of the count-then-insert sequence cannot close this on its own — the two calls run in
genuinely separate transactions that cannot see each other's uncommitted writes; only a lock that
serializes them does.

Scope note: this audit also surfaced two lower-severity findings (the concurrency lock being
session-scoped rather than customer-scoped, and no upper bound on a single tool-response row's
size). Per explicit scope agreement, this PR fixes only the medium-severity race above; the other
two are left untouched.

Implementation

• ChatOrchestrationService.begin_turn() now acquires a transaction-scoped
  pg_advisory_xact_lock(hashtext(:customer_id)::bigint) as the first statement inside its unit
  of work, before the cap check. pg_advisory_xact_lock (not the session-scoped variant) is
  released automatically on commit or rollback, so it can never leak across a pooled connection.
• ChatUsageLimiter.check() now takes the caller's already-open unit of work instead of opening
  its own — the count read and the message insert now share one transaction, serialized by the
  lock above, closing the race.
• _build_orchestration_service() updated for the new ChatUsageLimiter(daily_query_cap=...)
  constructor signature (no longer takes a uow_factory).
• The lock covers only this fast bookkeeping transaction — never the slow, network-bound LLM
  stream in stream_turn() — so a customer's second tab still works normally while the first is
  actively streaming; only the "am I allowed to start a turn" decision is serialized.
• As a disclosed side effect (not separately scoped work): because the lock key is per-customer
  rather than per-session, it also happens to narrow the session-scoped-lock finding noted above,
  since two sessions for the same customer now serialize through this same lock. No additional
  change was made to address that finding directly.

Validation

• uv run ruff check and uv run mypy --strict clean on all touched files (2 pre-existing,
  unrelated mypy errors in openai_agent_adapter.py confirmed present on origin/main before
  this change, via git stash).
• uv run lint-imports: all 5 layering contracts kept.
• All 12 existing tests/api/test_chat.py tests pass unchanged, including the daily-cap and
  concurrent-turn rejection tests.
• New tests/integration/test_chat_usage_cap_race.py: proves the serialization directly — one
  thread holds the same advisory lock key while inserting the message that fills the cap; a
  second thread's begin_turn() call is shown to genuinely block on that lock (asserted via wait
  duration, not scheduling luck) and, once unblocked, correctly sees the just-committed row and
  rejects. A second test confirms the lock doesn't wrongly block a second session's turn while
  still under the cap.
• Full backend suite (uv run pytest) run twice on this branch (based on origin/main) to rule
  out flakiness from the new threading-based test: 801 passed, 7 skipped, 2 pre-existing xpassed
  (unrelated test_valuation_api.py, tracked against a separate foundation bug) both times.

Security / DB / API impact

• Security: closes a real daily-rate-limit bypass (OWASP API4: Unrestricted Resource
  Consumption) — a customer could previously exceed the configured daily chat-query cap via
  concurrent requests.
• DB: no schema change. Uses Postgres's built-in transaction-scoped advisory lock
  (pg_advisory_xact_lock), no new table or column.
• API: no request/response contract change; the existing daily_chat_query_cap_exceeded 429
  error path is unchanged, just now race-free.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.