DEV Community

Cover image for React Doctor marcó 1,249 problemas en una SPA de React. Cinco valían la pena arreglar
AWS Community Builders profile image Franchesco Romero
Franchesco Romero for AWS Community Builders

Posted on

React Doctor marcó 1,249 problemas en una SPA de React. Cinco valían la pena arreglar

#javascript #react #security #tooling

React Doctor es un linter de cero instalación que escanea un codebase de React buscando problemas de correctitud, seguridad, accesibilidad,
rendimiento y mantenibilidad, luego los rankea y te entrega una lista de arreglos con forma de agente.

Este post es un reporte de campo: lo corrí sobre una SPA real de React 18 + Vite + TypeScript (~50 rutas, TanStack Query, react-hook-form) y separé lo que de verdad me atrapó.

El resultado honesto: 1,249 hallazgos, cinco que valía la pena arreglar, incluyendo dos bugs de seguridad reales. Los otros 1,244 fueron una mezcla de ruido, decisiones de criterio, y falsos positivos.

TL;DR

Categoría Reportados Vale la pena actuar Por qué la brecha
Seguridad 18 warnings 2 13 eran sinks saneados en el servidor (falsos positivos); 1 mina de código muerto + 1 XSS real fueron el oro
Bugs 24 errores, 487 warnings 2 1 fuga de timer, 1 key/spread; ~27 exhaustive-deps piden criterio humano
Accesibilidad 434 warnings, 1 error 1 el 1 error (aria-selected faltante) era real; los 434 son ruido de <Label>/<button type>
Rendimiento 110 warnings 0 nada caliente; candidatos pero sin impacto medido
Mantenibilidad 176 warnings 0 opiniones de estilo tipo "componente grande"
Total 1,249 5 señal-a-ruido ≈ 1 en 250

El puntaje que imprimió: 39 / 100. Dos cosas que ese encuadre esconde: los cinco que sacó a la superficie eran de alto valor (una fuga de token en localStorage y un sink de XSS almacenado), y el conteo no es determinista: una segunda corrida del mismo commit reportó 1,254, no 1,249.

El veredicto en una línea: excelente generador de hipótesis, pésima
compuerta. Úsalo para encontrar candidatos, verifica cada uno contra el código, y nunca conectes el conteo al CI.

Qué es React Doctor

Un solo binario que corres por npx / pnpm dlx, sin agregar dependencia a tu proyecto, sin archivo de configuración obligatorio. Parsea tu src/, hace match contra un conjunto de reglas (sus familias: Seguridad, Bugs, Accesibilidad, Rendimiento, Mantenibilidad), e imprime:

  • un Top 3 de lo que cree que deberías arreglar primero,
  • un desglose por categoría con conteos de error/warning,
  • una advertencia de escala de migración cuando una regla abarca docenas de archivos,
  • un bloque de guía para agente: la herramienta está diseñada de manera explícita para ser manejada por un agente LLM, no solo leída por un humano.

Esa última parte es el punto. React Doctor no arregla nada. Emite
hipótesis rankeadas y le dice al agente que "lea el código relevante antes de confirmar o suprimir cada hallazgo". Tomada al pie de la letra, esa instrucción es el flujo de trabajo completo.

Cómo correrlo

Sin instalación. Desde la raíz del paquete del frontend:

# Top-3 + resumen por categoría
npx react-doctor

# Cada hallazgo con archivo:línea y la receta de arreglo
npx react-doctor@latest --verbose

# Acotar a un directorio (recomendado para un repo grande)
npx react-doctor@latest src/components

# Solo los archivos que cambiaron vs la rama base (el modo amigable con CI)
npx react-doctor@latest --verbose --scope changed
Enter fullscreen mode Exit fullscreen mode

Si tu toolchain usa pnpm, pnpm dlx react-doctor es el equivalente. La primera corrida baja ~200 paquetes al caché de dlx; las corridas
siguientes son rápidas.

El default (sin flags) es la vista de triaje:

React Doctor v0.7.1

  Top 3 errors you should fix

  ✖ Bugs: Effect subscription or timer never cleaned up
    `setTimeout` creates a timer in useEffect without returning
    cleanup. Return a cleanup function so it does not leak
    after unmount.

    src/components/layout/SearchBar.tsx:114

  ...

  All 1249 issues

  Security › 18 warnings
  Bugs › 24 errors, 487 warnings
  Performance › 110 warnings
  Accessibility › 434 warnings
  Maintainability › 176 warnings
Enter fullscreen mode Exit fullscreen mode

--verbose expande cada regla en cada archivo:línea más una URL de docs con una "receta de chequeo de falso positivo". Lee esa frase como una etiqueta de advertencia: la herramienta sabe que sus hallazgos necesitan triaje.

Los cinco que importaron

1. Un setTimeout en useEffect sin cleanup (bug real)

Arriba de la lista, y correcto. Un timer de focus creado al abrir, nunca limpiado:

// antes: el timer se fuga si el componente se desmonta dentro de los 50ms
useEffect(() => {
  if (open) {
    setRecentSearches(getRecentSearches());
    setTimeout(() => inputRef.current?.focus(), 50);
  } else {
    setQuery("");
  }
}, [open]);
Enter fullscreen mode Exit fullscreen mode 
// después: el cleanup limpia el timer pendiente
useEffect(() => {
  if (!open) {
    setQuery("");
    return;
  }
  setRecentSearches(getRecentSearches());
  const focusTimer = setTimeout(() => inputRef.current?.focus(), 50);
  return () => clearTimeout(focusTimer);
}, [open]);
Enter fullscreen mode Exit fullscreen mode

Radio de impacto bajo (una llamada de focus de 50ms), pero una fuga real y un arreglo trivial. Esta es justo la clase en la que React Doctor es bueno: mecánica, local, verificable.

2. JSON.stringify hacia un sink de <script>: XSS almacenado (seguridad real)

Este solito justificó todo el ejercicio. El componente de SEO incrustaba JSON-LD de schema.org así:

{jsonLdPayloads.map((payload, i) => (
  <script key={i} type="application/ld+json">
    {JSON.stringify(payload)}
  </script>
))}
Enter fullscreen mode Exit fullscreen mode

JSON.stringify no escapa HTML. Los payloads cargan campos
controlados por el usuario: un nombre de perfil para mostrar, el título de un post de blog. Un nombre de </script><img src=x onerror=alert(1)> se sale del elemento script. Y como estas etiquetas aterrizan en una captura de SEO pre-renderizada que se sirve a rastreadores y en visitas directas, es XSS almacenado, no reflejado.

El arreglo es un serializador que escapa los caracteres que pueden
terminar el elemento o el contexto de string de JS, cada reemplazo una secuencia \uXXXX válida de JSON para que el dato viaje de ida y vuelta sin cambiar:

export function serializeJsonLd(payload: Record<string, unknown>): string {
  return JSON.stringify(payload)
    .replace(/</g, '\\u003c')
    .replace(/>/g, '\\u003e')
    .replace(/&/g, '\\u0026')
    .replace(/\u2028/g, '\\u2028')
    .replace(/\u2029/g, '\\u2029');
}
Enter fullscreen mode Exit fullscreen mode 
<script key={i} type="application/ld+json">
  {serializeJsonLd(payload)}
</script>
Enter fullscreen mode Exit fullscreen mode

La etiqueta que la propia React Doctor le puso a esta regla, "Unescaped JSON in HTML or script sink", fue precisa. La herramienta se ganó su lugar con este único hallazgo.

3. Un cliente axios muerto que acumulaba tokens en localStorage (seguridad real)

Regla: "Auth token in web storage". Apuntó a src/services/api/client.js:50:

// src/services/api/client.js (desde el primerísimo commit)
const refreshToken = localStorage.getItem('refresh_token')
const { data } = await apiClient.post('/auth/refresh', { refresh_token: refreshToken })
localStorage.setItem('access_token', data.access_token)
Enter fullscreen mode Exit fullscreen mode

Guardar un access token, ya no digamos un refresh token, en localStorage lo expone a cualquier XSS en la página. Esto contradecía de frente el modelo de auth real de la app: access token solo en memoria, refresh vía una cookie HttpOnly, implementado en un archivo distinto (src/lib/axios.ts).

El detalle: este archivo nunca se importaba en ningún lado. Código muerto del scaffold inicial. No una fuga activa, pero un arma cargada. Quien autocompletara el import equivocado de client reintroduciría en silencio la vulnerabilidad exacta que el codebase se construyó para evitar.

Arreglo: bórralo.

grep -rIn "services/api/client" src   # → cero imports. Seguro de quitar.
git rm src/services/api/client.js
Enter fullscreen mode Exit fullscreen mode

React Doctor encontró esto por patrón, no por alcanzabilidad. Que no pueda distinguir "sink vivo" de "mina muerta" es una limitación, pero aquí la mina muerta valía la pena quitarla igual.

4. role="option" sin aria-selected (a11y real, el único error de a11y)

De 435 hallazgos de accesibilidad, exactamente uno fue error, y estaba bien. Una opción de listbox sin estado de selección expuesto a la tecnología asistiva:

<button
  role="option"
  aria-selected={false}   // ← agregado
  onClick={() => handleSelect(u)}
>
Enter fullscreen mode Exit fullscreen mode

El padre ya cargaba role="listbox"; la opción nada más nunca declaraba su estado. Un atributo.

5. key antes de {...spread} (menor, en el límite) 

// marcado
<TourCard key={card.step} {...card} onCtaClick={...} />
// cambiado a
<TourCard {...card} key={card.step} onCtaClick={...} />
Enter fullscreen mode Exit fullscreen mode

La regla: un spread puede sobrescribir key. En la práctica React extrae key de manera especial y avisa ante un key en el spread, así que el original difícilmente se iba a portar mal. Reordenar es inofensivo y satisface al linter, pero este es el más débil de los cinco: del tipo de hallazgo que arreglas de pasada, no uno por el que abrirías un PR por su cuenta.

Lo que marcó y era ruido

13 sinks de "HTML injection": todos falsos positivos

El grupo de Seguridad más grande, "HTML injection sink with dynamic
content ×13", listó cada dangerouslySetInnerHTML de la app:

  ⚠ Security: HTML injection sink with dynamic content ×13
    HTML is injected from a dynamic-looking source, which can
    become XSS if the value is user-controlled or unsanitized.

    src/components/dm/MessageBody.tsx:25
    src/pages/BlogPostPage.tsx:397
    src/pages/ThreadPage.tsx:523
    ... (10 more)
Enter fullscreen mode Exit fullscreen mode

Cada uno de estos renderiza HTML que ya venía saneado del lado del
servidor (una pasada de nh3/ammonia en el backend), la frontera de confianza documentada de la app para contenido de blog, foro, y escrito por agentes. La herramienta ve el sink; no puede ver que el string llegó pre-saneado desde una API. Verificar el grupo me tomó más que arreglar los dos bugs reales. Los 13: descartados.

Esta es la debilidad de fondo de la herramienta. Un linter estático lee el sink, no el flujo de datos a través de la frontera de red, así que cada dangerouslySetInnerHTML legítimo es un warning que tienes que descartar a mano.

Ruido a escala de migración: type ×242, <Label> ×329

La propia React Doctor los marca como "muestrea antes de barrer":

  ⚠ Migration-scale change: sample before you sweep
    Button missing explicit type ×242 across 83 files
    Label missing associated control ×329 across 69 files
    Large component is hard to read and change ×62 across 60 files
Enter fullscreen mode Exit fullscreen mode

<button> toma por default type="submit" dentro de un form, así que un type explícito faltante puede morder, pero 242 instancias son un codemod mecánico y un diff de 60 archivos que nadie va a revisar con cuidado. Real, de bajo valor, y peligroso de arreglar en masa de una sola pasada. Buena decisión de la herramienta ponerlos en cuarentena; buena decisión tuya no tocarlos en un PR de arreglo de bugs.

exhaustive-deps ×27: necesita un humano cada vez

Aquí la herramienta es inusualmente honesta, imprimiendo anti-guía dentro del hallazgo:

  ⚠ Bugs: Missing effect dependencies ×27
    → Don't blindly add missing dependencies. Read the hook
      callback first.
Enter fullscreen mode Exit fullscreen mode

Una dependencia faltante a veces es un bug y a veces es intencional. No hay arreglo mecánico; cada uno de los 27 es una tarea de lectura de código. Útil como lista para irla trabajando, inútil como "arréglalo todo".

El conteo no es determinista

Dos corridas, el mismo commit, sin ediciones entre ellas:

run 1:  All 1254 issues   Bugs › 26 errors, 489 warnings
run 2:  All 1249 issues   Bugs › 24 errors, 487 warnings
Enter fullscreen mode Exit fullscreen mode

Cinco hallazgos aparecieron y desaparecieron entre corridas idénticas. Eso solo descalifica al número crudo como compuerta de CI: un build que falla ante "issues > N" andaría parpadeando.

Qué sirve, qué no

Sirve No sirve
Encontrar bugs locales y mecánicos (fugas de timer, cleanup faltante) Distinguir un sink vivo de código muerto
Nombrar un sink de XSS real con precisión (el acierto del JSON-LD) Ver el saneado del lado del servidor a través de la frontera de red
Cero instalación, cero configuración, un comando Producir un número estable y compuertable (conteo no determinista)
Rankear un Top 3 que casi siempre vale la pena leer Su propio puntaje (39/100) como métrica con significado
Emitir guía lista para agente + enlaces de docs por regla Cualquier cosa que requiera flujo de datos o intención (modelo de auth, frontera de confianza)
Acotar a una ruta o --scope changed Reglas a escala de migración: reales pero irrevisables en bloque

Cómo usarla de verdad

  1. Acótala. La salida de todo el repo son 1,200+ líneas. Corre --verbose --scope changed en un PR, o apúntala a un solo directorio.
  2. Dásela a un agente, no a un humano. La salida está hecha para eso: rankeada, con archivo:línea y una receta de arreglo por regla. Haz que el agente lea cada archivo marcado y etiquete el hallazgo como verdadero positivo / falso positivo / requiere revisión, con evidencia del archivo.
  3. Verifica cada hallazgo de seguridad a mano. Dos de los míos eran reales, trece no, y la proporción era invisible desde el resumen.
  4. Arregla los de alta confianza que preservan el comportamiento; anota el resto. La fuga de timer y el XSS salieron el mismo día. exhaustive-deps y las reglas de escala de migración se volvieron notas de backlog, no diff.
  5. Nunca condiciones el CI al conteo crudo. Parpadea. Condiciona sobre tu subconjunto triado y confirmado si de plano tienes que condicionar algo.
  6. Divide por familia de regla en PRs separados. Los arreglos de seguridad y un barrido de a11y tienen revisores distintos y riesgo distinto. No los amontones.

Lecciones

  1. El costo del triaje es el costo real. Correr la herramienta tomó segundos; descartar 13 banderas de XSS falso positivo tomó más que escribir los dos arreglos reales y sus pruebas. Presupuesta para la verificación, no para el escaneo.
  2. Un linter estático no puede ver tu frontera de confianza. Cada dangerouslySetInnerHTML legítimo alimentado por HTML saneado en el servidor es un falso positivo permanente. Eso no es un bug de la herramienta; es el techo del análisis estático.
  3. El mejor hallazgo estaba en código muerto. El cliente de token en localStorage era inalcanzable, así que no había señal de runtime, ni falla de prueba, ni comentario de revisión que lo hubiera atrapado. El match por patrón encontró lo que un análisis de alcanzabilidad habría despriorizado.
  4. Los nombres de regla precisos construyen confianza; un puntaje resumen la erosiona. "Unescaped JSON in HTML or script sink" era accionable. "39/100" no me dijo nada.
  5. El no-determinismo mata el condicionamiento. Un conteo que cambia entre corridas idénticas puede informar a un humano pero nunca puede tronar un build.
  6. Deja que la herramienta ponga en cuarentena su propio ruido. Su advertencia de "muestrea antes de barrer" fue lo más útil que imprimió: detuvo un PR de ruido de 60 archivos antes de que empezara.

Top comments (0)