Revisión Adversarial de Código
NutriFlow SaaS — GET /api/v1/patients/:id/records
🚫 BLOCK
Archivo revisado src/routes/patients.js
Tipo de cambio Nuevo endpoint REST
Líneas cambiadas +22 / −0
Método 3 personas adversariales
Fecha 2026-06-12
Revisado por CULTIVA IA / Code Review
🔴
3
CRÍTICOS — bloquean merge
🟡
2
ADVERTENCIAS
🔵
2
NOTAS
⬆️
2
PROMOVIDOS (2+ personas)
📄 Código bajo revisión — src/routes/patients.js
1// ADDED: endpoint para obtener registros de un paciente
2router.get('/patients/:id/records', async (req, res) => {
3 const { id } = req.params;
4 const { limit, from, to, format } = req.query;
5 
6 // Construir query con filtros opcionales
7 let query = `SELECT * FROM nutrition_records WHERE patient_id = ${id}`;
8 
9 if (from) query += ` AND recorded_at >= '${from}'`;
10 if (to) query += ` AND recorded_at <= '${to}'`;
11 
12 query += ` ORDER BY recorded_at DESC LIMIT ${limit || 100}`;
13 
14 const db = await getDbConnection();
15 const records = await db.query(query);
16 
17 if (format === 'csv') {
18 const csv = records.rows.map(r => Object.values(r).join(',')).join('\n');
19 console.log(`[DEBUG] CSV export for patient ${id}: ${csv}`);
20 res.set('Content-Type', 'text/csv');
21 return res.send(csv);
22 }
23 
24 res.json({ ok: true, data: records.rows });
25});
Hallazgo crítico Advertencia Nota
🔴 Hallazgos Críticos — BLOQUEAN MERGE 3 críticos
C-03 Datos de salud filtrados en logs de producción
CRÍTICO
🔒 Auditor de Seguridad
La línea console.log(`[DEBUG] CSV export for patient ${id}: ${csv}`) vuelca el CSV completo (todos los registros nutricionales del paciente) en los logs de aplicación. En cualquier entorno con log aggregation (Datadog, CloudWatch, Loggly), estos datos de salud quedan almacenados sin cifrar, accesibles a cualquier persona con acceso a logs.
💥 Impacto real: Datos de categoría especial (RGPD art. 9) expuestos en sistemas de terceros. Viola el principio de minimización de datos. Exposición indefinida incluso después de parchear el endpoint.
Fix: Eliminar el console.log completamente. Si se necesita auditoría de exportaciones, registrar solo metadatos: logger.info({ event:'csv_export', patientId: id, requestedBy: req.user.id, recordCount: records.rows.length })
🟡 Advertencias — Deben corregirse 2 warnings
W-01 Sin manejo de errores — crashea en producción
WARNING
💣 Saboteador
La función async no tiene try/catch. Si getDbConnection() falla (pool agotado, timeout, red caída) o db.query() lanza una excepción, Express devuelve un stack trace completo al cliente que incluye nombres de tablas, estructura de la base de datos y versión del servidor.
Fix: Envolver en try/catch y responder con res.status(500).json({error:'Internal server error'}). Logear el error interno sin exponerlo al cliente.
W-02 Sin cabecera Content-Disposition en export CSV
WARNING
👶 Nuevo Empleado
El export CSV no añade cabecera Content-Disposition: attachment; filename="patient-{id}-records.csv". El navegador renderizará el CSV en pantalla en lugar de descargarlo. Además, el CSV no tiene fila de cabeceras con nombres de columnas, lo que hace el archivo inutilizable sin contexto.
Fix: Añadir res.set('Content-Disposition', `attachment; filename="patient-${id}-records.csv"`) y añadir cabecera con Object.keys(records.rows[0]).join(',') como primera línea.
🔵 Notas — A discreción del autor 2 notas
N-01 Sin paginación real — solo LIMIT sin cursor/offset
NOTA
👶 Nuevo Empleado
Con LIMIT 100 no hay forma de obtener registros más allá de los 100 más recientes. Un paciente con historial de 2 años tiene ~730 registros y el nutricionista no puede acceder a los antiguos. Sin offset/cursor y sin metadatos de paginación en la respuesta.
N-02 getDbConnection() — posible leak de conexión
NOTA
💣 Saboteador
No se llama a db.release() ni db.end() después del query. Dependiendo de cómo implemente el pool getDbConnection(), esto puede agotar el pool de conexiones bajo carga. Sin el try/catch (W-01), si el query falla, la conexión nunca se devuelve al pool.
🎭 Perspectivas por persona
💣 El Saboteador
"Estoy intentando romper esto en producción"
[C-01] Payload SQL injection básico: GET /patients/1/records?from=2020-01-01' UNION SELECT * FROM users-- ejecuta una query arbitraria. No hay ninguna validación en el camino.
[C-02] Enumerar pacientes: el atacante hace un bucle for id in 1..10000 y cosecha el historial médico completo de todos los pacientes de la plataforma. Sin rate limiting, sin auth check por recurso.
[W-01] Si la base de datos cae durante un export CSV masivo, Express emite un UnhandledPromiseRejection con el stack trace completo. El atacante aprende la estructura interna del sistema.
[N-02] limit=99999999 en la URL fuerza una query que devuelve millones de registros, satura memoria del servidor y provoca OOM. No hay cota máxima de limit.
👶 El Nuevo Empleado
"Acabo de unirme al equipo y necesito entender esto"
[C-01] Veo interpolación de string en una query SQL pero no entiendo si hay algún middleware de sanitización upstream. Tuve que abrir 3 archivos más para confirmar que no lo hay. Esto no debería requerir arqueología.
[W-02] El endpoint devuelve text/csv pero sin cabecera de columnas. Cuando lo probé en Postman no sabía qué significaba cada valor. ¿Es id, patient_id, recorded_at, calories, protein...? No hay forma de saberlo sin mirar el schema de la tabla.
[N-01] No hay comentario ni JSDoc que explique qué devuelve exactamente records.rows. ¿Qué campos tiene cada objeto? ¿Hay campos calculados? ¿Por qué SELECT * y no campos explícitos?
🔒 El Auditor de Seguridad
"Este código va a ser atacado. Busco la vulnerabilidad antes que el atacante."
[C-01, OWASP A03] SQL Injection clásica. Ningún parámetro está parametrizado. Riesgo: exfiltración completa de la tabla nutrition_records y posiblemente otras tablas vía UNION attacks.
[C-02, OWASP A01] Broken Access Control / IDOR. No hay verificación de que el nutricionista autenticado tenga relación con el paciente :id. El endpoint asume que si el usuario está autenticado, puede ver cualquier paciente.
[C-03, OWASP A09] Logging inseguro de datos sensibles. Los registros de salud del paciente se vuelcan en logs. Violación directa del RGPD art. 9 (datos de salud = categoría especial). La retención en sistemas de log puede durar meses.
📋 Resumen ejecutivo

Este endpoint tiene tres vulnerabilidades críticas independientes, cada una suficiente por sí sola para bloquear el merge: inyección SQL sin parametrizar, ausencia total de control de acceso (IDOR) y volcado de datos médicos en logs. El PR fue desarrollado con buenas intenciones pero sin conocimiento de seguridad básica en APIs web.

El problema más urgente es la SQL injection: una vez desplegado, cualquier usuario autenticado puede exfiltrar la base de datos completa en minutos. El código "funciona en local con paciente ID 1" porque el happy path nunca activa las vulnerabilidades.

No mergear bajo ninguna circunstancia hasta resolver C-01, C-02 y C-03. El deadline de mañana no justifica exponer datos de salud de pacientes a una brecha trivialmente explotable.

🔀 Guía de merge
🚫 NO mergear hasta que
  • SQL injection corregida con queries parametrizadas (C-01)
  • Verificación de acceso por clinicId implementada (C-02)
  • console.log con datos de salud eliminado (C-03)
  • try/catch añadido al handler async (W-01)
✅ Safe to merge después de
  • Test unitario de SQL injection rechazada
  • Test de acceso cruzado entre pacientes de distintas clínicas
  • Revisor humano confirma el fix (no self-merge)
  • Escaneo de logs en staging sin datos PII/salud