From bd617dbf063c7531636820842eda393f63c4634c Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Tue, 1 Sep 2026 21:07:56 +0100 Subject: [PATCH] fix(multiplayer): validate ranked arena paths durably --- docs/MATCHMAKING.md | 3 ++- multiplayer-next.md | 2 +- server/domain/ranked.go | 12 ++++++++++++ server/domain/ranked_test.go | 13 +++++++++++++ server/store/allocation_match_sql.go | 4 ++-- server/store/match_sql.go | 4 ++-- server/store/proposal_sql.go | 2 +- server/store/proposal_sql_test.go | 29 ++++++++++++++++++++++++++++ 8 files changed, 62 insertions(+), 7 deletions(-) create mode 100644 server/store/proposal_sql_test.go diff --git a/docs/MATCHMAKING.md b/docs/MATCHMAKING.md index ee616801..036245d5 100644 --- a/docs/MATCHMAKING.md +++ b/docs/MATCHMAKING.md @@ -239,7 +239,8 @@ rating loss because no rated match began. the three floor-goal `ArenaRegistry` entries. Elevated goals remain excluded until the trained-policy restriction is lifted. It chooses from that list deterministically from the proposal ID, so retrying a proposal cannot change - its arena. The selected scene is + its arena. Every durable proposal, match, and allocation boundary rechecks + the same allowlist rather than accepting an arbitrary non-empty path. The selected scene is persisted with the proposal/match plan, included in the allocation identity, and passed through the Agones GameServer annotation into the allocated server's validated `--arena-path` flag. diff --git a/multiplayer-next.md b/multiplayer-next.md index bcd69f50..098d32e0 100644 --- a/multiplayer-next.md +++ b/multiplayer-next.md @@ -1448,7 +1448,7 @@ The same allocation path now carries the matcher-selected playlist, preventing a Ranked proposal admission no longer trusts the matcher’s `--ranked-random-arena` boolean. The Go domain now owns a named allowlist for the three floor-goal `ArenaRegistry` entries, and rejects unknown and elevated IDs before any proposal is created. -The arena hand-off is now durable: the matcher deterministically selects an eligible floor-goal arena from the proposal ID, migration 0008 stores that path on proposals and matches, allocation claims and idempotency digests retain it, Agones applies it as a match-scoped annotation, and the supervisor overlays the allocated child’s `--arena-path`. Godot accepts only the same floor-goal `ArenaRegistry` paths and requires one for allocated ranked matches, so a stale Fleet default, an elevated variant, or an altered retry cannot substitute a ranked arena. +The arena hand-off is now durable: the matcher deterministically selects an eligible floor-goal arena from the proposal ID, migration 0008 stores that path on proposals and matches, each durable transition rechecks the same allowlist, allocation claims and idempotency digests retain it, Agones applies it as a match-scoped annotation, and the supervisor overlays the allocated child’s `--arena-path`. Godot accepts only the same floor-goal `ArenaRegistry` paths and requires one for allocated ranked matches, so a stale Fleet default, an elevated variant, or an altered retry cannot substitute a ranked arena. The Godot control-plane client now retains the exact last idempotent mutation and exposes `retry_last_mutation()` for transport, timeout, rate-limit, and 5xx failures. Retries reuse the original idempotency key and expected revision, while 401 and 409 responses remain non-retryable; the harness covers the policy boundary. This closes the local duplicate-action recovery mechanism for heartbeat/cancel/proposal calls, with broader live UI retry verification still remaining. diff --git a/server/domain/ranked.go b/server/domain/ranked.go index 9b078b52..5417d135 100644 --- a/server/domain/ranked.go +++ b/server/domain/ranked.go @@ -49,6 +49,18 @@ func RankedArenaForProposal(proposalID string) RankedArena { return rankedArenas[rankedArenaOrder[int(digest[0])%len(rankedArenaOrder)]] } +// IsRankedArenaPath is the durable-store boundary for arena paths. Proposal +// and allocation records must not accept a merely non-empty caller supplied +// scene path, even when the caller bypasses matcher formation. +func IsRankedArenaPath(path string) bool { + for _, arena := range rankedArenas { + if arena.Path == path { + return true + } + } + return false +} + func ValidateRankedAdmission(participants []RankedParticipant, arena RankedArena) error { if len(participants) != 6 || !validRankedArena(arena) { return fmt.Errorf("ranked admission requirements not met") diff --git a/server/domain/ranked_test.go b/server/domain/ranked_test.go index 8b26790f..a7a0e9bf 100644 --- a/server/domain/ranked_test.go +++ b/server/domain/ranked_test.go @@ -48,6 +48,19 @@ func TestRankedArenaRegistryExcludesElevatedVariants(t *testing.T) { } } +func TestIsRankedArenaPathOnlyAllowsFloorGoalRegistry(t *testing.T) { + for _, path := range []string{"res://scenes/arena_01.tscn", "res://scenes/arena_02.tscn", "res://scenes/arena_03.tscn"} { + if !IsRankedArenaPath(path) { + t.Fatalf("eligible path %q rejected", path) + } + } + for _, path := range []string{"", "res://scenes/arena_01_elevated.tscn", "res://forged.tscn"} { + if IsRankedArenaPath(path) { + t.Fatalf("ineligible path %q accepted", path) + } + } +} + func TestRankedArenaForProposalIsStableAndRotatesEligibleRegistry(t *testing.T) { first := RankedArenaForProposal("proposal-stable") if first != RankedArenaForProposal("proposal-stable") { diff --git a/server/store/allocation_match_sql.go b/server/store/allocation_match_sql.go index c2ba3fd9..54f04c0d 100644 --- a/server/store/allocation_match_sql.go +++ b/server/store/allocation_match_sql.go @@ -234,8 +234,8 @@ func ClaimAllocatingMatch(ctx context.Context, db *sql.DB, transport string, now if build == "" { return fmt.Errorf("allocating match has no participants") } - if domain.Playlist(playlist) == domain.Ranked && !arenaPath.Valid { - return fmt.Errorf("ranked allocating match has no arena") + if domain.Playlist(playlist) == domain.Ranked && (!arenaPath.Valid || !domain.IsRankedArenaPath(arenaPath.String)) { + return fmt.Errorf("ranked allocating match has invalid arena") } item.Request = domain.AllocationRequest{AllocationID: claimedID, MatchID: matchID, Playlist: domain.Playlist(playlist), Region: region, Build: build, Protocol: protocol, ArenaPath: arenaPath.String, Transport: transport} found = true diff --git a/server/store/match_sql.go b/server/store/match_sql.go index f9c65d6a..0f7ae554 100644 --- a/server/store/match_sql.go +++ b/server/store/match_sql.go @@ -120,8 +120,8 @@ func CreateMatchFromAcceptedProposal(ctx context.Context, db *sql.DB, plan Accep if !validAcceptedPlaylistCount(domain.Playlist(playlist), len(plan.Players)) { return fmt.Errorf("accepted proposal playlist does not match player count") } - if domain.Playlist(playlist) == domain.Ranked && plan.ArenaPath == "" { - return fmt.Errorf("ranked accepted match plan has no arena") + if domain.Playlist(playlist) == domain.Ranked && !domain.IsRankedArenaPath(plan.ArenaPath) { + return fmt.Errorf("ranked accepted match plan has invalid arena") } participants, err := acceptedProposalParticipants(ctx, tx, plan) if err != nil { diff --git a/server/store/proposal_sql.go b/server/store/proposal_sql.go index b5f0422b..e3aa3ae2 100644 --- a/server/store/proposal_sql.go +++ b/server/store/proposal_sql.go @@ -75,7 +75,7 @@ func validProposalMatchPlan(proposal domain.Proposal) bool { if (proposal.Region != "EU" && proposal.Region != "NA") || proposal.Protocol < 1 { return false } - if proposal.Playlist == domain.Ranked && proposal.ArenaPath == "" { + if proposal.Playlist == domain.Ranked && !domain.IsRankedArenaPath(proposal.ArenaPath) { return false } seenSlots := make(map[int]struct{}, len(proposal.Participants)) diff --git a/server/store/proposal_sql_test.go b/server/store/proposal_sql_test.go new file mode 100644 index 00000000..353249ee --- /dev/null +++ b/server/store/proposal_sql_test.go @@ -0,0 +1,29 @@ +package store + +import ( + "testing" + + "github.com/cosmic-clash/cosmic-clash/server/domain" +) + +func TestRankedProposalMatchPlanRequiresRegisteredArenaPath(t *testing.T) { + proposal := domain.Proposal{ + Playlist: domain.Ranked, + Region: "EU", + Protocol: 1, + ArenaPath: "res://scenes/arena_01.tscn", + Participants: []domain.ProposalParticipant{ + {PlayerID: "player-a", Team: 0, Slot: 0}, + {PlayerID: "player-b", Team: 1, Slot: 3}, + }, + } + if !validProposalMatchPlan(proposal) { + t.Fatal("registered ranked arena rejected") + } + for _, path := range []string{"", "res://scenes/arena_01_elevated.tscn", "res://forged.tscn"} { + proposal.ArenaPath = path + if validProposalMatchPlan(proposal) { + t.Fatalf("ranked path %q accepted", path) + } + } +}