-
Notifications
You must be signed in to change notification settings - Fork 0
Adding anti-cheat safeguards to block scripted race input #10
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
Conversation
|
@codex review |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| socket.on('race:progress', (data = {}) => { | ||
| try { | ||
| // Client sends { position, total, isCompleted } | ||
| const { code, position, isCompleted, hasError = false } = data; | ||
| if (isSocketLocked()) { | ||
| return; | ||
| } |
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.
Keep client lock in sync with server anticheat state
When a socket is locked once, the server now exits every race:progress handler immediately (socket-handlers.js lines 1367-1371) and only clears suspiciousPlayers on disconnect, but the client resets anticheatState whenever a race is joined or reset (RaceContext.jsx lines 278-280, 987-988). This means a locked user can leave/rejoin without reconnecting, see the input re-enabled, and type, yet the server silently ignores all progress/results and never re-sends a lock message. Any false-positive lock effectively bricks racing until a full reconnect while giving no feedback, which is a serious UX/functional failure.
Useful? React with 👍 / 👎.
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.
Pull request overview
This PR implements a comprehensive anti-cheat system to prevent browser automation and scripted input during typing races. The solution combines server-side validation with client-side input guards to detect and block suspicious behavior.
Key Changes:
- Server-side anti-cheat detection with thresholds for progress rate, WPM (≤320), completion time (≥2.5s), and progress spikes (≤20 chars/update)
- Client-side synthetic event detection that blocks untrusted keyboard/input events and displays warning UI
- Socket lifecycle event system using CustomEvents to handle reconnection without exposing global socket
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| server/controllers/socket-handlers.js | Implements server-side anti-cheat logic with suspicion tracking, progress validation, and result verification against computed metrics |
| client/src/context/SocketContext.jsx | Replaces global window.socket with CustomEvent-based lifecycle event dispatching for better encapsulation |
| client/src/context/RaceContext.jsx | Adds anti-cheat state management with lock/flag mechanisms and server communication for suspicious behavior |
| client/src/context/AuthContext.jsx | Updates socket reconnect handler to use new CustomEvent-based system instead of global socket reference |
| client/src/components/Typing.jsx | Implements input guards for beforeInput, keyDown, and onChange events to detect synthetic/untrusted interactions |
| client/src/components/Typing.css | Adds styling for anti-cheat warning banner displayed when automation is detected |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }; | ||
| // Add raceState.snippet?.id to dependency array to reset typing state on snippet change | ||
| }, [socket, connected, raceState.type, raceState.manuallyStarted, raceState.snippet?.id]); | ||
| }, [socket, connected, raceState.type, raceState.manuallyStarted, raceState.snippet?.id, resetAnticheatState]); |
Copilot
AI
Dec 5, 2025
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.
The dependency array includes resetAnticheatState which is a callback defined with useCallback. This creates an unnecessary re-registration of all socket event listeners whenever resetAnticheatState changes (which should be stable). Since resetAnticheatState has no dependencies itself, this is safe to omit from the dependency array, or you can wrap the handlers that use it in their own useCallback hooks.
| }, [socket, connected, raceState.type, raceState.manuallyStarted, raceState.snippet?.id, resetAnticheatState]); | |
| }, [socket, connected, raceState.type, raceState.manuallyStarted, raceState.snippet?.id]); |
| const nativeEvent = e.nativeEvent || e; | ||
| if (nativeEvent && nativeEvent.isTrusted === false) { | ||
| e.preventDefault(); | ||
| flagSuspicious('synthetic-keydown', { key: e.key }); | ||
| return; |
Copilot
AI
Dec 5, 2025
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.
[nitpick] The isTrusted property check might not be reliable across all browsers or could be spoofed in certain scenarios. While this provides a good first line of defense against basic automation, consider logging or tracking patterns of suspicious activity (e.g., perfectly consistent timing between keystrokes) as an additional layer of protection beyond just checking isTrusted.
| const registerSuspicion = (reason, details = {}) => { | ||
| if (!reason) return; | ||
| const existing = suspiciousPlayers.get(socket.id) || { reasons: [], locked: false }; | ||
| const already = existing.reasons.some(entry => entry.reason === reason); | ||
| if (!already) { | ||
| existing.reasons.push({ reason, details, at: Date.now() }); | ||
| } | ||
| existing.locked = true; | ||
| suspiciousPlayers.set(socket.id, existing); | ||
|
|
||
| const progress = playerProgress.get(socket.id) || {}; | ||
| progress.suspicious = true; | ||
| progress.suspicionReasons = existing.reasons; | ||
| playerProgress.set(socket.id, progress); | ||
|
|
||
| console.warn(`[ANTICHEAT] Locked socket ${socket.id} (${netid}) for ${reason}`, details); | ||
| socket.emit('anticheat:lock', { | ||
| reason, | ||
| details, | ||
| message: details?.message || 'Suspicious typing detected. Automation is not allowed.' | ||
| }); | ||
| }; | ||
|
|
||
| const isSocketLocked = () => { | ||
| const entry = suspiciousPlayers.get(socket.id); | ||
| return entry?.locked; | ||
| }; | ||
|
|
||
| socket.on('anticheat:flag', (payload = {}) => { | ||
| try { | ||
| const { reason, metadata } = payload || {}; | ||
| if (typeof reason !== 'string' || !reason) { | ||
| return; | ||
| } | ||
| registerSuspicion(`client-${reason}`, metadata || {}); | ||
| } catch (err) { | ||
| console.error('Error processing anticheat flag from client:', err); | ||
| } | ||
| }); |
Copilot
AI
Dec 5, 2025
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.
The anti-cheat functionality lacks test coverage. Given that this is a security-critical feature with multiple detection thresholds and edge cases (WPM limits, progress validation, completion time checks), automated tests should be added to verify:
- Legitimate fast typing doesn't trigger false positives
- Each detection threshold correctly identifies suspicious behavior
- Client-side synthetic event detection works properly
- Lock state is properly maintained across race lifecycle events
Consider adding tests in server/tests/ for server-side anti-cheat logic and client/src/tests/ for client-side guards.
| const raceStart = race.startTime || now; | ||
| const elapsedMs = now - raceStart; | ||
| if (elapsedMs > 0) { | ||
| const elapsedMinutes = elapsedMs / 60000; |
Copilot
AI
Dec 5, 2025
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.
[nitpick] The WPM calculation uses position / 5 which assumes standard word length of 5 characters. However, this doesn't account for actual word boundaries or variations in word length. For very short snippets or snippets with unusually long/short words, this could produce inaccurate WPM calculations and potentially trigger false positives. Consider using the actual character count divided by 5 as is done, but add a comment explaining this is the standard typing test convention, or track actual words typed for more precise measurement.
| const elapsedMinutes = elapsedMs / 60000; | |
| const elapsedMinutes = elapsedMs / 60000; | |
| // WPM calculation uses the standard typing test convention: total characters typed divided by 5 (average word length). | |
| // This does not account for actual word boundaries, but provides a consistent metric for comparison. |
| if (!Number.isFinite(computedCompletion)) { | ||
| computedCompletion = Math.max(0, (finishTimestamp - raceStart) / 1000); | ||
| } | ||
|
|
Copilot
AI
Dec 5, 2025
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.
For timed tests, if progressRecord.wpm and progressRecord.accuracy are not set and the client doesn't provide valid wpm/accuracy values in the result submission, the computed values fall back to 0 (lines 1560-1565, then sanitized at 1568-1571). While this prevents invalid data, it allows a timed test result to be recorded with 0 WPM and 0 accuracy. Consider requiring that either the client provides valid metrics or that progress updates have been sent, or reject the result if both are missing.
| // Reject timed test result if both WPM and accuracy are zero and neither progressRecord nor client provided valid metrics | |
| if ( | |
| race.snippet?.is_timed_test && | |
| (!Number.isFinite(progressRecord.wpm) && !Number.isFinite(progressRecord.accuracy)) && | |
| (!Number.isFinite(wpm) && !Number.isFinite(accuracy)) && | |
| computedWpm === 0 && computedAccuracy === 0 | |
| ) { | |
| registerSuspicion('missing-metrics', { message: 'Timed test result missing WPM and accuracy' }); | |
| return; | |
| } |
| const allowableOverflow = Math.max(10, Math.floor(snippetLength * 0.1)); | ||
| if (position < 0 || position > snippetLength + allowableOverflow) { | ||
| registerSuspicion('progress-out-of-range', { position, snippetLength }); | ||
| return; | ||
| } |
Copilot
AI
Dec 5, 2025
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.
The allowable overflow calculation Math.max(10, Math.floor(snippetLength * 0.1)) allows up to 10% overshoot. For a 100-character snippet, this allows position values up to 110, which seems excessive. A user shouldn't legitimately exceed the snippet length by more than a few characters (due to input lag). Consider reducing this to a smaller fixed value (e.g., 3-5 characters) to catch manipulation attempts more effectively.
| history.push({ position, timestamp: now }); | ||
| const trimmedHistory = history.slice(-180); |
Copilot
AI
Dec 5, 2025
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.
The progress history is capped at 180 entries (last 180 updates), but there's no validation on the history.push() operation before trimming. If a malicious client sends many rapid updates, all would be pushed to the array before trimming occurs. Consider trimming before pushing the new entry or validating array size earlier to prevent temporary memory spikes.
| history.push({ position, timestamp: now }); | |
| const trimmedHistory = history.slice(-180); | |
| if (history.length >= 180) { | |
| history.shift(); | |
| } | |
| history.push({ position, timestamp: now }); | |
| const trimmedHistory = history; |
|
|
||
| socket.on('anticheat:flag', (payload = {}) => { | ||
| try { | ||
| const { reason, metadata } = payload || {}; | ||
| if (typeof reason !== 'string' || !reason) { | ||
| return; | ||
| } |
Copilot
AI
Dec 5, 2025
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.
The client can send an anticheat:flag event with arbitrary reason strings. While the server validates that reason is a string, it doesn't validate the content or limit the length. A malicious client could send extremely long strings or spam different reasons. Consider adding length validation (e.g., max 100 characters) and rate limiting on client-side flag reports.
| socket.on('anticheat:flag', (payload = {}) => { | |
| try { | |
| const { reason, metadata } = payload || {}; | |
| if (typeof reason !== 'string' || !reason) { | |
| return; | |
| } | |
| // Rate limit anticheat:flag events per socket | |
| const FLAG_RATE_LIMIT_WINDOW = 60 * 1000; // 1 minute | |
| const FLAG_RATE_LIMIT_MAX = 3; | |
| const flagEventTimestamps = []; | |
| socket.on('anticheat:flag', (payload = {}) => { | |
| try { | |
| const { reason, metadata } = payload || {}; | |
| if (typeof reason !== 'string' || !reason) { | |
| return; | |
| } | |
| // Length validation | |
| if (reason.length > 100) { | |
| // Optionally, you can truncate or reject | |
| console.warn(`[ANTICHEAT] Reason too long from socket ${socket.id}: length=${reason.length}`); | |
| return; | |
| } | |
| // Rate limiting | |
| const now = Date.now(); | |
| // Remove timestamps older than window | |
| while (flagEventTimestamps.length && flagEventTimestamps[0] < now - FLAG_RATE_LIMIT_WINDOW) { | |
| flagEventTimestamps.shift(); | |
| } | |
| if (flagEventTimestamps.length >= FLAG_RATE_LIMIT_MAX) { | |
| console.warn(`[ANTICHEAT] Rate limit exceeded for anticheat:flag from socket ${socket.id}`); | |
| return; | |
| } | |
| flagEventTimestamps.push(now); |
| if (delta > MAX_PROGRESS_STEP && !isCompleted) { | ||
| registerSuspicion('progress-spike', { prevPosition, position, delta }); | ||
| return; | ||
| } |
Copilot
AI
Dec 5, 2025
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.
The MAX_PROGRESS_STEP check should account for backspace/deletion scenarios. When a user realizes they made a mistake and uses backspace, then types the correct sequence, the delta could legitimately exceed 20 characters if they're typing quickly. Consider allowing slightly larger deltas when the user has corrected errors, or track whether backspacing occurred in the previous update.
| snippetDepartment | ||
| }) { | ||
| const { raceState, setRaceState, typingState, setTypingState, updateProgress, handleInput: raceHandleInput, loadNewSnippet } = useRace(); | ||
| const { raceState, setRaceState, typingState, setTypingState, updateProgress, handleInput: raceHandleInput, loadNewSnippet, anticheatState, flagSuspicious, markTrustedInteraction } = useRace(); |
Copilot
AI
Dec 5, 2025
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.
Unused variable updateProgress.
| const { raceState, setRaceState, typingState, setTypingState, updateProgress, handleInput: raceHandleInput, loadNewSnippet, anticheatState, flagSuspicious, markTrustedInteraction } = useRace(); | |
| const { raceState, setRaceState, typingState, setTypingState, handleInput: raceHandleInput, loadNewSnippet, anticheatState, flagSuspicious, markTrustedInteraction } = useRace(); |
…nticheat:lock event
|
@codex re review |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (delta > MAX_PROGRESS_STEP && !isCompleted) { | ||
| registerSuspicion('progress-spike', { prevPosition, position, delta }); | ||
| return; | ||
| } |
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.
Block single-packet completions bypassing anti-cheat
In race:progress the spike/interval guards are skipped whenever isCompleted is true (delta > MAX_PROGRESS_STEP && !isCompleted). A scripted client can therefore send its very first progress packet with position equal to the full snippet length and isCompleted: true after a modest delay (e.g., 8s to keep WPM <320). Because the spike and rate checks are bypassed, the packet is accepted and handlePlayerFinish runs without any incremental typing history, undermining the anti-automation protections this change is trying to add. Consider enforcing the same limits (or a dedicated check) even for completion packets to prevent single-update finishes.
Useful? React with 👍 / 👎.
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.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 15 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const flagSuspicious = useCallback((reason, metadata = {}) => { | ||
| if (!reason) return; | ||
| let shouldNotifyServer = false; | ||
|
|
||
| setAnticheatState(prev => { | ||
| const alreadyReported = prev.reasons.some(entry => entry.reason === reason); | ||
| if (!alreadyReported) { | ||
| shouldNotifyServer = true; | ||
| } | ||
| return { | ||
| locked: true, | ||
| reasons: alreadyReported | ||
| ? prev.reasons | ||
| : [...prev.reasons, { reason, metadata, at: Date.now(), source: 'client' }], | ||
| message: metadata?.message || prev.message || 'Suspicious automation detected' | ||
| }; | ||
| }); | ||
|
|
||
| if (shouldNotifyServer && socket && connected) { | ||
| socket.emit('anticheat:flag', { | ||
| reason, | ||
| metadata, | ||
| code: raceState.code, | ||
| lobbyId: raceState.lobbyId | ||
| }); | ||
| } | ||
| }, [socket, connected, raceState.code, raceState.lobbyId]); |
Copilot
AI
Dec 5, 2025
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.
The flagSuspicious callback depends on raceState.code and raceState.lobbyId, but these are not stable references - they're properties of the raceState object which changes frequently. This could cause the callback to be recreated unnecessarily on every raceState update, potentially leading to stale closures or performance issues.
Consider restructuring to only depend on stable values, or use a ref to access the latest race state values inside the callback without including them in the dependency array.
| const history = Array.isArray(prevProgress.history) ? prevProgress.history : []; | ||
| history.push({ position, timestamp: now }); | ||
| const trimmedHistory = history.slice(-180); | ||
|
|
||
| // Store player progress, using the client-provided completion status | ||
| playerProgress.set(socket.id, { | ||
| position, | ||
| completed: isCompleted, // Use the client-provided completion status | ||
| hasError: !!hasError, | ||
| timestamp: now | ||
| completed: isCompleted, | ||
| timestamp: now, | ||
| accuracy: Number.isFinite(accuracy) ? accuracy : prevProgress.accuracy, | ||
| errors: Number.isFinite(errors) ? errors : prevProgress.errors, | ||
| correctChars: Number.isFinite(correctChars) ? correctChars : prevProgress.correctChars, | ||
| hasError: currentHasError, | ||
| wpm: Number.isFinite(clientReportedWpm) ? clientReportedWpm : prevProgress.wpm, | ||
| history: trimmedHistory |
Copilot
AI
Dec 5, 2025
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.
The progress history is tracked and trimmed to the last 180 entries, but it's never actually used for anti-cheat validation. This consumes memory for every active player without providing value. Either implement velocity/pattern analysis using this history, or remove it to avoid unnecessary memory overhead in high-concurrency scenarios.
| const registerSuspicion = (reason, details = {}) => { | ||
| if (!reason) return; | ||
| const existing = suspiciousPlayers.get(socket.id) || { reasons: [], locked: false }; | ||
| const already = existing.reasons.some(entry => entry.reason === reason); | ||
| if (!already) { | ||
| existing.reasons.push({ reason, details, at: Date.now() }); | ||
| } | ||
| existing.locked = true; | ||
| suspiciousPlayers.set(socket.id, existing); | ||
|
|
||
| const progress = playerProgress.get(socket.id) || {}; | ||
| progress.suspicious = true; | ||
| progress.suspicionReasons = existing.reasons; | ||
| playerProgress.set(socket.id, progress); | ||
|
|
||
| console.warn(`[ANTICHEAT] Locked socket ${socket.id} (${netid}) for ${reason}`, details); | ||
| socket.emit('anticheat:lock', { | ||
| reason, | ||
| details, | ||
| message: details?.message || 'Suspicious typing detected. Automation is not allowed.' | ||
| }); | ||
| }; |
Copilot
AI
Dec 5, 2025
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.
The registerSuspicion function always sets locked: true (line 332), but there's no mechanism to unlock a player once they're flagged. Even if a player was flagged due to a false positive (e.g., network lag causing a spike), they remain permanently locked for the entire session.
Consider implementing either:
- A manual unlock mechanism for administrators
- A threshold system where minor infractions don't immediately lock
- A time-based unlock after a cooldown period
- Clearing the lock when a new race starts
Without such mechanisms, legitimate players affected by false positives have no recourse except refreshing the page.
| const MAX_PROGRESS_STEP = 20; // max characters allowed per progress update | ||
| const MIN_PROGRESS_INTERVAL = 25; // min ms between progress packets | ||
| const MAX_ALLOWED_WPM = 320; // anything above is flagged | ||
| const MIN_COMPLETION_TIME_MS = 2500; // cannot finish faster than this |
Copilot
AI
Dec 5, 2025
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.
The constant MIN_COMPLETION_TIME_MS is set to 2500ms (2.5 seconds), but it's only applied to snippets longer than 40 characters (line 1464). This naming doesn't reflect the conditional nature of its application. Consider renaming to something like MIN_LONG_SNIPPET_COMPLETION_MS or documenting the 40-character threshold in a comment above the constant.
| const MIN_COMPLETION_TIME_MS = 2500; // cannot finish faster than this | |
| // Only applied to snippets longer than 40 characters (see usage below) | |
| const MIN_LONG_SNIPPET_COMPLETION_MS = 2500; // cannot finish faster than this for long snippets |
| const handleSocketReconnect = () => { | ||
| if (authenticated && !isInitialConnection) { | ||
| console.log('Socket reconnected, refreshing user profile data'); | ||
| fetchUserProfile(); | ||
| } | ||
| isInitialConnection = false; | ||
| }; |
Copilot
AI
Dec 5, 2025
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.
[nitpick] The event handler is renamed from handleSocketConnect to handleSocketReconnect, which better reflects its purpose (it skips the initial connection). However, the event being listened to is still tigertype:connect which fires on every connect including the initial one. This creates a subtle mismatch between the event name and handler name that could confuse future maintainers.
Consider either renaming the handler back to handleSocketConnect (since it handles all connects, not just reconnects), or adjusting the logic/comments to clarify the relationship.
| const flagSuspicious = useCallback((reason, metadata = {}) => { | ||
| if (!reason) return; | ||
| let shouldNotifyServer = false; | ||
|
|
||
| setAnticheatState(prev => { | ||
| const alreadyReported = prev.reasons.some(entry => entry.reason === reason); | ||
| if (!alreadyReported) { | ||
| shouldNotifyServer = true; | ||
| } | ||
| return { | ||
| locked: true, | ||
| reasons: alreadyReported | ||
| ? prev.reasons | ||
| : [...prev.reasons, { reason, metadata, at: Date.now(), source: 'client' }], | ||
| message: metadata?.message || prev.message || 'Suspicious automation detected' | ||
| }; | ||
| }); | ||
|
|
||
| if (shouldNotifyServer && socket && connected) { | ||
| socket.emit('anticheat:flag', { | ||
| reason, | ||
| metadata, | ||
| code: raceState.code, | ||
| lobbyId: raceState.lobbyId | ||
| }); | ||
| } |
Copilot
AI
Dec 5, 2025
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.
The flagSuspicious function uses a local variable shouldNotifyServer that's set inside the state updater but used outside it. Since setState is asynchronous and the updater can be called multiple times, there's a potential race condition where shouldNotifyServer might be based on stale state if multiple flags occur in quick succession.
Consider moving the server emission logic inside a useEffect that watches for changes to anticheatState.reasons, or restructure to ensure the server is notified atomically when a new reason is added.
| const lastTrustedInteractionRef = useRef(Date.now()); | ||
|
|
||
| const markTrustedInteraction = useCallback(() => { | ||
| lastTrustedInteractionRef.current = Date.now(); | ||
| }, []); | ||
|
|
||
| const resetAnticheatState = useCallback(() => { | ||
| setAnticheatState(() => ({ | ||
| locked: false, | ||
| reasons: [], | ||
| message: null | ||
| })); | ||
| lastTrustedInteractionRef.current = Date.now(); |
Copilot
AI
Dec 5, 2025
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.
The lastTrustedInteractionRef is updated whenever a trusted interaction occurs, but it's never actually used for any validation logic. The ref is set in markTrustedInteraction and reset in resetAnticheatState, but there's no code that reads lastTrustedInteractionRef.current to validate time-based patterns or distinguish between legitimate bursts and automation.
If this ref isn't going to be used for validation, consider removing it to reduce complexity. Otherwise, implement the intended validation logic that checks the time since last trusted interaction.
| const lastTrustedInteractionRef = useRef(Date.now()); | |
| const markTrustedInteraction = useCallback(() => { | |
| lastTrustedInteractionRef.current = Date.now(); | |
| }, []); | |
| const resetAnticheatState = useCallback(() => { | |
| setAnticheatState(() => ({ | |
| locked: false, | |
| reasons: [], | |
| message: null | |
| })); | |
| lastTrustedInteractionRef.current = Date.now(); | |
| const resetAnticheatState = useCallback(() => { | |
| setAnticheatState(() => ({ | |
| locked: false, | |
| reasons: [], | |
| message: null | |
| })); |
| return; | ||
| } | ||
| const nativeEvent = e.nativeEvent || e; | ||
| if (nativeEvent && nativeEvent.isTrusted === false) { |
Copilot
AI
Dec 5, 2025
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.
Similar to the onBeforeInput handler, the isTrusted check uses strict inequality (=== false) which only catches explicitly false values. If isTrusted is undefined or missing, the check will pass. Consider using !== true instead for more robust detection of synthetic events.
| if (nativeEvent && nativeEvent.isTrusted === false) { | |
| if (nativeEvent && nativeEvent.isTrusted !== true) { |
| if (nativeEvent && nativeEvent.isTrusted === false) { | ||
| e.preventDefault(); | ||
| flagSuspicious('synthetic-input-change', { length: e.target?.value?.length ?? 0 }); | ||
| return; |
Copilot
AI
Dec 5, 2025
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.
Same issue with the isTrusted check - using strict inequality (=== false) only catches explicitly false values. If isTrusted is undefined or missing, the check will pass. Consider using !== true for more robust detection of synthetic events.
| if (isSocketLocked()) { | ||
| const entry = suspiciousPlayers.get(socket.id); | ||
| if (entry && entry.reasons.length > 0) { | ||
| const lastReason = entry.reasons[entry.reasons.length - 1]; | ||
| socket.emit('anticheat:lock', { | ||
| reason: lastReason.reason, | ||
| details: lastReason.details, | ||
| message: lastReason.details?.message || 'Suspicious typing detected. Automation is not allowed.' | ||
| }); | ||
| } | ||
| return; | ||
| } |
Copilot
AI
Dec 5, 2025
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.
When a socket is locked, this block re-emits the lock event on every progress update. This could spam the client with duplicate lock events. Consider tracking whether the lock event has already been sent to avoid unnecessary network traffic, or simply return early without re-emitting since the client should already have been notified by the initial lock.
Summary
Stops browser automation (e.g., Tampermonkey scripts) from typing or posting impossible progress during races, freezing input and warning the user whenever suspicious behavior
is detected.
Implementation Details
can also trigger locks.
tracked to avoid false positives.
Closes TIG-139