-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Reject version-mismatched joins and prompt refresh on homepage after deploys #4672
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -451,6 +451,26 @@ export async function startWorker() { | |
| return; | ||
| } | ||
|
|
||
| // The sim is deterministic only when every client in a game runs | ||
| // identical code, so a client built from a different commit (e.g. a | ||
| // tab left open across a deploy) would desync the game. Reject it | ||
| // with a typed error the client answers by refreshing. A missing | ||
| // commit means a pre-feature bundle, which is stale by definition. | ||
| if (clientMsg.gitCommit !== ServerEnv.gitCommit()) { | ||
| log.info("rejecting version-mismatched client", { | ||
| gameID: clientMsg.gameID, | ||
| clientCommit: clientMsg.gitCommit, | ||
| }); | ||
| ws.send( | ||
| JSON.stringify({ | ||
| type: "error", | ||
| error: "version_mismatch", | ||
| } satisfies ServerErrorMessage), | ||
| ); | ||
| ws.close(1002, "Version mismatch"); | ||
| return; | ||
| } | ||
|
Comment on lines
+454
to
+472
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== tracked files matching Worker/ClientGameRunner/Transport =="
git ls-files | rg '(^|/)Worker\.ts$|ClientGameRunner\.ts$|Transport\.ts$|ServerErrorMessage' || true
echo
echo "== search version_mismatch and close handling =="
rg -n 'version_mismatch|onclose|on Close|close\(|1002|connection refused|refresh|reload|alert|error.*version' -S src || trueRepository: openfrontio/OpenFrontIO Length of output: 28139 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Worker.ts relevant section =="
sed -n '430,485p' src/server/Worker.ts | nl -ba -v 430
echo
echo "== ClientGameRunner.ts version_mismatch and socket initialization section =="
sed -n '200,250p' src/client/ClientGameRunner.ts | nl -ba -v 200
echo
echo "== Transport.ts onclose section =="
sed -n '380,415p' src/client/Transport.ts | nl -ba -v 380
echo
echo "== deterministic ordering check from source snippets =="
python3 - <<'PY'
from pathlib import Path
import re
worker = Path("src/server/Worker.ts").read_text()
client = Path("src/client/ClientGameRunner.ts").read_text()
transport = Path("src/client/Transport.ts").read_text()
version_send = "JSON.stringify({\n type: \"error\",\n error: \"version_mismatch\""
if version_send in worker:
line = worker.splitlines().index(version_send.splitlines()[0]) + 1
print("Worker sends version_mismatch:", line)
error_handler = "message.error === \"version_mismatch\""
if error_handler in client:
line = client.splitlines().index(error_handler.splitlines()[0]) + 1
nearby = client.splitlines()[max(0,line+12-1):min(len(client.splitlines()),line+18)]
print("ClientGameRunner handles version_mismatch:", line)
print(" nearby:", nearby)
onclose = re.search(r"this\.socket\.onclose\s*=\s*\(\w+:\s*CloseEvent\)\s*=>\s*\{", transport)
if onclose:
print("Transport attaches socket onclose:", onclose.start())
onclose_line = transport[:onclose.start()].count("\n")+1
print("Transport onclose handler starts:", onclose_line)
# find next return or function after onclose assignment
PYRepository: openfrontio/OpenFrontIO Length of output: 234 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Worker.ts relevant section =="
sed -n '430,485p' src/server/Worker.ts
echo
echo "== ClientGameRunner.ts version_mismatch handler =="
sed -n '200,255p' src/client/ClientGameRunner.ts
echo
echo "== Transport.ts onclose handler =="
sed -n '380,415p' src/client/Transport.ts
echo
echo "== Read-only source ordering probe =="
python3 - <<'PY'
from pathlib import Path
worker = Path("src/server/Worker.ts").read_text()
client = Path("src/client/ClientGameRunner.ts").read_text()
transport = Path("src/client/Transport.ts").read_text()
print({
"version_mismatch_text_in_worker": "error: \"version_mismatch\"" in worker,
"version_mismatch_handler_in_client": "message.error === \"version_mismatch\"" in client,
"socket_onclose_handler_in_transport": "this.socket.onclose" in transport and "event.code === 1002" in transport,
})
print("transport first 1000 bytes includes onclose setup:", "this.socket.onclose" in transport[:1000])
PYRepository: openfrontio/OpenFrontIO Length of output: 5318 Handle version mismatches in the message path, not the 1002 close path. The worker sends 🤖 Prompt for AI Agents |
||
|
|
||
| // Verify token signature | ||
| const result = await verifyClientToken(clientMsg.token); | ||
| if (result.type === "error") { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| import { | ||
| ClientJoinMessageSchema, | ||
| ClientRejoinMessageSchema, | ||
| PublicLobbyMessageSchema, | ||
| } from "../src/core/Schemas"; | ||
|
|
||
| const COMMIT = "a".repeat(40); | ||
|
|
||
| const baseJoin = { | ||
| type: "join", | ||
| token: "123e4567-e89b-12d3-a456-426614174000", | ||
| gameID: "abcd1234", | ||
| username: "TestPlayer", | ||
| clanTag: null, | ||
| turnstileToken: null, | ||
| }; | ||
|
|
||
| const baseRejoin = { | ||
| type: "rejoin", | ||
| gameID: "abcd1234", | ||
| lastTurn: 10, | ||
| token: "123e4567-e89b-12d3-a456-426614174000", | ||
| }; | ||
|
|
||
| describe("gitCommit on join/rejoin messages", () => { | ||
| test("join parses with gitCommit", () => { | ||
| const result = ClientJoinMessageSchema.safeParse({ | ||
| ...baseJoin, | ||
| gitCommit: COMMIT, | ||
| }); | ||
| expect(result.success).toBe(true); | ||
| expect(result.data?.gitCommit).toBe(COMMIT); | ||
| }); | ||
|
|
||
| test("join parses without gitCommit (pre-feature clients)", () => { | ||
| const result = ClientJoinMessageSchema.safeParse(baseJoin); | ||
| expect(result.success).toBe(true); | ||
| expect(result.data?.gitCommit).toBeUndefined(); | ||
| }); | ||
|
|
||
| test("join rejects an oversized gitCommit", () => { | ||
| const result = ClientJoinMessageSchema.safeParse({ | ||
| ...baseJoin, | ||
| gitCommit: "a".repeat(65), | ||
| }); | ||
| expect(result.success).toBe(false); | ||
| }); | ||
|
|
||
| test("rejoin parses with and without gitCommit", () => { | ||
| expect( | ||
| ClientRejoinMessageSchema.safeParse({ ...baseRejoin, gitCommit: COMMIT }) | ||
| .success, | ||
| ).toBe(true); | ||
| expect(ClientRejoinMessageSchema.safeParse(baseRejoin).success).toBe(true); | ||
| }); | ||
| }); | ||
|
|
||
| describe("gitCommit on the public lobby feed", () => { | ||
| test("full message parses with and without gitCommit", () => { | ||
| const full = { type: "full", serverTime: 123, games: {} }; | ||
| expect(PublicLobbyMessageSchema.safeParse(full).success).toBe(true); | ||
| const withCommit = PublicLobbyMessageSchema.safeParse({ | ||
| ...full, | ||
| gitCommit: COMMIT, | ||
| }); | ||
| expect(withCommit.success).toBe(true); | ||
| if (withCommit.success && withCommit.data.type === "full") { | ||
| expect(withCommit.data.gitCommit).toBe(COMMIT); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: openfrontio/OpenFrontIO
Length of output: 10894
🏁 Script executed:
Repository: openfrontio/OpenFrontIO
Length of output: 1493
Reject empty build identifiers.
z.string().max(64).optional()accepts"", and the worker comparesclientMsg.gitCommitdirectly withServerEnv.gitCommit(). If the server is misconfigured with an empty commit, a malformed client can bypass the version gate. Keep the field optional, but require a non-empty value when present, and add empty-string tests.Proposed schema change
Also applies to: 886-890, 899-900
🤖 Prompt for AI Agents