Repository navigation
fix(security): unbiased maxai X-Random nonce + stricter URL/regex test assertions (destrava o gate codeql-ratchet) - #12502
Merged
Conversation
…ertions Drains 7 of the 13 open CodeQL alerts that put the `codeql-ratchet` gate into regression (13 > baseline 11) on every open PR. The alerts arrived with the recent provider/media merges (#11461 MaxAI, #11513 UC, #12365 prefix shadowing), not with the work they are currently blocking. Production fix (js/biased-cryptographic-random): - open-sse/executors/maxai/signing.ts: the 6-digit `X-Random` wire slot was drawn as `randomBytes(4).readUInt32BE(0) % 900000`. 2^32 does not divide evenly by 900000, so the low ~4772 values of the range came out marginally more often. Extracted as `maxaiRandomSlot()` over `crypto.randomInt`, which rejection-samples internally. The emitted shape is unchanged (6 digits). Test assertions strengthened (never weakened): - tests/unit/helpers/ucClerkUrl.ts (new): `isUcClerkMintUrl()` matches the Clerk mint call by parsed origin (against `UC_CLERK_FAPI`) plus the `/v1/client/sessions/{sid}/tokens` path shape. - tests/unit/uc-image.test.ts, tests/unit/uc-video.test.ts: the mock fetch routers dispatched on `url.includes("clerk.uncensored.com")`, so any host merely embedding the name was served the mint response — a malformed URL built by the executor could not fail the test (js/incomplete-url-substring-sanitization x4). - tests/unit/maxai-image.test.ts: `new RegExp(PATH.replace(/\//g, "\\/"))` escaped only slashes (which need no escaping) and matched the path anywhere in a wrong URL; replaced by exact URL equality (js/incomplete-sanitization). - tests/unit/custom-provider-prefix-shadowing-11943.test.ts: the expected node mention was a RegExp with only `()` hand-escaped; replaced by an exact substring check (js/incomplete-sanitization). - tests/unit/maxai.test.ts: regression guard for the X-Random slot (6 digits, in range, spread across both halves of the range). The remaining 6 alerts are not defects and are left for an operator dismissal with justification (Hard Rule #14): the MaxAI HMAC-SHA1/SM3 signature and the CryptoJS `EVP_BytesToKey(MD5)` derivation are wire-protocol requirements — changing either breaks the provider — and `open-sse/utils/error.ts:749` already routes through `sanitizeErrorMessage()` (documented CodeQL sanitizer blind spot, docs/security/ERROR_SANITIZATION.md).
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Collaborator
Author
|
Mergeando com 2 checks vermelhos que não podem ficar verdes antes do merge — ambos são estado do repositório, não conteúdo desta PR (15 checks verdes):
O que esta PR de fato faz: corrige 7 alertas reais — nonce enviesado no assinador MaxAI ( Os 6 alertas restantes seguem abertos, com justificativa técnica por alerta no corpo desta PR, aguardando dispensa do operador (Hard Rule #14 — não dispenso alerta de segurança por conta própria). |
This was referenced Sep 2, 2026
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…ertions (cheaper-inference#12502) Drains 7 of the 13 open CodeQL alerts that put the `codeql-ratchet` gate into regression (13 > baseline 11) on every open PR. The alerts arrived with the recent provider/media merges (cheaper-inference#11461 MaxAI, cheaper-inference#11513 UC, cheaper-inference#12365 prefix shadowing), not with the work they are currently blocking. Production fix (js/biased-cryptographic-random): - open-sse/executors/maxai/signing.ts: the 6-digit `X-Random` wire slot was drawn as `randomBytes(4).readUInt32BE(0) % 900000`. 2^32 does not divide evenly by 900000, so the low ~4772 values of the range came out marginally more often. Extracted as `maxaiRandomSlot()` over `crypto.randomInt`, which rejection-samples internally. The emitted shape is unchanged (6 digits). Test assertions strengthened (never weakened): - tests/unit/helpers/ucClerkUrl.ts (new): `isUcClerkMintUrl()` matches the Clerk mint call by parsed origin (against `UC_CLERK_FAPI`) plus the `/v1/client/sessions/{sid}/tokens` path shape. - tests/unit/uc-image.test.ts, tests/unit/uc-video.test.ts: the mock fetch routers dispatched on `url.includes("clerk.uncensored.com")`, so any host merely embedding the name was served the mint response — a malformed URL built by the executor could not fail the test (js/incomplete-url-substring-sanitization x4). - tests/unit/maxai-image.test.ts: `new RegExp(PATH.replace(/\//g, "\\/"))` escaped only slashes (which need no escaping) and matched the path anywhere in a wrong URL; replaced by exact URL equality (js/incomplete-sanitization). - tests/unit/custom-provider-prefix-shadowing-11943.test.ts: the expected node mention was a RegExp with only `()` hand-escaped; replaced by an exact substring check (js/incomplete-sanitization). - tests/unit/maxai.test.ts: regression guard for the X-Random slot (6 digits, in range, spread across both halves of the range). The remaining 6 alerts are not defects and are left for an operator dismissal with justification (Hard Rule cheaper-inference#14): the MaxAI HMAC-SHA1/SM3 signature and the CryptoJS `EVP_BytesToKey(MD5)` derivation are wire-protocol requirements — changing either breaks the provider — and `open-sse/utils/error.ts:749` already routes through `sanitizeErrorMessage()` (documented CodeQL sanitizer blind spot, docs/security/ERROR_SANITIZATION.md). Co-authored-by: Markus Hartung <diegosouzapw@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contexto
O gate
codeql-ratchetestá em REGRESSÃO e reprova toda PR aberta do repositório:Os 13 alertas não vieram do trabalho que eles estão bloqueando — chegaram com merges recentes de provider/mídia de outros autores:
cabbbe41feat(providers): add MaxAI — signed OpenAI-compatible provider …530096a3feat(providers): add UC (uncensored.com) — persona + direct752aac65/ #12423)3a564183fix(sse): name the shadowed custom provider node …84504e3fchore(providers): retire Felo Web on provenance holdEsta PR corrige em código os 7 alertas que são defeito real e documenta os 6 restantes para dispensa do operador com justificativa (Hard Rule #14 — nenhum alerta foi dispensado via API por esta sessão).
Projeção: 13 → 6 alertas abertos, abaixo do baseline 11. O rebaseline (
--update) fica a cargo do operador, depois que a contagem assentar.Triagem completa — 13 alertas
js/biased-cryptographic-randomopen-sse/executors/maxai/signing.ts:128randomBytes(4).readUInt32BE(0) % 900000enviesa o slotX-Random(2^32 não divide 900000). Trocado porcrypto.randomInt(100000, 1000000), que faz rejection sampling internamente. Forma na rede idêntica: sempre 6 dígitos.js/incomplete-sanitizationtests/unit/maxai-image.test.ts:115new RegExp(MAXAI_IMAGE_PATH.replace(/\//g, "\\/"))escapava só/(que nem precisa de escape em RegExp) e aceitava o path em qualquer posição de uma URL errada. Trocado por igualdade exataassert.equal(capturedUrl, MAXAI_BASE_URL + MAXAI_IMAGE_PATH).js/incomplete-sanitizationtests/unit/custom-provider-prefix-shadowing-11943.test.ts:100NAME.replace(/[()]/g, "\\$&")— só parênteses escapados; qualquer outro metacaractere num nome futuro seria interpretado em vez de casado literalmente. Trocado pormessage.includes(...)com a string exata.js/incomplete-url-substring-sanitizationtests/unit/uc-image.test.ts:147fetchmockado despachava porurl.includes("clerk.uncensored.com")— qualquer host que apenas contivesse o nome (ex.:https://evil.example/?next=clerk.uncensored.com) recebia a resposta do mint, ou seja, o teste não conseguia falhar se o executor montasse a URL errada. Trocado pelo helper novoisUcClerkMintUrl():new URL(url).origin === UC_CLERK_FAPI+ shape do path/v1/client/sessions/{sid}/tokens.js/incomplete-url-substring-sanitizationtests/unit/uc-image.test.ts:268js/incomplete-url-substring-sanitizationtests/unit/uc-video.test.ts:151js/incomplete-url-substring-sanitizationtests/unit/uc-video.test.ts:342js/weak-cryptographic-algorithmopen-sse/executors/maxai/signing.ts:43sha1 = HMAC_SHA1(sign_str, key=reqTime:hmacKey)). Trocar o algoritmo invalida toda assinatura e derruba o provider. HMAC-SHA1 também não é afetado pelos ataques de colisão contra SHA-1.js/weak-cryptographic-algorithmopen-sse/executors/maxai/constants.ts:279reproduceProof()é a reimplementação pura do mesmo cálculo (HMAC-SHA1 + SM3), usada para validar constantes extraídas. Mesma justificativa de #973.js/weak-cryptographic-algorithmtests/unit/helpers/maxaiMockConstants.ts:52referenceProof()é a implementação de referência independente do proof (deliberadamente não importada da produção) para provar que a matemática bate com a spec externa. Arquivo de teste, chaves MOCK, nenhum segredo real.js/insufficient-password-hashopen-sse/executors/maxai/signing.ts:43hmacKeycomo "password" e o HMAC como hash de senha. Não é: é a chave de um MAC de assinatura de requisição. Nada é armazenado nem comparado como hash de senha, e ohmacKeyé uma constante pública que o próprio bundle JS da MaxAI publica (ver cabeçalho designing.ts). Um KDF com custo (bcrypt/scrypt/argon2) é semanticamente inaplicável aqui e quebraria o protocolo.js/insufficient-password-hashopen-sse/executors/maxai/signing.ts:61evpBytesToKey()é oEVP_BytesToKey(MD5)do OpenSSL — exatamente a derivação queCryptoJS.AES.encrypt(text, passphrase)usa. É obrigatória para produzir um envelopeSalted__que a MaxAI consiga decifrar. A "passphrase" é oaesKeypúblico extraído do bundle, não uma senha de usuário.js/stack-trace-exposureopen-sse/utils/error.ts:749docs/security/ERROR_SANITIZATION.md): o CodeQL não reconhece sanitizadores customizados. A linha éJSON.stringify({ error: { message: sanitizeErrorMessage(message), … } })e a única entrada tainted passa porsanitizeErrorMessage()(open-sse/utils/error.ts:64), que corta a string no primeiro\n(descartando os framesat …), substitui paths absolutos por<path>, trunca emMAX_ERROR_LENe rodaredactSensitiveErrorText().Resumo: 7 corrigidos · 2 falsos positivos (#969, #984) · 4 exigências de protocolo (#970, #972, #973, #974).
Precisa de ação do operador
Os 6 alertas abaixo não foram dispensados por esta sessão (Hard Rule #14 reserva o dismissal ao operador). Sugestão de motivo para cada um:
js/insufficient-password-hashfalse positive— chave de MAC de assinatura de requisição (constante pública do bundle MaxAI), não hash de senha.js/insufficient-password-hashwon't fix—EVP_BytesToKey(MD5)exigido pelo formatoSalted__do CryptoJS que a MaxAI decifra.js/weak-cryptographic-algorithmwon't fix— HMAC-SHA1 + SM3 é o esquema de assinatura da MaxAI; mudar quebra o provider.js/weak-cryptographic-algorithmwon't fix— idem #972 (caminho de produção).js/weak-cryptographic-algorithmused in tests— implementação de referência do proof, com chaves MOCK.js/stack-trace-exposurefalse positive— já sanitizado porsanitizeErrorMessage(); verdocs/security/ERROR_SANITIZATION.md.Depois que a contagem assentar, o rebaseline é
node scripts/check/check-codeql-ratchet.mjs --update(não rodei — é decisão do operador).Mudanças
open-sse/executors/maxai/signing.tsmaxaiRandomSlot()sobrecrypto.randomInttests/unit/helpers/ucClerkUrl.tsisUcClerkMintUrl(), recognizer estrito do mint do Clerktests/unit/uc-image.test.ts,tests/unit/uc-video.test.tstests/unit/maxai-image.test.tstests/unit/custom-provider-prefix-shadowing-11943.test.tstests/unit/maxai.test.tsmaxaiRandomSlot emits an unbiased 6-digit X-Random slotNenhuma asserção existente foi enfraquecida — todas as trocas em teste são estritamente mais restritivas do que o que estava lá. O
prettierdolint-stagedreformatou algumas linhas pré-existentes designing.ts/maxai.test.ts/maxai-image.test.ts(esses arquivos entraram no repo não formatados); é ruído de formatação, não mudança de comportamento.Verificação (foreground)
npm run typecheck:corenode --import tsx/esm --test tests/unit/maxai.test.ts tests/unit/maxai-image.test.ts tests/unit/uc-image.test.ts tests/unit/uc-video.test.ts tests/unit/custom-provider-prefix-shadowing-11943.test.tstests 99 · pass 99 · fail 0node --import tsx/esm --test tests/unit/maxai-documents.test.tstests 14 · pass 14 · fail 0npx eslint <7 arquivos alterados>Base
release/v3.8.51verificada verde na abertura (sem issueRelease branch not greenaberta) e semrelease-freezeativo.