Skip to content

fix(security): allowlist de language/difficulty nas rotas de sala (#35) - #38

Merged
caioross merged 1 commit into
mainfrom
auto/issue-35-allowlist-lang-diff
Jul 22, 2026
Merged

fix(security): allowlist de language/difficulty nas rotas de sala (#35)#38
caioross merged 1 commit into
mainfrom
auto/issue-35-allowlist-lang-diff

Conversation

@caioross

Copy link
Copy Markdown
Owner

Contexto

As duas rotas de sala gravavam language/difficulty como string crua do cliente, sem allowlist:

  • POST /api/rooms (criar) — language: settings.language || "javascript".
  • action settings em POST /api/rooms/[code]language: s.language || room.language.

persistMatch copia esses campos para matches/scores, que alimentam o leaderboard global público. Ou seja, um POST direto injetava linguagem/dificuldade arbitrária no ranking (poluição de dado + abuso de cardinalidade nos filtros) e gravava payloads sem teto de tamanho — depois replicados por Realtime a toda a sala. Não é XSS (Supabase parametriza); é integridade de dado e storage abuse.

O que mudou e por quê

  • src/lib/room.tsisValidLang/isValidDifficulty (predicados puros, type-guards) + resolveLang/resolveDifficulty (política de fronteira compartilhada). Reusam LANGUAGES/DIFFICULTIES de src/lib/languages.tsuma fonte de verdade, nunca duas listas para dessincronizar. Ficam junto de sanitizeResults, a outra fronteira anti-cheat da mesma tabela.
  • Rotas — valor presente inválido → 400 com mensagem clara; ausente (undefined/null/"") cai no default (create) ou no valor atual da sala (settings). maxPlayers segue como estava.
  • supabase/migrations/0004_settings_allowlist.sql — cinto e suspensório no banco: CHECK difficulty in ('easy','medium','hard') + teto de comprimento em language nas 3 tabelas (rooms/matches/scores). Aditiva e idempotente; NOT VALID de propósito, para valer em INSERT/UPDATE novos sem falhar a aplicação por linhas legadas. Apenas criei o arquivo — o dono aplica (CLAUDE.md / HANDBOOK §8).
  • scripts/validate-persistence.mjs — cobre os validadores puros: válido preserva · inválido rejeita · ausente cai no default · action settings mantém o valor atual.

Divergência do Parecer do Conselho (1 linha)

O Conselho sugeriu coerção silenciosa (inválido → default). Segui o acceptance criteria da issue, que é explícito e mais robusto: inválido → 400 (falhar alto revela cliente quebrado ou ataque; ausente continua no default, sem quebrar o fluxo legítimo).

Resultado do gate

  • pnpm install --frozen-lockfile
  • pnpm typecheck
  • pnpm build ✓ (8/8 páginas)
  • node scripts/validate-persistence.mjs49 passaram, 0 falharam
  • lint — N/A (sem config ESLint no repo; a CI não roda lint)
  • validate-metrics — N/A (não toca a engine de digitação)

Riscos

  • NOT VALID não limpa dado histórico já gravado; protege daqui pra frente. O dono pode validate constraint depois de higienizar, se quiser.
  • Fora de escopo (registrado na issue): rate limiting em POST /api/rooms — exige serviço novo (HANDBOOK §7.1).

Toca src/app/api/rooms/** + migration aditiva.

Solicito quórum (HANDBOOK §7)

Closes #35

As rotas de criação (POST /api/rooms) e de ajuste (action `settings` em
POST /api/rooms/[code]) gravavam `language`/`difficulty` como string crua do
cliente, sem allowlist. Esses campos são copiados por `persistMatch` para
`matches`/`scores` — o leaderboard global público —, então um POST direto
injetava dimensões arbitrárias no ranking e gravava payloads sem teto de tamanho
(replicados por Realtime a toda a sala).

- src/lib/room.ts: `isValidLang`/`isValidDifficulty` (predicados puros) e
  `resolveLang`/`resolveDifficulty` (política de fronteira), reusando
  `LANGUAGES`/`DIFFICULTIES` de src/lib/languages.ts — uma fonte de verdade.
- Rotas: valor presente inválido → 400 com mensagem clara; ausente cai no
  default (create) ou no valor atual da sala (settings), sem quebrar o fluxo.
- supabase/migrations/0004: CHECK aditivo/idempotente de difficulty e teto de
  comprimento de language nas 3 tabelas (NOT VALID para não falhar em dados
  legados). Apenas criado — o dono aplica.
- scripts/validate-persistence.mjs: cobre válido/inválido/ausente dos validadores.

Closes #35
@vercel

vercel Bot commented Jul 15, 2026

Copy link
Copy Markdown

@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:

  • Make your repository public. Collaboration is free for open source and public repositories.
  • Upgrade to pro and add @caioross as a member. A Pro subscription is required to access Vercel's collaborative features.
    • If you're the owner of the team, click here to upgrade and add @caioross as a member.
    • If you're the user who initiated this build request, click here to request access.
    • If you're already a member of the caioross' projects team, make sure that your Vercel account is connected to your GitHub account.

To read more about collaboration on Vercel, click here.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces robust validation for room settings (language and difficulty) across the API endpoints, test scripts, and database constraints to prevent invalid values or storage abuse. The reviewer feedback suggests a defensive programming improvement to validate the fallback parameters in resolveLang and resolveDifficulty (both in the main implementation and the test mirror) to avoid potential 500 errors if legacy invalid data exists in the database.

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.

Comment thread src/lib/room.ts
Comment on lines +121 to +132
export function resolveLang(raw: unknown, fallback: LangId): FieldResolution<LangId> {
if (raw == null || raw === "") return { ok: true, value: fallback };
return isValidLang(raw) ? { ok: true, value: raw } : { ok: false };
}

export function resolveDifficulty(
raw: unknown,
fallback: Difficulty
): FieldResolution<Difficulty> {
if (raw == null || raw === "") return { ok: true, value: fallback };
return isValidDifficulty(raw) ? { ok: true, value: raw } : { ok: false };
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Se a sala atual possuir dados legados inválidos no banco de dados (por exemplo, um language ou difficulty corrompido ou com tamanho maior que o permitido), o uso de room.language ou room.difficulty como fallback nas rotas de API pode propagar esse valor inválido durante uma atualização de configurações (como alterar o número máximo de jogadores). Isso fará com que a nova constraint do banco de dados rejeite a transação, resultando em erro 500 para o usuário.

Para evitar isso e garantir programação defensiva, podemos validar o fallback em tempo de execução dentro de resolveLang e resolveDifficulty, caindo para um valor padrão seguro caso o fallback seja inválido.

Suggested change
export function resolveLang(raw: unknown, fallback: LangId): FieldResolution<LangId> {
if (raw == null || raw === "") return { ok: true, value: fallback };
return isValidLang(raw) ? { ok: true, value: raw } : { ok: false };
}
export function resolveDifficulty(
raw: unknown,
fallback: Difficulty
): FieldResolution<Difficulty> {
if (raw == null || raw === "") return { ok: true, value: fallback };
return isValidDifficulty(raw) ? { ok: true, value: raw } : { ok: false };
}
export function resolveLang(raw: unknown, fallback: LangId): FieldResolution<LangId> {
if (raw == null || raw === "") {
const safeFallback = isValidLang(fallback) ? fallback : "javascript";
return { ok: true, value: safeFallback };
}
return isValidLang(raw) ? { ok: true, value: raw } : { ok: false };
}
export function resolveDifficulty(
raw: unknown,
fallback: Difficulty
): FieldResolution<Difficulty> {
if (raw == null || raw === "") {
const safeFallback = isValidDifficulty(fallback) ? fallback : "medium";
return { ok: true, value: safeFallback };
}
return isValidDifficulty(raw) ? { ok: true, value: raw } : { ok: false };
}

Comment on lines +81 to +88
function resolveLang(raw, fallback) {
if (raw == null || raw === '') return { ok: true, value: fallback };
return isValidLang(raw) ? { ok: true, value: raw } : { ok: false };
}
function resolveDifficulty(raw, fallback) {
if (raw == null || raw === '') return { ok: true, value: fallback };
return isValidDifficulty(raw) ? { ok: true, value: raw } : { ok: false };
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Para manter o espelho de testes em sincronia com a implementação de src/lib/room.ts, atualize as funções resolveLang e resolveDifficulty para também validarem o fallback em tempo de execução.

function resolveLang(raw, fallback) {
  if (raw == null || raw === '') {
    const safeFallback = isValidLang(fallback) ? fallback : 'javascript';
    return { ok: true, value: safeFallback };
  }
  return isValidLang(raw) ? { ok: true, value: raw } : { ok: false };
}
function resolveDifficulty(raw, fallback) {
  if (raw == null || raw === '') {
    const safeFallback = isValidDifficulty(fallback) ? fallback : 'medium';
    return { ok: true, value: safeFallback };
  }
  return isValidDifficulty(raw) ? { ok: true, value: raw } : { ok: false };
}

@caioross

Copy link
Copy Markdown
Owner Author

🩺 Quórum adversarial (HANDBOOK §7.2) — 3× APROVA ✅

Classificação: quórum — toca src/app/api/rooms/**, a fronteira do leaderboard e adiciona migration aditiva; o corpo solicita quórum. Pré-requisitos conferidos: CI Install, typecheck & build verde, mergeable=MERGEABLE, diff lido inteiro no head 6f4e782. (Check Vercel vermelho = limitação de permissão do autor no projeto, pré-existente, não falha de código.)

Três lentes adversariais em paralelo (default VETAR, vetor arquivo:linha obrigatório), lendo o código commitado do head — não o working tree do dono:

  • AppSec ✅ — a allowlist fecha a injeção real: no baseline settings.language || "javascript" gravava string crua, copiada por persistMatch para matches/scores; agora as duas rotas passam por resolveLang/resolveDifficulty (src/lib/room.ts:117-131), presente-inválido → 400, ausente → default já-allowlistado. Fonte de verdade única (LANGUAGES/DIFFICULTIES), com a constraint SQL como cinto-e-suspensório (não segunda enumeração). Payload gigante barrado no app e no banco (char_length ≤ 32). Migration 0004 é nova/aditiva/idempotente (if not exists por conname+conrelid)/NOT VALID, sem DROP/DELETE/UPDATE → fora do núcleo §7.1; só o arquivo foi criado (o dono aplica).
  • Ofensiva ✅ — nenhum abuso construível passa: case ("Rust"/"Easy"), não-string, array/objeto, __proto__ e homoglyph unicode caem em ok:false → 400 (Set.has + typeof === "string", room.ts:121-132); storage abuse barrado em duas camadas (server 400 + 0004:39). Divergir do Parecer do Conselho (400 vs coerção silenciosa) é mais estrito, não abre brecha. Resíduo não-vetável e já documentado: uma sala legada (janela de 3h, auto-expira) poderia ecoar um language curto inválido via o fallback de settings — difficulty legada já é bloqueada pela check em matches/scores.
  • Domínio ✅ — política de fronteira correta: ausente (null/undefined/"") → fallback; inválido → 400; válido preservado. A action settings mantém o valor atual da sala, idêntico ao antigo s.x || room.x, exceto por falhar alto em valor inválido (o fix pedido). Migration é do-block plpgsql válido (format('%I') + ::regclass), idempotente e aditiva; char_length ≤ 32 é folga suficiente (maior nome = 10 chars). Closes #35 apropriado (cobre o AC; rate limiting fica fora de escopo pela própria issue). Gate real no head: pnpm typecheck ✓ · pnpm build ✓ · node scripts/validate-persistence.mjs49/49 ✓.

⏸️ Parking-lot — pronto, merge parado pelo gate de produção

O que falta: nada de código. Quórum 3×APROVA, CI verde, mergeable. Por que não mergeei: merge na main = deploy Vercel de produção; em rodada autônoma não-supervisionada o classificador do harness barra esse merge (mesma condição de #30/#33). Quem destrava: o dono, numa sessão supervisionada — gh pr merge 38 --squash --delete-branch, e depois aplicar a migration supabase/migrations/0004_settings_allowlist.sql.

@caioross

Copy link
Copy Markdown
Owner Author

🅿️ Parking-lot do PR Doctor

PR pronta e MERGEABLE, mas não mergeada nesta rodada — teto de 2 merges/rodada já usado (#42, #33).

Bloqueio de coordenação — colisão de migração 0004: esta PR cria supabase/migrations/0004_settings_allowlist.sql e a #44 cria 0004_room_kicked_ids.sql. Só uma pode ficar 0004. Convenção (esta é a mais antiga): #38 mantém 0004; a #44 renumera para 0005 no merge dela.

Próxima rodada: quórum §7.2 (toca src/app/api/rooms/** + migração aditiva idempotente). Lembrete: o dono aplica a migração (§8) — o merge da PR não a aplica; até lá o CHECK só vale para escritas novas.
Destrava: PR Doctor (quórum + merge) + dono (aplicar 0004).

@caioross

Copy link
Copy Markdown
Owner Author

🏛️ Quórum adversarial (HANDBOOK §7.2) — 3× APROVA · head 6f4e782

Três lentes independentes, cada uma com default VETAR e obrigação de vetor concreto arquivo:linha, lendo o worktree da branch (não a working tree do clone do dono).

🔐 AppSec — APROVA

  • Allowlist derivada da fonte única: src/lib/room.ts:96-97 monta os Sets a partir de LANGUAGES/DIFFICULTIES — nenhuma segunda lista em produção. typeof x === "string" antes do Set.has derruba coerção (42, true, objeto com toString), case ('Easy') e homóglifos.
  • Prototype pollution não passa: settings vem só de JSON.parse; __proto__ em JSON vira propriedade própria e ainda teria de estar na allowlist.
  • SUPABASE_SERVICE_ROLE_KEY segue confinado em src/lib/supabase.tslanguages.ts é metadado público, nada de segredo entra no bundle.
  • Migration 0004: só ADD CONSTRAINT com guarda em pg_constraint — aditiva, idempotente, reaplicável. Sem injeção no bloco do $$: o único interpolado vem do array literal ['rooms','matches','scores'], identificadores por %I.

⚔️ Ofensiva — APROVA

  • O buraco fecha de fato: mapeados todos os escritores de language/difficultyroute.ts:72-73 e [code]/route.ts:71-72. persistMatch copia da linha rooms, que agora só aceita valor da allowlist → leaderboard blindado por transitividade, sem caminho lateral.
  • Sem estado parcial: os dois campos resolvem antes do .update(), então um 400 não deixa a sala meio-atualizada nem serve para travá-la.
  • RLS de rooms/matches/scores só tem select using (true) — anon não escreve; escrita só via service_role nas rotas.

🎯 Domínio — APROVA

  • Espelho do validador confere hoje, item a item, com as 14 linguagens e os 3 níveis de languages.ts.
  • Fluxo legítimo intacto: HomeView.tsx:118 e Lobby.tsx:130,157 mandam sempre l.id/d.id canônicos; patch parcial do Lobby ({maxPlayers} sozinho) segue funcionando pelo fallback.
  • Sala legada com valor fora da allowlist: o fallback preserva o valor atual sem revalidar — não reintroduz dado inválido novo e não tranca o líder fora dos settings.
  • Gate reproduzido no worktree: node scripts/validate-persistence.mjs49/49, tsc --noEmit limpo.

Registrado como follow-up (não bloqueia — as três lentes convergiram nisso)

As rotas de sala não conferem o error do .update() ([code]/route.ts:68,83,95,107). Depois que o dono aplicar a 0004, um UPDATE numa sala legada envenenada passa a violar o CHECK e a API responde ok:true em silêncio. Correção barata numa fatia própria; não é regressão deste PR.

Também pré-existentes na main (fora do escopo, valem issue): claim-leader sem prova ([code]/route.ts:114), finish sem checar playerId (:90-103) e ausência de teto/rate limit em POST /api/rooms.

⚠️ Ordem de deploy: a migration 0004_settings_allowlist.sql é cinto-e-suspensório opcional — o código já valida na fronteira, então o merge é seguro antes de o dono aplicar. CI Install, typecheck & build verde; o check Vercel vermelho é permissão da conta ("Git author caioross must have access to the project"), idêntico em toda PR aberta.

Mergeando em squash.

@caioross
caioross merged commit 4a82a40 into main Jul 22, 2026
2 of 3 checks passed
@caioross
caioross deleted the auto/issue-35-allowlist-lang-diff branch July 22, 2026 13:21
caioross added a commit that referenced this pull request Jul 22, 2026
União com origin/main (conflito em Race.tsx) mantendo uma só fonte de verdade
em vez de reintroduzir cópias que a main acabou de eliminar.

- Race.tsx fica na forma composta desta branch (RaceTrack + TypingCore + chat);
  o cálculo inline que o #32 tirou de lá não volta.
- TypingCore.tsx passa a importar countCorrectChars/computeWpm/computeAccuracy/
  computeProgress de @/lib/metrics (#32, mergeada no PR #48). A extração era 1:1
  do Race.tsx pré-#32; sem isso o merge recriava a duplicação de fórmula que a
  #32 existiu para matar. `correctChars` segue no useMemo (área sagrada §2).
- /api/snippet usa resolveLang/resolveDifficulty de @/lib/room (#35, mergeada no
  PR #38) no lugar da checagem própria contra LANGUAGES/DIFFICULTIES — mesma
  política de fronteira das rotas de sala, uma implementação só.

Gate: typecheck OK · build OK (/practice 149 kB estática, /api/snippet dinâmica)
· vitest 34/34 · validate-metrics 37/0 · validate-persistence 62/0.

Refs #25

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Segurança/API: language e difficulty entram no banco sem allowlist e vazam para matches/scores

1 participant