fix(anti-cheat): rejeita finish temporalmente impossível e 409 honesto (#34) - #36
fix(anti-cheat): rejeita finish temporalmente impossível e 409 honesto (#34)#36caioross wants to merge 1 commit into
Conversation
… não ok:true (#34) O leaderboard global só é alimentado pela action `finish`, que hoje exige apenas a sala em `racing`. `sanitizeResults` (#7) fecha valores absurdos, mas é cego ao relógio: um payload "plausível" (WPM 349) postado logo após o `start` entra no ranking sem nenhuma tecla digitada. - `plausibleWpmCeiling(startAtISO, snippetChars, now)` e `validateFinishTiming` (puras, em src/lib/room.ts): teto físico por-corrida `(chars/5)/elapsedMin` com 10% de folga p/ clock-skew; linhas acima são DESCARTADAS (não clampadas). `start_at` já embute o countdown (a rota grava `now + COUNTDOWN_MS`), então o tempo decorrido é `now - start_at` — sem subtrair COUNTDOWN_MS de novo. - Rota `finish`: 409 se a sala não está em `racing`, 409 se ainda no countdown (`now <= start_at`, sem flipar/persistir), e 409 quando a transição condicional não casa — antes respondia `ok:true` mentindo p/ o cliente. - `start_at`/snippet ausentes numa sala `racing` → descarta tudo (nunca "passa tudo"), encerrando a sala sem alimentar o leaderboard. - Cobertura em scripts/validate-persistence.mjs (+12 casos, 45/45 verde). Refs #34 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@caioross is attempting to deploy a commit to the caioross' projects team on Vercel, but is not a member of this team. To resolve this issue, you can:
To read more about collaboration on Vercel, click here. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces anti-cheat temporal validation to prevent forged race results by verifying that the submitted WPM is physically plausible given the elapsed time. However, the review identified a critical security vulnerability where extremely short elapsed times allow bots to bypass the WPM ceiling, which should be resolved by validating actual typing speed against a maximum plausible limit. Additionally, a UX regression was flagged in the /finish endpoint where concurrent multiplayer finishes would incorrectly trigger 409 Conflict errors for subsequent finishers instead of returning a successful response.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| export function validateFinishTiming( | ||
| results: ResultRow[], | ||
| room: Pick<RoomRow, "start_at" | "snippet">, | ||
| now: number | ||
| ): ResultRow[] { | ||
| if (!Array.isArray(results) || results.length === 0) return []; | ||
| const ceiling = plausibleWpmCeiling(room.start_at ?? null, room.snippet?.code.length ?? 0, now); | ||
| if (ceiling <= 0) return []; // corrida impossível/sem dado → nada plausível | ||
| return results.filter(r => r.wpm <= ceiling); | ||
| } |
There was a problem hiding this comment.
🚨 Brecha de Segurança / Bypass do Anti-Cheat
A lógica atual de validação temporal possui uma vulnerabilidade crítica que permite que trapaceiros enviem resultados instantâneos com o WPM máximo permitido (350 WPM).
O Problema:
O teto de WPM (plausibleWpmCeiling) é calculado como ((chars / 5) / elapsedMin) * CLOCK_SKEW_SLACK.
Quando o tempo decorrido (elapsedMin) é extremamente curto (por exemplo, 2 segundos), o teto calculado torna-se absurdamente alto (ex: 1980 WPM para um snippet de 300 caracteres).
Como o WPM enviado pelo cliente (r.wpm) já foi sanitizado e limitado a 350 WPM por sanitizeResults, a verificação r.wpm <= ceiling (ex: 350 <= 1980) será verdadeira. Isso permite que um bot termine a corrida em 2 segundos e seu resultado de 350 WPM seja aceito e persistido no leaderboard global!
A Solução:
Devemos validar a velocidade real de digitação do jogador com base no seu progresso e no tempo decorrido, garantindo que essa velocidade real não exceda o limite humano plausível (MAX_PLAUSIBLE_WPM * CLOCK_SKEW_SLACK). O cheater pode forjar o campo r.wpm, mas não pode forjar o tempo decorrido no servidor e o progresso (que precisa ser 1 para constar como finalizado).
export function validateFinishTiming(
results: ResultRow[],
room: Pick<RoomRow, "start_at" | "snippet">,
now: number
): ResultRow[] {
if (!Array.isArray(results) || results.length === 0) return [];
if (!room.start_at) return [];
const startMs = Date.parse(room.start_at);
if (!Number.isFinite(startMs)) return [];
const elapsedMin = (now - startMs) / 60000;
if (elapsedMin <= 0) return [];
const chars = Math.max(0, Math.round(Number(room.snippet?.code.length)) || 0);
if (chars <= 0) return [];
const ceiling = ((chars / 5) / elapsedMin) * CLOCK_SKEW_SLACK;
return results.filter(r => {
// 1. O WPM declarado não pode exceder o teto calculado para o tempo decorrido
if (r.wpm > ceiling) return false;
// 2. A velocidade real de digitação baseada no progresso não pode exceder o limite humano plausível
const actualWpm = ((chars * (r.progress ?? 0)) / 5) / elapsedMin;
return actualWpm <= MAX_PLAUSIBLE_WPM * CLOCK_SKEW_SLACK;
});
}| function validateFinishTiming(results, room, now) { | ||
| if (!Array.isArray(results) || results.length === 0) return []; | ||
| const ceiling = plausibleWpmCeiling(room.start_at ?? null, room.snippet?.code.length ?? 0, now); | ||
| if (ceiling <= 0) return []; | ||
| return results.filter(r => r.wpm <= ceiling); | ||
| } |
There was a problem hiding this comment.
🚨 Brecha de Segurança / Bypass do Anti-Cheat (Espelho de Testes)
Esta função precisa ser atualizada para espelhar a correção de segurança sugerida em src/lib/room.ts. Sem isso, a validação de persistência nos scripts de teste não cobrirá o cenário real de bypass temporal onde um bot envia 350 WPM em poucos segundos.
function validateFinishTiming(results, room, now) {
if (!Array.isArray(results) || results.length === 0) return [];
if (!room.start_at) return [];
const startMs = Date.parse(room.start_at);
if (!Number.isFinite(startMs)) return [];
const elapsedMin = (now - startMs) / 60000;
if (elapsedMin <= 0) return [];
const chars = Math.max(0, Math.round(Number(room.snippet?.code.length)) || 0);
if (chars <= 0) return [];
const ceiling = ((chars / 5) / elapsedMin) * CLOCK_SKEW_SLACK;
return results.filter(r => {
if (r.wpm > ceiling) return false;
const actualWpm = ((chars * (r.progress ?? 0)) / 5) / elapsedMin;
return actualWpm <= MAX_PLAUSIBLE_WPM * CLOCK_SKEW_SLACK;
});
}| case "finish": { | ||
| // Fronteira anti-cheat: `results` vem do cliente e alimenta o leaderboard | ||
| // global. Sanitiza/clampa/descarta linhas forjadas antes de persistir. | ||
| const results: ResultRow[] = sanitizeResults(body.results, room as RoomRow); | ||
| // global. Só a sala em `racing` pode terminar — responder ok:true fora disso | ||
| // mentiria para o cliente e esconderia ataque/bug. | ||
| if (room.status !== "racing") | ||
| return NextResponse.json({ ok: false, error: "Sala não está em corrida" }, { status: 409 }); | ||
|
|
||
| const now = Date.now(); | ||
| const startMs = room.start_at ? Date.parse(room.start_at) : NaN; | ||
| // Ainda no countdown (start_at no futuro): a corrida não pode ter terminado. | ||
| // Não flipa nem persiste. (start_at ausente/inválido cai adiante: a sala | ||
| // encerra com results vazio, sem alimentar o leaderboard.) | ||
| if (Number.isFinite(startMs) && now <= startMs) | ||
| return NextResponse.json({ ok: false, error: "Corrida ainda não começou" }, { status: 409 }); | ||
|
|
||
| // Sanitiza valores forjados e então descarta linhas temporalmente | ||
| // impossíveis (WPM alto demais para o tempo real desde `start_at`). | ||
| const sanitized: ResultRow[] = sanitizeResults(body.results, room as RoomRow); | ||
| const results: ResultRow[] = validateFinishTiming(sanitized, room as RoomRow, now); | ||
| // Conditional transition racing→finished so only the first caller persists. | ||
| const { data: flipped } = await sb | ||
| .from("rooms") | ||
| .update({ status: "finished", results }) | ||
| .eq("code", code) | ||
| .eq("status", "racing") | ||
| .select("code"); | ||
| if (flipped && flipped.length) await persistMatch(sb, room as RoomRow, results); | ||
| // Perdeu a corrida da transição (outro `finish` concorrente já flipou) → 409. | ||
| if (!flipped || !flipped.length) | ||
| return NextResponse.json({ ok: false, error: "Sala não está em corrida" }, { status: 409 }); | ||
| if (results.length) await persistMatch(sb, room as RoomRow, results); | ||
| return NextResponse.json({ ok: true }); | ||
| } |
There was a problem hiding this comment.
⚠️ Regressão de UX: Erros 409 para Jogadores Legítimos
A alteração para retornar 409 Conflict quando a sala não está em corrida ou quando a transição condicional falha (!flipped || !flipped.length) introduz uma regressão grave na experiência do usuário.
O Problema:
Em uma corrida multiplayer, múltiplos jogadores terminam a corrida em instantes muito próximos e seus respectivos clientes enviam requisições para o endpoint /finish.
- O cliente do primeiro colocado envia a requisição, atualiza o status da sala para
finishede retorna200 OK. - Os clientes dos demais colocados enviam suas requisições logo em seguida. Como o status da sala já foi alterado para
finishedpelo primeiro colocado, a verificaçãoroom.status !== "racing"ou a transição condicional falhará, fazendo com que todos os outros jogadores recebam um erro409("Sala não está em corrida"). - Isso fará com que a interface do usuário exiba mensagens de erro falsas para jogadores honestos que simplesmente não terminaram em primeiro lugar.
A Solução:
Se a sala já estiver no status finished, o endpoint deve retornar 200 OK de forma silenciosa e bem-sucedida, pois o objetivo final (finalizar a corrida) já foi atingido. Devemos retornar 409 apenas se a sala estiver no status lobby (onde finalizar é impossível).
case "finish": {
// Fronteira anti-cheat: `results` vem do cliente e alimenta o leaderboard
// global. Só a sala em `racing` pode terminar — responder ok:true fora disso
// mentiria para o cliente e esconderia ataque/bug.
if (room.status === "lobby")
return NextResponse.json({ ok: false, error: "Sala não está em corrida" }, { status: 409 });
if (room.status === "finished")
return NextResponse.json({ ok: true });
const now = Date.now();
const startMs = room.start_at ? Date.parse(room.start_at) : NaN;
// Ainda no countdown (start_at no futuro): a corrida não pode ter terminado.
// Não flipa nem persiste. (start_at ausente/inválido cai adiante: a sala
// encerra com results vazio, sem alimentar o leaderboard.)
if (Number.isFinite(startMs) && now <= startMs)
return NextResponse.json({ ok: false, error: "Corrida ainda não começou" }, { status: 409 });
// Sanitiza valores forjados e então descarta linhas temporalmente
// impossíveis (WPM alto demais para o tempo real desde `start_at`).
const sanitized: ResultRow[] = sanitizeResults(body.results, room as RoomRow);
const results: ResultRow[] = validateFinishTiming(sanitized, room as RoomRow, now);
// Conditional transition racing→finished so only the first caller persists.
const { data: flipped } = await sb
.from("rooms")
.update({ status: "finished", results })
.eq("code", code)
.eq("status", "racing")
.select("code");
// Perdeu a corrida da transição (outro `finish` concorrente já flipou) → retorna sucesso silencioso.
if (flipped && flipped.length) {
if (results.length) await persistMatch(sb, room as RoomRow, results);
}
return NextResponse.json({ ok: true });
}
🩺 Quórum adversarial (HANDBOOK §7.2) — 1 APROVA / 2 VETO ❌ (não mergeado)Classificação: quórum — anti-cheat + Vetos (confirmados, com vetor):
AppSec ✅ — service_role server-side, sem injeção PostgREST, Veredito: a mudança não entrega o anti-cheat que promete e, pior, o teto invertido/global pode descartar corrida honesta em produção. O gate ficou verde só porque os testes cobrem apenas configs (snippet curto / elapsed longo) onde o forjado ultrapassa o teto — nunca a janela real de ataque. Por que devolvo ao Resolvedor (DRAFT) em vez de reparar inline: a correção não é 1 linha — o modelo correto precisa (a) inverter para piso e (b) ser por-jogador, usando |
|
Passagem do PR Doctor (sem parecer novo — o veredito do quórum em Só um registro de estado para quem assumir: a branch agora está CONFLICTING. A Lembrando o que ainda bloqueia, para não se perder: o teto temporal precisa virar piso e ser por-jogador ( |
🩺 PR Doctor — encerro esta PR (a necessidade #34 continua viva; o branch não)Terceira rodada em que esta PR aparece como "assumo depois". O HANDBOOK me obriga a 1. O que ainda vale (e não é pouco): a issue #34 está aberta e o furo está abertoReconferido no head da 2. Por que o branch não é mais reparável por união(a) A lógica foi vetada e precisa ser reescrita inteira — o quórum em (b) A parte de
A (c) O (d) A aritmética da dívida. Desde a base comum
Conflito nos três, e 100% do que sairia do merge seria reescrito ou revertido em seguida. 3. Encaminhamento
Não é |
…da (#124) O §5 dizia que uma issue com branch remota `auto/issue-<N>-*` não é elegível, sem qualificar o estado da PR. Como o §8 proíbe apagar branch, toda PR fechada sem merge congelava sua issue para sempre. Custo medido: a #34 (P1, furo de anti-cheat aberto na `main`) ficou top-1 do Curador por três rodadas e foi pulada duas vezes — a branch `auto/issue-34-finish-timing` era resíduo da PR #36, CLOSED com `mergedAt:null` desde 28/07. Só destravou porque o Curador escreveu um ruling dentro da issue em 31/07 e pediu a correção da letra ao Meta. - HANDBOOK §5: o bloqueio passa a ser "branch remota COM PR aberta vinculada"; branch órfã = resíduo, e o caminho correto é abrir branch nova. - cr-fleet-ops §2: a checagem (b)/(c) vira uma só, com o comando que decide (`gh pr list --state all --head <branch>`). Somente markdown — nenhum arquivo executável tocado, gate §6 não se aplica. Closes nada; melhoria de processo pedida pelo Curador em 2026-07-31. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Contexto
finish(src/app/api/rooms/[code]/route.ts) é a única porta do leaderboard global e hoje exige apenas a sala emracing.sanitizeResults(#7) fecha valores absurdos, mas é cego ao relógio: um payload plausível (WPM 349) postado logo após ostartentra no ranking sem nenhuma tecla digitada — e o mesmo request encerra a corrida de terceiros.Segui o 🏛️ Parecer do Conselho (APROVADA PARA EXECUÇÃO): entrego o teto temporal + piso de countdown agora; a auth-por-participante (roster server-side) fica para follow-up, como o Conselho recomendou.
O que mudou e por quê
src/lib/room.ts(puro, determinístico):plausibleWpmCeiling(startAtISO, snippetChars, now)→ teto físico por-corrida(chars/5)/elapsedMin, comCLOCK_SKEW_SLACK(10%) p/ clock-skew/latência. Retorna0quando nada é plausível (semstart_at, inválido, ou ainda no countdown).validateFinishTiming(results, room, now)→ descarta (não clampa) linhas acima do teto; teto0⇒ descarta tudo.route.tsfinish:409se a sala não está emracing;409se ainda no countdown (now <= start_at, sem flipar/persistir);409quando a transição condicional não casa (antes respondiaok:true, mentindo p/ o cliente). Pipeline:sanitizeResults→validateFinishTiming→ persiste.scripts/validate-persistence.mjs: +12 casos cobrindo o teto, countdown,start_at/snippet ausentes, corrida honesta preservada e corrida lenta sem falso-positivo.Divergências (transparência)
COUNTDOWN_MS. A AC sugeriaelapsedMin = (now - start_at - COUNTDOWN_MS)/60000, mas no código real ostartgravastart_at = now + COUNTDOWN_MS(o countdown já está embutido — confirmado emuseRoom.ts:269-333, onde o cliente conta regressivo atéstart_ate só postafinishquandonow >= start_at). Subtrair de novo seria double-count e descartaria os primeiros 4 s de corrida honesta. Usoelapsed = now - start_at, exatamente como o Parecer do Conselho derivou.COUNTDOWN_MScontinua importado e usado nostart; nada hardcoded.scripts/validate-persistence.mjs, não em.claude/skills/cr-multiplayer/scripts/...como a AC citou. Esse é o arquivo que o gate roda (node scripts/validate-persistence.mjs, HANDBOOK §6 / CLAUDE.md) e o único atualizado (espelha osanitizeResultsde Segurança: finish aceita results não validados → leaderboard global forjável (anti-cheat) #7). A cópia em.claude/skills/está defasada (pré-Segurança: finish aceita results não validados → leaderboard global forjável (anti-cheat) #7, espelha umsanitizeResultsingular com teto 300) e não é rodada pelo gate — sincronizá-la seria refactor fora de escopo. Fica como candidato a dedup/follow-up.Escopo / follow-up
Griefing (encerrar corrida alheia) é mitigado pelo piso temporal, não fechado: a autorização plena (só participante/líder encerra) exige um roster server-side que hoje não existe (presença vive no Realtime). Por isso uso
Refs #34e nãoCloses— deixo ao dono/PR Doctor decidir fechar ou abrir o follow-up de auth-por-participante.Validação (gate real)
pnpm typecheck✅ ·pnpm build✅node scripts/validate-persistence.mjs✅ 45/45 (12 novos)node .claude/skills/cr-typing-engine/scripts/validate-metrics.mjs✅ 27/27pnpm lint= N/A (sem config ESLint no repo; a CI não roda lint)Riscos
sanitizeResultsjá capa em 350 WPM global. Folga de 10% cobre clock-skew/rede em corridas realistas.nowancorado nostart_atdo servidor, nunca nofinishedAtdo cliente (o relógio do atacante não é régua).Solicito quórum (HANDBOOK §7)
Refs #34