diff --git a/src/core/execution/AttackExecution.ts b/src/core/execution/AttackExecution.ts index 0e3d6eb065..e6593cb898 100644 --- a/src/core/execution/AttackExecution.ts +++ b/src/core/execution/AttackExecution.ts @@ -205,11 +205,14 @@ export class AttackExecution implements Execution { const survivors = this.attack.troops() - deaths; this._owner.addTroops(survivors); + + // BUG-01: Check retreated() before delete() to preserve attack state for stat recording + const wasRetreated = this.attack.retreated(); this.attack.delete(); this.active = false; // Not all retreats are canceled attacks - if (this.attack.retreated()) { + if (wasRetreated) { // Record stats this.mg.stats().attackCancel(this._owner, this.target, survivors); } diff --git a/src/core/execution/WinCheckExecution.ts b/src/core/execution/WinCheckExecution.ts index 8ea4a001ab..64330b0295 100644 --- a/src/core/execution/WinCheckExecution.ts +++ b/src/core/execution/WinCheckExecution.ts @@ -68,9 +68,14 @@ export class WinCheckExecution implements Execution { (this.mg.ticks() - this.mg.config().numSpawnPhaseTurns()) / 10; const numTilesWithoutFallout = this.mg.numLandTiles() - this.mg.numTilesWithFallout(); + // Guard against division by zero when all land tiles have fallout + // Use integer cross-multiplication to avoid floating-point determinism issues + const exceedsThreshold = + numTilesWithoutFallout > 0 && + max.numTilesOwned() * 100 > + this.mg.config().percentageTilesOwnedToWin() * numTilesWithoutFallout; if ( - (max.numTilesOwned() / numTilesWithoutFallout) * 100 > - this.mg.config().percentageTilesOwnedToWin() || + exceedsThreshold || (this.mg.config().gameConfig().maxTimerValue !== undefined && timeElapsed - this.mg.config().gameConfig().maxTimerValue! * 60 >= 0) || timeElapsed >= WinCheckExecution.HARD_TIME_LIMIT_SECONDS @@ -104,9 +109,14 @@ export class WinCheckExecution implements Execution { (this.mg.ticks() - this.mg.config().numSpawnPhaseTurns()) / 10; const numTilesWithoutFallout = this.mg.numLandTiles() - this.mg.numTilesWithFallout(); - const percentage = (max[1] / numTilesWithoutFallout) * 100; + // Guard against division by zero when all land tiles have fallout + // Use integer cross-multiplication to avoid floating-point determinism issues + const exceedsThreshold = + numTilesWithoutFallout > 0 && + max[1] * 100 > + this.mg.config().percentageTilesOwnedToWin() * numTilesWithoutFallout; if ( - percentage > this.mg.config().percentageTilesOwnedToWin() || + exceedsThreshold || (this.mg.config().gameConfig().maxTimerValue !== undefined && timeElapsed - this.mg.config().gameConfig().maxTimerValue! * 60 >= 0) || timeElapsed >= WinCheckExecution.HARD_TIME_LIMIT_SECONDS diff --git a/src/core/game/GameMap.ts b/src/core/game/GameMap.ts index 592d02ca40..f59a7b8069 100644 --- a/src/core/game/GameMap.ts +++ b/src/core/game/GameMap.ts @@ -373,6 +373,12 @@ export class GameMapImpl implements GameMap { } return tiles; } + /** + * Flood-fill search starting from `tile`, collecting all connected tiles + * accepted by `filter`. Uses stack-based (DFS) traversal for performance + * — Array.pop() is O(1) vs Array.shift() O(n). The returned Set is + * identical regardless of traversal order. + */ bfs( tile: TileRef, filter: (gm: GameMap, tile: TileRef) => boolean, @@ -385,8 +391,7 @@ export class GameMapImpl implements GameMap { } while (q.length > 0) { - const curr = q.pop(); - if (curr === undefined) continue; + const curr = q.pop()!; for (const n of this.neighbors(curr)) { if (!seen.has(n) && filter(this, n)) { seen.add(n); diff --git a/src/server/ClientMsgRateLimiter.ts b/src/server/ClientMsgRateLimiter.ts index d243b64607..d3c8f5a3b8 100644 --- a/src/server/ClientMsgRateLimiter.ts +++ b/src/server/ClientMsgRateLimiter.ts @@ -5,12 +5,14 @@ const INTENTS_PER_SECOND = 10; const INTENTS_PER_MINUTE = 150; const MAX_INTENT_SIZE = 500; const MAX_CONFIG_INTENT_SIZE = 2000; -const TOTAL_BYTES = 2 * 1024 * 1024; // 2MB per client +const TOTAL_BYTES = 2 * 1024 * 1024; // 2MB per client per window +const BYTE_WINDOW_MS = 60_000; // Reset byte counter every 60 seconds export type RateLimitResult = "ok" | "limit" | "kick"; interface ClientBucket { perSecond: RateLimiter; perMinute: RateLimiter; + byteEvents: Array<{ at: number; bytes: number }>; totalBytes: number; } @@ -24,6 +26,18 @@ export class ClientMsgRateLimiter { intentType?: string, ): RateLimitResult { const bucket = this.getOrCreate(clientID); + + // Rolling-window byte accounting: evict events older than BYTE_WINDOW_MS + // so throughput is measured over a true sliding window instead of + // a fixed window that allows burst bypass at boundaries. + const now = Date.now(); + const cutoff = now - BYTE_WINDOW_MS; + while (bucket.byteEvents.length > 0 && bucket.byteEvents[0].at < cutoff) { + const evicted = bucket.byteEvents.shift()!; + bucket.totalBytes -= evicted.bytes; + } + + bucket.byteEvents.push({ at: now, bytes }); bucket.totalBytes += bytes; if (bucket.totalBytes >= TOTAL_BYTES) return "kick"; @@ -68,6 +82,7 @@ export class ClientMsgRateLimiter { tokensPerInterval: INTENTS_PER_MINUTE, interval: "minute", }), + byteEvents: [], totalBytes: 0, }; this.buckets.set(clientID, bucket); diff --git a/src/server/GameServer.ts b/src/server/GameServer.ts index 169c159722..8c5675dbd1 100644 --- a/src/server/GameServer.ts +++ b/src/server/GameServer.ts @@ -48,7 +48,7 @@ export class GameServer { private maxGameDuration = 3 * 60 * 60 * 1000; // 3 hours - private disconnectedTimeout = 1 * 30 * 1000; // 30 seconds + private disconnectedTimeout = 30_000; // 30 seconds private turns: Turn[] = []; private intents: StampedIntent[] = []; @@ -576,12 +576,10 @@ export class GameServer { } } } catch (error) { - this.log.info( - `error handline websocket request in game server: ${error}`, - { - clientID: client.clientID, - }, - ); + this.log.error(`error handling websocket request in game server`, { + clientID: client.clientID, + error: String(error), + }); } }); client.ws.on("close", () => { @@ -826,7 +824,9 @@ export class GameServer { turn: pastTurn, } satisfies ServerTurnMessage); this.activeClients.forEach((c) => { - c.ws.send(msg); + if (c.ws.readyState === WebSocket.OPEN) { + c.ws.send(msg); + } }); } diff --git a/src/server/Worker.ts b/src/server/Worker.ts index 13cf1f32c2..cf31b1b452 100644 --- a/src/server/Worker.ts +++ b/src/server/Worker.ts @@ -106,7 +106,7 @@ export async function startWorker() { app.set("trust proxy", 3); app.use(compression()); - app.use(express.json()); + // Note: express.json({ limit: "5mb" }) is already applied above (line 51) app.use( express.static(path.join(__dirname, "../../out"), { @@ -210,6 +210,63 @@ export async function startWorker() { res.json(game.gameInfo()); }); + // Add other endpoints from your original server + app.post("/api/start_game/:id", async (req, res) => { + // SEC-02: Verify the caller before touching any game state. + const authHeader = req.headers.authorization; + if (!authHeader?.startsWith("Bearer ")) { + return res + .status(401) + .json({ error: "Authorization header required to start a game" }); + } + const token = authHeader.substring("Bearer ".length); + const result = await verifyClientToken(token, config); + if (result.type === "error") { + log.warn(`Invalid token for start_game: ${result.message}`); + return res.status(401).json({ error: "Invalid token" }); + } + + log.info(`starting private lobby with id ${req.params.id}`); + const game = gm.game(req.params.id); + if (!game) { + return res.status(404).json({ error: "Game not found" }); + } + if (game.isPublic()) { + // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing + const clientIP = req.ip || req.socket.remoteAddress || "unknown"; + log.info( + `cannot start public game ${game.id}, game is public, ip: ${ipAnonymize(clientIP)}`, + ); + return res.status(400).json({ error: "Cannot start public game" }); + } + + const callerPersistentId = result.persistentId; + const existingClientId = + game.getClientIdForPersistentId(callerPersistentId); + if ( + existingClientId === null || + existingClientId !== game.gameInfo().lobbyCreatorClientID + ) { + log.warn( + `Unauthorized start_game attempt by ${callerPersistentId.substring(0, 8)}`, + ); + return res + .status(403) + .json({ error: "Only the lobby creator can start the game" }); + } + + try { + if (game.hasStarted()) { + return res.status(409).json({ error: "Game already started" }); + } + game.start(); + res.status(200).json({ success: true }); + } catch (error) { + log.error(`Error starting game ${req.params.id}:`, error); + return res.status(500).json({ error: "Failed to start game" }); + } + }); + app.get("/api/game/:id/exists", async (req, res) => { const lobbyId = req.params.id; res.json({ diff --git a/tests/AttackRetreatStats.test.ts b/tests/AttackRetreatStats.test.ts new file mode 100644 index 0000000000..cf3a68e740 --- /dev/null +++ b/tests/AttackRetreatStats.test.ts @@ -0,0 +1,89 @@ +import { AttackExecution } from "../src/core/execution/AttackExecution"; +import { RetreatExecution } from "../src/core/execution/RetreatExecution"; +import { SpawnExecution } from "../src/core/execution/SpawnExecution"; +import { Game, Player, PlayerInfo, PlayerType } from "../src/core/game/Game"; +import { GameID } from "../src/core/Schemas"; +import { setup } from "./util/Setup"; + +let game: Game; +const gameID: GameID = "game_id"; +let player1: Player; +let player2: Player; + +describe("AttackRetreatStats", () => { + beforeEach(async () => { + game = await setup("plains", {}, [ + new PlayerInfo("player1", PlayerType.Human, "player1", "player1"), + new PlayerInfo("player2", PlayerType.Human, "player2", "player2"), + ]); + + player1 = game.player("player1"); + player2 = game.player("player2"); + + game.addExecution( + new SpawnExecution(gameID, player1.info(), game.ref(50, 50)), + ); + game.addExecution( + new SpawnExecution(gameID, player2.info(), game.ref(50, 55)), + ); + + while (game.inSpawnPhase()) { + game.executeNextTick(); + } + }); + + test("should call attackCancel when attack is retreated", () => { + // Attack terraNullius so the attack doesn't end quickly from troop loss + const attackCancelSpy = vi.spyOn(game.stats(), "attackCancel"); + + game.addExecution( + new AttackExecution(player1.troops(), player1, game.terraNullius().id()), + ); + + // Execute one tick so the attack is initialized + game.executeNextTick(); + + const attacks = player1.outgoingAttacks(); + expect(attacks.length).toBeGreaterThan(0); + const attackId = attacks[0].id(); + + // Add retreat execution immediately + game.addExecution(new RetreatExecution(player1, attackId)); + + // Execute ticks until the attack finishes retreating (cancelDelay=20 + a few more) + for (let i = 0; i < 50; i++) { + game.executeNextTick(); + } + + // Verify attackCancel was called (retreat stats recorded) + expect(attackCancelSpy).toHaveBeenCalled(); + expect(attackCancelSpy).toHaveBeenCalledWith( + player1, + expect.anything(), + expect.any(Number), + ); + }); + + test("should NOT call attackCancel when attack completes without retreat", () => { + expect(player1.sharesBorderWith(player2)).toBeTruthy(); + + const attackCancelSpy = vi.spyOn(game.stats(), "attackCancel"); + + // Start a full-troop attack against the other player (no retreat) + game.addExecution( + new AttackExecution(player1.troops(), player1, player2.id()), + ); + + // Execute until attack completes + let maxTicks = 5000; + while (player1.outgoingAttacks().length > 0 && maxTicks > 0) { + game.executeNextTick(); + maxTicks--; + } + // Make sure the loop ended because the attack finished, not because we ran out of ticks. + expect(player1.outgoingAttacks().length).toBe(0); + + // Verify attackCancel was NOT called + expect(attackCancelSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/core/executions/WinCheckExecution.test.ts b/tests/core/executions/WinCheckExecution.test.ts index 6b5b07d09e..0dd9e07457 100644 --- a/tests/core/executions/WinCheckExecution.test.ts +++ b/tests/core/executions/WinCheckExecution.test.ts @@ -87,6 +87,55 @@ describe("WinCheckExecution", () => { it("should return false for activeDuringSpawnPhase", () => { expect(winCheck.activeDuringSpawnPhase()).toBe(false); }); + + it("should not set winner via tile percentage when all land tiles have fallout (FFA)", () => { + const player = { + numTilesOwned: vi.fn(() => 100), + name: vi.fn(() => "P1"), + }; + mg.players = vi.fn(() => [player]); + mg.numLandTiles = vi.fn(() => 100); + // All tiles have fallout => numTilesWithoutFallout === 0 + mg.numTilesWithFallout = vi.fn(() => 100); + mg.config = vi.fn(() => ({ + gameConfig: vi.fn(() => ({ + gameMode: GameMode.FFA, + maxTimerValue: undefined, + })), + percentageTilesOwnedToWin: vi.fn(() => 80), + numSpawnPhaseTurns: vi.fn(() => 0), + })); + mg.ticks = vi.fn(() => 0); + mg.stats = vi.fn(() => ({ stats: () => ({}) })); + winCheck.init(mg, 0); + winCheck.checkWinnerFFA(); + expect(mg.setWinner).not.toHaveBeenCalled(); + }); + + it("should not set winner via tile percentage when all land tiles have fallout (Team)", () => { + const player = { + numTilesOwned: vi.fn(() => 100), + name: vi.fn(() => "P1"), + team: vi.fn(() => "Red"), + }; + mg.players = vi.fn(() => [player]); + mg.numLandTiles = vi.fn(() => 100); + // All tiles have fallout => numTilesWithoutFallout === 0 + mg.numTilesWithFallout = vi.fn(() => 100); + mg.config = vi.fn(() => ({ + gameConfig: vi.fn(() => ({ + gameMode: GameMode.Team, + maxTimerValue: undefined, + })), + percentageTilesOwnedToWin: vi.fn(() => 80), + numSpawnPhaseTurns: vi.fn(() => 0), + })); + mg.ticks = vi.fn(() => 0); + mg.stats = vi.fn(() => ({ stats: () => ({}) })); + winCheck.init(mg, 0); + winCheck.checkWinnerTeam(); + expect(mg.setWinner).not.toHaveBeenCalled(); + }); }); describe("WinCheckExecution - Nation Winners", () => {