Skip to content

sec(server): restrict custom names for guest players and validate lobby ID endpoints (#1636)#4018

Open
berkelmali wants to merge 2 commits into
openfrontio:mainfrom
berkelmali:pr-1636
Open

sec(server): restrict custom names for guest players and validate lobby ID endpoints (#1636)#4018
berkelmali wants to merge 2 commits into
openfrontio:mainfrom
berkelmali:pr-1636

Conversation

@berkelmali
Copy link
Copy Markdown
Contributor

Description:

Unauthenticated guest players could submit any custom username and clan tag when joining a lobby, enabling them to impersonate registered players or administrators. Additionally, server endpoints for creating or querying games used raw unvalidated URL parameters, making them susceptible to cache poisoning or path enjections.

This fix updates Worker.ts to strictly override the username of unauthenticated guest connections to AnonXXX (generated deterministically from a short hash of their persistent UUID) and clear their clan tag. It also introduces GAME_ID_REGEX validations on all Express endpoints receiving lobby IDs to guarantee safe, malformed-free parameters.

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
  • I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

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

barfires

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 26, 2026

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: 9fcb7e05-e327-4a9d-a738-b009bae1b768

📥 Commits

Reviewing files that changed from the base of the PR and between dd8b705 and 6d3e4ae.

📒 Files selected for processing (1)
  • src/server/Worker.ts

Walkthrough

The Worker validates game ID formats on REST endpoints and normalizes/censors WebSocket identity fields (username, clanTag) up-front, generating deterministic guest names for unauthenticated connections and using censored fields during reconnection.

Changes

Game ID validation and identity normalization

Layer / File(s) Summary
Game ID format validation on REST endpoints
src/server/Worker.ts
Import GAME_ID_REGEX and validate :id parameters on POST /api/create_game/:id, GET /api/game/:id/exists, and GET /api/game/:id; reject invalid formats with 400 { error: "Invalid game ID format" } before calling the game manager.
WebSocket join/rejoin identity normalization
src/server/Worker.ts
Normalize and censor username and clanTag via privilegeRefresher.get().censor(...) before join/reconnect logic; for unauthenticated connections (claims === null) derive a deterministic guest censoredUsername from persistentId, clear censoredClanTag, and pass these censored values to gm.rejoinClient and reconnect paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • openfrontio/OpenFrontIO#4000: Both PRs touch clan-tag visibility/censorship handling; this PR centralizes censorship for WebSocket joins and clears clan tags for guests.

Suggested labels

Bugfix, Translation

Suggested reviewers

  • evanpelle
  • ryanbarlow97

Poem

🎮 Game IDs checked at the door,
Names censored, guests no more a chore.
Persistent IDs spawn friendly guise,
Rejoins use the censored ties.
Clean flows now hum—no surprise.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main security fixes: restricting guest player names and validating lobby ID endpoints.
Description check ✅ Passed The description clearly explains the security vulnerabilities addressed and the specific fixes implemented in the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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 and usage tips.

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

1 participant