fix(deps): cierra 19 de 20 alertas y endurece la cuarentena - #8
Conversation
Vulnerabilidades: 20 -> 1
-------------------------
hono@<4.12.34 -> ^4.12.34 7 avisos: ReDoS en el middleware
CORS, fuga de SSR entre peticiones
en memo(), DoS en el middleware de
idioma... (rainbowkit > wagmi > porto)
undici@>=7.0.0 <7.29.0 -> ^7.29.0 5 avisos, uno HIGH (fuga de info
entre usuarios). Dev, via jsdom.
react-router-dom ^7.18.1 -> ^7.18.3 HIGH: bypass de CSRF en modo
RSC permite ejecutar actions
nanoid@<3.3.18 -> ^3.3.18 HIGH: los generadores personalizados
pueden entrar en bucle infinito
brace-expansion 4.x/5.x -> ^5.0.9 HIGH: DoS arrays intermedios
postcss@<8.5.23 -> ^8.5.23 parche incompleto de GHSA-6g55
uuid@<11.1.1 -> ^11.1.1 bounds check en v3/v5/v6
vitest 4.1.10 -> ^4.1.11 path traversal en el mocker
Todos los overrides se quedan dentro del major del propio parche, para no
colar un cambio de API por la puerta de atras.
Queda 1 sin arreglo posible: decode-uri-component (moderate, DoS por
decodificacion exponencial). El aviso pide >=0.4.3, version que no existe;
el unico parche es 0.5.0 y es ESM pura. query-string@7.1.3 --quien la usa,
via @walletconnect/utils-- la carga con require(), asi que recibe el
namespace del modulo en vez de la funcion y revienta con "decodeComponent
is not a function" al parsear cualquier query string.
Se intento y se revertio. Importante: el build de vite y los 64 tests pasan
igualmente con el override puesto; el fallo solo aparece al ejecutar
query-string. Queda documentado en pnpm-workspace.yaml.
Se verificaron a mano los dos saltos de varios majors: uuid 8.3.2 -> 11.1.1
carga y genera v4() desde @metamask/utils, y query-string sigue parseando
bien tras revertir decode-uri-component.
Cuarentena
----------
Se anade minimumReleaseAgeIgnoreMissingTime: false. El fichero ya explicaba
que pnpm <11.18 aborta con ERR_PNPM_MISSING_TIME por pedir el metadata
abreviado; esto cubre el otro lado del problema, que es que pnpm se salte la
comprobacion con un simple WARN cuando el campo "time" no esta.
Verificacion
------------
64/64 tests, typecheck limpio, build OK. Comprobados contra el registry los
718 paquetes del lockfile: los 718 tienen 7 dias o mas, 0 violaciones.
`pnpm lint` falla con 25 problemas, pero ya fallaba en main con los mismos:
son reglas de react-hooks sobre codigo existente, ajenas a este cambio.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@kilocode-bot, por que no has revisado?? |
|
There was a problem hiding this comment.
Code review — verificación completa
He revisado el diff real del PR y reproducido las verificaciones en local (pnpm 11.18.0, exactamente el packageManager de package.json y el pin de pnpm/action-setup en CI).
Resultados de las verificaciones ejecutadas
| Comando | Resultado |
|---|---|
pnpm install --frozen-lockfile |
✅ OK (lockfile sincronizado con manifiestos y overrides, sin warnings) |
pnpm typecheck |
✅ limpio |
pnpm test |
✅ 64/64 (vitest 4.1.11) |
pnpm build |
✅ OK (warning de chunk >500 kB, preexistente) |
pnpm audit |
✅ 1 moderada: decode-uri-component (la documentada) — 19/20 cerradas |
pnpm audit --prod |
✅ 1 moderada (misma; es runtime vía walletconnect) |
pnpm lint |
❌ 25 problemas (21 errores, 4 warnings) — idénticos byte a byte a main, preexistentes (he comparado las salidas con diff) |
Coherencia lockfile ↔ overrides
- Los 6 overrides nuevos resuelven dentro del major del propio parche:
hono4.12.32→4.13.8,undici7.28.0→7.29.1,nanoid3.3.16→3.3.19,postcss8.5.22→8.5.28,brace-expansion5.0.8→5.0.12,uuid(única copia) 11.1.1;react-router-dom/react-router7.18.3 yvitest4.1.11 (+ todo el familia@vitest/*) actualizados de forma consistente. - No queda ningún paquete resolviendo a versión vulnerable salvo
decode-uri-component@0.2.2, decisión documentada. - Spot-check de cuarentena contra el registry: todas las versiones resueltas tienen ≥7 días (hono 4.13.8: 15-sep, brace-expansion 5.0.12: 14-sep, etc.).
Sobre el riesgo de ^ frente a =
Matiz sobre la premisa: el PR no convierte overrides existentes de = a ^ (qr/ws/axios no se tocan); añade 6 overrides nuevos con selector pkg@<rango-vulnerable>: ^fijo. Ese diseño me parece correcto:
- Los selectores se delimitan a la ventana vulnerable, así que el override es auto-limpiable: cuando el dependiente declare ≥ la versión parcheada, deja de aplicar.
- El objetivo
^solo abre el rango dentro del major; junto con--frozen-lockfileen CI (que instala el lockfile pineado, nunca re-resuelve) y la cuarentena de 7 días en resolución fresca, la superficie de supply-chain es razonable. El riesgo residual (release comprometida con >7 días de antigüedad entrando por una re-resolución) es el mismo que con cualquier rango^y se mitiga revisando las PRs de actualización de lockfile. No veo necesidad de volver a pins=.
Puntos dudosos, verificados
- undici: queda confirmado dev-only (
pnpm why undici→ solojsdom← vitest ← devDependencies;audit --prodno lo reporta). minimumReleaseAgeIgnoreMissingTime: la clave existe desde pnpm v11.0.0 (documentación de settings);pnpm config listla lee del workspace con valorfalse.false= fail-closed (fallar si faltatime) frente al defaulttrue(saltarse con WARN): correcto para un repo endurecido. Con registry.npmjs.org no hay riesgo de CI; el único escenario de falsos fallos sería un mirror/registro privado que omitatime.- CI: no hay ningún paso
pnpm auditni allowlists que oculten alertas; el PR no toca workflows. Sugerencia opcional para otro PR: añadir un paso de audit programado. - Alcance: solo
package.json,pnpm-lock.yamlypnpm-workspace.yaml; no toca código de aplicación.
Los comentarios en línea tienen el detalle de cada punto. Veredicto: aprobable tal cual; nada de lo que señalo es bloqueante.
| # Llega via rainbowkit > wagmi > porto. | ||
| hono@<4.12.34: ^4.12.34 | ||
| # DoS por arrays intermedios sin limite. Solo dev, via eslint. | ||
| brace-expansion@>=4.0.0 <5.0.9: ^5.0.9 |
There was a problem hiding this comment.
Verificado que este override es seguro respecto al cruce de majors: el selector brace-expansion@>=4.0.0 <5.0.9 solo matchea dependientes cuya versión declarada caiga en la ventana vulnerable. En el árbol el único consumidor es minimatch@10.2.5 (declara ^5.0.5) → resuelve 5.0.12, mismo major. No hay ningún brace-expansion 1.x/2.x/4.x en el lockfile, y si apareciera uno nuevo declarando ^1.1.11 (p. ej. un glob/minimatch@3 futuro) el selector no lo tocaría. Segmentation correcta, no override global.
|
|
||
| # Si pese a todo a un paquete le falta el campo "time", fallar en vez de | ||
| # saltarse la comprobacion en silencio con un WARN. | ||
| minimumReleaseAgeIgnoreMissingTime: false |
There was a problem hiding this comment.
Verificado contra la versión pinneada (pnpm@11.18.0 en packageManager y en pnpm/action-setup): la clave existe desde pnpm v11.0.0 y pnpm config list la lee del workspace con este valor, así que no es una clave fantasma que pnpm ignore en silencio.
Con false el comportamiento es fail-closed: si al paquete le falta el campo time en el metadata, la instalación falla en vez de saltarse la cuarentena con un WARN (que es el default true). Para el objetivo de supply-chain de este repo me parece el valor correcto. Único riesgo teórico: un mirror/registro privado que recorte el metadata rompería los installs — con registry.npmjs.org no hay problema (verificado: instalación limpia).
| nanoid@<3.3.18: ^3.3.18 | ||
| # Cinco avisos contra undici 7.x, uno de ellos high (fuga de informacion | ||
| # entre usuarios). Solo dev: llega via jsdom. | ||
| undici@>=7.0.0 <7.29.0: ^7.29.0 |
There was a problem hiding this comment.
Confirmado el "solo dev": pnpm why undici → única cadena jsdom ← vitest ← devDependencies, y pnpm audit --prod no reporta undici. No alcanza runtime ni el bundle de producción. El rango >=7.0.0 <7.29.0 en el selector deja además fuera una futura 8.x, igual que hace el override de ws con 7.x.
|
Revisión completada y publicada en el PR: #8 (review) Veredicto: aprobable tal cual, nada bloqueante. Verificaciones ejecutadas con pnpm 11.18.0 (el
Puntos que validé específicamente:
No se hicieron cambios de código ni se modificó el estado del PR. |
Dos imprecisiones que senalo la review, ninguna funcional: el lockfile no cambia. uuid: el comentario daba una sola ruta (@gemini-wallet/core > @metamask/utils) cuando el selector alcanza cinco dependientes: @metamask/sdk y @metamask/sdk-communication-layer, que declaran ^8.3.2, y @metamask/utils 8.5.0 / 9.3.0 / 11.11.0, que declaran ^9.0.1 y entran por tres caminos distintos (@metamask/rpc-errors > @gemini-wallet/core, @coinbase/wallet-sdk y @metamask/providers). Comprobado con `pnpm why uuid`. Importa para saber que mirar si algo se rompe. decode-uri-component: se anade el aviso, GHSA-vcc3-ghjq-m6fr (CVE-2026-45822), con la advertencia de que pnpm audit dice "patched >=0.4.3" y esa version no existe; el GHSA lista 0.5.0 como parche real. Y se aclara que lo instalado es 0.2.2, que es lo que resuelve el ^0.2.2 de query-string: el 0.4.1 que citaba el texto es solo la ultima vulnerable publicada, no lo que hay en el arbol. 64/64 tests y pnpm audit en la moderada documentada. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Revisado — PR mergeable tal cual: #8 (review) Veredicto: aprobable, nada bloqueante.
Menores no bloqueantes (follow-up): documentar fail-closed con mirrors sin |
…closed Los dos menores que quedaban de la review. El de uuid ya iba en 8c7d200. brace-expansion: el selector solo cubria >=4.0.0 <5.0.9, que es la unica rama presente hoy (5.0.12, via eslint > minimatch@10, dev). Se anaden las ramas 1.x y 2.x, que no estan en el arbol: si mañana entra un glob o un minimatch@3 declarando ^1.1.x o ^2.x, el selector de la 5.x no lo alcanzaria y volveria a colarse una version vulnerable. Cada rango sube dentro de su propio major, asi que no puede colar un cambio de API. La 3.x no lleva entrada porque no aparece en ningun rango vulnerable de los avisos actuales. Como era de esperar, ninguna resolucion cambia: el lockfile solo registra las dos lineas nuevas de overrides. minimumReleaseAgeIgnoreMissingTime: se documenta la contrapartida que senalaba la review. Al ser fail-closed, un registry que sirva metadata recortado sin "time" rompe el install en vez de dejarlo pasar sin comprobar. Con registry.npmjs.org no ocurre porque pnpm se baja el documento completo cuando el abreviado no lo trae, pero si algun dia se instala desde un mirror o un registro privado conviene saber que la solucion es arreglar el mirror, no poner la opcion en `true`. 64/64 tests, typecheck limpio y audit en la moderada documentada. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Gracias por la verificación completa — y por reproducir el fallo de Los tres follow-ups menores están aplicados, en
También añadido en La única sugerencia que no he aplicado es el issue de seguimiento para cuando WalletConnect suba a Estado: 64/64 tests, typecheck limpio, 🤖 Addressed by Claude Code |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Changes IntroducedEste PR endurece la cadena de suministro: alertas de
Verificación de esta revisión
Files Reviewed (3 files)
Reviewed by balanced · Input: 0 · Output: 0 · Cached: 0 |
Vulnerabilidades: 20 → 1
hono<4.12.34→^4.12.34memo(), DoS en el middleware de idioma,parseBody()sin límite de anidamiento… (rainbowkit > wagmi > porto)undici>=7.0.0 <7.29.0→^7.29.0jsdomreact-router-dom^7.18.1→^7.18.3nanoid<3.3.18→^3.3.18brace-expansion4.x/5.x→^5.0.9postcss<8.5.23→^8.5.23uuid<11.1.1→^11.1.1vitest4.1.10→^4.1.11@vitest/mockerTodos los overrides se quedan dentro del major del propio parche, para no colar un cambio de API por la puerta de atrás.
La que no se puede arreglar
decode-uri-component(moderate, DoS por decodificación exponencial).El aviso pide
>=0.4.3— esa versión no existe; el salto real sería de0.4.1a0.5.0, la única publicada con parche. Y0.5.0es ESM pura ("type": "module"), mientras quequery-string@7.1.3— quien la usa, vía@walletconnect/utils— la carga conrequire():Con 0.5.0 recibe el namespace del módulo en vez de la función:
Es decir: rompe el parseo de cualquier query string en el flujo de WalletConnect.
Warning
Lo apliqué, y el build de vite y los 64 tests pasaron igual. El fallo solo aparece ejecutando
query-string. Revertido y documentado enpnpm-workspace.yamlpara que nadie lo "arregle" sin darse cuenta.Se revisará cuando WalletConnect suba a
query-string8+, que ya usa la API nueva.Verificación de los saltos grandes
Después de lo anterior, comprobé a mano los overrides que cruzan varios majors en vez de fiarme de que la suite pasara:
uuid8.3.2 → 11.1.1 desde@metamask/utilsv4()query-stringtras revertirdecode-uri-component%E2%82%AC→€)Cuarentena
Se añade
minimumReleaseAgeIgnoreMissingTime: false. El fichero ya explicaba muy bien que pnpm <11.18 aborta conERR_PNPM_MISSING_TIMEpor pedir el metadata abreviado; esto cubre el otro lado del mismo problema: que pnpm se salte la comprobación con un simple WARN cuando el campotimeno está, en vez de fallar.Verificación
pnpm test→ 64/64 ✅pnpm typecheck→ limpiopnpm build→ OKNota
pnpm lintfalla con 25 problemas (21 errores), pero ya fallaba enmaincon exactamente los mismos: son reglas dereact-hookssobre código existente (set-state-in-effecty similares), ajenas a este cambio. Lo comprobé con stash para descartar que fuera cosa de las actualizaciones. Si queréis, lo abordo en un PR aparte.🤖 Generated with Claude Code