Skip to content

fix(core): gate all client spawn intents on the spawn phase (follow-up to #4657) #4660

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
iiamlewis wants to merge 2 commits into
base: main
Choose a base branch
from
+155 −19
Open

fix(core): gate all client spawn intents on the spawn phase (follow-up to #4657) #4660

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f5a4b3
fix(core): gate all client spawn intents on the spawn phase (follow-u…
iiamlewis Jul 21, 2026
49c3cf0
test(core): cover Executor.createExec marking spawn intents fromIntent
iiamlewis Jul 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/core/execution/ExecutionManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,12 @@ export class Executor {
case "move_warship":
return new MoveWarshipExecution(player, intent.unitIds, intent.tile);
case "spawn":
return new SpawnExecution(this.gameID, player.info(), intent.tile);
return new SpawnExecution(
this.gameID,
player.info(),
intent.tile,
true,
);
Comment on lines +73 to +78

@coderabbitai coderabbitai Bot Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Look for any existing test exercising Executor.createExec with a "spawn" intent.
rg -nP -C3 '\bcreateExec(s)?\s*\(' --type=ts -g '*.test.ts'
rg -nP -C3 '"spawn"' --type=ts -g '*.test.ts'

Repository: openfrontio/OpenFrontIO

Length of output: 161

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## Relevant files\n'
git ls-files 'src/core/execution/*' 'tests/core/execution/*' 'tests/core/*' | sed -n '1,200p'

printf '\n## Find Executor.createExec and spawn handling\n'
rg -n -C 3 'createExec|spawn|fromIntent' src/core/execution tests/core/execution tests/core --type ts

printf '\n## Show ExecutionManager.ts around spawn branch\n'
sed -n '1,140p' src/core/execution/ExecutionManager.ts

printf '\n## Show SpawnExecution tests\n'
sed -n '1,220p' tests/core/execution/SpawnExecution.test.ts

Repository: openfrontio/OpenFrontIO

Length of output: 50379

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- file list ---'
git ls-files 'src/core/execution/*' 'tests/core/execution/*' 'tests/core/*' | sed -n '1,200p'

echo
echo '--- createExec / spawn / fromIntent references ---'
rg -n -C 3 'createExec|spawn|fromIntent' src/core/execution tests/core/execution tests/core --type ts || true

echo
echo '--- ExecutionManager.ts ---'
sed -n '1,160p' src/core/execution/ExecutionManager.ts

echo
echo '--- SpawnExecution.test.ts ---'
sed -n '1,240p' tests/core/execution/SpawnExecution.test.ts

Repository: openfrontio/OpenFrontIO

Length of output: 50379

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- tests mentioning ExecutionManager / StampedIntent / createExec ---'
rg -n -C 2 'ExecutionManager|StampedIntent|createExec|createExecs|fromIntent' tests src/core --type ts | sed -n '1,220p'

echo
echo '--- ExecutionManager.ts spawn branch only ---'
sed -n '45,82p' src/core/execution/ExecutionManager.ts

echo
echo '--- SpawnExecution.test.ts spawn-intent tests only ---'
sed -n '120,250p' tests/core/execution/SpawnExecution.test.ts

Repository: openfrontio/OpenFrontIO

Length of output: 11590

Add a small test for the "spawn" intent path.

tests/core/execution/SpawnExecution.test.ts covers fromIntent = true by building SpawnExecution directly, but it does not exercise ExecutionManager.createExec for a stamped "spawn" intent. Add one test there so this wiring cannot regress.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/execution/ExecutionManager.ts` around lines 73 - 78, Add a focused
test for the stamped "spawn" intent branch in ExecutionManager.createExec,
verifying it returns a SpawnExecution with fromIntent set to true and passes the
expected game, player, and tile data. Keep the existing direct SpawnExecution
tests unchanged.

Source: Coding guidelines

case "boat":
return new TransportShipExecution(player, intent.dst, intent.troops);
case "allianceRequest":
Expand Down
26 changes: 17 additions & 9 deletions src/core/execution/SpawnExecution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ export class SpawnExecution implements Execution {
gameID: GameID,
private playerInfo: PlayerInfo,
public tile?: TileRef,
// True when this spawn came from a client "spawn" intent (a player choosing
// where to spawn). Internal spawns — nations, bots, random-spawn placement —
// leave this false and are never gated by the spawn phase.
private fromIntent: boolean = false,
) {
this.random = new PseudoRandom(
simpleHash(playerInfo.id) + simpleHash(gameID),
Expand All @@ -40,22 +44,26 @@ export class SpawnExecution implements Execution {
tick(ticks: number) {
this.active = false;

// Security: a client-requested spawn is only valid during the spawn phase.
// Once the phase has ended, ignore the intent so a player can neither spawn
// for the first time nor re-spawn mid-game. This closes the "teleport"
// exploit, where a crafted spawn intent relinquished a player's territory
// and re-conquered it elsewhere. Internal spawns (nations, bots, random
// spawn placement) are queued during the spawn phase by trusted code and
// may land a tick later — they set fromIntent=false and are not gated here.
// inSpawnPhase() is deterministic game state, so the intent is an identical
// no-op on every client.
if (this.fromIntent && !this.mg.inSpawnPhase()) {
return;
}

let player: Player | null = null;
if (this.mg.hasPlayer(this.playerInfo.id)) {
player = this.mg.player(this.playerInfo.id);
} else {
player = this.mg.addPlayer(this.playerInfo);
}

// Security: a spawn intent may only place or relocate a player's starting
// territory during the spawn phase. Once the game is underway, an
// already-spawned player who sends a spawn intent is attempting to
// teleport — relinquishing their territory and re-conquering it elsewhere.
// Ignore it so the intent is a deterministic no-op on every client.
if (!this.mg.inSpawnPhase() && player.hasSpawned()) {
return;
}

// Security: If random spawn is enabled, prevent players from re-rolling their spawn location
if (this.mg.config().isRandomSpawn() && player.hasSpawned()) {
return;
Expand Down
104 changes: 95 additions & 9 deletions tests/core/execution/SpawnExecution.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,22 @@
import path from "path";
import { SpawnExecution } from "../../../src/core/execution/SpawnExecution";
import { PlayerInfo, PlayerType } from "../../../src/core/game/Game";
import { GameConfig } from "../../../src/core/Schemas";
import { setup } from "../../util/Setup";
import { TestConfig } from "../../util/TestConfig";

// tests/util, so Setup resolves its map paths relative to it (../testdata/maps).
const UTIL_DIR = path.join(__dirname, "..", "..", "util");

// Keep the game in the spawn phase (setup() ends it by default), for asserting
// that a client-requested spawn is accepted while the phase is open.
function setupSpawnPhase(
mapName: string,
gameConfig: Partial<GameConfig>,
players: PlayerInfo[],
) {
return setup(mapName, gameConfig, players, UTIL_DIR, TestConfig, false);
}

describe("Spawn execution", () => {
// Manually calculated based on number of tiles in manifest of each map
Expand Down Expand Up @@ -94,16 +110,39 @@ describe("Spawn execution", () => {

const game = await setup("half_land_half_ocean", {}, [playerInfo]);

game.addExecution(new SpawnExecution("game_id", playerInfo, 10));
game.addExecution(new SpawnExecution("game_id", playerInfo, 20));
game.executeNextTick();
game.executeNextTick();

expect(game.playerByClientID("client_id")?.spawnTile()).toBe(20);
// Previous territory from first spawn should be relinquished
expect(game.owner(10).isPlayer()).toBe(false);
});

test("Client spawn intent is accepted during the spawn phase", async () => {
const playerInfo = new PlayerInfo(
`player`,
PlayerType.Human,
`client_id`,
`player_id`,
);

const game = await setupSpawnPhase("half_land_half_ocean", {}, [
playerInfo,
]);

// fromIntent = true simulates a client "spawn" intent.
game.addExecution(new SpawnExecution("game_id", playerInfo, 20, true));
game.executeNextTick();
game.executeNextTick();

const player = game.playerByClientID("client_id")!;
expect(player.spawnTile()).toBe(20);
expect(player.numTilesOwned()).toBeGreaterThan(0);
});

test("Spawn intent after the spawn phase cannot relocate territory (anti-teleport)", async () => {
test("Client spawn intent after the spawn phase is ignored (anti-teleport)", async () => {
const playerInfo = new PlayerInfo(
`player`,
PlayerType.Human,
Expand All @@ -114,7 +153,7 @@ describe("Spawn execution", () => {
// setup() ends the spawn phase by default, so the game is already underway.
const game = await setup("half_land_half_ocean", {}, [playerInfo]);

// Establish the player's territory with a legitimate first spawn.
// Establish the player's territory (an internal/trusted spawn).
game.addExecution(new SpawnExecution("game_id", playerInfo, 20));
game.executeNextTick();
game.executeNextTick();
Expand All @@ -124,17 +163,60 @@ describe("Spawn execution", () => {
const tilesBefore = player.numTilesOwned();
expect(tilesBefore).toBeGreaterThan(0);

// Malicious "teleport": a spawn intent to a new tile after the game has
// started must be a deterministic no-op — the player keeps their original
// Malicious "teleport": a client spawn intent to a new tile after the phase
// ended must be a deterministic no-op — the player keeps their original
// spawn location and territory rather than relinquishing and re-conquering.
game.addExecution(new SpawnExecution("game_id", playerInfo, 10));
game.addExecution(new SpawnExecution("game_id", playerInfo, 10, true));
game.executeNextTick();
game.executeNextTick();

expect(player.spawnTile()).toBe(20);
expect(player.numTilesOwned()).toBe(tilesBefore);
});

test("Client spawn intent after the spawn phase cannot spawn a never-spawned player", async () => {
const playerInfo = new PlayerInfo(
`player`,
PlayerType.Human,
`client_id`,
`player_id`,
);

const game = await setup("half_land_half_ocean", {}, [playerInfo]);

// A client "spawn" intent once the phase has ended is ignored entirely.
game.addExecution(new SpawnExecution("game_id", playerInfo, 20, true));
game.executeNextTick();
game.executeNextTick();

const player = game.playerByClientID("client_id");
expect(player?.hasSpawned() ?? false).toBe(false);
expect(game.owner(20).isPlayer()).toBe(false);
});

test("Internal spawns are not gated by the spawn phase", async () => {
// Nations, bots and random-spawn placement queue a SpawnExecution during
// the spawn phase, but it may land a tick after the phase ends (e.g. a
// singleplayer human's spawn ends the phase early). These trusted spawns
// (fromIntent = false) must still succeed.
const playerInfo = new PlayerInfo(
`player`,
PlayerType.Human,
`client_id`,
`player_id`,
);

const game = await setup("half_land_half_ocean", {}, [playerInfo]);

game.addExecution(new SpawnExecution("game_id", playerInfo, 20));
game.executeNextTick();
game.executeNextTick();

const player = game.playerByClientID("client_id")!;
expect(player.spawnTile()).toBe(20);
expect(player.numTilesOwned()).toBeGreaterThan(0);
});

test("Random spawn ignores client-specified tile", async () => {
const playerInfo = new PlayerInfo(
`player`,
Expand All @@ -143,13 +225,17 @@ describe("Spawn execution", () => {
`player_id`,
);

const game = await setup("half_land_half_ocean", { randomSpawn: true }, [
playerInfo,
]);
const game = await setupSpawnPhase(
"half_land_half_ocean",
{ randomSpawn: true },
[playerInfo],
);

// Simulate a malicious client sending a spawn intent with a specific tile
const maliciousTile = 10;
game.addExecution(new SpawnExecution("game_id", playerInfo, maliciousTile));
game.addExecution(
new SpawnExecution("game_id", playerInfo, maliciousTile, true),
);
game.executeNextTick();
game.executeNextTick();

Expand Down
Loading