Skip to content
Merged

pull #81

Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
# misc
.DS_Store
*.pem
tmpclaude*
logs

#visual studio
Expand Down
6 changes: 4 additions & 2 deletions api/eloRank.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,11 @@ export default async function handler(req, res) {
try {

let user;
if(username) {
if(username && typeof username === 'string') {
// Prevent NoSQL injection - username must be a string
user = await User.findOne({ username: username }).collation(USERNAME_COLLATION).cache(120);
} else if(secret) {
} else if(secret && typeof secret === 'string') {
// Prevent NoSQL injection - secret must be a string
user = await User.findOne({ secret }).cache(120);
}

Expand Down
3 changes: 2 additions & 1 deletion api/googleAuth.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ export default async function handler(req, res) {

const { code, secret } = req.body;
if (!code) {
if(!secret) {
// Prevent NoSQL injection - secret must be a string
if(!secret || typeof secret !== 'string') {
return res.status(400).json({ error: 'Invalid' });
}

Expand Down
5 changes: 5 additions & 0 deletions api/leaderboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ export default async function handler(req, res) {
const isXp = req.query.mode === 'xp';
console.log(`[API] leaderboard: mode=${isXp ? 'xp' : 'elo'}, pastDay=${pastDay}, user=${myUsername || 'none'}`);

// Prevent NoSQL injection - username must be a string if provided
if (myUsername && typeof myUsername !== 'string') {
return res.status(400).json({ message: 'Invalid username' });
}

if (req.method !== 'GET') {
return res.status(405).json({ message: 'Method not allowed' });
}
Expand Down
7 changes: 4 additions & 3 deletions api/map/mapHome.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,13 @@ export default async function handler(req, res) {

// Skip user lookup for anonymous requests
if(secret && !isAnon) {
const startUser = Date.now();
user = await User.findOne({ secret: secret });
timings.userLookup = Date.now() - startUser;
// Prevent NoSQL injection - validate secret type BEFORE the query
if(typeof secret !== 'string') {
return res.status(400).json({ message: 'Invalid input' });
}
const startUser = Date.now();
user = await User.findOne({ secret: secret });
timings.userLookup = Date.now() - startUser;
if(!user) {
return res.status(404).json({ message: 'User not found' });
}
Expand Down
68 changes: 67 additions & 1 deletion components/home.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ export default function Home({ }) {
const [dismissedNameChangeBanner, setDismissedNameChangeBanner] = useState(false)
const [dismissedBanBanner, setDismissedBanBanner] = useState(false)
const [timeOffset, setTimeOffset] = useState(0)
const timeSyncRef = useRef({ bestRtt: Infinity, lastSyncAt: 0, lastServerNow: 0 })
const [loginQueued, setLoginQueued] = useState(false);
const [options, setOptions] = useState({
});
Expand Down Expand Up @@ -967,6 +968,37 @@ export default function Home({ }) {
const [multiplayerChatOpen, setMultiplayerChatOpen] = useState(false);
const [multiplayerChatEnabled, setMultiplayerChatEnabled] = useState(false);

const updateTimeOffsetFromSync = (serverNow, clientSentAt) => {
if (!serverNow || !clientSentAt) return;
const now = Date.now();
const rtt = Math.max(0, now - clientSentAt);
const offset = serverNow - (clientSentAt + rtt / 2);
const sync = timeSyncRef.current;
const tooOld = now - sync.lastSyncAt > 60000;
const betterRtt = rtt <= sync.bestRtt + 25;
if (sync.lastSyncAt === 0 || betterRtt || tooOld) {
const prevBestRtt = sync.bestRtt;
sync.bestRtt = Math.min(sync.bestRtt, rtt);
sync.lastSyncAt = now;
sync.lastServerNow = serverNow;
if (window.debugTimeSync) {
console.log("[TimeSync] update", {
offset,
rtt,
serverNow,
clientSentAt,
prevBestRtt
});
}
setTimeOffset(offset);
}
};
Comment on lines +971 to +995
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

The updateTimeOffsetFromSync function is called from the WebSocket message handler (line 1442), but it's not wrapped in useCallback. This means a new function instance is created on every render. While this doesn't directly break functionality because it's called synchronously from the message handler, it's inconsistent with best practices. Consider wrapping it in useCallback for consistency and to prevent potential issues if it's used elsewhere in the future.

Copilot uses AI. Check for mistakes.

const sendTimeSync = () => {
if (!ws || ws.readyState !== WebSocket.OPEN) return;
ws.send(JSON.stringify({ type: "timeSync", clientSentAt: Date.now() }));
};


// Auto-close connection error modal when connected
useEffect(() => {
Expand All @@ -985,6 +1017,27 @@ export default function Home({ }) {
}
}, [session?.token?.secret, ws])

useEffect(() => {
if (!ws) return;
timeSyncRef.current = { bestRtt: Infinity, lastSyncAt: 0, lastServerNow: 0 };
setTimeOffset(0);
sendTimeSync();
const interval = setInterval(() => {
sendTimeSync();
}, 30000);
return () => clearInterval(interval);
}, [ws])

useEffect(() => {
const handleVisibility = () => {
if (document.visibilityState === "visible") {
sendTimeSync();
}
};
document.addEventListener("visibilitychange", handleVisibility);
return () => document.removeEventListener("visibilitychange", handleVisibility);
}, [ws])
Comment on lines +1020 to +1039
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

The sendTimeSync function is referenced in the dependency arrays of the useEffect hooks on lines 1020-1029 and 1031-1039, but it's not included in the dependency arrays. Since sendTimeSync is a function defined in the component body that depends on ws, it should either be wrapped in useCallback with appropriate dependencies, or the dependency arrays should be updated. Without this, the function references in the intervals and event listeners may become stale.

Copilot uses AI. Check for mistakes.

const { t: text } = useTranslation("common");

useEffect(() => {
Expand Down Expand Up @@ -1371,10 +1424,23 @@ export default function Home({ }) {
}
if (data.type === "t") {
const offset = data.t - Date.now();
if (Math.abs(offset) > 1000 && ((Math.abs(offset) < Math.abs(timeOffset)) || !timeOffset)) {
const sync = timeSyncRef.current;
const now = Date.now();
const useFallback = sync.lastSyncAt === 0 || (now - sync.lastSyncAt) > 60000;
if (useFallback && Math.abs(offset) < 300000) {
if (window.debugTimeSync) {
console.log("[TimeSync] fallback", {
offset,
serverNow: data.t,
lastSyncAt: sync.lastSyncAt
});
}
setTimeOffset(offset)
}
}
if (data.type === "timeSync") {
updateTimeOffsetFromSync(data.serverNow, data.clientSentAt);
}

if (data.type === "elo") {
setEloData((prev) => ({
Expand Down
5 changes: 5 additions & 0 deletions serverUtils/validateSecret.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
export default async function validateSecret(secret, User) {
// Prevent NoSQL injection - secret must be a string
if (typeof secret !== 'string') {
return null;
}

const user = await User.findOne({
secret
});
Expand Down
49 changes: 27 additions & 22 deletions ws/classes/Game.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export default class Game {
eloChanges: this.eloChanges,
pIds: this.pIds,
accountIds: this.accountIds,
oldElos: this.oldElos,
gameCount: this.gameCount,
location: this.location,
saveInProgress: this.saveInProgress,
Expand Down Expand Up @@ -414,7 +415,6 @@ export default class Game {
// For ranked duels: if someone leaves during "getready" (countdown before first round),
// cancel the game without ELO penalties - no actual gameplay has happened yet
const isPreGameLeave = this.public && this.duel && this.state === 'getready';

// Track disconnection for ranked duels (only if actual gameplay has started)
if(this.public && this.duel && !isPreGameLeave) {
this.disconnectedPlayer = tag;
Expand Down Expand Up @@ -462,7 +462,7 @@ export default class Game {
console.log('Cannot start game: not in waiting state', this.state);
return;
}

if (Object.keys(this.players).length < 2) {
console.log('Cannot start game: not enough players', Object.keys(this.players).length);
if (hostPlayer) {
Expand All @@ -474,7 +474,7 @@ export default class Game {
}
return;
}

if (this.rounds !== this.locations.length) {
console.log('Cannot start game: locations not loaded', this.rounds, this.locations.length);
if (hostPlayer) {
Expand Down Expand Up @@ -555,7 +555,7 @@ export default class Game {
}


if (allFinal && (this.nextEvtTime - Date.now()) > 5000) {
if (allFinal && (this.nextEvtTime - Date.now()) > 1000) {
this.nextEvtTime = Date.now() + 1000;
this.sendStateUpdate();
}
Expand Down Expand Up @@ -694,7 +694,7 @@ export default class Game {
p.send(json);
}
}
end(leftUser) {
async end(leftUser) {
console.log(`Ending game ${this.id} - duel: ${this.duel}, public: ${this.public}, players: ${Object.keys(this.players).length}`);
// For duels, only save the final round if it was actually completed (all players made guesses)
// For regular games, save if the round was started but not yet saved
Expand Down Expand Up @@ -773,21 +773,26 @@ export default class Game {
}


let p1NewElo = null;
let p2NewElo = null;

let p1OldElo = p1obj?.elo || null;
let p2OldElo = p2obj?.elo || null;
const p1EloResult = await User.findById(this.accountIds.p1).select('elo').lean();
const p2EloResult = await User.findById(this.accountIds.p2).select('elo').lean();

// Use DB value if available, otherwise fall back to stored oldElos from game creation
// This prevents null ELO bugs while still handling external ELO updates
let p1OldElo = p1EloResult?.elo ?? this.oldElos?.p1 ?? null;
let p2OldElo = p2EloResult?.elo ?? this.oldElos?.p2 ?? null;

let p1NewElo = p1OldElo;
let p2NewElo = p2OldElo;
// elo changes
if(this.eloChanges) {
if(this.eloChanges && p1OldElo && p2OldElo) {
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

The condition check includes p1OldElo && p2OldElo to ensure both ELO values exist, but the condition on line 860 also requires these values before saving the duel. However, if the database queries on lines 777-778 fail or return null/undefined ELO values, but the oldElos fallback also doesn't exist (e.g., if the game was created before this field was added), the ELO changes will be skipped silently without logging or notifying players. Consider adding logging when ELO changes are skipped due to missing values to help debug issues.

Copilot uses AI. Check for mistakes.
if(draw) {

const changes = this.eloChanges.draw;
// { newRating1, newRating2 }

p1NewElo = changes.newRating1;
p2NewElo = changes.newRating2;
p1NewElo += changes.newRating1;
p2NewElo += changes.newRating2;

if(p1obj) {

Expand All @@ -797,27 +802,27 @@ export default class Game {
}

if(p2obj) {
p2obj.setElo(changes.newRating2, { draw: true, oldElo: p2OldElo });
p2obj.setElo(p2NewElo, { draw: true, oldElo: p2OldElo });
} else {
setElo(this.accountIds.p2, changes.newRating2, { draw: true, oldElo: p2OldElo });
setElo(this.accountIds.p2, p2NewElo, { draw: true, oldElo: p2OldElo });
}
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

The closing brace on line 808 closes the if(p2obj) block that starts on line 804, but the indentation and structure suggest it should close the outer if(draw) block. This causes the else on line 806 to be incorrectly paired with if(p2obj) instead of being part of the p2obj conditional logic. The correct structure should have a closing brace after line 808 to close the if(draw) block before the else if(winner) on line 809.

Suggested change
}
}

Copilot uses AI. Check for mistakes.
} else if(winner) {

const changes = this.eloChanges[winner.id];
// { newRating1, newRating2 }
p1NewElo = changes.newRating1;
p2NewElo = changes.newRating2;
p1NewElo += changes.newRating1;
p2NewElo += changes.newRating2;

if(p1obj) {
p1obj.setElo(changes.newRating1, { winner: winner.tag === 'p1', oldElo: p1OldElo });
p1obj.setElo(p1NewElo, { winner: winner.tag === 'p1', oldElo: p1OldElo });
} else {
setElo(this.accountIds.p1, changes.newRating1, { winner: winner.tag === 'p1', oldElo: p1OldElo });
setElo(this.accountIds.p1, p1NewElo, { winner: winner.tag === 'p1', oldElo: p1OldElo });
}

if(p2obj) {
p2obj.setElo(changes.newRating2, { winner: winner.tag === 'p2', oldElo: p2OldElo });
p2obj.setElo(p2NewElo, { winner: winner.tag === 'p2', oldElo: p2OldElo });
} else {
setElo(this.accountIds.p2, changes.newRating2, { winner: winner.tag === 'p2', oldElo: p2OldElo });
setElo(this.accountIds.p2, p2NewElo, { winner: winner.tag === 'p2', oldElo: p2OldElo });
}

}
Expand Down Expand Up @@ -852,7 +857,7 @@ export default class Game {
}

// Save duel game to MongoDB for history tracking
if(this.duel && this.accountIds?.p1 && this.accountIds?.p2) {
if(this.duel && this.accountIds?.p1 && this.accountIds?.p2 && p1OldElo && p2OldElo) {
this.saveInProgress = true;
const p1Xp = this.calculatePlayerXp(this.pIds?.p1);
const p2Xp = this.calculatePlayerXp(this.pIds?.p2);
Expand Down Expand Up @@ -1128,7 +1133,7 @@ export default class Game {
// Create player summaries sorted by total points (highest first) - include ALL players
// Calculate XP for public games only (not parties), and only for registered users (not guests)
const awardXp = this.public;

const playerSummaries = allPlayers
.map(player => {
const playerXp = (awardXp && player.accountId) ? this.calculatePlayerXp(player.id) : 0;
Expand Down
3 changes: 2 additions & 1 deletion ws/classes/Player.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ export default class Player {
// account verification
if((!json.secret) ||(json.secret === 'not_logged_in')) {
if(!this.verified) {
if(json.rejoinCode) {
if(typeof json.rejoinCode === 'string' && json.rejoinCode.includes('-')) {
// Only accept UUID format rejoinCodes (contain dashes), reject MongoDB ObjectIds
const dcPlayerId = disconnectedPlayers.get(json.rejoinCode);
if(dcPlayerId) {
handleReconnect(dcPlayerId, json.rejoinCode);
Expand Down
29 changes: 26 additions & 3 deletions ws/ws.js
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,16 @@ app.ws('/wg', {
player.lastPong = Date.now();
return;
}
if (json.type === "timeSync") {
if (typeof json.clientSentAt === "number") {
player.send({
type: "timeSync",
clientSentAt: json.clientSentAt,
serverNow: Date.now()
});
}
return;
}
if (json.type === 'verify') {
player.verify(json);
return;
Expand Down Expand Up @@ -1566,10 +1576,23 @@ try {
const eloDraw = calculateOutcomes(p1.elo, p2.elo, 0.5);
const eloP2Win = calculateOutcomes(p1.elo, p2.elo, 0);

const deltaP1Win = {newRating1: eloP1Win.newRating1 - p1.elo, newRating2: eloP1Win.newRating2 - p2.elo};
const deltaP2Win = {newRating1: eloP2Win.newRating1 - p1.elo, newRating2: eloP2Win.newRating2 - p2.elo};
const deltaDraw = {newRating1: eloDraw.newRating1 - p1.elo, newRating2: eloDraw.newRating2 - p2.elo};

game.eloChanges = {
[p1.id]: eloP1Win,
[p2.id]: eloP2Win,
draw: eloDraw
[p1.id]: deltaP1Win,
[p2.id]: deltaP2Win,
draw: deltaDraw
}

game.oldElos = {
p1: p1.elo,
p2: p2.elo
}

if (process.env.DEBUG_ELO_CHANGES === 'true') {
console.log('game.eloChanges', game.eloChanges, game.oldElos);
}

if(p1.elo > 2000 && p2.elo > 2000) {
Expand Down
Loading