Saltar al contenido
Development

La suite de pruebas en verde que no probaba lo que yo pensaba que probaba

Por Victor Da Luz
dependenciessecuritytestingdev-logsite

Apareció una alerta de Dependabot de severidad alta: brace-expansion, una utilidad minúscula de globbing de strings, tres niveles adentro del árbol de dependencias, tenía un bug de denegación de servicio. El movimiento obvio es forzar la versión parcheada y seguir adelante. Ese movimiento obvio hubiera estado mal.

El contexto

brace-expansion tiene tres consumidores bloqueados en este repo, todos fijados a un minimatch@3.1.5 más viejo que pide brace-expansion@^1.1.7: eslint-plugin-jsx-a11y y pa11y-ci directamente, y eslint-plugin-astro indirectamente a través de una dependencia peer del primero de esos dos, en vez de un enlace de dependencia normal. Por eso un npm ls simple no lo muestra como consumidor, pero el propio resolver de Dependabot, que sí cuenta las dependencias peer, lo muestra.

eslint-plugin-astro está fijado a 2.x en vez de 3.x por una razón sin relación, encontrada el día anterior: el nuevo parser de 3.x descarta atributos de accesibilidad antes de que el linter los vea. Un pin desactualizado, dos problemas separados.

Mientras tanto, eslint mismo trae un minimatch moderno que ya pide brace-expansion@^5.0.5. La versión parcheada que arregla el DoS es 5.0.8. La cadena de eslint llega ahí gratis con solo refrescar el lockfile. Las otras tres no, porque 5.0.8 no satisface ^1.1.7. Por eso justamente el PR automático de Dependabot seguía fallando: no hay ningún cambio de manifiesto que parchee a todas sin forzar un conflicto de versiones.

El instinto que hubiera enviado un bug a producción

El arreglo fácil es una entrada overrides de npm que fuerce brace-expansion a ^5.0.8 en todas partes, sin importar el conflicto. Lo probé. npm install funcionó. Todo el gate local, type-check, lint, build, y la suite de accesibilidad de 39 páginas, pasó limpio.

Ese resultado se sintió como confirmación. No lo era.

Lo que el gate en verde en realidad probaba

Revisé qué había cambiado entre las dos versiones mayores, y la forma de exportación del módulo es distinta. La versión 1.x hace module.exports = expand, una función directamente invocable. La versión 5.x hace exports.expand = expand, una propiedad con nombre dentro de un objeto. El minimatch viejo fijado en el proyecto la llama a la manera antigua.

Al forzar el override, esa llamada se convierte en {expand: [Function]}(pattern), que lanza una excepción. Lo confirmé de forma directa en vez de razonarlo:

const mm = require('./node_modules/eslint-plugin-jsx-a11y/node_modules/minimatch')
mm.braceExpand('a{b,c}d')
// TypeError: expand is not a function

Entonces, ¿por qué la suite pasaba con el override activo? Porque ninguno de los objetivos de lint ni las URLs de las pruebas de accesibilidad de este repo pasan hoy por esa llamada interna de una forma que la dispare. El gate no estaba probando que el arreglo fuera seguro. Estaba probando que la ruta de código que dispara el crash no se ejecuta actualmente, una afirmación completamente distinta, y que se invierte en el momento en que una futura configuración o patrón de ignore contenga un grupo con llaves.

Lo que se publicó al final

Dos decisiones separadas en vez de una. Actualicé brace-expansion para la cadena propia de eslint: riesgo cero, satisface su rango declarado. Dejé las otras tres cadenas como estaban y en su lugar descarté la alerta, con el razonamiento adjunto al descarte: es una devDependency, nunca se empaqueta dentro del Worker desplegado, y el exploit necesita strings controlados por un atacante llegando a expand() en tiempo de ejecución. Estas cadenas solo procesan patrones de glob estáticos, escritos por quien desarrolla, en tiempo de build y de CI. No existe ninguna ruta por la que la entrada de un atacante llegue al código vulnerable en absoluto.

Lección

Una suite de pruebas que pasa responde “esto rompe algo que la suite ya verifica”. No responde “este cambio es seguro”, sobre todo para un cambio que altera el contrato de llamada de un módulo en vez de su comportamiento. Cuando esas dos preguntas tienen respuestas distintas, conviene confiar en la que se puede razonar de forma directa, no en la que resultó pasar.

Lecturas relacionadas