From 5c546a1473ff6e4bf06d63ca4e1062f3967b3b62 Mon Sep 17 00:00:00 2001 From: jwinterm Date: Sun, 31 May 2026 09:36:09 -0400 Subject: [PATCH] fix: recover unconsumed single-game payments; make game preservation deterministic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two disconnect-related money/UX gaps: 1. A confirmed single_game payment whose game never started (user paid, then disconnected) was money-in-nothing-given with no recovery path. recover- PendingPayments now detects confirmed single_game payments with no linked games row and grants the equivalent credits — idempotently (per-payment reason key, row-locked re-check of both the game link and prior recovery). 2. In-progress games could be dropped on disconnect when a fresh getBySocket lookup failed. The stable dbUserId is now stamped on the game at creation, so suspend/restore keys on it directly without a failure-prone disconnect- time lookup. Any game (paid or free) is preserved/reconnectable whenever a session exists; the remaining drop-branch is only the true no-session edge (e.g. DB unavailable at connect), which is logged for audit rather than masked. Adds singleGameRecovery.test.js. Full suite: 13 suites / 119 tests. --- docs/REMEDIATION_PLAN.md | 2 +- src/network/sessionManager.js | 73 ++++++++++++++++++++++++++-- src/network/socketHandlers.js | 59 +++++++++++++++-------- test/singleGameRecovery.test.js | 85 +++++++++++++++++++++++++++++++++ 4 files changed, 193 insertions(+), 26 deletions(-) create mode 100644 test/singleGameRecovery.test.js diff --git a/docs/REMEDIATION_PLAN.md b/docs/REMEDIATION_PLAN.md index da4a9b0..5b51cfd 100644 --- a/docs/REMEDIATION_PLAN.md +++ b/docs/REMEDIATION_PLAN.md @@ -21,7 +21,7 @@ Key: `S` ≤ half day · `M` ~1–3 days · `L` ~1 week+ · 🔴 launch-blocker - [x] **0.2 🔴 M — Make provably-fair real.** ✅ Per-game seeded RNG (`seedToInt`) threaded through `dungeon.js` (Digger reseed + all `Math.random()` → `rng`), `monster.js` (`seededShuffle`), `game.js` (chase + generation). `DungeonGenerator.regenerateFromSeed` + `layoutFingerprint`; `/api/verify` regenerates & returns fingerprint. Determinism test suite added (`provablyFair.test.js`). _Verifier can now reproduce the exact dungeon from the seed._ - [x] **0.3 🔴 M — Fix batch-payout `tx_hash` collision + double-pay.** ✅ Migration 015 drops the payouts `tx_hash` unique index (batches legitimately share a tx_hash) and extends the one-per-game guard to cover `processing`. Batch completion now happens in ONE transaction via `id = ANY` (no row stranded mid-batch); BIGINT stat math (no float). Ambiguous batch failures → `needs_review` (retry service won't re-send) + operator alert. New `batchPayout.test.js` covers both paths. - [x] **0.4 🔴 S — Harden identity transport.** ✅ Client sends `anon_token` in handshake `auth` (not the URL query → no proxy/referer/access-log leakage); server reads `auth` first with a query-string fallback for older clients. Token is now **rotated on every resume** (`rotateToken`), so a leaked token is invalidated when the real owner reconnects; client already persists the returned token. Rotation + old-token-replay-rejection test added to `security.test.js`. -- [ ] **0.5 🟠 S — Single-game disconnect recovery.** Extend `recoverPendingPayments` (`src/network/sessionManager.js:158`) to handle `single_game`; stop silently deleting in-progress paid games (`src/network/socketHandlers.js:787`). +- [x] **0.5 🟠 S — Single-game disconnect recovery.** ✅ `recoverPendingPayments` now also recovers confirmed-but-unconsumed `single_game` payments (no `games.payment_id` link) by granting the equivalent credits, idempotently (per-payment reason key, row-locked). Disconnect preservation made deterministic: the stable `dbUserId` is **stamped on the game at creation**, so suspend/restore never needs a fragile disconnect-time DB lookup — every game (paid or free) is reconnectable whenever a session exists; the drop-branch is now only the genuine no-session edge. New `singleGameRecovery.test.js` (grant / consumed / idempotent). - [ ] **0.6 🟠 S — Client + admin XSS and CSP.** Escape chat/admin fields (`html/js/network/socketHandlers.js:291,323`; `html/admin.html`); add CSP headers. ## Phase 1 — Money correctness hardening (🟠) diff --git a/src/network/sessionManager.js b/src/network/sessionManager.js index 647cb9c..8a2a65a 100644 --- a/src/network/sessionManager.js +++ b/src/network/sessionManager.js @@ -213,11 +213,9 @@ class SessionManager { }); } - if (unprocessedPayments.rows.length === 0) { - return result; - } - - for (const payment of unprocessedPayments.rows) { + // NOTE: do not early-return when there are no credits_package payments — the + // single_game recovery below must still run. + for (const payment of (unprocessedPayments.rows.length === 0 ? [] : unprocessedPayments.rows)) { try { // Determine credits: use stored value from orphaned record, or parse from description let creditsToAdd = payment._creditsFromRecord || 0; @@ -304,6 +302,71 @@ class SessionManager { } } + // PHASE 0.5: Recover confirmed single_game payments that were never consumed. + // If a user paid for a single game and disconnected before a game started (no + // games row references the payment), the money was taken with nothing given and + // there was previously no recovery path. Grant the equivalent credits (one paid + // game) so they get what they paid for. Idempotent via a per-payment reason key. + try { + const creditsPerGame = (this.gameModeManager && this.gameModeManager.creditsPerGameCost) || 1; + const unconsumed = await this.db.query(` + SELECT p.id, p.confirmed_at + FROM payments p + WHERE p.user_id = $1 + AND p.status = 'confirmed' + AND p.payment_type = 'single_game' + AND p.confirmed_at > NOW() - INTERVAL '7 days' + AND NOT EXISTS (SELECT 1 FROM games g WHERE g.payment_id = p.id) + AND NOT EXISTS ( + SELECT 1 FROM credit_transactions ct + WHERE ct.user_id = p.user_id AND ct.reason = 'single_game_recovered:' || p.id + ) + ORDER BY p.confirmed_at ASC + `, [userId]); + + for (const payment of unconsumed.rows) { + try { + const recovered = await this.db.withTransaction(async (client) => { + // Lock the payment and re-check under the lock that it is still unconsumed + // and not already recovered (prevents double-credit across instances/races). + const lock = await client.query(`SELECT id FROM payments WHERE id = $1 FOR UPDATE`, [payment.id]); + if (!lock.rows[0]) return null; + + const gameCheck = await client.query(`SELECT 1 FROM games WHERE payment_id = $1 LIMIT 1`, [payment.id]); + if (gameCheck.rows.length > 0) return null; // a game was started for it after all + + const reason = `single_game_recovered:${payment.id}`; + const dup = await client.query( + `SELECT 1 FROM credit_transactions WHERE user_id = $1 AND reason = $2 LIMIT 1`, + [userId, reason] + ); + if (dup.rows.length > 0) return null; // already recovered + + const upd = await client.query( + `UPDATE users SET credits = credits + $1 WHERE id = $2 RETURNING credits`, + [creditsPerGame, userId] + ); + const newBalance = upd.rows[0]?.credits ?? 0; + await client.query(` + INSERT INTO credit_transactions (user_id, amount, reason, balance_after, transaction_type) + VALUES ($1, $2, $3, $4, 'recovery') + `, [userId, creditsPerGame, reason, newBalance]); + return { creditsToAdd: creditsPerGame }; + }); + + if (recovered) { + result.creditsRecovered += recovered.creditsToAdd; + result.paymentsProcessed++; + console.log(`💰 [PaymentRecovery] Recovered unconsumed single_game payment ${payment.id} as ${recovered.creditsToAdd} credit(s) for user ${userId}`); + } + } catch (e) { + console.error(`[PaymentRecovery] Failed to recover single_game payment ${payment.id}:`, e.message); + } + } + } catch (e) { + console.error('[PaymentRecovery] single_game recovery error:', e.message); + } + if (result.creditsRecovered > 0) { console.log(`💰 [PaymentRecovery] Total recovered for user ${userId}: ${result.creditsRecovered} credits from ${result.paymentsProcessed} payments`); } diff --git a/src/network/socketHandlers.js b/src/network/socketHandlers.js index aa4ff00..bb847bd 100644 --- a/src/network/socketHandlers.js +++ b/src/network/socketHandlers.js @@ -86,7 +86,19 @@ class SocketHandlers { debugManager: this.debugManager, broadcastManager: this.broadcastManager, io: this.io, - createGameForUser: (userObj, gameType, options) => this.gameManager.createGameForUser(userObj, gameType, options), + createGameForUser: async (userObj, gameType, options) => { + const game = await this.gameManager.createGameForUser(userObj, gameType, options); + // Stamp the stable DB user id onto the game at creation. Suspend/restore keys + // on this id; capturing it now (when we reliably have the session) means the + // disconnect path never needs a fresh, failure-prone DB lookup to preserve a + // game — so ANY game, paid or free, is reconnectable. + try { + const sid = userObj?.id; + const dbUid = sid != null ? this.sessionManager?.sessions?.get(sid)?.id : null; + if (game && dbUid != null) game.dbUserId = dbUid; + } catch (_) {} + return game; + }, getUserBySocket: (socketId) => this.connectionHandler.getUserBySocket(socketId), activeGames: this.activeGames, gameModeManager: this.gameModeManager, @@ -752,38 +764,45 @@ class SocketHandlers { this.connectionHandler.handleDisconnect(socket, async (socket) => { const socketId = socket.id; - // Try to get DB user ID for session-based preservation - let dbUserId = null; - if (this.sessionManager) { - try { - const user = await this.sessionManager.getBySocket(socketId); - dbUserId = user?.id; - } catch (e) { - // Non-fatal, proceed without DB user + const activeGame = this.activeGames.get(socketId); + + // Resolve the stable DB user id that suspend/restore keys on. Preference order: + // 1. the id stamped on the game at creation (always present for any game + // started with a session) — no lookup, can't fail, + // 2. the in-memory session cache, + // 3. a DB lookup as a last resort. + // Every game — paid or free — is preserved and reconnectable as long as a stable + // id resolves, which it does whenever a session was ever established. + let dbUserId = activeGame?.dbUserId || null; + if (!dbUserId && this.sessionManager) { + try { dbUserId = this.sessionManager.sessions?.get(socketId)?.id || null; } catch (_) {} + if (!dbUserId) { + try { const u = await this.sessionManager.getBySocket(socketId); dbUserId = u?.id || null; } catch (_) {} } } - - // Check if user has an active game that should be suspended - const activeGame = this.activeGames.get(socketId); + if (activeGame && dbUserId && this.suspendedGameManager) { - // Suspend the game for potential reconnection + // Suspend the game so it can be restored on reconnect (keyed by dbUserId). const suspended = this.suspendedGameManager.suspendGame(dbUserId, socketId, activeGame, { paymentMonitoringActive: this.paymentHandlers?.hasActiveMonitoring?.(socketId) || false }); - + this.activeGames.delete(socketId); if (suspended) { - // Remove from activeGames (now in suspended state) - this.activeGames.delete(socketId); - if (this.debugManager.CONSOLE_LOGGING) { console.log(`[SocketHandlers] Game suspended for user ${dbUserId} (socket: ${socketId})`); } } else { - // Couldn't suspend, just delete - this.activeGames.delete(socketId); + console.error(`[SocketHandlers] suspendGame returned false for user ${dbUserId} (socket ${socketId}); game ${activeGame.id || 'unknown'} not restorable.`); } } else { - // No DB user or no game, just clean up + if (activeGame) { + // Reachable only when no stable identity resolved at all — i.e. a session + // was never established (e.g. DB unavailable at connect), so there is + // genuinely no id to restore the game under. Record it for auditing. + console.error(`[SocketHandlers] Active game dropped without suspension — no stable user id ` + + `(socket=${socketId}, mode=${activeGame.paymentMode || activeGame.gameMode || 'unknown'}, ` + + `gameId=${activeGame.id || 'unknown'}). Cannot restore a game without a session identity.`); + } this.activeGames.delete(socketId); } diff --git a/test/singleGameRecovery.test.js b/test/singleGameRecovery.test.js new file mode 100644 index 0000000..f3d434e --- /dev/null +++ b/test/singleGameRecovery.test.js @@ -0,0 +1,85 @@ +/** + * Single-game disconnect recovery tests (Phase 0.5). + * + * If a user pays for a single game and disconnects before a game starts (no games row + * references the payment), the money was taken with nothing given. recoverPendingPayments + * must grant the equivalent credits — exactly once (idempotent), and never for a payment + * that was actually consumed by a game. + */ + +const SessionManager = require('../src/network/sessionManager'); + +function makeDb({ singleGamePayments = [], gamesForPayment = new Set(), recoveredReasons = new Set() }) { + let credits = 0; + const handler = async (text, params = []) => { + // credits_package recovery queries (unprocessed + orphaned) — none in these tests + if (/FROM payments[\s\S]*credits_package/i.test(text)) return { rows: [] }; + // outer "unconsumed single_game" discovery query + if (/payment_type = 'single_game'/i.test(text)) { + return { + rows: singleGamePayments + .filter(p => !gamesForPayment.has(p.id) && !recoveredReasons.has(`single_game_recovered:${p.id}`)) + .map(p => ({ id: p.id, confirmed_at: p.confirmed_at })) + }; + } + if (/SELECT id FROM payments WHERE id = \$1 FOR UPDATE/i.test(text)) return { rows: [{ id: params[0] }] }; + if (/SELECT 1 FROM games WHERE payment_id/i.test(text)) return { rows: gamesForPayment.has(params[0]) ? [{}] : [] }; + if (/FROM credit_transactions WHERE user_id = \$1 AND reason = \$2/i.test(text)) { + return { rows: recoveredReasons.has(params[1]) ? [{}] : [] }; + } + if (/UPDATE users SET credits = credits \+/i.test(text)) { credits += Number(params[0]); return { rows: [{ credits }] }; } + if (/INSERT INTO credit_transactions/i.test(text)) { recoveredReasons.add(params[2]); return { rows: [] }; } + return { rows: [] }; + }; + return { + _getCredits: () => credits, + query: jest.fn(handler), + withTransaction: jest.fn(async (cb) => cb({ query: jest.fn(handler) })) + }; +} + +function makeSession(db) { + return new SessionManager({ + db, + debugManager: { CONSOLE_LOGGING: false }, + gameModeManager: { creditsPerGameCost: 1 } + }); +} + +describe('Single-game disconnect recovery', () => { + test('grants a credit for a confirmed-but-unconsumed single_game payment', async () => { + const db = makeDb({ singleGamePayments: [{ id: 42, confirmed_at: new Date() }] }); + const sm = makeSession(db); + + const result = await sm.recoverPendingPayments(7, 'sock-1'); + + expect(result.creditsRecovered).toBe(1); + expect(result.paymentsProcessed).toBe(1); + expect(db._getCredits()).toBe(1); + }); + + test('does NOT grant when a game already consumed the payment', async () => { + const db = makeDb({ + singleGamePayments: [{ id: 42, confirmed_at: new Date() }], + gamesForPayment: new Set([42]) + }); + const sm = makeSession(db); + + const result = await sm.recoverPendingPayments(7, 'sock-1'); + + expect(result.creditsRecovered).toBe(0); + expect(db._getCredits()).toBe(0); + }); + + test('is idempotent — a second recovery does not double-credit', async () => { + const db = makeDb({ singleGamePayments: [{ id: 42, confirmed_at: new Date() }] }); + const sm = makeSession(db); + + const first = await sm.recoverPendingPayments(7, 'sock-1'); + const second = await sm.recoverPendingPayments(7, 'sock-2'); + + expect(first.creditsRecovered).toBe(1); + expect(second.creditsRecovered).toBe(0); + expect(db._getCredits()).toBe(1); + }); +});