Skip to content

sec: require strict majority in 1v1 winner-vote consensus (backport)#4581

Merged
Celant merged 1 commit into
mainfrom
fix/1v1-winner-vote-consensus-main
Jul 12, 2026
Merged

sec: require strict majority in 1v1 winner-vote consensus (backport)#4581
Celant merged 1 commit into
mainfrom
fix/1v1-winner-vote-consensus-main

Conversation

@Celant

@Celant Celant commented Jul 12, 2026

Copy link
Copy Markdown
Member

Add approved & assigned issue number here:

Resolves #4136

Description:

Backport of #4580 to main. VoteRound.result() (used by handleWinner in GameServer.ts to reach consensus on a reported game winner) accepted an exact tie as a majority: votes * 2 >= totalUniqueIPs. In a 1v1 game the electorate is 2 unique IPs, so a single client's vote alone satisfied 1 * 2 >= 2 and was accepted immediately — letting one player unilaterally declare themselves the winner without any agreement from the other player, and without the server ever checking the report against the actual simulation outcome.

This fixes the comparison to require a strict majority (votes * 2 > totalUniqueIPs), so a 1v1 now requires both clients to agree. Legitimate forfeit-on-disconnect is unaffected: once a player actually disconnects, the electorate shrinks to 1 unique IP, and the sole remaining vote still resolves immediately (1 * 2 > 1).

The v32 hotfix (#4580) is being deployed first; this PR keeps main in sync with the same fix.

Please complete the following:

  • I have added screenshots for all UI updates
  • I process any text displayed to the user through translateText() and I've added it to the en.json file
  • I have added relevant tests to the test directory

Please put your Discord username so you can be contacted if a bug or regression is found:

jish

…er spoof

VoteRound.result() treated an exact tie (votes * 2 >= total) as a
majority. With a 1v1 game's electorate of 2 unique IPs, a single
client's vote alone satisfied that check (1 * 2 >= 2), letting one
player unilaterally declare themselves the winner without the other
player agreeing. Requiring a strict majority (votes * 2 > total) closes
this while still resolving immediately once the electorate shrinks to 1
(e.g. the other player actually disconnects).

Resolves #4136
@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 622bd718-01ff-47a3-9a01-3a400738276c

📥 Commits

Reviewing files that changed from the base of the PR and between d766913 and 2eaa32e.

📒 Files selected for processing (2)
  • src/server/VoteTally.ts
  • tests/server/VoteTally.test.ts

Walkthrough

VoteRound.result now requires a strict majority of unique IPs. Documentation and tests were updated to reject ties, accept only votes above half the electorate, and resolve a single-elector round.

Changes

Strict majority resolution

Layer / File(s) Summary
Majority rule and validation
src/server/VoteTally.ts, tests/server/VoteTally.test.ts
The winner threshold changed from inclusive to strict, with tests covering ties, competing candidates, and a single remaining elector.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

Halfway votes now softly wait,
A true majority seals their fate.
Two become one, the round is clear,
Strict tallies steer the winner near.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: requiring strict majority for 1v1 winner consensus.
Description check ✅ Passed The description is directly about the same strict-majority fix and its 1v1 disconnect behavior.
Linked Issues check ✅ Passed The code and tests enforce strict majority, preventing unilateral 1v1 winner claims while preserving disconnect resolution.
Out of Scope Changes check ✅ Passed The changes stay focused on the consensus threshold and matching tests, with no clear unrelated additions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Celant Celant added this to the Backlog milestone Jul 12, 2026
@Celant Celant merged commit e57939a into main Jul 12, 2026
16 of 17 checks passed
@github-project-automation github-project-automation Bot moved this from Triage to Complete in OpenFront Release Management Jul 12, 2026
@Celant Celant deleted the fix/1v1-winner-vote-consensus-main branch July 12, 2026 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

Security: 1v1 ranked winner consensus spoofing vulnerability

1 participant