From 66fd2151af7ae75e61b5776a1296974e5530de67 Mon Sep 17 00:00:00 2001
From: saritahimthani <sarita.himthani123@gmail.com>
Date: Tue, 6 Oct 2026 16:06:55 -0700
Subject: [PATCH 1/4] PvP results (PR 1): middleware records who plays whom

Challenge accept now saves a PvpGame (gameId, white, black, status),
and GET /challenge/game/:gameId lets the chess server verify a joining
player's identity against it instead of trusting the client.

Adds the service-key-gated POST /internal/gameResults, validated
against the saved PvpGame (unknown gameId -> 404, mismatched players
-> 400), idempotent on gameId. Removes the player-facing POST
/gameResults entirely - results can now only be written by the chess
server, authenticated with CHESS_SERVICE_KEY.

See "PvP game results: server-authoritative reporting" (v2), T1-T3.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 .../src/middleware/requireServiceKey.js       |  38 +++++
 middlewareNode/src/models/PvpGame.js          |  36 +++++
 middlewareNode/src/models/gameResults.js      |  13 +-
 middlewareNode/src/routes/challenge.js        |  53 ++++++-
 middlewareNode/src/routes/gameResults.js      |  60 ++------
 .../src/routes/internalGameResults.js         |  79 ++++++++++
 middlewareNode/src/server.js                  |   9 ++
 .../tests/challenge.pvpgame.test.js           | 108 +++++++++++++
 middlewareNode/tests/gameResults.test.js      | 117 +-------------
 .../tests/internalGameResults.test.js         | 145 ++++++++++++++++++
 10 files changed, 499 insertions(+), 159 deletions(-)
 create mode 100644 middlewareNode/src/middleware/requireServiceKey.js
 create mode 100644 middlewareNode/src/models/PvpGame.js
 create mode 100644 middlewareNode/src/routes/internalGameResults.js
 create mode 100644 middlewareNode/tests/challenge.pvpgame.test.js
 create mode 100644 middlewareNode/tests/internalGameResults.test.js

diff --git a/middlewareNode/src/middleware/requireServiceKey.js b/middlewareNode/src/middleware/requireServiceKey.js
new file mode 100644
index 00000000..5ce6bcff
--- /dev/null
+++ b/middlewareNode/src/middleware/requireServiceKey.js
@@ -0,0 +1,38 @@
+/**
+ * Require Service Key Middleware
+ *
+ * Gates the chess server's internal result-reporting endpoint. The caller is
+ * not a student — it's the chess server itself, authenticating with a shared
+ * secret (CHESS_SERVICE_KEY) instead of a player JWT. See the PvP results
+ * plan (v2), target design point 4.
+ *
+ * Uses crypto.timingSafeEqual to compare keys, which requires equal-length
+ * buffers — lengths are compared first so a wrong-length key takes the same
+ * code path as a wrong-value key instead of throwing.
+ */
+
+const crypto = require("crypto");
+
+const requireServiceKey = (req, res, next) => {
+  const expected = process.env.CHESS_SERVICE_KEY;
+  const provided = req.headers["x-service-key"];
+
+  if (!expected || !provided) {
+    return res.status(401).json({ error: "Unauthorized" });
+  }
+
+  const expectedBuf = Buffer.from(expected);
+  const providedBuf = Buffer.from(provided);
+
+  const matches =
+    expectedBuf.length === providedBuf.length &&
+    crypto.timingSafeEqual(expectedBuf, providedBuf);
+
+  if (!matches) {
+    return res.status(401).json({ error: "Unauthorized" });
+  }
+
+  next();
+};
+
+module.exports = requireServiceKey;
diff --git a/middlewareNode/src/models/PvpGame.js b/middlewareNode/src/models/PvpGame.js
new file mode 100644
index 00000000..858f2110
--- /dev/null
+++ b/middlewareNode/src/models/PvpGame.js
@@ -0,0 +1,36 @@
+/**
+ * PvpGame Schema
+ *
+ * Created the moment a challenge is accepted. It is the middleware's record of
+ * who the two real players in a gameId are, so the chess server can verify a
+ * joining socket against it instead of trusting whatever username the client
+ * sends. See documentation/student-vs-student-design.md §10 and the
+ * "PvP game results: server-authoritative reporting" plan (v2), target design.
+ *
+ * `gameId` is the same id the challenge flow (routes/challenge.js) hands out
+ * when the challenge is created — unique here too, so an accept can only ever
+ * produce one PvpGame per game.
+ */
+
+const mongoose = require("mongoose");
+
+const PvpGameSchema = new mongoose.Schema(
+  {
+    gameId: { type: String, required: true, unique: true, index: true },
+
+    // The challenger is always seated white, the opponent black — matches the
+    // chess server's GameManager.createOrJoinPvpGame convention.
+    white: { type: String, required: true },
+    black: { type: String, required: true },
+
+    // "active" while the game is being played; "finished" once a result has
+    // been accepted for it. Internal results can only flip active -> finished.
+    status: { type: String, enum: ["active", "finished"], default: "active", index: true },
+
+    acceptedAt: { type: Date, default: Date.now },
+    finishedAt: { type: Date, default: null },
+  },
+  { timestamps: true }
+);
+
+module.exports = mongoose.model("PvpGame", PvpGameSchema);
diff --git a/middlewareNode/src/models/gameResults.js b/middlewareNode/src/models/gameResults.js
index 69e900d5..6e35812d 100644
--- a/middlewareNode/src/models/gameResults.js
+++ b/middlewareNode/src/models/gameResults.js
@@ -14,7 +14,12 @@
  * or both clients reporting the same game can never double-count it.
  *
  * Records are written only by the chessServer at game end (checkmate, resign,
- * or disconnect-forfeit) via POST /gameResults.
+ * or disconnect-forfeit) via POST /internal/gameResults, authenticated with
+ * CHESS_SERVICE_KEY rather than a player JWT — see the PvP results plan (v2).
+ * `source` records how a record got here: "chessServer" for everything
+ * reported through that path, "legacy-unverified" for records that predate
+ * it (the old player-facing POST never actually worked — see finding #1 of
+ * that plan — so any pre-existing record is suspect, not verified history).
  */
 
 const mongoose = require("mongoose");
@@ -52,6 +57,12 @@ const GameResultsSchema = new mongoose.Schema(
     },
 
     playedAt: { type: Date, default: Date.now, index: true },
+
+    source: {
+      type: String,
+      enum: ["chessServer", "legacy-unverified"],
+      required: true,
+    },
   },
   { timestamps: true }
 );
diff --git a/middlewareNode/src/routes/challenge.js b/middlewareNode/src/routes/challenge.js
index 7aea391d..cbaef150 100644
--- a/middlewareNode/src/routes/challenge.js
+++ b/middlewareNode/src/routes/challenge.js
@@ -14,8 +14,9 @@
 
 const express = require('express');
 const crypto = require('crypto');
-const router = express.Router({ mergeParams: true });
 const requireAuth = require('../middleware/requireAuth');
+const PvpGame = require('../models/PvpGame');
+const router = express.Router({ mergeParams: true });
 
 // challengeId -> { id, gameId, fromUsername, toUsername, status, createdAt }
 // status: "pending" | "accepted" | "declined"
@@ -136,8 +137,14 @@ router.get('/:id', requireAuth, (req, res) => {
 /**
  * POST /challenge/:id/accept
  * Opponent accepts; both sides now share `gameId`.
+ *
+ * Also persists a PvpGame record (white = challenger, black = opponent) —
+ * this is the middleware's own record of who the two real players are, so
+ * GET /challenge/game/:gameId can later verify a joining chess-server socket
+ * against it instead of trusting a client-supplied username. See the PvP
+ * results plan (v2), target design.
  */
-router.post('/:id/accept', requireAuth, (req, res) => {
+router.post('/:id/accept', requireAuth, async (req, res) => {
     sweepExpired();
     const challenge = challenges.get(req.params.id);
     if (!challenge) {
@@ -153,6 +160,24 @@ router.post('/:id/accept', requireAuth, (req, res) => {
         return res.status(409).json({ error: `Challenge already ${challenge.status}` });
     }
     challenge.status = 'accepted';
+
+    try {
+        await PvpGame.create({
+            gameId: challenge.gameId,
+            white: challenge.fromUsername,
+            black: challenge.toUsername,
+        });
+    } catch (err) {
+        // Unique-index race: two accepts for the same challenge can't both get
+        // here in practice (the in-memory status guard above already blocks a
+        // second accept), but if they somehow did, the duplicate key means a
+        // PvpGame already exists for this gameId — nothing more to do.
+        if (!err || err.code !== 11000) {
+            console.error('challenge accept — failed to save PvpGame:', err && err.message);
+            return res.status(500).json({ error: 'Server error' });
+        }
+    }
+
     return res.status(200).json({
         gameId: challenge.gameId,
         challenger: challenge.fromUsername,
@@ -160,6 +185,30 @@ router.post('/:id/accept', requireAuth, (req, res) => {
     });
 });
 
+/**
+ * GET /challenge/game/:gameId
+ * Lets the chess server verify who the two real players in a game are before
+ * seating a joining socket. Behind requireAuth: the caller's own JWT decides
+ * `you`, so a socket can never claim to be someone else's seat.
+ */
+router.get('/game/:gameId', requireAuth, async (req, res) => {
+    const game = await PvpGame.findOne({ gameId: req.params.gameId });
+    if (!game) {
+        return res.status(404).json({ error: 'Game not found' });
+    }
+    const you = req.user.username;
+    if (you !== game.white && you !== game.black) {
+        return res.status(403).json({ error: 'You are not a player in this game' });
+    }
+    return res.status(200).json({
+        gameId: game.gameId,
+        you,
+        white: game.white,
+        black: game.black,
+        status: game.status,
+    });
+});
+
 /**
  * POST /challenge/:id/decline
  */
diff --git a/middlewareNode/src/routes/gameResults.js b/middlewareNode/src/routes/gameResults.js
index 154c013d..f8cfd395 100644
--- a/middlewareNode/src/routes/gameResults.js
+++ b/middlewareNode/src/routes/gameResults.js
@@ -18,17 +18,21 @@
  *     reporting the same game is a no-op, not a double count.
  *
  * Endpoints:
- *   POST /gameResults              -> record one finished game (idempotent)
  *   GET  /gameResults/:username    -> that student's W/D/L + chess score
  *
- * Mounted behind requireAuth (any logged-in role). POST additionally requires
- * that the caller was one of the two players — a student cannot report a game
- * they did not play.
+ * Mounted behind requireAuth (any logged-in role).
+ *
+ * There is no player-facing POST here. A student cannot write their own game
+ * result — only the chess server can, via the separately-mounted, service-key
+ * -gated POST /internal/gameResults (routes/internalGameResults.js), which
+ * reuses buildRecord/serialize below for validation and response shape. See
+ * the PvP results plan (v2): the old player-facing POST never actually
+ * reached the database (finding #1), so a student posting to the legacy path
+ * now just gets a 404 — there is no route left to match.
  */
 
 const express = require("express");
 const router = express.Router();
-const GameResults = require("../models/gameResults");
 const { getChessRecord } = require("../utils/studentStats");
 
 const VALID_RESULTS = ["win", "draw"];
@@ -44,6 +48,7 @@ function serialize(doc) {
     loserUsername: doc.loserUsername,
     reason: doc.reason,
     playedAt: doc.playedAt,
+    source: doc.source,
   };
 }
 
@@ -102,48 +107,6 @@ function buildRecord(body) {
   };
 }
 
-/**
- * POST /gameResults
- * Body (win):  { gameId, result: "win", reason, winnerUsername, loserUsername, playedAt? }
- * Body (draw): { gameId, result: "draw", reason: "draw", players: [a, b], playedAt? }
- *
- * Idempotent on gameId: re-reporting a recorded game returns 200 with
- * duplicate: true and changes nothing.
- */
-router.post("/", async (req, res) => {
-  try {
-    const { error, record } = buildRecord(req.body);
-    if (error) return res.status(400).json({ success: false, error });
-
-    // Only a participant may report the game.
-    const caller = req.user && req.user.username;
-    if (!caller || !record.players.includes(caller)) {
-      return res
-        .status(403)
-        .json({ success: false, error: "Only a player in this game may report its result" });
-    }
-
-    const existing = await GameResults.findOne({ gameId: record.gameId });
-    if (existing) {
-      return res.json({ success: true, duplicate: true, gameResult: serialize(existing) });
-    }
-
-    const created = await GameResults.create(record);
-    return res.status(201).json({ success: true, duplicate: false, gameResult: serialize(created) });
-  } catch (err) {
-    // Unique-index race: the other client reported the same game first. That's
-    // the idempotency guarantee doing its job, not an error.
-    if (err && err.code === 11000) {
-      const existing = await GameResults.findOne({ gameId: req.body.gameId });
-      if (existing) {
-        return res.json({ success: true, duplicate: true, gameResult: serialize(existing) });
-      }
-    }
-    console.error("gameResults POST /:", err.message);
-    return res.status(500).json({ success: false, error: "Server error" });
-  }
-});
-
 /**
  * GET /gameResults/:username
  * That student's competitive record and derived chess score.
@@ -159,3 +122,6 @@ router.get("/:username", async (req, res) => {
 });
 
 module.exports = router;
+// Reused by routes/internalGameResults.js for body validation and response shape.
+module.exports.buildRecord = buildRecord;
+module.exports.serialize = serialize;
diff --git a/middlewareNode/src/routes/internalGameResults.js b/middlewareNode/src/routes/internalGameResults.js
new file mode 100644
index 00000000..43d69242
--- /dev/null
+++ b/middlewareNode/src/routes/internalGameResults.js
@@ -0,0 +1,79 @@
+/**
+ * Internal Game Results Routes  —  /internal/gameResults
+ *
+ * The chess server's own reporting path, authenticated with CHESS_SERVICE_KEY
+ * (middleware/requireServiceKey) instead of a player JWT. See the PvP results
+ * plan (v2), target design points 3-4.
+ *
+ * The middleware only accepts a result for a PvpGame it knows about — one
+ * created when a challenge was accepted (routes/challenge.js) — and only when
+ * the reported winner/loser (or both drawn players) match that game's two
+ * real players. This is what makes a result trustworthy: the chess server
+ * decides who won, but the middleware decides whether a game was real.
+ *
+ * gameId remains the idempotency key: a second report for an already-recorded
+ * game returns 200 duplicate: true and writes nothing, same guarantee as the
+ * old player-facing POST had.
+ */
+
+const express = require("express");
+const router = express.Router();
+const requireServiceKey = require("../middleware/requireServiceKey");
+const GameResults = require("../models/gameResults");
+const PvpGame = require("../models/PvpGame");
+const { buildRecord, serialize } = require("./gameResults");
+
+/** True when the reported participants are exactly this PvpGame's two players. */
+function playersMatch(pvpGame, players) {
+  const expected = [pvpGame.white, pvpGame.black].sort();
+  const actual = [...players].sort();
+  return expected[0] === actual[0] && expected[1] === actual[1];
+}
+
+/**
+ * POST /internal/gameResults
+ * Body: same shape as the old player-facing POST /gameResults.
+ * Behind requireServiceKey — the caller is the chess server, not a player.
+ */
+router.post("/", requireServiceKey, async (req, res) => {
+  try {
+    const { error, record } = buildRecord(req.body);
+    if (error) return res.status(400).json({ success: false, error });
+
+    const pvpGame = await PvpGame.findOne({ gameId: record.gameId });
+    if (!pvpGame) {
+      return res.status(404).json({ success: false, error: "Game not found" });
+    }
+    if (!playersMatch(pvpGame, record.players)) {
+      return res
+        .status(400)
+        .json({ success: false, error: "Reported players do not match the accepted game" });
+    }
+
+    const existing = await GameResults.findOne({ gameId: record.gameId });
+    if (existing) {
+      return res.json({ success: true, duplicate: true, gameResult: serialize(existing) });
+    }
+
+    // Flip the game to finished before inserting — this is the one write that
+    // decides a report "wins", so a concurrent duplicate falls through to the
+    // GameResults unique-index race below instead of double-finishing.
+    await PvpGame.updateOne({ gameId: record.gameId, status: "active" }, { $set: { status: "finished", finishedAt: new Date() } });
+
+    const created = await GameResults.create({ ...record, source: "chessServer" });
+    return res.status(201).json({ success: true, duplicate: false, gameResult: serialize(created) });
+  } catch (err) {
+    // Unique-index race: another report for the same game won first. That's
+    // the idempotency guarantee doing its job, not an error.
+    if (err && err.code === 11000) {
+      const existing = await GameResults.findOne({ gameId: req.body.gameId });
+      if (existing) {
+        return res.json({ success: true, duplicate: true, gameResult: serialize(existing) });
+      }
+    }
+    console.error("internalGameResults POST /:", err.message);
+    return res.status(500).json({ success: false, error: "Server error" });
+  }
+});
+
+module.exports = router;
diff --git a/middlewareNode/src/server.js b/middlewareNode/src/server.js
index 91c651ef..295cd1fb 100644
--- a/middlewareNode/src/server.js
+++ b/middlewareNode/src/server.js
@@ -141,7 +141,16 @@ app.use("/streak", streakRoutes);
 app.use("/badges", require("./routes/badges"));
 app.use("/chat", require("./routes/chat"));
 app.use("/challenge", require("./routes/challenge"));
+// PvP results plan (v2): the only write path left under /gameResults was the
+// player-facing POST, now removed (a student can no longer report their own
+// game — see routes/gameResults.js). currencyEventLimiter stays here since
+// GET is still mounted behind it and there's no harm rate-limiting a read,
+// but it's no longer guarding a write. The actual write path moved to
+// /internal/gameResults below, protected by CHESS_SERVICE_KEY instead of a
+// per-user limiter, since its only legitimate caller is the chess server.
 app.use("/gameResults", currencyEventLimiter, requireAuth, require("./routes/gameResults"));
+// Chess server only — authenticated with CHESS_SERVICE_KEY, not a player JWT.
+app.use("/internal/gameResults", require("./routes/internalGameResults"));
 app.use("/analytics", analyticsLimiter, adminGuard, require("./routes/analytics"));
 app.use("/leaderboard", leaderboardLimiter, requireAuth, require("./routes/leaderboard"));
 
diff --git a/middlewareNode/tests/challenge.pvpgame.test.js b/middlewareNode/tests/challenge.pvpgame.test.js
new file mode 100644
index 00000000..3703f4e4
--- /dev/null
+++ b/middlewareNode/tests/challenge.pvpgame.test.js
@@ -0,0 +1,108 @@
+/**
+ * Integration tests — challenge accept persists a PvpGame, and
+ * GET /challenge/game/:gameId verifies a joining player against it.
+ *
+ * See the PvP results plan (v2), T1 and T2. Uses mongodb-memory-server + the
+ * real PvpGame model (no mocks) so the unique-index and lookup behavior is
+ * verified directly, matching the pattern in tests/badges.concurrency.test.js.
+ * requireAuth is mocked the same way tests/gameResults.test.js does it: the
+ * caller's username comes from an x-test-user header instead of a real JWT.
+ */
+
+jest.mock("../src/middleware/requireAuth", () => (req, res, next) => {
+  const username = req.headers["x-test-user"];
+  if (!username) return res.status(401).json({ error: "Unauthorized" });
+  req.user = { username, role: "student" };
+  next();
+});
+
+const { MongoMemoryServer } = require("mongodb-memory-server");
+const mongoose = require("mongoose");
+const express = require("express");
+const request = require("supertest");
+
+jest.setTimeout(60000);
+
+let mongod;
+let app;
+let challenge;
+
+beforeAll(async () => {
+  mongod = await MongoMemoryServer.create({ instance: { launchTimeout: 30000 } });
+  await mongoose.connect(mongod.getUri() + "ystem");
+
+  challenge = require("../src/routes/challenge");
+  app = express();
+  app.use(express.json());
+  app.use("/challenge", challenge);
+});
+
+afterAll(async () => {
+  await mongoose.disconnect();
+  await mongod.stop();
+});
+
+afterEach(async () => {
+  challenge._reset();
+  const collections = mongoose.connection.collections;
+  await Promise.all(Object.values(collections).map((c) => c.deleteMany({})));
+});
+
+/** Creates and accepts a challenge, returning its gameId. */
+async function acceptedGame(fromUsername = "alice", toUsername = "bob") {
+  const created = await request(app).post("/challenge").send({ fromUsername, toUsername });
+  const accept = await request(app).post(`/challenge/${created.body.challengeId}/accept`);
+  return { challengeId: created.body.challengeId, gameId: accept.body.gameId, accept };
+}
+
+describe("POST /challenge/:id/accept — persists a PvpGame", () => {
+  test("accept creates exactly one PvpGame with white=challenger, black=opponent", async () => {
+    const { gameId } = await acceptedGame("alice", "bob");
+
+    const PvpGame = require("../src/models/PvpGame");
+    const docs = await PvpGame.find({ gameId });
+    expect(docs).toHaveLength(1);
+    expect(docs[0].white).toBe("alice");
+    expect(docs[0].black).toBe("bob");
+    expect(docs[0].status).toBe("active");
+  });
+
+  test("a second accept of the same challenge returns 409 and creates no extra PvpGame", async () => {
+    const { challengeId, gameId } = await acceptedGame("alice", "bob");
+
+    const second = await request(app).post(`/challenge/${challengeId}/accept`);
+    expect(second.status).toBe(409);
+
+    const PvpGame = require("../src/models/PvpGame");
+    const docs = await PvpGame.find({ gameId });
+    expect(docs).toHaveLength(1);
+  });
+});
+
+describe("GET /challenge/game/:gameId", () => {
+  test("200 — either player gets you/white/black/status", async () => {
+    const { gameId } = await acceptedGame("alice", "bob");
+
+    const asWhite = await request(app).get(`/challenge/game/${gameId}`).set("x-test-user", "alice");
+    expect(asWhite.status).toBe(200);
+    expect(asWhite.body).toEqual({ gameId, you: "alice", white: "alice", black: "bob", status: "active" });
+
+    const asBlack = await request(app).get(`/challenge/game/${gameId}`).set("x-test-user", "bob");
+    expect(asBlack.status).toBe(200);
+    expect(asBlack.body.you).toBe("bob");
+  });
+
+  test("403 — a student who is not one of the two players", async () => {
+    const { gameId } = await acceptedGame("alice", "bob");
+
+    const res = await request(app).get(`/challenge/game/${gameId}`).set("x-test-user", "mallory");
+    expect(res.status).toBe(403);
+  });
+
+  test("404 — an unknown gameId", async () => {
+    const res = await request(app)
+      .get("/challenge/game/00000000-0000-0000-0000-000000000000")
+      .set("x-test-user", "alice");
+    expect(res.status).toBe(404);
+  });
+});
diff --git a/middlewareNode/tests/gameResults.test.js b/middlewareNode/tests/gameResults.test.js
index 81733080..a2807432 100644
--- a/middlewareNode/tests/gameResults.test.js
+++ b/middlewareNode/tests/gameResults.test.js
@@ -1,13 +1,13 @@
 /**
- * Integration tests — POST/GET /gameResults
+ * Integration tests — GET /gameResults
  *
  * The GameResults model is mocked; utils/studentStats is NOT, so the scoring
  * these tests assert is the real formula the leaderboard and analytics use.
  *
- * Covers the three properties the design depends on:
- *   - idempotency on gameId (a game can never be counted twice),
- *   - only a participant may report a game,
- *   - win/draw bodies are validated so junk can't reach the collection.
+ * There is no player-facing POST to test here anymore — a student can't write
+ * their own game result. See tests/internalGameResults.test.js for the chess
+ * server's service-key-gated reporting path (POST /internal/gameResults), and
+ * the "legacy POST is gone" case below for the 404 that replaces it.
  */
 
 jest.mock("../src/middleware/requireAuth", () => (req, _res, next) => {
@@ -36,111 +36,10 @@ const WIN_BODY = {
   loserUsername: "bob",
 };
 
-const DRAW_BODY = {
-  gameId: "game-2",
-  result: "draw",
-  reason: "draw",
-  players: ["alice", "bob"],
-};
-
-describe("POST /gameResults — recording", () => {
-  beforeEach(() => {
-    GameResults.findOne.mockResolvedValue(null);
-    GameResults.create.mockImplementation(async (doc) => doc);
-  });
-
-  test("201 — records a decisive game", async () => {
-    const res = await request(app).post("/gameResults").send(WIN_BODY);
-    expect(res.status).toBe(201);
-    expect(res.body.success).toBe(true);
-    expect(res.body.duplicate).toBe(false);
-    expect(res.body.gameResult.winnerUsername).toBe("alice");
-    expect(res.body.gameResult.players).toEqual(["alice", "bob"]);
-  });
-
-  test("201 — records a draw with both players and no winner", async () => {
-    const res = await request(app).post("/gameResults").send(DRAW_BODY);
-    expect(res.status).toBe(201);
-    expect(res.body.gameResult.result).toBe("draw");
-    expect(res.body.gameResult.winnerUsername).toBeNull();
-    expect(res.body.gameResult.loserUsername).toBeNull();
-    expect(res.body.gameResult.players).toEqual(["alice", "bob"]);
-  });
-
-  test("a resign is recorded as a win, not a special result", async () => {
-    const res = await request(app)
-      .post("/gameResults")
-      .send({ ...WIN_BODY, reason: "resign" });
-    expect(res.status).toBe(201);
-    expect(res.body.gameResult.result).toBe("win");
-    expect(res.body.gameResult.reason).toBe("resign");
-  });
-});
-
-describe("POST /gameResults — idempotency on gameId", () => {
-  test("re-reporting a recorded game returns 200 duplicate and writes nothing", async () => {
-    GameResults.findOne.mockResolvedValue({ ...WIN_BODY, players: ["alice", "bob"] });
-
-    const res = await request(app).post("/gameResults").send(WIN_BODY);
-    expect(res.status).toBe(200);
-    expect(res.body.duplicate).toBe(true);
-    expect(GameResults.create).not.toHaveBeenCalled();
-  });
-
-  test("a unique-index race is resolved as a duplicate, not a 500", async () => {
-    // Both clients report at once: findOne sees nothing, create loses the race.
-    GameResults.findOne
-      .mockResolvedValueOnce(null)
-      .mockResolvedValueOnce({ ...WIN_BODY, players: ["alice", "bob"] });
-    GameResults.create.mockRejectedValue({ code: 11000 });
-
+describe("POST /gameResults — legacy player-facing path is gone", () => {
+  test("404 — a student token posting to the old route finds no route to match", async () => {
     const res = await request(app).post("/gameResults").send(WIN_BODY);
-    expect(res.status).toBe(200);
-    expect(res.body.duplicate).toBe(true);
-  });
-});
-
-describe("POST /gameResults — authorization", () => {
-  beforeEach(() => {
-    GameResults.findOne.mockResolvedValue(null);
-    GameResults.create.mockImplementation(async (doc) => doc);
-  });
-
-  test("403 — a student who did not play the game cannot report it", async () => {
-    const res = await request(app)
-      .post("/gameResults")
-      .set("x-test-user", "mallory")
-      .send(WIN_BODY);
-    expect(res.status).toBe(403);
-    expect(GameResults.create).not.toHaveBeenCalled();
-  });
-
-  test("the loser may report the game", async () => {
-    const res = await request(app).post("/gameResults").set("x-test-user", "bob").send(WIN_BODY);
-    expect(res.status).toBe(201);
-  });
-});
-
-describe("POST /gameResults — validation", () => {
-  beforeEach(() => {
-    GameResults.findOne.mockResolvedValue(null);
-    GameResults.create.mockImplementation(async (doc) => doc);
-  });
-
-  const badBodies = [
-    ["missing gameId", { ...WIN_BODY, gameId: undefined }],
-    ["unknown result", { ...WIN_BODY, result: "forfeit" }],
-    ["unknown reason", { ...WIN_BODY, reason: "boredom" }],
-    ["win with no loser", { ...WIN_BODY, loserUsername: undefined }],
-    ["win against yourself", { ...WIN_BODY, loserUsername: "alice" }],
-    ["draw with one player", { ...DRAW_BODY, players: ["alice"] }],
-    ["draw with a non-draw reason", { ...DRAW_BODY, reason: "checkmate" }],
-  ];
-
-  test.each(badBodies)("400 — %s", async (_label, body) => {
-    const res = await request(app).post("/gameResults").send(body);
-    expect(res.status).toBe(400);
-    expect(GameResults.create).not.toHaveBeenCalled();
+    expect(res.status).toBe(404);
   });
 });
 
diff --git a/middlewareNode/tests/internalGameResults.test.js b/middlewareNode/tests/internalGameResults.test.js
new file mode 100644
index 00000000..6ba83fd3
--- /dev/null
+++ b/middlewareNode/tests/internalGameResults.test.js
@@ -0,0 +1,145 @@
+/**
+ * Integration tests — POST /internal/gameResults
+ *
+ * The chess server's own reporting path (PvP results plan v2, T3). Models are
+ * mocked here the same way tests/gameResults.test.js mocks GameResults — the
+ * real-database equivalent of the happy path is covered end-to-end by
+ * tests/contract.chessServerReport.test.js (T4b), which mounts this route for
+ * real against mongodb-memory-server.
+ */
+
+jest.mock("../src/models/gameResults");
+jest.mock("../src/models/PvpGame");
+
+const express = require("express");
+const request = require("supertest");
+const internalGameResults = require("../src/routes/internalGameResults");
+const GameResults = require("../src/models/gameResults");
+const PvpGame = require("../src/models/PvpGame");
+
+const app = express();
+app.use(express.json());
+app.use("/internal/gameResults", internalGameResults);
+
+const SERVICE_KEY = "test-service-key";
+
+beforeEach(() => {
+  process.env.CHESS_SERVICE_KEY = SERVICE_KEY;
+});
+
+afterEach(() => {
+  jest.clearAllMocks();
+  delete process.env.CHESS_SERVICE_KEY;
+});
+
+const WIN_BODY = {
+  gameId: "game-1",
+  result: "win",
+  reason: "checkmate",
+  winnerUsername: "alice",
+  loserUsername: "bob",
+};
+
+const ACTIVE_GAME = { gameId: "game-1", white: "alice", black: "bob", status: "active" };
+
+describe("POST /internal/gameResults — service key", () => {
+  test("401 — missing key", async () => {
+    const res = await request(app).post("/internal/gameResults").send(WIN_BODY);
+    expect(res.status).toBe(401);
+  });
+
+  test("401 — wrong key", async () => {
+    const res = await request(app)
+      .post("/internal/gameResults")
+      .set("X-Service-Key", "wrong-key")
+      .send(WIN_BODY);
+    expect(res.status).toBe(401);
+  });
+});
+
+describe("POST /internal/gameResults — game lookup and validation", () => {
+  test("404 — unknown gameId", async () => {
+    PvpGame.findOne.mockResolvedValue(null);
+
+    const res = await request(app)
+      .post("/internal/gameResults")
+      .set("X-Service-Key", SERVICE_KEY)
+      .send(WIN_BODY);
+    expect(res.status).toBe(404);
+  });
+
+  test("400 — winner/loser do not match the accepted game's players", async () => {
+    PvpGame.findOne.mockResolvedValue({ gameId: "game-1", white: "alice", black: "carol", status: "active" });
+
+    const res = await request(app)
+      .post("/internal/gameResults")
+      .set("X-Service-Key", SERVICE_KEY)
+      .send(WIN_BODY); // reports bob, not carol
+    expect(res.status).toBe(400);
+  });
+
+  test("400 — invalid body never reaches the game lookup", async () => {
+    const res = await request(app)
+      .post("/internal/gameResults")
+      .set("X-Service-Key", SERVICE_KEY)
+      .send({ ...WIN_BODY, result: "forfeit" });
+    expect(res.status).toBe(400);
+    expect(PvpGame.findOne).not.toHaveBeenCalled();
+  });
+});
+
+describe("POST /internal/gameResults — recording", () => {
+  beforeEach(() => {
+    PvpGame.findOne.mockResolvedValue({ ...ACTIVE_GAME });
+    PvpGame.updateOne.mockResolvedValue({ acknowledged: true });
+    GameResults.findOne.mockResolvedValue(null);
+    GameResults.create.mockImplementation(async (doc) => doc);
+  });
+
+  test("201 — happy path stores source: chessServer", async () => {
+    const res = await request(app)
+      .post("/internal/gameResults")
+      .set("X-Service-Key", SERVICE_KEY)
+      .send(WIN_BODY);
+
+    expect(res.status).toBe(201);
+    expect(res.body.duplicate).toBe(false);
+    expect(res.body.gameResult.source).toBe("chessServer");
+    expect(GameResults.create).toHaveBeenCalledWith(
+      expect.objectContaining({ gameId: "game-1", source: "chessServer" })
+    );
+    expect(PvpGame.updateOne).toHaveBeenCalledWith(
+      { gameId: "game-1", status: "active" },
+      { $set: expect.objectContaining({ status: "finished" }) }
+    );
+  });
+
+  test("200 duplicate:true — second report for the same game writes nothing", async () => {
+    GameResults.findOne.mockResolvedValue({ ...WIN_BODY, players: ["alice", "bob"], source: "chessServer" });
+
+    const res = await request(app)
+      .post("/internal/gameResults")
+      .set("X-Service-Key", SERVICE_KEY)
+      .send(WIN_BODY);
+
+    expect(res.status).toBe(200);
+    expect(res.body.duplicate).toBe(true);
+    expect(GameResults.create).not.toHaveBeenCalled();
+    expect(PvpGame.updateOne).not.toHaveBeenCalled();
+  });
+
+  test("a unique-index race is resolved as a duplicate, not a 500", async () => {
+    GameResults.findOne
+      .mockResolvedValueOnce(null)
+      .mockResolvedValueOnce({ ...WIN_BODY, players: ["alice", "bob"], source: "chessServer" });
+    GameResults.create.mockRejectedValue({ code: 11000 });
+
+    const res = await request(app)
+      .post("/internal/gameResults")
+      .set("X-Service-Key", SERVICE_KEY)
+      .send(WIN_BODY);
+
+    expect(res.status).toBe(200);
+    expect(res.body.duplicate).toBe(true);
+  });
+});

From cb11da03e5a28ee7f4be3db6d3d5a27377673156 Mon Sep 17 00:00:00 2001
From: saritahimthani <sarita.himthani123@gmail.com>
Date: Tue, 6 Oct 2026 16:07:14 -0700
Subject: [PATCH 2/4] PvP results (PR 2): chess server verifies identity and
 reports with its own key

newpvpgame now calls GET /challenge/game/:gameId with the joining
player's own JWT and seats them under the username the middleware's
auth resolves (`you`), never the client-claimed username - a mismatch
is rejected with gameerror instead of silently corrected.
createOrJoinPvpGame takes white/black from that response only.

Deletes the Authentication header bug at the old EventHandlers.js:60
rather than renaming it - the whole player-token report path is gone.
reportGameResult now builds its request with the new pure
reporting/resultRequest.js and authenticates with CHESS_SERVICE_KEY
instead of relaying a player's token. Seats no longer store player
credentials for reporting.

Adds validateEnvironment.js (new - no such startup check existed) so
a production boot without CHESS_SERVICE_KEY/MIDDLEWARE_URL fails fast
instead of silently skipping every report, plus .env.example.

Adds the cross-service contract test (T4b): it imports resultRequest.js
by relative path and sends its exact output through the real
/internal/gameResults route (real service-key check, real models) so
the two services can't each pass their own tests while disagreeing -
which is how the header bug went unnoticed in v1.

Updates the design doc and feature guide to describe the new trust
model, replacing the known v1 limitation.

See "PvP game results: server-authoritative reporting" (v2), T4, T4b, T7.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 chessServer/.env.example                      |  17 ++
 chessServer/src/managers/EventHandlers.js     |  96 ++++++------
 chessServer/src/managers/GameManager.js       |  55 +++----
 chessServer/src/reporting/resultRequest.js    |  43 +++++
 .../src/tests/EventHandlers.pvp.test.js       | 148 ++++++++++++++++++
 chessServer/src/tests/GameManager.test.js     |  47 +++---
 .../src/tests/validateEnvironment.test.js     |  66 ++++++++
 chessServer/src/validateEnvironment.js        |  16 +-
 documentation/student-vs-student-design.md    |  66 ++++++--
 .../student-vs-student-feature-guide.md       |  94 +++++++----
 .../tests/contract.chessServerReport.test.js  | 111 +++++++++++++
 11 files changed, 612 insertions(+), 147 deletions(-)
 create mode 100644 chessServer/src/reporting/resultRequest.js
 create mode 100644 chessServer/src/tests/EventHandlers.pvp.test.js
 create mode 100644 chessServer/src/tests/validateEnvironment.test.js
 create mode 100644 middlewareNode/tests/contract.chessServerReport.test.js

diff --git a/chessServer/.env.example b/chessServer/.env.example
index 4e25e7cc..6b187836 100644
--- a/chessServer/.env.example
+++ b/chessServer/.env.example
@@ -1,5 +1,22 @@
 NODE_ENV=development
+
+# Port the socket/HTTP server listens on. Defaults to 3001 if unset.
 PORT=3001
+
+# Comma-separated list of origins allowed to connect (CORS + Socket.IO).
+# Falls back to a hardcoded dev/prod allowlist in src/index.js if unset.
+# Required in production (one of CORS_ORIGIN or ALLOWED_ORIGINS).
 CORS_ORIGIN=http://localhost:3000,http://localhost:3002
 ALLOWED_ORIGINS=
+
+# Base URL of middlewareNode, used to verify PvP game identity
+# (GET /challenge/game/:gameId) and to report finished game results
+# (POST /internal/gameResults). Required in production.
 MIDDLEWARE_URL=http://localhost:8000
+
+# Shared secret the chess server authenticates with when reporting a PvP
+# result to the middleware (X-Service-Key header). Must match the same
+# value configured on middlewareNode. Required in production. See the PvP
+# results plan (v2), T5 — generate one random 32-byte value and store it as
+# a secret on both services, not in source control.
+CHESS_SERVICE_KEY=
diff --git a/chessServer/src/managers/EventHandlers.js b/chessServer/src/managers/EventHandlers.js
index a6958ba7..86c208c3 100644
--- a/chessServer/src/managers/EventHandlers.js
+++ b/chessServer/src/managers/EventHandlers.js
@@ -1,4 +1,5 @@
 const GameManager = require("./GameManager");
+const buildResultRequest = require("../reporting/resultRequest");
 
 const gameManager = new GameManager();
 
@@ -10,55 +11,35 @@ const gameManager = new GameManager();
  * balance to credit — the coin idea was dropped in favour of a separate
  * leaderboard stat). See documentation/student-vs-student-design.md §7.
  *
+ * Authenticated with CHESS_SERVICE_KEY, not a player's token — the chess
+ * server reports with its own identity, and the middleware checks the result
+ * against the PvpGame it saved when the challenge was accepted. See the PvP
+ * results plan (v2), target design point 4.
+ *
  * Idempotency is the middleware's job, keyed on `gameId`: a reconnect, a retry,
  * or both clients reporting the same game is a no-op there. This side just
  * reports once per decided game and tolerates failure — a lost report costs one
  * game's stats, it must never break the players' "game over" experience.
  *
- * Mirrors the existing activity pattern in the "move" handler, which PUTs to
- * `${MIDDLEWARE_URL}/activities/:username/activity` with a Bearer credential.
- *
  * @param {Object} game - the finished game (supplies gameId and both players)
  * @param {Object} outcome - { over, reason, winnerUsername?, loserUsername? }
- * @param {string} [credentials] - Bearer token of a player in this game
  */
-const reportGameResult = async (game, outcome, credentials) => {
+const reportGameResult = async (game, outcome) => {
     if (!process.env.MIDDLEWARE_URL) {
         console.log("[gameResults] MIDDLEWARE_URL unset — skipping report");
         return;
     }
-    // The middleware only accepts a report from a player in the game, so fall
-    // back to a seated player's token when the ending event carried none
-    // (resign and disconnect have no payload).
-    const token = credentials || (game.players || []).map((p) => p.credentials).find(Boolean);
-    if (!token) {
-        console.log(`[gameResults] no credentials for game ${game.gameId} — skipping report`);
+    if (!process.env.CHESS_SERVICE_KEY) {
+        console.log(`[gameResults] CHESS_SERVICE_KEY unset — skipping report for ${game.gameId}`);
         return;
     }
 
-    const isDraw = !outcome.winnerUsername;
-    const body = isDraw
-        ? {
-              gameId: game.gameId,
-              result: "draw",
-              reason: "draw",
-              players: game.players.map((p) => p.username),
-          }
-        : {
-              gameId: game.gameId,
-              result: "win",
-              reason: outcome.reason,
-              winnerUsername: outcome.winnerUsername,
-              loserUsername: outcome.loserUsername,
-          };
+    const { path, headers, body } = buildResultRequest(game, outcome, process.env.CHESS_SERVICE_KEY);
 
     try {
-        const response = await fetch(`${process.env.MIDDLEWARE_URL}/gameResults`, {
+        const response = await fetch(`${process.env.MIDDLEWARE_URL}${path}`, {
             method: "POST",
-            headers: {
-                "Content-Type": "application/json",
-                Authentication: `Bearer ${token}`,
-            },
+            headers,
             body: JSON.stringify(body),
         });
         if (!response.ok) {
@@ -81,9 +62,8 @@ const reportGameResult = async (game, outcome, credentials) => {
  * @param {Object} game - the finished game
  * @param {Object} outcome - { over, reason, winnerUsername?, loserUsername? }
  * @param {Server} io
- * @param {string} [credentials] - Bearer token of the reporting client
  */
-const emitGameOver = async (game, outcome, io, credentials) => {
+const emitGameOver = async (game, outcome, io) => {
     const payload = JSON.stringify(outcome);
     [game.student.id, game.mentor.id].forEach((id) => {
         if (id) io.to(id).emit("gameover", payload);
@@ -92,7 +72,7 @@ const emitGameOver = async (game, outcome, io, credentials) => {
     // A draw still counts as a played game, so report it too — only the
     // opponent-never-joined case (no usernames at all) is skipped.
     if (game.isPvp && (outcome.winnerUsername || outcome.reason === "draw")) {
-        await reportGameResult(game, outcome, credentials);
+        await reportGameResult(game, outcome);
     }
 };
 
@@ -134,20 +114,48 @@ const registerSocketHandlers = (socket, io) => {
 
     /**
      * Handles creating or joining a student-vs-student (PvP) game by gameId.
-     * The gameId + both usernames come from an accepted challenge (middleware).
-     * Expected payload: { gameId, challenger, opponent, username, credentials }
+     *
+     * Identity is verified against the middleware, never trusted from the
+     * client: the chess server calls GET /challenge/game/:gameId with the
+     * joining player's own token, and seats the player under the `you` the
+     * middleware returns (derived from that token), not the `username` field
+     * the client sent. If the two differ, the client is lying about who it
+     * is — the join is rejected rather than silently corrected, so a client
+     * bug surfaces instead of being hidden. `white`/`black` likewise come
+     * only from that response. See the PvP results plan (v2), target design
+     * point 2 and T4.
+     *
+     * Expected payload: { gameId, username, credentials }
      */
-    socket.on("newpvpgame", (msg) => {
+    socket.on("newpvpgame", async (msg) => {
         try {
             const parsed = JSON.parse(msg);
+            const { gameId, username, credentials } = parsed;
+
+            if (!process.env.MIDDLEWARE_URL) {
+                throw new Error("MIDDLEWARE_URL unset — cannot verify game");
+            }
+
+            const response = await fetch(`${process.env.MIDDLEWARE_URL}/challenge/game/${gameId}`, {
+                headers: { Authorization: `Bearer ${credentials}` },
+            });
+            if (!response.ok) {
+                socket.emit("gameerror", `Unable to verify game (${response.status})`);
+                return;
+            }
+            const { you, white, black } = await response.json();
+
+            if (username !== you) {
+                socket.emit("gameerror", "username does not match your login");
+                return;
+            }
 
             const result = gameManager.createOrJoinPvpGame({
-                gameId: parsed.gameId,
-                challenger: parsed.challenger,
-                opponent: parsed.opponent,
-                username: parsed.username,
-                socketId: socket.id,
-                credentials: parsed.credentials
+                gameId,
+                username: you,
+                white,
+                black,
+                socketId: socket.id
             });
 
             socket.emit(
@@ -222,7 +230,7 @@ const registerSocketHandlers = (socket, io) => {
             if (outcome && outcome.over) {
                 const game = gameManager.getGameBySocketId(socket.id);
                 if (game) {
-                    await emitGameOver(game, outcome, io, credentials);
+                    await emitGameOver(game, outcome, io);
                 }
             }
             if(!computerMove && credentials) {
diff --git a/chessServer/src/managers/GameManager.js b/chessServer/src/managers/GameManager.js
index 815b1bec..7cd6c930 100644
--- a/chessServer/src/managers/GameManager.js
+++ b/chessServer/src/managers/GameManager.js
@@ -90,29 +90,33 @@ class GameManager {
      * `game.isPvp` marks that the slot NAMES carry no meaning in this game.
      * See student-vs-student-design.md §5a.
      *
-     * The challenger takes white. Whichever client connects first creates the
-     * game; the second joins it by gameId. Reconnecting with the same username
-     * reclaims the original seat and color rather than creating a new game.
+     * `username`, `white` and `black` must come from the middleware's
+     * GET /challenge/game/:gameId response — never from the client socket
+     * message directly. The caller (EventHandlers' `newpvpgame` handler)
+     * verifies the joining socket against that response before calling this,
+     * so by the time this method runs, identity has already been checked.
+     * Whichever client connects first creates the game; the second joins it
+     * by gameId. Reconnecting with the same username reclaims the original
+     * seat and color rather than creating a new game.
      *
-     * Each seat also carries the joining client's `credentials` (bearer token).
-     * The result report at game end is sent to the middleware as one of the two
-     * players, and resign/disconnect endings are triggered by a socket event
-     * that carries no body — so the token has to be captured at join time.
+     * Seats no longer carry a player token: results are reported to the
+     * middleware with CHESS_SERVICE_KEY, not a player's bearer token, so
+     * there is nothing to capture here. See the PvP results plan (v2), T4.
      *
-     * @param {Object} param0 - Contains gameId, challenger, opponent, username, socketId, credentials
+     * @param {Object} param0 - Contains gameId, username, white, black, socketId
      * @returns {Object} Game object, assigned color, and new game status
      */
-    createOrJoinPvpGame({ gameId, challenger, opponent, username, socketId, credentials }) {
+    createOrJoinPvpGame({ gameId, username, white, black, socketId }) {
         if (!gameId) {
             throw new Error("A gameId is required to join a student-vs-student game!");
         }
-        if (!challenger || !opponent) {
-            throw new Error("Both challenger and opponent usernames are required!");
+        if (!white || !black) {
+            throw new Error("Both white and black usernames are required!");
         }
-        if (challenger === opponent) {
+        if (white === black) {
             throw new Error("A student cannot challenge themselves!");
         }
-        if (username !== challenger && username !== opponent) {
+        if (username !== white && username !== black) {
             throw new Error("You are not a player in this game!");
         }
 
@@ -125,30 +129,27 @@ class GameManager {
                 throw new Error("You are not a player in this game!");
             }
             seat.id = socketId;
-            if (credentials) seat.credentials = credentials;
             return { game, color: seat.color, newGame: false };
         }
 
         // First player in creates the game; both seats are known upfront from
-        // the accepted challenge, so the opponent's seat just waits for a socket.
+        // the accepted challenge, so the second player's seat just waits for a socket.
         const board = new Chess();
-        const challengerPlayer = {
-            username: challenger,
-            id: username === challenger ? socketId : null,
-            credentials: username === challenger ? credentials : null,
+        const whitePlayer = {
+            username: white,
+            id: username === white ? socketId : null,
             color: "white"
         };
-        const opponentPlayer = {
-            username: opponent,
-            id: username === opponent ? socketId : null,
-            credentials: username === opponent ? credentials : null,
+        const blackPlayer = {
+            username: black,
+            id: username === black ? socketId : null,
             color: "black"
         };
 
         const newGame = {
-            student: challengerPlayer,
-            mentor: opponentPlayer,
-            players: [challengerPlayer, opponentPlayer],
+            student: whitePlayer,
+            mentor: blackPlayer,
+            players: [whitePlayer, blackPlayer],
             gameId,
             isPvp: true,
             boardState: board,
@@ -159,7 +160,7 @@ class GameManager {
 
         return {
             game: newGame,
-            color: username === challenger ? "white" : "black",
+            color: username === white ? "white" : "black",
             newGame: true
         };
     }
diff --git a/chessServer/src/reporting/resultRequest.js b/chessServer/src/reporting/resultRequest.js
new file mode 100644
index 00000000..3f66dc54
--- /dev/null
+++ b/chessServer/src/reporting/resultRequest.js
@@ -0,0 +1,43 @@
+/**
+ * Builds the chess server's game-result report to the middleware.
+ *
+ * A pure function with no `require` calls, so middlewareNode's own tests can
+ * import this file directly by relative path and assert against the exact
+ * request it produces (see middlewareNode/tests/contract.chessServerReport.test.js,
+ * the PvP results plan v2's T4b). If the header name or the path ever drifts
+ * between the two services, that contract test fails in CI instead of both
+ * sides quietly agreeing with themselves. See target design point 5.
+ *
+ * @param {Object} game - the finished game; supplies gameId and both players
+ * @param {Object} outcome - { reason, winnerUsername?, loserUsername? }
+ * @param {string} key - the shared secret (CHESS_SERVICE_KEY)
+ * @returns {{ path: string, headers: Object, body: Object }}
+ */
+function buildResultRequest(game, outcome, key) {
+  const isDraw = !outcome.winnerUsername;
+  const body = isDraw
+    ? {
+        gameId: game.gameId,
+        result: "draw",
+        reason: "draw",
+        players: game.players.map((p) => p.username),
+      }
+    : {
+        gameId: game.gameId,
+        result: "win",
+        reason: outcome.reason,
+        winnerUsername: outcome.winnerUsername,
+        loserUsername: outcome.loserUsername,
+      };
+
+  return {
+    path: "/internal/gameResults",
+    headers: {
+      "Content-Type": "application/json",
+      "X-Service-Key": key,
+    },
+    body,
+  };
+}
+
+module.exports = buildResultRequest;
diff --git a/chessServer/src/tests/EventHandlers.pvp.test.js b/chessServer/src/tests/EventHandlers.pvp.test.js
new file mode 100644
index 00000000..d2d3b5be
--- /dev/null
+++ b/chessServer/src/tests/EventHandlers.pvp.test.js
@@ -0,0 +1,148 @@
+/**
+ * Unit tests — "newpvpgame" verifies identity against the middleware before
+ * seating a player (never trusts the client's own claim), and a finished PvP
+ * game reports its result with the chess server's own service key. See the
+ * PvP results plan (v2), T4.
+ *
+ * global.fetch is mocked; everything else (GameManager, the socket handlers)
+ * is real. Sockets are plain objects with a jest.fn() emit and an on() that
+ * records handlers, driven directly — no real network/socket.io needed for
+ * this layer (src/tests/index.test.js already covers the real-socket path).
+ */
+
+const registerSocketHandlers = require("../managers/EventHandlers");
+
+function fakeSocket(id) {
+  const handlers = {};
+  return {
+    id,
+    on: (event, handler) => {
+      handlers[event] = handler;
+    },
+    emit: jest.fn(),
+    _handlers: handlers,
+  };
+}
+
+const fakeIo = {
+  to: () => ({ emit: jest.fn() }),
+  // GameManager.broadcastBoardState reaches into io.sockets.sockets.get(id);
+  // no real sockets are registered here, so every lookup is a harmless miss.
+  sockets: { sockets: { get: () => undefined } },
+};
+
+beforeEach(() => {
+  process.env.MIDDLEWARE_URL = "http://middleware.test";
+  global.fetch = jest.fn();
+});
+
+afterEach(() => {
+  jest.resetAllMocks();
+  delete process.env.MIDDLEWARE_URL;
+  delete process.env.CHESS_SERVICE_KEY;
+});
+
+describe("newpvpgame — identity is verified against the middleware", () => {
+  test("rejects when the client-claimed username does not match the middleware's", async () => {
+    global.fetch.mockResolvedValue({
+      ok: true,
+      json: async () => ({ gameId: "g1", you: "alice", white: "alice", black: "bob", status: "active" }),
+    });
+
+    const socket = fakeSocket("s1");
+    registerSocketHandlers(socket, fakeIo);
+
+    await socket._handlers["newpvpgame"](
+      JSON.stringify({ gameId: "g1", username: "mallory", credentials: "tok" })
+    );
+
+    expect(socket.emit).toHaveBeenCalledWith("gameerror", "username does not match your login");
+  });
+
+  test("rejects an unaccepted/unknown gameId", async () => {
+    global.fetch.mockResolvedValue({ ok: false, status: 404 });
+
+    const socket = fakeSocket("s1");
+    registerSocketHandlers(socket, fakeIo);
+
+    await socket._handlers["newpvpgame"](
+      JSON.stringify({ gameId: "ghost", username: "alice", credentials: "tok" })
+    );
+
+    expect(socket.emit).toHaveBeenCalledWith("gameerror", expect.stringContaining("404"));
+  });
+
+  test("seats the verified player using white/black from the middleware response only", async () => {
+    global.fetch.mockResolvedValue({
+      ok: true,
+      json: async () => ({ gameId: "g1", you: "alice", white: "alice", black: "bob", status: "active" }),
+    });
+
+    const socket = fakeSocket("s1");
+    registerSocketHandlers(socket, fakeIo);
+
+    // The client claims the opposite colors — they must be ignored.
+    await socket._handlers["newpvpgame"](
+      JSON.stringify({ gameId: "g1", username: "alice", white: "mallory", black: "mallory", credentials: "tok" })
+    );
+
+    expect(socket.emit).toHaveBeenCalledWith("boardstate", expect.any(String));
+    const [, payload] = socket.emit.mock.calls.find(([event]) => event === "boardstate");
+    expect(JSON.parse(payload).color).toBe("white");
+  });
+});
+
+describe("a finished PvP game reports once, with the chess server's own key", () => {
+  test("checkmate reports exactly once to /internal/gameResults with X-Service-Key", async () => {
+    process.env.CHESS_SERVICE_KEY = "test-key";
+
+    global.fetch.mockImplementation((url, opts) => {
+      if (String(url).endsWith("/challenge/game/g1")) {
+        const you = opts.headers.Authorization.replace("Bearer ", "");
+        return Promise.resolve({
+          ok: true,
+          json: async () => ({ gameId: "g1", you, white: "alice", black: "bob", status: "active" }),
+        });
+      }
+      if (String(url).endsWith("/internal/gameResults")) {
+        return Promise.resolve({ ok: true, status: 201 });
+      }
+      return Promise.resolve({ ok: false, status: 404 });
+    });
+
+    const alice = fakeSocket("sAlice");
+    const bob = fakeSocket("sBob");
+    registerSocketHandlers(alice, fakeIo);
+    registerSocketHandlers(bob, fakeIo);
+
+    await alice._handlers["newpvpgame"](
+      JSON.stringify({ gameId: "g1", username: "alice", credentials: "alice" })
+    );
+    await bob._handlers["newpvpgame"](
+      JSON.stringify({ gameId: "g1", username: "bob", credentials: "bob" })
+    );
+
+    // Fool's Mate: 1. f3 e5 2. g4 Qh4# — white (alice) is checkmated.
+    await alice._handlers["move"](JSON.stringify({ from: "f2", to: "f3" }));
+    await bob._handlers["move"](JSON.stringify({ from: "e7", to: "e5" }));
+    await alice._handlers["move"](JSON.stringify({ from: "g2", to: "g4" }));
+    await bob._handlers["move"](JSON.stringify({ from: "d8", to: "h4" }));
+
+    const reportCalls = global.fetch.mock.calls.filter(([url]) =>
+      String(url).endsWith("/internal/gameResults")
+    );
+    expect(reportCalls).toHaveLength(1);
+
+    const [url, opts] = reportCalls[0];
+    expect(url).toBe("http://middleware.test/internal/gameResults");
+    expect(opts.headers["X-Service-Key"]).toBe("test-key");
+    const body = JSON.parse(opts.body);
+    expect(body).toEqual({
+      gameId: "g1",
+      result: "win",
+      reason: "checkmate",
+      winnerUsername: "bob",
+      loserUsername: "alice",
+    });
+  });
+});
diff --git a/chessServer/src/tests/GameManager.test.js b/chessServer/src/tests/GameManager.test.js
index d99e1613..d4abfa31 100644
--- a/chessServer/src/tests/GameManager.test.js
+++ b/chessServer/src/tests/GameManager.test.js
@@ -143,10 +143,15 @@ describe('GameManager', () => {
   });
 
   // --- Student-vs-student (PvP) join by gameId (§5) -----------------------
+  //
+  // gameId, username, white and black all come from the middleware's
+  // GET /challenge/game/:gameId response (verified by EventHandlers before
+  // calling this) — never from the client directly. See the PvP results
+  // plan (v2), T4.
 
-  test('creates a PvP game and seats the challenger as white', () => {
+  test('creates a PvP game and seats white as white', () => {
     const res = gameManager.createOrJoinPvpGame({
-      gameId: 'g1', challenger: 'Alice', opponent: 'Cara',
+      gameId: 'g1', white: 'Alice', black: 'Cara',
       username: 'Alice', socketId: 'sA'
     });
     expect(res.newGame).toBe(true);
@@ -155,44 +160,28 @@ describe('GameManager', () => {
     expect(gameManager.getGameByGameId('g1')).toBe(res.game);
   });
 
-  test('each PvP seat keeps its own credentials for the end-of-game report', () => {
-    // Resign and disconnect carry no payload, so the token has to be captured
-    // at join time or the result can never be reported to the middleware.
+  test('reconnecting with the same username reclaims the original seat', () => {
     gameManager.createOrJoinPvpGame({
-      gameId: 'g1', challenger: 'Alice', opponent: 'Cara',
-      username: 'Alice', socketId: 'sA', credentials: 'token-alice'
-    });
-    const res = gameManager.createOrJoinPvpGame({
-      gameId: 'g1', challenger: 'Alice', opponent: 'Cara',
-      username: 'Cara', socketId: 'sC', credentials: 'token-cara'
-    });
-
-    const seats = Object.fromEntries(res.game.players.map((p) => [p.username, p.credentials]));
-    expect(seats).toEqual({ Alice: 'token-alice', Cara: 'token-cara' });
-  });
-
-  test('reconnecting refreshes the seat credentials rather than blanking them', () => {
-    gameManager.createOrJoinPvpGame({
-      gameId: 'g1', challenger: 'Alice', opponent: 'Cara',
-      username: 'Alice', socketId: 'sA', credentials: 'token-alice'
+      gameId: 'g1', white: 'Alice', black: 'Cara',
+      username: 'Alice', socketId: 'sA'
     });
-    // Reconnect with no token supplied — keep the one we already had.
     const res = gameManager.createOrJoinPvpGame({
-      gameId: 'g1', challenger: 'Alice', opponent: 'Cara',
+      gameId: 'g1', white: 'Alice', black: 'Cara',
       username: 'Alice', socketId: 'sA2'
     });
     const alice = res.game.players.find((p) => p.username === 'Alice');
     expect(alice.id).toBe('sA2');
-    expect(alice.credentials).toBe('token-alice');
+    expect(res.color).toBe('white');
+    expect(res.newGame).toBe(false);
   });
 
   test('second PvP player joins the same game by gameId as black', () => {
     gameManager.createOrJoinPvpGame({
-      gameId: 'g1', challenger: 'Alice', opponent: 'Cara',
+      gameId: 'g1', white: 'Alice', black: 'Cara',
       username: 'Alice', socketId: 'sA'
     });
     const res = gameManager.createOrJoinPvpGame({
-      gameId: 'g1', challenger: 'Alice', opponent: 'Cara',
+      gameId: 'g1', white: 'Alice', black: 'Cara',
       username: 'Cara', socketId: 'sC'
     });
     expect(res.newGame).toBe(false);
@@ -202,7 +191,7 @@ describe('GameManager', () => {
 
   test('rejects a non-player trying to join a PvP game', () => {
     expect(() => gameManager.createOrJoinPvpGame({
-      gameId: 'g1', challenger: 'Alice', opponent: 'Cara',
+      gameId: 'g1', white: 'Alice', black: 'Cara',
       username: 'Mallory', socketId: 'sM'
     })).toThrow(/not a player/);
   });
@@ -217,11 +206,11 @@ describe('GameManager', () => {
 
   test('a forfeit in a PvP game awards the win to the opponent', () => {
     gameManager.createOrJoinPvpGame({
-      gameId: 'g1', challenger: 'Alice', opponent: 'Cara',
+      gameId: 'g1', white: 'Alice', black: 'Cara',
       username: 'Alice', socketId: 'sA'
     });
     gameManager.createOrJoinPvpGame({
-      gameId: 'g1', challenger: 'Alice', opponent: 'Cara',
+      gameId: 'g1', white: 'Alice', black: 'Cara',
       username: 'Cara', socketId: 'sC'
     });
     const res = gameManager.resign('sC', 'disconnect'); // Cara drops
diff --git a/chessServer/src/tests/validateEnvironment.test.js b/chessServer/src/tests/validateEnvironment.test.js
new file mode 100644
index 00000000..b5bcbce6
--- /dev/null
+++ b/chessServer/src/tests/validateEnvironment.test.js
@@ -0,0 +1,66 @@
+const validateEnvironment = require("../validateEnvironment");
+
+const ORIGINAL_ENV = process.env;
+
+beforeEach(() => {
+  process.env = { ...ORIGINAL_ENV };
+  jest.spyOn(console, "error").mockImplementation(() => {});
+  jest.spyOn(console, "log").mockImplementation(() => {});
+  jest.spyOn(process, "exit").mockImplementation(() => {});
+});
+
+afterEach(() => {
+  jest.restoreAllMocks();
+});
+
+afterAll(() => {
+  process.env = ORIGINAL_ENV;
+});
+
+describe("validateEnvironment", () => {
+  test("does nothing outside production", () => {
+    process.env.NODE_ENV = "test";
+    delete process.env.CHESS_SERVICE_KEY;
+    delete process.env.MIDDLEWARE_URL;
+    validateEnvironment();
+    expect(process.exit).not.toHaveBeenCalled();
+  });
+
+  test("exits in production when CHESS_SERVICE_KEY is missing", () => {
+    process.env.NODE_ENV = "production";
+    process.env.MIDDLEWARE_URL = "https://middleware.example.com";
+    process.env.CORS_ORIGIN = "https://ystemandchess.com";
+    delete process.env.CHESS_SERVICE_KEY;
+
+    validateEnvironment();
+
+    expect(process.exit).toHaveBeenCalledWith(1);
+    expect(console.error).toHaveBeenCalledWith(expect.stringContaining("CHESS_SERVICE_KEY"));
+  });
+
+  test("exits in production when neither CORS_ORIGIN nor ALLOWED_ORIGINS is set", () => {
+    process.env.NODE_ENV = "production";
+    process.env.MIDDLEWARE_URL = "https://middleware.example.com";
+    process.env.CHESS_SERVICE_KEY = "a-real-key";
+    delete process.env.CORS_ORIGIN;
+    delete process.env.ALLOWED_ORIGINS;
+
+    validateEnvironment();
+
+    expect(process.exit).toHaveBeenCalledWith(1);
+    expect(console.error).toHaveBeenCalledWith(
+      expect.stringContaining("CORS_ORIGIN or ALLOWED_ORIGINS")
+    );
+  });
+
+  test("passes in production when every required var is set", () => {
+    process.env.NODE_ENV = "production";
+    process.env.MIDDLEWARE_URL = "https://middleware.example.com";
+    process.env.CHESS_SERVICE_KEY = "a-real-key";
+    process.env.CORS_ORIGIN = "https://ystemandchess.com";
+
+    validateEnvironment();
+
+    expect(process.exit).not.toHaveBeenCalled();
+  });
+});
diff --git a/chessServer/src/validateEnvironment.js b/chessServer/src/validateEnvironment.js
index 14a9bdc7..7cb6ded1 100644
--- a/chessServer/src/validateEnvironment.js
+++ b/chessServer/src/validateEnvironment.js
@@ -1,6 +1,16 @@
-const REQUIRED_PRODUCTION_VARS = [
-  "MIDDLEWARE_URL",
-];
+/**
+ * Fails fast in production when required secrets/config are missing, instead
+ * of booting and silently degrading. CHESS_SERVICE_KEY unset has exactly that
+ * silent-failure shape today: the PvP result report just logs and skips (see
+ * EventHandlers.reportGameResult) — this is the production-startup guard the
+ * PvP results plan (v2), T4, asks for, folded into the env check this repo's
+ * "signed-environment-separation" work already added for MIDDLEWARE_URL/CORS.
+ *
+ * Only enforced when NODE_ENV === "production" — local/dev/test runs are
+ * unaffected, same as the other "skip and log" call sites already behave.
+ */
+
+const REQUIRED_PRODUCTION_VARS = ["MIDDLEWARE_URL", "CHESS_SERVICE_KEY"];
 
 function hasCorsConfiguration() {
   return Boolean(
diff --git a/documentation/student-vs-student-design.md b/documentation/student-vs-student-design.md
index 76b94982..fe13bee8 100644
--- a/documentation/student-vs-student-design.md
+++ b/documentation/student-vs-student-design.md
@@ -172,25 +172,37 @@ The `gameover` event triggers one side-effect: the chessServer reports the
 finished game to the middleware. No balance is credited — the result is stored
 and the score is derived from it.
 
+**v2 update** ("PvP game results: server-authoritative reporting"): there is
+no player-facing write anymore. Only the chess server can report a result,
+authenticated with its own service credential, and only for a game the
+middleware already knows was accepted by two real players. See §10 for why.
+
 ### Route naming
 
-`POST /gameResults` — plural, resource-first, matching the existing
-`/leaderboard`, `/badges`, `/activities` convention.
+`/gameResults` — plural, resource-first, matching the existing `/leaderboard`,
+`/badges`, `/activities` convention. The chess server's own write path is
+nested under `/internal` to mark it as not player-facing.
 
 ### Contract
 
 ```
-POST /gameResults          (requireAuth; caller must be one of the two players)
+POST /internal/gameResults   (requireServiceKey — X-Service-Key: CHESS_SERVICE_KEY)
 
   win:   { gameId, result: "win",  reason: "checkmate"|"resign"|"disconnect",
            winnerUsername, loserUsername, playedAt? }
   draw:  { gameId, result: "draw", reason: "draw", players: [a, b], playedAt? }
 
-  -> 201 { success, duplicate: false, gameResult }
+  -> 201 { success, duplicate: false, gameResult }   // gameResult.source: "chessServer"
   -> 200 { success, duplicate: true,  gameResult }   // already recorded
+  -> 404 unknown gameId (no accepted PvpGame)
+  -> 400 reported players don't match the accepted game
 
-GET /gameResults/:username
+GET /gameResults/:username    (requireAuth — any logged-in role)
   -> { success, data: { wins, draws, losses, gamesPlayed, chessScore } }
+
+GET /challenge/game/:gameId   (requireAuth)
+  -> { gameId, you, white, black, status }   // `you` is derived from the caller's own JWT
+  -> 403 if the caller is not one of the two players; 404 if unknown
 ```
 
 ### Why a record, not a counter
@@ -241,12 +253,26 @@ duplicate, not a 500.
 ## 9. Testing
 
 - Unit: `chessServer/src/tests/GameManager.test.js` — checkmate/draw/resign
-  outcomes, winner resolution by color, PvP pairing, per-seat credentials.
+  outcomes, winner resolution by color, PvP pairing by `white`/`black`.
+- Unit: `chessServer/src/tests/EventHandlers.pvp.test.js` — `newpvpgame`
+  rejects a client-claimed username that doesn't match the middleware's, seats
+  only from the middleware's `white`/`black`, and a finished game reports
+  exactly once with `CHESS_SERVICE_KEY`.
 - Integration: two socket clients play a scholar's-mate line; assert both
   receive `gameover` with the correct `winnerUsername`.
-- Result API: `middlewareNode/tests/gameResults.test.js` — idempotency on
-  `gameId` (including the unique-index race), the participant-only guard, and
-  win/draw body validation.
+- Result API: `middlewareNode/tests/internalGameResults.test.js` — service-key
+  auth, unknown-gameId / player-mismatch rejection, idempotency on `gameId`
+  (including the unique-index race). `middlewareNode/tests/gameResults.test.js`
+  covers the read side and that the legacy player-facing POST is gone (404).
+- Contract: `middlewareNode/tests/contract.chessServerReport.test.js` imports
+  `chessServer/src/reporting/resultRequest.js` directly and sends its exact
+  output through the real `/internal/gameResults` route (real service-key
+  check, real models via mongodb-memory-server) — the two services can't pass
+  their own tests in isolation while disagreeing with each other, which is how
+  the v1 header bug went unnoticed.
+- Challenge persistence: `middlewareNode/tests/challenge.pvpgame.test.js` —
+  accepting a challenge persists exactly one `PvpGame`, and
+  `GET /challenge/game/:gameId` returns it only to one of its two players.
 - Separation: `middlewareNode/tests/leaderboard.test.js` asserts chess results
   never move the engagement `score`, and that `sortBy=chess` ranks
   independently of it.
@@ -258,9 +284,19 @@ duplicate, not a 500.
 Matchmaking queue / ELO, spectators, rematch, anti-cheat, and any spendable
 currency. Direct-challenge only.
 
-Known limitation carried into v1: the reporting client is trusted to report
-honestly. `POST /gameResults` requires the caller to be one of the two players,
-which stops a third party fabricating results, but a player could still report
-a game they lost as a win. Closing that means the chessServer holding its own
-service credential rather than relaying a player's token — worth doing before
-the chess score is used for anything that matters.
+**Closed in v2** ("PvP game results: server-authoritative reporting"): v1's
+known limitation — a player's own token reported their own game's result, so a
+losing player could report themselves a win — is gone. The chess server now
+holds its own service credential (`CHESS_SERVICE_KEY`) and reports through
+`POST /internal/gameResults`, which only the chess server can call. Identity
+is verified where players join, not trusted from reporting: on `newpvpgame`
+the chess server calls `GET /challenge/game/:gameId` with the joining player's
+own JWT and seats them under the username the middleware's own auth resolves,
+never the username the client claims. A result is only accepted if its
+winner/loser match the `PvpGame` the middleware recorded when the challenge
+was accepted — so a fabricated `gameId` or mismatched players is rejected
+before anything is stored. See §7 for the current contract.
+
+Still out of scope: win trading between two cooperating real accounts (a
+separate follow-up plan), matchmaking, ELO, spectators and rematch (unchanged
+from v1, still gated on usage data this fix makes possible to collect).
diff --git a/documentation/student-vs-student-feature-guide.md b/documentation/student-vs-student-feature-guide.md
index c45582f4..5737f6e7 100644
--- a/documentation/student-vs-student-feature-guide.md
+++ b/documentation/student-vs-student-feature-guide.md
@@ -19,33 +19,48 @@ engagement score — see the design doc §1). Three layers cooperate:
 Student A profile ──"Challenge cara"──▶ middleware /challenge ──▶ B's incoming list
         (PlayStudent.tsx)                    (in-memory)                (B short-polls)
                                                                             │ Accept
-        both sides now hold the same gameId ◀───────────────────────────────┘
+        both sides now hold the same gameId, and the middleware
+        saves a PvpGame {gameId, white: A, black: B, status: "active"} ◀───┘
                                     │
-   each client ─"newpvpgame {gameId, challenger, opponent, username, credentials}"─▶ chessServer
-                                    │  (createOrJoinPvpGame pairs them: challenger = white)
+   each client ─"newpvpgame {gameId, username, credentials}"─▶ chessServer
+                                    │  chessServer calls GET /challenge/game/:gameId
+                                    │  with the player's own JWT, gets back {you, white, black}
+                                    │  from the MIDDLEWARE's auth — never trusts the client's
+                                    │  own claim. Mismatch → rejected, nobody seated.
+                                    │  createOrJoinPvpGame pairs them from that response: white = white seat.
                         ...moves sync over sockets...
                                     │
         a move causes checkmate ──▶ detectOutcome() resolves winner BY COLOR
                                     │
         chessServer emits "gameover" {winnerUsername, loserUsername, reason} to BOTH
                                     │
-        POST /gameResults ──▶ one immutable record, idempotent on gameId
+        POST /internal/gameResults (X-Service-Key: CHESS_SERVICE_KEY) ──▶
+          middleware checks winner/loser match the saved PvpGame, then stores
+          one immutable record, idempotent on gameId, source: "chessServer"
                                     │
         leaderboard / analytics compute W-D-L + chessScore on read
 ```
 
+Players never write their own result — only the chess server can, and only
+for a game the middleware already knows two real players accepted. See the
+design doc §7 and §10.
+
 ### Key files
 
 | File | Responsibility |
 |---|---|
 | `react-ystemandchess/src/features/student/student-profile/PlayStudent.tsx` | "Play a Student" tab: send challenge, poll for acceptance, accept/decline incoming |
 | `react-ystemandchess/.../Modals/LeaderboardModal.tsx` | Sortable **Chess** column (score + W–D–L), separate from Score |
-| `middlewareNode/src/routes/challenge.js` | Challenge handshake endpoints (in-memory, TTL-swept) |
-| `middlewareNode/src/routes/gameResults.js` | `POST /gameResults` (idempotent, participant-only), `GET /gameResults/:username` |
-| `middlewareNode/src/models/gameResults.js` | One immutable record per finished game; `gameId` unique |
+| `middlewareNode/src/routes/challenge.js` | Challenge handshake endpoints (in-memory, TTL-swept); accept also saves a `PvpGame`; `GET /challenge/game/:gameId` lets the chess server verify a joining player |
+| `middlewareNode/src/models/PvpGame.js` | The middleware's record of who the two real players in a `gameId` are; `active` → `finished` |
+| `middlewareNode/src/middleware/requireServiceKey.js` | Gates `/internal/gameResults` on `CHESS_SERVICE_KEY` instead of a player JWT |
+| `middlewareNode/src/routes/internalGameResults.js` | `POST /internal/gameResults` — the chess server's only write path, validated against the saved `PvpGame` |
+| `middlewareNode/src/routes/gameResults.js` | `GET /gameResults/:username` only — no player-facing POST |
+| `middlewareNode/src/models/gameResults.js` | One immutable record per finished game; `gameId` unique; `source: "chessServer" \| "legacy-unverified"` |
 | `middlewareNode/src/utils/studentStats.js` | `getChessRecord` / `getChessRecords` / `chessScoreFrom` — the single scoring source |
-| `chessServer/src/managers/GameManager.js` | `createOrJoinPvpGame`, `detectOutcome`, `resign`, the `isOver` latch |
-| `chessServer/src/managers/EventHandlers.js` | `newpvpgame` / `resign` socket events, `emitGameOver` + `reportGameResult` |
+| `chessServer/src/managers/GameManager.js` | `createOrJoinPvpGame` (seats from `white`/`black` only), `detectOutcome`, `resign`, the `isOver` latch |
+| `chessServer/src/managers/EventHandlers.js` | `newpvpgame` (verifies identity via the middleware) / `resign` socket events, `emitGameOver` + `reportGameResult` |
+| `chessServer/src/reporting/resultRequest.js` | Pure function building the `{path, headers, body}` the chess server sends; imported directly by the middleware's contract test |
 
 ---
 
@@ -55,14 +70,18 @@ Student A profile ──"Challenge cara"──▶ middleware /challenge ──
 
 ```bash
 cd chessServer && npx jest src/tests/GameManager.test.js
-# → Tests: 17 passed
+cd chessServer && npx jest src/tests/EventHandlers.pvp.test.js   # identity verification + one report
 ```
 
-### Result API — idempotency, participant guard, scoring
+### Result API — service-key auth, idempotency, scoring
 
 ```bash
-cd middlewareNode && npx jest tests/gameResults.test.js
-# → Tests: 16 passed
+cd middlewareNode && npx jest tests/internalGameResults.test.js   # the chess server's write path
+cd middlewareNode && npx jest tests/gameResults.test.js           # read path + legacy POST is gone
+cd middlewareNode && npx jest tests/challenge.pvpgame.test.js     # accept persists a PvpGame
+
+# Cross-service: the chess server's actual request shape against the real route
+cd middlewareNode && npx jest tests/contract.chessServerReport.test.js
 
 # The separation guarantee (chess results never move the engagement score):
 cd middlewareNode && npx jest tests/leaderboard.test.js
@@ -114,30 +133,37 @@ curl -sX POST localhost:8000/challenge -H 'Content-Type: application/json' \
   -d '{"fromUsername":"alice","toUsername":"cara"}'          # → {challengeId, gameId}
 curl -s localhost:8000/challenge/incoming/cara               # cara sees it
 curl -sX POST localhost:8000/challenge/<challengeId>/accept  # → {gameId, challenger, opponent}
+                                                               #   (also saves a PvpGame)
+
+# either player can verify the game — this is what the chess server calls on join
+curl -s localhost:8000/challenge/game/<gameId> -H "Authorization: Bearer $ALICE_TOKEN"
+# → {gameId, you: "alice", white: "alice", black: "cara", status: "active"}
 ```
 
 ### Watch the result API (no browser)
 
-```bash
-TOKEN=<a player's JWT>
+Only the chess server can write a result now — a player's own token can't.
 
-# record a game (as one of the two players)
-curl -sX POST localhost:8000/gameResults \
-  -H 'Content-Type: application/json' -H "Authentication: Bearer $TOKEN" \
+```bash
+# record a game — chess server identity, not a player's token
+curl -sX POST localhost:8000/internal/gameResults \
+  -H 'Content-Type: application/json' -H "X-Service-Key: $CHESS_SERVICE_KEY" \
   -d '{"gameId":"g1","result":"win","reason":"checkmate",
        "winnerUsername":"alice","loserUsername":"cara"}'   # → 201 {duplicate:false}
+# (requires a PvpGame already saved for "g1" — i.e. a real accepted challenge)
 
 # report it again — idempotent, nothing changes
-curl -sX POST localhost:8000/gameResults \
-  -H 'Content-Type: application/json' -H "Authentication: Bearer $TOKEN" \
+curl -sX POST localhost:8000/internal/gameResults \
+  -H 'Content-Type: application/json' -H "X-Service-Key: $CHESS_SERVICE_KEY" \
   -d '{"gameId":"g1","result":"win","reason":"checkmate",
        "winnerUsername":"alice","loserUsername":"cara"}'   # → 200 {duplicate:true}
 
-curl -s localhost:8000/gameResults/alice -H "Authentication: Bearer $TOKEN"
+# a player's own token no longer works for writing — only reading
+curl -s localhost:8000/gameResults/alice -H "Authorization: Bearer $TOKEN"
 # → {wins, draws, losses, gamesPlayed, chessScore}
 
 # the leaderboard shows it as its own column, and can rank by it
-curl -s 'localhost:8000/leaderboard?sortBy=chess' -H "Authentication: Bearer $TOKEN"
+curl -s 'localhost:8000/leaderboard?sortBy=chess' -H "Authorization: Bearer $TOKEN"
 ```
 
 ---
@@ -149,9 +175,12 @@ curl -s 'localhost:8000/leaderboard?sortBy=chess' -H "Authentication: Bearer $TO
 | Winner resolved correctly (by color, both game types) | unit: Fool's-mate → `cara` wins |
 | Draws detected, no winner | unit: insufficient-material |
 | Resign / disconnect forfeit to opponent | unit + E2E |
-| Both seats keep their own token for the end-of-game report | unit: per-seat credentials |
+| A joining player is seated under their own verified identity, not a client-claimed username | `EventHandlers.pvp.test.js` — mismatch rejected, seat comes from the middleware response only |
 | No double-count after a decided game | unit: "cannot be resigned again" + `gameId` idempotency tests |
-| A non-participant cannot report a game | `gameResults.test.js` — 403 |
+| Only the chess server (service key) can report a result | `internalGameResults.test.js` — 401 without/with wrong key |
+| A result is only accepted for a real accepted game, with matching players | `internalGameResults.test.js` — 404 unknown gameId, 400 player mismatch |
+| The two services agree on the report's shape without being mocked at each other | `contract.chessServerReport.test.js` |
+| A player can no longer write their own result | `gameResults.test.js` — legacy POST returns 404 |
 | Chess results never move the engagement score | `leaderboard.test.js` — separation tests |
 | Full handshake → paired game → gameover on both clients | E2E 11/11 |
 
@@ -162,8 +191,11 @@ curl -s 'localhost:8000/leaderboard?sortBy=chess' -H "Authentication: Bearer $TO
 **Works end-to-end today:** the entire **challenge handshake** — send / accept /
 decline, live polling, self-challenge rejection, duplicate dedup — all of the
 **game and outcome logic** (pairing by `gameId`, move sync, checkmate/draw/resign/
-disconnect detection, single-count guarantee), and **result recording + scoring**
-(`POST /gameResults` → leaderboard Chess column and analytics `chess` block).
+disconnect detection, single-count guarantee), **server-authoritative result
+recording** (`POST /internal/gameResults`, service-key gated, validated against
+the accepted `PvpGame` → leaderboard Chess column and analytics `chess` block),
+and **identity verification on join** (a player is seated under the username
+the middleware's own auth resolves, never a client-supplied one).
 
 **Pending:**
 
@@ -171,6 +203,10 @@ disconnect detection, single-count guarantee), and **result recording + scoring*
   chess client; embedding the board in the profile via `postMessage` waits on the
   chess-client refactor. Until a client emits `newpvpgame`, the reporting path is
   exercised by tests and the E2E driver rather than by real browser play.
-- **Trust model.** The reporting client is a player, so a determined student
-  could report a loss as a win. See the design doc §10 — the fix is a chessServer
-  service credential instead of a relayed player token.
+- **Deploy.** `CHESS_SERVICE_KEY` needs to exist as a matching secret on both
+  services in production before this reaches real traffic — see the PvP results
+  plan (v2), T5. Until then, reports fail exactly as they do today (no regression,
+  just no fix yet).
+- **Win trading.** The v1 trust gap (a player reporting their own result) is
+  closed, but two cooperating real accounts can still play and throw real games
+  to farm chess score — see the separate "PvP follow-ups" plan.
diff --git a/middlewareNode/tests/contract.chessServerReport.test.js b/middlewareNode/tests/contract.chessServerReport.test.js
new file mode 100644
index 00000000..12c46954
--- /dev/null
+++ b/middlewareNode/tests/contract.chessServerReport.test.js
@@ -0,0 +1,111 @@
+/**
+ * Cross-service contract test — the chess server's result-report request
+ * shape against the middleware's actual internal route. See the PvP results
+ * plan (v2), T4b.
+ *
+ * This is the test that makes "the header name and path live in
+ * resultRequest.js and nowhere else on the chess server side" actually true:
+ * it imports that file directly from the chess server's source tree (CI
+ * checks out the whole monorepo, so the relative require works even though
+ * the two services are built into separate Docker images), builds a request
+ * with it, and sends that exact request through the real middleware route —
+ * real requireServiceKey, real PvpGame/GameResults models via
+ * mongodb-memory-server, nothing mocked. If either side changes the header
+ * name or the path without the other, this fails in CI instead of each
+ * service's own tests passing in isolation (which is exactly how the
+ * Authentication/Authorization header bug this plan fixes went unnoticed).
+ */
+
+const buildResultRequest = require("../../chessServer/src/reporting/resultRequest");
+
+const { MongoMemoryServer } = require("mongodb-memory-server");
+const mongoose = require("mongoose");
+const express = require("express");
+const request = require("supertest");
+
+jest.setTimeout(60000);
+
+const SERVICE_KEY = "contract-test-service-key";
+
+let mongod;
+let app;
+let PvpGame;
+let GameResults;
+
+beforeAll(async () => {
+  mongod = await MongoMemoryServer.create({ instance: { launchTimeout: 30000 } });
+  await mongoose.connect(mongod.getUri() + "ystem");
+
+  process.env.CHESS_SERVICE_KEY = SERVICE_KEY;
+
+  PvpGame = require("../src/models/PvpGame");
+  GameResults = require("../src/models/gameResults");
+  const internalGameResults = require("../src/routes/internalGameResults");
+
+  app = express();
+  app.use(express.json());
+  app.use("/internal/gameResults", internalGameResults);
+});
+
+afterAll(async () => {
+  delete process.env.CHESS_SERVICE_KEY;
+  await mongoose.disconnect();
+  await mongod.stop();
+});
+
+afterEach(async () => {
+  const collections = mongoose.connection.collections;
+  await Promise.all(Object.values(collections).map((c) => c.deleteMany({})));
+});
+
+/** Sends a { path, headers, body } request (as built by resultRequest.js) through supertest. */
+function sendBuiltRequest({ path, headers, body }) {
+  return request(app).post(path).set(headers).send(body);
+}
+
+describe("resultRequest.js output against the real /internal/gameResults route", () => {
+  test("happy path — a real chess-server-shaped request is accepted and stored once", async () => {
+    await PvpGame.create({ gameId: "contract-g1", white: "alice", black: "bob" });
+
+    const game = { gameId: "contract-g1", players: [{ username: "alice" }, { username: "bob" }] };
+    const outcome = { reason: "checkmate", winnerUsername: "bob", loserUsername: "alice" };
+    const built = buildResultRequest(game, outcome, SERVICE_KEY);
+
+    const res = await sendBuiltRequest(built);
+
+    expect(res.status).toBe(201);
+    expect(res.body.success).toBe(true);
+    expect(res.body.gameResult.source).toBe("chessServer");
+
+    const stored = await GameResults.find({ gameId: "contract-g1" });
+    expect(stored).toHaveLength(1);
+    expect(stored[0].winnerUsername).toBe("bob");
+  });
+
+  test("a draw-shaped request is also accepted", async () => {
+    await PvpGame.create({ gameId: "contract-g2", white: "alice", black: "bob" });
+
+    const game = { gameId: "contract-g2", players: [{ username: "alice" }, { username: "bob" }] };
+    const outcome = { reason: "draw" }; // no winnerUsername => resultRequest builds a draw
+    const built = buildResultRequest(game, outcome, SERVICE_KEY);
+
+    const res = await sendBuiltRequest(built);
+
+    expect(res.status).toBe(201);
+    expect(res.body.gameResult.result).toBe("draw");
+  });
+
+  test("wrong key — proves the service-key check actually runs, not just the happy path", async () => {
+    await PvpGame.create({ gameId: "contract-g3", white: "alice", black: "bob" });
+
+    const game = { gameId: "contract-g3", players: [{ username: "alice" }, { username: "bob" }] };
+    const outcome = { reason: "checkmate", winnerUsername: "alice", loserUsername: "bob" };
+    const built = buildResultRequest(game, outcome, "a-completely-wrong-key");
+
+    const res = await sendBuiltRequest(built);
+
+    expect(res.status).toBe(401);
+    const stored = await GameResults.find({ gameId: "contract-g3" });
+    expect(stored).toHaveLength(0);
+  });
+});

From edaba4d492d0a800fea9887b69df088129c62faf Mon Sep 17 00:00:00 2001
From: saritahimthani <sarita.himthani123@gmail.com>
Date: Tue, 6 Oct 2026 16:07:25 -0700
Subject: [PATCH 3/4] PvP results T6: script to mark pre-fix gameResults
 legacy-unverified

Not run yet - needs the production count check and Devin's call on
whether legacy records should be excluded from chess score first.
Idempotent (only touches documents missing `source`), safe to run once
those are answered.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 .../src/scripts/markLegacyGameResults.js      | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 middlewareNode/src/scripts/markLegacyGameResults.js

diff --git a/middlewareNode/src/scripts/markLegacyGameResults.js b/middlewareNode/src/scripts/markLegacyGameResults.js
new file mode 100644
index 00000000..13dbb1c5
--- /dev/null
+++ b/middlewareNode/src/scripts/markLegacyGameResults.js
@@ -0,0 +1,43 @@
+/**
+ * One-time migration: mark every pre-existing gameResults record
+ * source: "legacy-unverified".
+ *
+ * See the PvP results plan (v2), T6. The player-facing POST /gameResults never
+ * actually reached the database before this plan (the chess server sent an
+ * `Authentication` header; passport only reads `Authorization` — see finding
+ * #1), so any record that predates CHESS_SERVICE_KEY reporting did not come
+ * through the verified chessServer path. "legacy-unverified" doesn't claim
+ * those games didn't happen — it just means they weren't recorded through a
+ * path that checked a real accepted PvpGame. Devin decides whether chess
+ * score should exclude them; Karthik's backfill should read only
+ * source: "chessServer" (see the plan's T6/T8 notes).
+ *
+ * Usage:
+ *   node src/scripts/markLegacyGameResults.js
+ *
+ * Safe to run multiple times — only touches documents missing `source`.
+ * New records from the chess server always set `source` themselves, so this
+ * script never overwrites one.
+ */
+
+require("dotenv").config();
+const mongoose = require("mongoose");
+const config = require("config");
+
+async function run() {
+  await mongoose.connect(config.get("mongoURI"));
+  console.log("Connected to MongoDB");
+
+  const result = await mongoose.connection.collection("gameresults").updateMany(
+    { source: { $exists: false } },
+    { $set: { source: "legacy-unverified" } }
+  );
+
+  console.log(`Migration complete: ${result.modifiedCount} documents marked legacy-unverified`);
+  await mongoose.disconnect();
+}
+
+run().catch((err) => {
+  console.error("Migration failed:", err);
+  process.exit(1);
+});

From 53609cd39bffb9dc7f797e2956b4a7ae125b2bec Mon Sep 17 00:00:00 2001
From: saritahimthani <sarita.himthani123@gmail.com>
Date: Thu, 8 Oct 2026 15:02:00 -0700
Subject: [PATCH 4/4] PvP results: fix integration regressions found after
 rebasing onto main

Rebasing the 3 PvP commits onto current upstream/main (now includes
Karthik's/Jimmy's merged currency-ledger work and a separate
signed-environment-separation effort) surfaced two real breaks,
beyond the straightforward conflicts in challenge.js/server.js and
the duplicate validateEnvironment.js/.env.example already resolved
in the rebase itself:

- gameResults.source was required: true, which broke the already-
  merged tests/backfillCurrencyLedger.test.js - its fixtures create
  GameResults documents with no source field. Changed to
  default: "legacy-unverified" instead of required: true: any write
  that doesn't explicitly claim source: "chessServer" (only
  internalGameResults.js does) now safely falls back to unverified
  rather than failing. Strictly more correct than required, and
  non-breaking for existing callers.

- challenge.pvpgame.test.js's acceptedGame() helper called POST
  /challenge and POST /challenge/:id/accept with no identity header.
  Both routes are now behind requireAuth with identity enforcement
  (added upstream, independently of this plan) - the caller must be
  fromUsername to create a challenge, and only toUsername may accept
  it. Updated the helper to authenticate as the right player on each
  call.

All 346 middleware + 40 chessServer tests pass on top of current main.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 middlewareNode/src/models/gameResults.js      |  7 ++++++-
 .../tests/challenge.pvpgame.test.js           | 19 +++++++++++++++----
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/middlewareNode/src/models/gameResults.js b/middlewareNode/src/models/gameResults.js
index 6e35812d..5eca85f9 100644
--- a/middlewareNode/src/models/gameResults.js
+++ b/middlewareNode/src/models/gameResults.js
@@ -58,10 +58,15 @@ const GameResultsSchema = new mongoose.Schema(
 
     playedAt: { type: Date, default: Date.now, index: true },
 
+    // Defaults to "legacy-unverified" rather than being required: any write
+    // that doesn't explicitly claim source: "chessServer" (only
+    // routes/internalGameResults.js does) should safely fall back to
+    // unverified, not fail. This also keeps older code paths that create a
+    // GameResults doc without knowing about this field working unchanged.
     source: {
       type: String,
       enum: ["chessServer", "legacy-unverified"],
-      required: true,
+      default: "legacy-unverified",
     },
   },
   { timestamps: true }
diff --git a/middlewareNode/tests/challenge.pvpgame.test.js b/middlewareNode/tests/challenge.pvpgame.test.js
index 3703f4e4..28904a7e 100644
--- a/middlewareNode/tests/challenge.pvpgame.test.js
+++ b/middlewareNode/tests/challenge.pvpgame.test.js
@@ -48,10 +48,19 @@ afterEach(async () => {
   await Promise.all(Object.values(collections).map((c) => c.deleteMany({})));
 });
 
-/** Creates and accepts a challenge, returning its gameId. */
+/**
+ * Creates and accepts a challenge, returning its gameId. Both calls now need
+ * an identity header: POST /challenge requires the caller to be fromUsername,
+ * and only the recipient (toUsername) may accept.
+ */
 async function acceptedGame(fromUsername = "alice", toUsername = "bob") {
-  const created = await request(app).post("/challenge").send({ fromUsername, toUsername });
-  const accept = await request(app).post(`/challenge/${created.body.challengeId}/accept`);
+  const created = await request(app)
+    .post("/challenge")
+    .set("x-test-user", fromUsername)
+    .send({ fromUsername, toUsername });
+  const accept = await request(app)
+    .post(`/challenge/${created.body.challengeId}/accept`)
+    .set("x-test-user", toUsername);
   return { challengeId: created.body.challengeId, gameId: accept.body.gameId, accept };
 }
 
@@ -70,7 +79,9 @@ describe("POST /challenge/:id/accept — persists a PvpGame", () => {
   test("a second accept of the same challenge returns 409 and creates no extra PvpGame", async () => {
     const { challengeId, gameId } = await acceptedGame("alice", "bob");
 
-    const second = await request(app).post(`/challenge/${challengeId}/accept`);
+    const second = await request(app)
+      .post(`/challenge/${challengeId}/accept`)
+      .set("x-test-user", "bob");
     expect(second.status).toBe(409);
 
     const PvpGame = require("../src/models/PvpGame");