fix(02-bot-skeleton-initdata-security): WR-03 honest rotate-on-new invariant + best-effort post-mint reconciliation (fold in IN-01b dead branch, IN-01c Phase-4 notes)

bot/src/kv/sessions.ts

@@ -10,8 +10,26 @@
  *     language_code. Nothing else is ever serialized to storage.
  *   - Tokens are opaque by construction: `crypto.randomUUID()` (T-02-05).
  *   - Rotate-on-new (D-07): a fresh validated identity for a chat deletes
- *     the previous `session:<token>` record before minting — one live
- *     token per chat, structurally closing the replay mint window.
+ *     the previous `session:<token>` record before minting, then runs one
+ *     best-effort post-mint reconciliation (WR-03).
+ *
+ * Honest invariant under KV semantics (no transactions, eventual
+ * consistency — WR-03 corrected the earlier "structurally closed" claim):
+ *   - one *intended* live token per chat: the `chat:<chat_id>` pointer's
+ *     target at any sequential moment;
+ *   - interleaved mints (two requests racing the same pre-write pointer)
+ *     converge at mint time or at next use — the per-mint sweep deletes
+ *     the pointer's predecessor, the post-mint reconciliation re-reads
+ *     and sweeps when a racing mint's pointer put won, and any record
+ *     orphaned by a lost race is invisible to both sweeps and dies with
+ *     the 900 s TTL (PRIV-02), never anything longer;
+ *   - this is not a privilege escalation: minting requires an identity
+ *     that is already valid-by-HMAC (D-13) and the claim stored is that
+ *     same identity's own (D-08) — an extra concurrent token only extends
+ *     validity the holder already has, 15 minutes at most;
+ *   - Phase 4 must NOT build a nonce-store or single-session assumption
+ *     on top of the sequential shape alone: worst case is a handful of
+ *     concurrently live same-identity tokens, every one bounded by 900 s.
  *
  * Key scheme:
  *   session:<token>   → JSON SessionClaim record
@@ -42,28 +60,46 @@ export interface MintSessionKv extends KvLike {}
 
 /**
  * Mint an opaque token for `claim` and rotate (D-07): read the chat
- * pointer, delete the previous session record, write the new session
- * record, write the new pointer. Both writes carry `expirationTtl`
- * (PRIV-02 — see the call lines; anything less is a review-blocking gate).
+ * pointer, sweep the previous session record, write the new session
+ * record, write the new pointer — then reconcile (WR-03): re-read the
+ * pointer once and sweep whatever it names if it is not this mint. Under
+ * sequential operation the reconciliation is a no-op; if an interleaved
+ * mint's pointer put landed after ours, its record is swept here (that
+ * client re-mints on next use). Reconciliation is best-effort — it must
+ * never fail a mint that already succeeded; any residue TTLs out at
+ * 900 s regardless. Both writes carry `expirationTtl` (PRIV-02 — see the
+ * call lines; anything less is a review-blocking gate).
  */
 export async function mintSession(kv: MintSessionKv, claim: SessionClaim): Promise<string> {
   const token = crypto.randomUUID();
   const previousToken = await kv.get(`chat:${claim.chat_id}`);
-  if (previousToken !== null && previousToken !== token) {
+  if (previousToken !== null) {
     await kv.delete(`session:${previousToken}`); // D-07 rotate-on-new
   }
   await kv.put(`session:${token}`, JSON.stringify(claim), { expirationTtl: SESSION_TTL_SECONDS });
   await kv.put(`chat:${claim.chat_id}`, token, { expirationTtl: SESSION_TTL_SECONDS });
+  try {
+    const settled = await kv.get(`chat:${claim.chat_id}`);
+    if (settled !== null && settled !== token) {
+      await kv.delete(`session:${settled}`); // WR-03 best-effort reconciliation sweep
+    }
+  } catch {
+    // best-effort: TTL (SESSION_TTL_SECONDS) bounds any unreconciled residue
+  }
   return token;
 }
 
-/** Current live token for a chat (the pointer target), or null if none. */
+/** Current live token for a chat (the pointer target), or null if none.
+ *  Tests consume it today; the production caller is the Phase 4 Mini App
+ *  flow's claim validation (IN-01 — do not delete as dead code). */
 export async function getTokenForChat(kv: MintSessionKv, chatId: number): Promise<string | null> {
   return kv.get(`chat:${chatId}`);
 }
 
 /** Claim for a token, or null when unknown/expired — callers treat null
- *  as the "expired" rejection class (D-14 vocabulary). Reads never write. */
+ *  as the "expired" rejection class (D-14 vocabulary). Reads never write.
+ *  Tests consume it today; the production caller is the Phase 4 Mini App
+ *  flow (IN-01 — do not delete as dead code). */
 export async function getClaimByToken(kv: MintSessionKv, token: string): Promise<SessionClaim | null> {
   const raw = await kv.get(`session:${token}`);
   if (raw === null) return null;

bot/src/webhook.ts

@@ -16,10 +16,11 @@
  *     posts a verbatim Telegram.WebApp.initData string — capped at 4 KB
  *     (WR-01: over-cap bodies are rejected 413 before any parsing; real
  *     initData is ≪ 1 KB, so this touches no legitimate client); only an
- *     HMAC-validated,
- *     fresh identity obtains a one-time opaque session token (D-06). Rotation
- *     of any prior token for the chat is mintSession's own invariant (D-07) —
- *     this endpoint adds no extra branch and no extra write path (T-02-06).
+ *     HMAC-validated, fresh identity obtains a one-time opaque session
+ *     token (D-06). Rotation of any prior token for the chat is mintSession's
+ *     own invariant (D-07 — best-effort under KV eventual consistency, see
+ *     sessions.ts) — this endpoint adds no extra branch and no extra write
+ *     path (T-02-06).
  * Anything else → 404.
  *
  * D-14: the ONLY log statements are category-only — malformed/tampered/stale

bot/test/sessions.test.mjs

@@ -12,6 +12,9 @@
  *   SM2 mint: every put carries expirationTtl === SESSION_TTL_SECONDS (900)
  *   SR1 rotate: second mint for same chat removes the old session key
  *   SR2 rotate: pointer targets the new token; exactly one live record
+ *   SR3 the honest KV limit (WR-03): interleaved mints racing the same
+ *       pointer can orphan a loser record for at most SESSION_TTL_SECONDS;
+ *       the next sequential mint sweeps the pointer's predecessor
  *   SG1 getClaimByToken round-trips (language_code "ms" and null)
  *   SG2 unknown token → null (the "expired" rejection class, D-14)
  */
@@ -82,7 +85,7 @@ test("SM2 mint: every put carries expirationTtl === 900 (PRIV-02 by construction
   }
 });
 
-/* ————— SR1/SR2 — rotate-on-new (T-02-02 replay close) ————— */
+/* ————— SR1/SR2/SR3 — rotate-on-new (T-02-02 replay close) ————— */
 
 test("SR1 rotate: minting again for the same chat removes the old session key", async () => {
   const kv = makeKV();
@@ -96,17 +99,80 @@ test("SR1 rotate: minting again for the same chat removes the old session key",
   );
 });
 
-test("SR2 rotate: pointer targets the new token; exactly one live session record", async () => {
+test("SR2 rotate (sequential): pointer targets the new token; exactly one live session record", async () => {
   const kv = makeKV();
   const token1 = await mod.mintSession(kv, claim(104, "en"));
   const token2 = await mod.mintSession(kv, claim(104, "en"));
   const pointer = await mod.getTokenForChat(kv, 104);
   assert.equal(pointer, token2, "chat pointer must point at the newest token");
   const liveSessions = [...kv.map.keys()].filter((k) => k.startsWith("session:"));
-  assert.equal(liveSessions.length, 1, "one live token per chat (D-07 invariant)");
+  assert.equal(
+    liveSessions.length,
+    1,
+    "one live token per chat (D-07) — sequential shape; see sessions.ts header for the WR-03 eventual-consistency caveat",
+  );
   assert.equal(liveSessions[0], `session:${token2}`);
 });
 
+/** SR3 emulation of the KV race the review confirmed (no transactions,
+ *  eventual consistency): both racing mints read the SAME pre-write
+ *  pointer, so neither sees the other's writes. B's pointer read is parked
+ *  mid-flight — it snapshots `chat:<id>` before any write — while A's
+ *  entire mint lands; when B resumes, its stale view makes it blind to
+ *  A's token, exactly the stale-replica shape under KV. */
+test("SR3 (WR-03, documented KV limit): interleaved mints can orphan a loser record for ≤ SESSION_TTL_SECONDS; the next sequential mint sweeps the pointer's predecessor", async () => {
+  const kv = makeKV();
+  const token1 = await mod.mintSession(kv, claim(108, "en"));
+  assert.equal(await mod.getTokenForChat(kv, 108), token1);
+
+  let releaseB;
+  const gate = new Promise((resolve) => (releaseB = resolve));
+  const origGet = kv.get.bind(kv);
+  let pointerReads = 1; // token1's mint consumed read #1 of chat:108
+  kv.get = async (key) => {
+    if (key === "chat:108" && (pointerReads += 1) === 3) {
+      // B's read: snapshot the pre-write pointer (token1), then park until
+      // A's entire mint has landed — a replica serving stale data.
+      const stale = await origGet(key);
+      await gate;
+      return stale;
+    }
+    return origGet(key);
+  };
+
+  const promiseA = mod.mintSession(kv, claim(108, "en", 1700000050)); // A: read #2, unparked
+  const promiseB = mod.mintSession(kv, claim(108, "en", 1700000060)); // B: read #3, parked
+  const tokenA = await promiseA; // A's mint lands fully while B is parked
+  releaseB(); // B's stale snapshot resolves — it saw token1, never tokenA
+  const tokenB = await promiseB;
+
+  // last pointer write wins
+  assert.equal(await mod.getTokenForChat(kv, 108), tokenB);
+  // THE HONEST LIMIT: A lost the pointer-write race, so its record is
+  // unreferenced yet live — bounded by the 900 s TTL, nothing more.
+  assert.equal(
+    kv.map.has(`session:${tokenA}`),
+    true,
+    "losers of a pointer-write race survive until TTL (WR-03 corrected claim — NOT structurally closed)",
+  );
+  assert.equal(kv.map.has(`session:${tokenB}`), true, "the pointer-named token is live");
+
+  // next sequential mint sweeps the pointer's predecessor (reconcile at
+  // next use — the shape that keeps the chat converging)
+  const tokenC = await mod.mintSession(kv, claim(108, "en", 1700000070));
+  assert.equal(await mod.getTokenForChat(kv, 108), tokenC);
+  assert.equal(
+    kv.map.has(`session:${tokenB}`),
+    false,
+    "the pointer's predecessor is swept at the next mint",
+  );
+  assert.equal(
+    kv.map.has(`session:${tokenA}`),
+    true,
+    "an unreferenced orphan is invisible to the pointer sweep — TTL (900 s) is the only bound (see sessions.ts header)",
+  );
+});
+
 /* ————— SG1/SG2 — claim reads (expired class, D-14) ————— */
 
 test("SG1 getClaimByToken round-trips language_code 'ms' and null", async () => {