diff --git a/__tests__/unit/server/services/credit.service.test.ts b/__tests__/unit/server/services/credit.service.test.ts index ecfb725b..130e3a40 100644 --- a/__tests__/unit/server/services/credit.service.test.ts +++ b/__tests__/unit/server/services/credit.service.test.ts @@ -8,6 +8,11 @@ import { FREE_WEEKLY_LIMIT, ONE_WEEK_MS } from '../../../../lib/credits' import type { UserProfile, CreditBalance } from '../../../../lib/types' // Mock the database module +const mockTxn = { + get: jest.fn(), + update: jest.fn(), +} + const mockDb = { get: jest.fn(), set: jest.fn(), @@ -21,6 +26,7 @@ const mockDb = { batchDelete: jest.fn(), getAll: jest.fn(), count: jest.fn(), + runTransaction: jest.fn((fn: (txn: typeof mockTxn) => Promise) => fn(mockTxn)), } jest.mock('../../../../server/db', () => ({ @@ -56,15 +62,17 @@ beforeEach(() => { describe('checkAndDeductCredit', () => { it('should deduct from free credits first', async () => { const user = makeUser() + // ensureCreditsExist uses mockDb.get; transaction uses mockTxn.get mockDb.get.mockResolvedValue(user) - mockDb.update.mockResolvedValue(undefined) + mockTxn.get.mockResolvedValue(makeUser()) + mockTxn.update.mockResolvedValue(undefined) const result = await checkAndDeductCredit('user-1') expect(result.success).toBe(true) expect(result.source).toBe('free') expect(result.remaining).toBeDefined() - expect(mockDb.update).toHaveBeenCalledWith('users', 'user-1', expect.objectContaining({ + expect(mockTxn.update).toHaveBeenCalledWith('users', 'user-1', expect.objectContaining({ credits: expect.objectContaining({ free: expect.objectContaining({ used: 1 }) }) @@ -72,32 +80,30 @@ describe('checkAndDeductCredit', () => { }) it('should deduct from banked credits when free are exhausted', async () => { - const user = makeUser({ - credits: { - free: { used: FREE_WEEKLY_LIMIT, limit: FREE_WEEKLY_LIMIT, lastResetDate: new Date().toISOString() }, - banked: 10 - } - }) - mockDb.get.mockResolvedValue(user) - mockDb.update.mockResolvedValue(undefined) + const credits = { + free: { used: FREE_WEEKLY_LIMIT, limit: FREE_WEEKLY_LIMIT, lastResetDate: new Date().toISOString() }, + banked: 10 + } + mockDb.get.mockResolvedValue(makeUser({ credits })) + mockTxn.get.mockResolvedValue(makeUser({ credits: { ...credits, banked: 10 } })) + mockTxn.update.mockResolvedValue(undefined) const result = await checkAndDeductCredit('user-1') expect(result.success).toBe(true) expect(result.source).toBe('banked') - expect(mockDb.update).toHaveBeenCalledWith('users', 'user-1', expect.objectContaining({ + expect(mockTxn.update).toHaveBeenCalledWith('users', 'user-1', expect.objectContaining({ credits: expect.objectContaining({ banked: 9 }) })) }) it('should fail when both buckets are empty', async () => { - const user = makeUser({ - credits: { - free: { used: FREE_WEEKLY_LIMIT, limit: FREE_WEEKLY_LIMIT, lastResetDate: new Date().toISOString() }, - banked: 0 - } - }) - mockDb.get.mockResolvedValue(user) + const credits = { + free: { used: FREE_WEEKLY_LIMIT, limit: FREE_WEEKLY_LIMIT, lastResetDate: new Date().toISOString() }, + banked: 0 + } + mockDb.get.mockResolvedValue(makeUser({ credits })) + mockTxn.get.mockResolvedValue(makeUser({ credits: { ...credits } })) const result = await checkAndDeductCredit('user-1') @@ -108,14 +114,13 @@ describe('checkAndDeductCredit', () => { it('should apply lazy weekly reset before deducting', async () => { const weekAgo = new Date(Date.now() - ONE_WEEK_MS - 1000).toISOString() - const user = makeUser({ - credits: { - free: { used: FREE_WEEKLY_LIMIT, limit: FREE_WEEKLY_LIMIT, lastResetDate: weekAgo }, - banked: 0 - } - }) - mockDb.get.mockResolvedValue(user) - mockDb.update.mockResolvedValue(undefined) + const credits = { + free: { used: FREE_WEEKLY_LIMIT, limit: FREE_WEEKLY_LIMIT, lastResetDate: weekAgo }, + banked: 0 + } + mockDb.get.mockResolvedValue(makeUser({ credits })) + mockTxn.get.mockResolvedValue(makeUser({ credits: { ...credits } })) + mockTxn.update.mockResolvedValue(undefined) const result = await checkAndDeductCredit('user-1') @@ -162,26 +167,29 @@ describe('getCredits', () => { describe('addBankedCredits', () => { it('should add credits to banked bucket', async () => { - const user = makeUser({ credits: { free: { used: 0, limit: 5, lastResetDate: new Date().toISOString() }, banked: 5 } }) - mockDb.get.mockResolvedValue(user) - mockDb.update.mockResolvedValue(undefined) + const credits = { free: { used: 0, limit: 5, lastResetDate: new Date().toISOString() }, banked: 5 } + // ensureCreditsExist uses mockDb.get; transaction uses mockTxn.get + mockDb.get.mockResolvedValue(makeUser({ credits })) + mockTxn.get.mockResolvedValue(makeUser({ credits: { ...credits, banked: 5 } })) + mockTxn.update.mockResolvedValue(undefined) await addBankedCredits('user-1', 20, 1000) - expect(mockDb.update).toHaveBeenCalledWith('users', 'user-1', expect.objectContaining({ + expect(mockTxn.update).toHaveBeenCalledWith('users', 'user-1', expect.objectContaining({ credits: expect.objectContaining({ banked: 25 }), lifetimeSpend: 1000 })) }) it('should accumulate lifetime spend', async () => { - const user = makeUser({ lifetimeSpend: 500, credits: { free: { used: 0, limit: 5, lastResetDate: new Date().toISOString() }, banked: 0 } }) - mockDb.get.mockResolvedValue(user) - mockDb.update.mockResolvedValue(undefined) + const credits = { free: { used: 0, limit: 5, lastResetDate: new Date().toISOString() }, banked: 0 } + mockDb.get.mockResolvedValue(makeUser({ lifetimeSpend: 500, credits })) + mockTxn.get.mockResolvedValue(makeUser({ lifetimeSpend: 500, credits: { ...credits } })) + mockTxn.update.mockResolvedValue(undefined) await addBankedCredits('user-1', 10, 1000) - expect(mockDb.update).toHaveBeenCalledWith('users', 'user-1', expect.objectContaining({ + expect(mockTxn.update).toHaveBeenCalledWith('users', 'user-1', expect.objectContaining({ lifetimeSpend: 1500 })) }) diff --git a/contexts/AuthContext.tsx b/contexts/AuthContext.tsx index 4707c21b..43e7476d 100644 --- a/contexts/AuthContext.tsx +++ b/contexts/AuthContext.tsx @@ -67,7 +67,7 @@ function getAnonymousPlayerId(): string { let id = localStorage.getItem('plottwists_player_id') if (!id) { - id = `anon_${Date.now()}_${Math.random().toString(36).substr(2, 9)}` + id = `anon_${crypto.randomUUID()}` localStorage.setItem('plottwists_player_id', id) } return id diff --git a/contexts/SocketContext.tsx b/contexts/SocketContext.tsx index d708253a..6449a466 100644 --- a/contexts/SocketContext.tsx +++ b/contexts/SocketContext.tsx @@ -172,10 +172,6 @@ export function SocketProvider({ children }: { children: React.ReactNode }) { }) } - // Expose socket for NativeBootstrap app-lifecycle reconnect - // eslint-disable-next-line @typescript-eslint/no-explicit-any - if (typeof window !== 'undefined') (window as any).__plotTwistsSocket = globalSocket - setSocket(globalSocket) } diff --git a/lib/types.ts b/lib/types.ts index f02bdede..7c8c60a9 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -110,6 +110,7 @@ export interface CardPackInput { name: string description: string author: string + authorId?: string theme: string isMature: boolean isBuiltIn: boolean diff --git a/server.ts b/server.ts index 2df491a7..2f962aa7 100644 --- a/server.ts +++ b/server.ts @@ -417,11 +417,19 @@ app.prepare().then(async () => { return } + // Only allow submissions during SELECTION phase + if (room.gameState !== 'SELECTION') { + callback({ success: false, error: 'Card selection is not active' }) + return + } + // Find player by socket ID let playerId: string | undefined + let playerRole: string | undefined for (const [id, player] of room.players.entries()) { if (player.socketId === socket.id) { playerId = id + playerRole = player.role break } } @@ -431,7 +439,20 @@ app.prepare().then(async () => { return } - room.selections.set(playerId, selections) + // Only players (not spectators) can submit card selections + if (playerRole === 'SPECTATOR') { + callback({ success: false, error: 'Spectators cannot submit card selections' }) + return + } + + // Sanitize card selections to prevent prompt injection + const sanitizedSelections = { + character: sanitizeUserInput(selections.character || '', 100), + setting: sanitizeUserInput(selections.setting || '', 100), + circumstance: sanitizeUserInput(selections.circumstance || '', 100), + } + + room.selections.set(playerId, sanitizedSelections) const player = room.players.get(playerId) if (player) { player.hasSubmittedSelection = true @@ -489,6 +510,9 @@ app.prepare().then(async () => { const room = roomService.getRoomFromCache(roomCode) if (!room) return + // Only allow voting during VOTING phase + if (room.gameState !== 'VOTING') return + // Find voter by socket ID let voterId: string | undefined for (const [id, player] of room.players.entries()) { @@ -500,6 +524,12 @@ app.prepare().then(async () => { if (!voterId) return + // Prevent self-voting + if (voterId === targetPlayerId) return + + // Validate target is an actual player in the room + if (!room.players.has(targetPlayerId)) return + room.votes.set(voterId, targetPlayerId) const voter = room.players.get(voterId) if (voter) { @@ -673,6 +703,19 @@ app.prepare().then(async () => { socket.on('player_jump_to_line', withErrorHandler(socket, 'player_jump_to_line', (roomCode, lineIndex) => { const room = validateRoom(roomCode, socket) if (!room || room.gameState !== 'PERFORMING' || !room.script) return + if (!requireRoomMember(room, socket)) return + + // Only players (not spectators) can navigate the teleprompter + let isPlayer = requireHost(room, socket) + if (!isPlayer) { + for (const player of room.players.values()) { + if (player.socketId === socket.id && player.role === 'PLAYER') { + isPlayer = true + break + } + } + } + if (!isPlayer) return // Validate line index if (lineIndex < 0 || lineIndex >= room.script.lines.length) return @@ -1260,6 +1303,8 @@ app.prepare().then(async () => { } try { + // Enforce authorId from authenticated user to prevent spoofing + packData.authorId = socket.data.uid || undefined const result = await createCardPack(packData) callback(result) } catch (error) { @@ -1294,7 +1339,7 @@ app.prepare().then(async () => { } try { - const result = await updateCardPack(packId, updates) + const result = await updateCardPack(packId, updates, socket.data.uid) callback(result) } catch (error) { logger.error('Error updating card pack:', error) @@ -1311,7 +1356,7 @@ app.prepare().then(async () => { } try { - const result = await deleteCardPack(packId) + const result = await deleteCardPack(packId, socket.data.uid) callback(result) } catch (error) { logger.error('Error deleting card pack:', error) @@ -1814,6 +1859,12 @@ app.prepare().then(async () => { // Helper function to start script generation async function startScriptGeneration(room: Room, io: SocketIOServer) { + // Guard against double-invocation race condition (two players submitting final card simultaneously) + if (room.gameState === 'LOADING' || room.gameState === 'PERFORMING') { + logger.warn(`Script generation skipped: room ${room.code} already in ${room.gameState}`) + return + } + // Rate limiting for script generation (use host socket ID) const hostSocketId = room.host.socketId if (!scriptGenerationLimiter.check(hostSocketId)) { diff --git a/server/middleware/rateLimiter.ts b/server/middleware/rateLimiter.ts index c15e34e4..498fab7b 100644 --- a/server/middleware/rateLimiter.ts +++ b/server/middleware/rateLimiter.ts @@ -50,8 +50,8 @@ export class SocketRateLimiter { ) { this.attempts = new Map() - // Cleanup old entries every minute - setInterval(() => { + // Cleanup old entries every minute (unref so it doesn't prevent process exit) + const cleanup = setInterval(() => { const now = Date.now() for (const [key, value] of this.attempts.entries()) { if (now > value.resetTime) { @@ -59,6 +59,7 @@ export class SocketRateLimiter { } } }, 60000) + cleanup.unref() } /** diff --git a/server/services/audience.service.ts b/server/services/audience.service.ts index f61e30aa..fc1e90a6 100644 --- a/server/services/audience.service.ts +++ b/server/services/audience.service.ts @@ -17,12 +17,22 @@ import type { SpectatorMessage } from '../../lib/types' import { logger } from '../../lib/logger' +import { sanitizeInput } from '../utils/validation' // Initialize Anthropic client for AI-powered twists const anthropic = new Anthropic({ apiKey: process.env.ANTHROPIC_API_KEY, }) +// Valid reaction types (runtime check since TypeScript types are erased) +const VALID_REACTION_TYPES = new Set([ + 'laugh', 'cheer', 'gasp', 'boo', 'applause', 'cringe', 'love', 'mindblown' +]) + +export function isValidReactionType(type: string): type is AudienceReactionType { + return VALID_REACTION_TYPES.has(type as AudienceReactionType) +} + // Rate limiting for reactions (per user) const reactionCooldowns = new Map() const REACTION_COOLDOWN_MS = 2000 // 2 seconds between reactions @@ -102,6 +112,11 @@ export function recordReaction( senderId: string, senderName: string ): AudienceReaction | null { + // Runtime validation: prevent arbitrary key injection into reactionCounts + if (!isValidReactionType(type)) { + return null + } + if (!canSendReaction(senderId)) { return null } @@ -316,7 +331,7 @@ export function recordSpectatorMessage( if (!canSendSpectatorMessage(senderId)) return null // Sanitize and limit text length - const sanitized = text.trim().slice(0, 100) + const sanitized = sanitizeInput(text, 100) if (!sanitized) return null const message: SpectatorMessage = { @@ -368,8 +383,9 @@ export function cleanupCooldowns(): void { } } -// Run cleanup every 5 minutes -setInterval(cleanupCooldowns, 5 * 60 * 1000) +// Run cleanup every 5 minutes (unref so it doesn't prevent process exit) +const cleanupInterval = setInterval(cleanupCooldowns, 5 * 60 * 1000) +cleanupInterval.unref() // ============================================================ // AI-Powered Plot Twist Generation diff --git a/server/services/cardpack.service.ts b/server/services/cardpack.service.ts index 3f438a39..b9ad7b5f 100644 --- a/server/services/cardpack.service.ts +++ b/server/services/cardpack.service.ts @@ -186,12 +186,19 @@ export async function createCardPack( return { success: true, packId } } +// Allowed fields for card pack updates (prevents field injection) +const ALLOWED_UPDATE_FIELDS = new Set([ + 'name', 'description', 'theme', 'isMature', 'isPublic', + 'characters', 'settings', 'circumstances' +]) + /** * Update an existing card pack */ export async function updateCardPack( packId: string, - updates: Record + updates: Record, + requesterId?: string | null ): Promise<{ success: boolean, error?: string }> { const db = getDatabase() const pack = await db.get(Collections.CARD_PACKS, packId) @@ -204,28 +211,41 @@ export async function updateCardPack( return { success: false, error: 'Cannot modify built-in packs' } } + // Authorization: only the author can update their pack + if (!requesterId || pack.authorId !== requesterId) { + return { success: false, error: 'Only the pack author can update this pack' } + } + + // Whitelist allowed fields to prevent overwriting id, isBuiltIn, rating, authorId, etc. + const safeUpdates: Record = {} + for (const key of Object.keys(updates)) { + if (ALLOWED_UPDATE_FIELDS.has(key)) { + safeUpdates[key] = updates[key] + } + } + // Ensure cards have IDs - if (updates.characters && Array.isArray(updates.characters)) { - updates.characters = (updates.characters as Array<{id?: string, name: string, description?: string}>).map(c => ({ + if (safeUpdates.characters && Array.isArray(safeUpdates.characters)) { + safeUpdates.characters = (safeUpdates.characters as Array<{id?: string, name: string, description?: string}>).map(c => ({ ...c, id: c.id || uuidv4() })) } - if (updates.settings && Array.isArray(updates.settings)) { - updates.settings = (updates.settings as Array<{id?: string, name: string, description?: string}>).map(s => ({ + if (safeUpdates.settings && Array.isArray(safeUpdates.settings)) { + safeUpdates.settings = (safeUpdates.settings as Array<{id?: string, name: string, description?: string}>).map(s => ({ ...s, id: s.id || uuidv4() })) } - if (updates.circumstances && Array.isArray(updates.circumstances)) { - updates.circumstances = (updates.circumstances as Array<{id?: string, name: string, description?: string}>).map(c => ({ + if (safeUpdates.circumstances && Array.isArray(safeUpdates.circumstances)) { + safeUpdates.circumstances = (safeUpdates.circumstances as Array<{id?: string, name: string, description?: string}>).map(c => ({ ...c, id: c.id || uuidv4() })) } - // Apply updates - const updatedPack = { ...pack, ...updates, updatedAt: Date.now() } + // Apply only whitelisted updates + const updatedPack = { ...pack, ...safeUpdates, updatedAt: Date.now() } await db.set(Collections.CARD_PACKS, packId, updatedPack) return { success: true } @@ -234,7 +254,10 @@ export async function updateCardPack( /** * Delete a card pack */ -export async function deleteCardPack(packId: string): Promise<{ success: boolean, error?: string }> { +export async function deleteCardPack( + packId: string, + requesterId?: string | null +): Promise<{ success: boolean, error?: string }> { const db = getDatabase() const pack = await db.get(Collections.CARD_PACKS, packId) @@ -246,6 +269,11 @@ export async function deleteCardPack(packId: string): Promise<{ success: boolean return { success: false, error: 'Cannot delete built-in packs' } } + // Authorization: only the author can delete their pack + if (!requesterId || pack.authorId !== requesterId) { + return { success: false, error: 'Only the pack author can delete this pack' } + } + await db.delete(Collections.CARD_PACKS, packId) logger.info(`Deleted card pack: ${packId}`)