Security: prevent winner stats spoofing and auth-gate singleplayer archive

src/client/LocalServer.ts

@@ -18,7 +18,7 @@ import {
   decompressGameRecord,
   replacer,
 } from "../core/Util";
-import { getPersistentID } from "./Auth";
+import { getAuthHeader, getPersistentID } from "./Auth";
 import { LobbyConfig } from "./ClientGameRunner";
 import {
   GameSpeedDownIntentEvent,
@@ -228,7 +228,7 @@ export class LocalServer {
     }
     if (clientMsg.type === "winner") {
       this.winner = clientMsg;
-      this.allPlayersStats = clientMsg.allPlayersStats;
+      this.allPlayersStats = clientMsg.allPlayersStats ?? {};
     }
   }
 
@@ -303,16 +303,23 @@ export class LocalServer {
 
     const jsonString = JSON.stringify(result.data, replacer);
 
-    compress(jsonString)
-      .then((compressedData) => {
+    getAuthHeader()
+      .then(async (authHeader) => {
+        // Anonymous users have no JWT. The archive endpoint requires auth to
+        // prevent spoofing, so skip silently rather than sending a request
+        // that would be rejected with 401. Logged-in singleplayer archiving
+        // is intentionally preserved; anonymous singleplayer is not archived.
+        if (!authHeader) return;
+        const compressedData = await compress(jsonString);
         return fetch(`/${workerPath}/api/archive_singleplayer_game`, {
           method: "POST",
           headers: {
             "Content-Type": "application/json",
             "Content-Encoding": "gzip",
+            Authorization: authHeader,
           },
           body: compressedData,
-          keepalive: true, // Ensures request completes even if page unloads
+          keepalive: true,
         });
       })
       .catch((error) => {

src/client/Transport.ts

@@ -574,11 +574,18 @@ export class Transport {
 
   private onSendWinnerEvent(event: SendWinnerEvent) {
     if (this.isLocal || this.socket?.readyState === WebSocket.OPEN) {
-      this.sendMsg({
-        type: "winner",
-        winner: event.winner,
-        allPlayersStats: event.allPlayersStats,
-      } satisfies ClientSendWinnerMessage);
+      // allPlayersStats is stripped on the multiplayer path: the server does
+      // not trust client-reported stats (prevents fake stat injection).
+      // For local (singleplayer) games allPlayersStats must be kept — it is
+      // the only source of stats that LocalServer.endGame() archives.
+      const msg: ClientSendWinnerMessage = this.isLocal
+        ? {
+            type: "winner",
+            winner: event.winner,
+            allPlayersStats: event.allPlayersStats,
+          }
+        : { type: "winner", winner: event.winner };
+      this.sendMsg(msg);
     } else {
       console.log(
         "WebSocket is not open. Current state:",

src/core/Schemas.ts

@@ -649,7 +649,11 @@ export const ServerMessageSchema = z.discriminatedUnion("type", [
 export const ClientSendWinnerSchema = z.object({
   type: z.literal("winner"),
   winner: WinnerSchema,
-  allPlayersStats: AllPlayersStatsSchema,
+  // allPlayersStats is optional and intentionally ignored by the multiplayer
+  // server (GameServer.handleWinner). It is only used by LocalServer for
+  // offline singleplayer archiving. Clients must not rely on the server
+  // accepting or trusting this field in multiplayer.
+  allPlayersStats: AllPlayersStatsSchema.optional(),
 });
 
 export const ClientHashSchema = z.object({

src/server/GameServer.ts

@@ -23,6 +23,7 @@ import {
   ServerTurnMessage,
   StampedIntent,
   Turn,
+  Winner,
 } from "../core/Schemas";
 import { createPartialGameRecord } from "../core/Util";
 import { archive, finalizeGameRecord } from "./Archive";
@@ -66,7 +67,7 @@ export class GameServer {
 
   private lastPingUpdate = 0;
 
-  private winner: ClientSendWinnerMessage | null = null;
+  private winner: Winner | null = null;
 
   // Note: This can be undefined if accessed before the game starts.
   private gameStartInfo!: GameStartInfo;
@@ -86,10 +87,8 @@ export class GameServer {
 
   private websockets: Set<WebSocket> = new Set();
 
-  private winnerVotes: Map<
-    string,
-    { winner: ClientSendWinnerMessage; ips: Set<string> }
-  > = new Map();
+  private winnerVotes: Map<string, { winner: Winner; ips: Set<string> }> =
+    new Map();
 
   private _hasEnded = false;
 
@@ -1095,23 +1094,22 @@ export class GameServer {
   private archiveGame() {
     this.log.info("archiving game", {
       gameID: this.id,
-      winner: this.winner?.winner,
+      winner: this.winner,
     });
 
     // Players must stay in the same order as the game start info.
+    // Stats are intentionally left undefined: client-reported allPlayersStats
+    // were removed from ClientSendWinnerMessage to prevent fake stat injection.
+    // A future server-side stats tracker should populate this field instead.
     const playerRecords: PlayerRecord[] = this.gameStartInfo.players.map(
       (player) => {
-        const stats = this.winner?.allPlayersStats[player.clientID];
-        if (stats === undefined) {
-          this.log.warn(`Unable to find stats for clientID ${player.clientID}`);
-        }
         return {
           clientID: player.clientID,
           username: player.username,
           clanTag: player.clanTag,
           persistentID:
             this.allClients.get(player.clientID)?.persistentID ?? "",
-          stats,
+          stats: undefined,
           cosmetics: player.cosmetics,
         } satisfies PlayerRecord;
       },
@@ -1125,7 +1123,7 @@ export class GameServer {
           this.turns,
           this._startTime ?? 0,
           Date.now(),
-          this.winner?.winner,
+          this.winner ?? undefined,
           this.createdAt,
           this.visibleAt,
         ),
@@ -1248,7 +1246,10 @@ export class GameServer {
     // Add client vote
     const winnerKey = JSON.stringify(clientMsg.winner);
     if (!this.winnerVotes.has(winnerKey)) {
-      this.winnerVotes.set(winnerKey, { ips: new Set(), winner: clientMsg });
+      this.winnerVotes.set(winnerKey, {
+        ips: new Set(),
+        winner: clientMsg.winner,
+      });
     }
     const potentialWinner = this.winnerVotes.get(winnerKey)!;
     potentialWinner.ips.add(client.ip);

src/server/Worker.ts

@@ -233,6 +233,24 @@ export async function startWorker() {
 
   app.post("/api/archive_singleplayer_game", async (req, res) => {
     try {
+      // Require a valid JWT so only the actual player can archive their own game.
+      // Without this, any unauthenticated client could submit fake records for
+      // arbitrary persistentIDs (see security audit finding #1).
+      const authHeader = req.headers.authorization;
+      if (!authHeader?.startsWith("Bearer ")) {
+        log.warn("archive_singleplayer_game: missing Authorization header");
+        return res.status(401).json({ error: "Unauthorized" });
+      }
+      const tokenResult = await verifyClientToken(
+        authHeader.substring("Bearer ".length),
+      );
+      if (tokenResult.type === "error") {
+        log.warn(
+          `archive_singleplayer_game: invalid token — ${tokenResult.message}`,
+        );
+        return res.status(401).json({ error: "Unauthorized" });
+      }
+
       const record = req.body;
 
       const result = PartialGameRecordSchema.safeParse(record);
@@ -260,6 +278,16 @@ export async function startWorker() {
         return res.status(400).json({ error: "Invalid request" });
       }
 
+      // Ensure the token owner matches the player in the record.
+      const recordPersistentID = result.data.info.players[0]?.persistentID;
+      if (recordPersistentID !== tokenResult.persistentId) {
+        log.warn(
+          `archive_singleplayer_game: persistentID mismatch — token: ${tokenResult.persistentId?.substring(0, 8)}, record: ${recordPersistentID?.substring(0, 8)}`,
+          { gameID: gameRecord.info.gameID },
+        );
+        return res.status(403).json({ error: "Forbidden" });
+      }
+
       log.info("archiving singleplayer game", {
         gameID: gameRecord.info.gameID,
       });

tests/ClientSendWinnerSchema.test.ts

@@ -0,0 +1,62 @@
+import { ClientSendWinnerSchema } from "../src/core/Schemas";
+
+describe("ClientSendWinnerSchema", () => {
+  // ID must match /^[A-Za-z0-9]{8}$/
+  const id1 = "AAAAAAAA";
+  const id2 = "BBBBBBBB";
+  const validWinner = ["player", id1, id2] as const;
+
+  test("accepts a winner message without allPlayersStats", () => {
+    const result = ClientSendWinnerSchema.safeParse({
+      type: "winner",
+      winner: validWinner,
+    });
+    expect(result.success).toBe(true);
+  });
+
+  test("accepts a winner message with allPlayersStats (singleplayer path)", () => {
+    const result = ClientSendWinnerSchema.safeParse({
+      type: "winner",
+      winner: validWinner,
+      allPlayersStats: {
+        [id1]: {},
+      },
+    });
+    expect(result.success).toBe(true);
+  });
+
+  test("accepts a winner message with undefined winner (draw)", () => {
+    const result = ClientSendWinnerSchema.safeParse({
+      type: "winner",
+      winner: undefined,
+    });
+    expect(result.success).toBe(true);
+  });
+
+  test("rejects a message with wrong type", () => {
+    const result = ClientSendWinnerSchema.safeParse({
+      type: "not_winner",
+      winner: validWinner,
+    });
+    expect(result.success).toBe(false);
+  });
+
+  test("rejects a message with invalid winner format", () => {
+    const result = ClientSendWinnerSchema.safeParse({
+      type: "winner",
+      winner: "invalid",
+    });
+    expect(result.success).toBe(false);
+  });
+
+  test("allPlayersStats is absent (undefined) when not provided by sender", () => {
+    const result = ClientSendWinnerSchema.safeParse({
+      type: "winner",
+      winner: validWinner,
+    });
+    expect(result.success).toBe(true);
+    if (result.success) {
+      expect(result.data.allPlayersStats).toBeUndefined();
+    }
+  });
+});

tests/server/WinnerSecurity.test.ts

@@ -0,0 +1,126 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+vi.mock("../../src/core/Schemas", async () => {
+  const actual = (await vi.importActual("../../src/core/Schemas")) as any;
+  return {
+    ...actual,
+    GameStartInfoSchema: {
+      safeParse: (data: any) => ({ success: true, data: data }),
+    },
+    ServerPrestartMessageSchema: {
+      safeParse: (data: any) => ({ success: true, data: data }),
+    },
+    ClientMessageSchema: {
+      safeParse: (data: any) => ({ success: true, data: data }),
+    },
+  };
+});
+
+vi.mock("../../src/server/Archive", () => ({
+  archive: vi.fn(),
+  finalizeGameRecord: (r: any) => r,
+}));
+
+import { GameType } from "../../src/core/game/Game";
+import { archive } from "../../src/server/Archive";
+import { Client } from "../../src/server/Client";
+import { GameServer } from "../../src/server/GameServer";
+
+function makeMockWs(ip = "1.2.3.4") {
+  const handlers: Record<string, (...args: any[]) => any> = {};
+  return {
+    on: (event: string, handler: (...args: any[]) => any) => {
+      handlers[event] = handler;
+    },
+    removeAllListeners: (_event: string) => {},
+    send: vi.fn(),
+    close: vi.fn(),
+    readyState: 1,
+    trigger: (event: string, ...args: any[]) => handlers[event]?.(...args),
+  };
+}
+
+function makeClient(
+  clientID: string,
+  persistentID: string,
+  ip = "1.2.3.4",
+): { client: Client; ws: ReturnType<typeof makeMockWs> } {
+  const ws = makeMockWs(ip);
+  const client = new Client(
+    clientID,
+    persistentID,
+    null,
+    null,
+    undefined,
+    ip,
+    "TestUser",
+    null,
+    ws as any,
+    undefined,
+    undefined,
+    [],
+  );
+  return { client, ws };
+}
+
+describe("GameServer - winner message security", () => {
+  let mockLogger: any;
+
+  beforeEach(() => {
+    vi.useFakeTimers();
+    mockLogger = {
+      child: vi.fn().mockReturnThis(),
+      info: vi.fn(),
+      warn: vi.fn(),
+      error: vi.fn(),
+    };
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+    vi.clearAllTimers();
+  });
+
+  it("archives with undefined stats regardless of client-supplied allPlayersStats", async () => {
+    const game = new GameServer("test-game", mockLogger, Date.now(), {
+      gameType: GameType.Private,
+    } as any);
+
+    const { client: c1, ws: ws1 } = makeClient("cid-1", "pid-1", "1.2.3.4");
+    const { client: c2, ws: ws2 } = makeClient("cid-2", "pid-2", "5.6.7.8");
+    game.joinClient(c1);
+    game.joinClient(c2);
+    game.start();
+
+    // Both clients vote for the same winner and attach fabricated stats.
+    // Majority threshold is met so archiveGame() is triggered.
+    const fabricatedStats = {
+      "cid-1": { kills: 9999, gold: 9999 } as any,
+      "cid-2": { kills: 9999, gold: 9999 } as any,
+    };
+
+    await ws1.trigger(
+      "message",
+      JSON.stringify({
+        type: "winner",
+        winner: ["player", "cid-1"],
+        allPlayersStats: fabricatedStats,
+      }),
+    );
+    await ws2.trigger(
+      "message",
+      JSON.stringify({
+        type: "winner",
+        winner: ["player", "cid-1"],
+        allPlayersStats: fabricatedStats,
+      }),
+    );
+
+    expect(archive).toHaveBeenCalledOnce();
+    const archivedRecord = (archive as ReturnType<typeof vi.fn>).mock
+      .calls[0][0];
+    for (const player of archivedRecord.info.players) {
+      expect(player.stats).toBeUndefined();
+    }
+  });
+});