Code Review Report — NutriTrack API PR #247

Critical

High

Medium

Low

Total

⛔ Critical Findings

CR-001 JWT secret hardcodeado en código fuente Critical Security

Ubicación src/api/auth/jwt_handler.py:14

Descripción La clave secreta JWT SECRET_KEY = "nutritrack_secret_2026" está hardcodeada directamente en el módulo. Cualquier persona con acceso al repositorio puede firmar tokens arbitrarios y suplantar a cualquier usuario.

Impacto Compromiso total de autenticación. Atacante puede acceder a datos de cualquier paciente con un token forjado. Violación directa del Art. 32 GDPR (medidas técnicas apropiadas).

Corrección recomendada

Mover a variable de entorno: SECRET_KEY = os.environ["JWT_SECRET_KEY"]. Añadir validación al arrancar que falle si no está definida. Rotar el secreto inmediatamente en producción. Revisar si el secreto fue expuesto en algún commit anterior con git log -S "nutritrack_secret".

CR-002 Inyección SQL en query de exportación de historial Critical Security Performance ⟳ Merged (2 revisores)

Ubicación src/db/queries/patient_queries.py:67

Descripción La query construye el filtro de fechas por concatenación de strings: f"WHERE patient_id={patient_id} AND date BETWEEN '{start}' AND '{end}'". Los parámetros start y end provienen del query string de la request sin sanitizar.

Impacto Exfiltración de toda la base de datos de pacientes mediante SQL injection clásico. Detectado independientemente por revisor Security y revisor Performance (que identificó el riesgo de full-table scan como vector de DoS). Severidad escalada a Critical.

Corrección recomendada

Usar parámetros preparados: cursor.execute("WHERE patient_id = %s AND date BETWEEN %s AND %s", (patient_id, start, end)). Validar el formato de fechas con datetime.fromisoformat() antes de pasarlas a la query. Añadir test de regresión de inyección.

🔴 High Findings

HI-001 Endpoint de exportación sin verificación de autorización por clínica High Security

Ubicación src/api/patients/export.py:31

Descripción El endpoint GET /patients/{id}/export valida que el token JWT sea válido pero no comprueba que el paciente id pertenezca a la clínica del usuario autenticado. Un dietista de la clínica A puede exportar datos de pacientes de la clínica B.

Corrección recomendada

Añadir check: assert patient.clinic_id == current_user.clinic_id antes de procesar la exportación. Incluir test de autorización cruzada entre clínicas.

HI-002 Exportación de dataset completo cargado en memoria (OOM risk) High Performance

Ubicación src/api/patients/export.py:78–102

Descripción El endpoint carga todos los registros del paciente en una lista Python antes de serializar. Pacientes con historial largo (>2 años de mediciones diarias ≈ 730 rows) en exportaciones paralelas pueden agotar la RAM del servidor.

Corrección recomendada

Usar streaming con StreamingResponse de FastAPI + generador que lea en chunks de 100 rows. Para PDF, generar mediante ReportLab con paginación, no cargar todo en buffer.

HI-003 Refresh token sin rotación — riesgo de token theft persistente High Security ⟳ Merged (Security + Architecture)

Ubicación src/api/auth/jwt_handler.py:88–110

Descripción Los refresh tokens son válidos indefinidamente y no se invalidan al usarse. Si un token es robado, el atacante mantiene acceso sin límite temporal. Architecture también señala que la lógica de refresh está embebida en el handler en lugar de en un servicio dedicado.

Corrección recomendada

Implementar refresh token rotation: al usar un refresh token, invalidarlo e emitir uno nuevo. Almacenar tokens en tabla refresh_tokens con invalidated_at. Extraer lógica a AuthService.

HI-004 Cobertura de tests insuficiente en rutas de error del export High Testing

Ubicación tests/test_export.py — cobertura: 34%

Descripción Solo existen tests del happy path (exportación exitosa). No hay tests para: paciente sin datos, rango de fechas inválido, timeout de BD, exportación de paciente de otra clínica, o formato inválido solicitado.

Corrección recomendada

Añadir al menos: test_export_empty_patient, test_export_invalid_date_range, test_export_cross_clinic_forbidden, test_export_db_timeout_returns_503. Objetivo mínimo: 80% cobertura en export.py.

🟡 Medium Findings

ME-001 N+1 queries en carga de métricas por paciente Medium Performance

Ubicación src/db/queries/patient_queries.py:112–130

Descripción Para cada registro nutricional se lanza una query separada para obtener la categoría del alimento. Con 100 registros = 101 queries a BD.

Corrección recomendada

JOIN en la query principal: SELECT nr.*, fc.name as category FROM nutrition_records nr JOIN food_categories fc ON nr.category_id = fc.id.

ME-002 PatientModel acumula responsabilidades GDPR, validación y persistencia Medium Architecture

Ubicación src/models/patient.py

Descripción El modelo mezcla lógica de anonimización GDPR, validación de campos y lógica de persistencia en la misma clase. Violación de SRP (Single Responsibility Principle).

Corrección recomendada

Separar en: Patient (solo datos), PatientGDPRService (anonimización/derecho al olvido), PatientRepository (persistencia).

ME-003 ExportModal sin aria-label ni manejo de focus trap Medium Accessibility

Ubicación src/components/ExportModal.tsx:8, 45

Descripción El modal carece de role="dialog", aria-modal="true" y aria-labelledby. El foco no queda atrapado dentro del modal al abrirlo, permitiendo navegar con Tab a elementos del fondo. Viola WCAG 2.1 AA (4.1.2, 2.1.2).

Corrección recomendada

Añadir role="dialog" aria-modal="true" aria-labelledby="export-modal-title". Implementar focus trap con @radix-ui/react-dialog o custom hook useFocusTrap.

ME-004 Logs de export incluyen datos de salud del paciente en claro Medium Security

Ubicación src/api/patients/export.py:55

Descripción logger.debug(f"Exporting {records} for patient {patient.full_name}, BMI={patient.bmi}") — datos personales y de salud en logs de aplicación. Incumple minimización de datos GDPR.

Corrección recomendada

Loguear solo IDs: logger.info("export_started patient_id=%s records=%d", patient.id, len(records)). Implementar política de retención de logs ≤30 días.

ME-005 Sin test de contrato para el esquema de exportación CSV Medium Testing

Ubicación tests/test_export.py

Descripción No existe ningún test que valide las columnas del CSV exportado ni su orden. Un cambio en Patient puede alterar el esquema de exportación silenciosamente y romper integraciones downstream de las clínicas.

Corrección recomendada

Añadir test_export_csv_schema que parsea el CSV resultante y verifica las columnas esperadas contra una lista fija definida como constante.

ME-006 Botones del modal sin texto visible para usuarios de lector de pantalla Medium Accessibility co-located con ME-003

Ubicación src/components/ExportModal.tsx:89, 97

Descripción Los botones "Cancelar" y "Exportar" usan solo iconos SVG sin aria-label. Lectores de pantalla anuncian el botón como vacío.

Corrección recomendada

Añadir aria-label="Cancelar exportación" y aria-label="Confirmar exportación", o incluir texto oculto con clase sr-only.

🟢 Low Findings

LO-001 Nombres de variables inconsistentes (snake_case vs camelCase en Python) Low Architecture

Descripción patientId, startDate en export.py mezclan camelCase con el resto del codebase que usa patient_id, start_date. Aplicar Ruff/Black con config consistente.

LO-002 Docstrings ausentes en funciones públicas del módulo export Low Architecture

Descripción Las 4 funciones públicas de export.py carecen de docstrings. Añadir Google-style docstrings con tipos de parámetros y retorno.

LO-003 Contraste de color insuficiente en texto del modal (ratio 3.8:1) Low Accessibility

Descripción El texto de descripción del modal usa #888 sobre fondo blanco. Ratio 3.8:1 — por debajo del mínimo WCAG AA de 4.5:1. Cambiar a #767676 o más oscuro.

LO-004 Constante JWT_EXPIRY duplicada en dos módulos Low Architecture

Descripción JWT_EXPIRY = 3600 definida en jwt_handler.py:8 y también en router.py:5. Centralizar en settings.py.

LO-005 Import no utilizado: from typing import Optional Low Architecture

Descripción src/models/patient.py:3 importa Optional sin usarlo. Eliminar. Configurar Ruff con F401 en CI para prevenir regresiones.

Resumen por Dimensión

Dimensión	Critical	High	Medium	Low	Total
Security	2	2	1	0	5
Performance	1*	1	1	0	3
Architecture	0	1*	1	4	6
Testing	0	1	1	0	2
Accessibility	0	0	2	1	3
Total	2	4	6	5	17

* Hallazgo contabilizado en dimensión primaria. Merged findings contribuyen a ambas dimensiones pero cuentan una sola vez en Total.

Recomendación General

Bloquear merge hasta resolver CR-001 y CR-002. El JWT hardcodeado y la inyección SQL en el endpoint de exportación representan riesgos críticos para datos de salud de pacientes (categoría especial GDPR). Requieren corrección y nueva revisión antes de cualquier despliegue.

Sprint de seguridad (HI-001, HI-003, ME-004). La ausencia de autorización multi-tenant y los logs con datos personales deben resolverse en el mismo sprint. Programar auditoría GDPR completa del módulo de exportación con DPO.

Deuda técnica en Testing (HI-004, ME-005). Elevar cobertura de export.py al 80% antes del siguiente release. Añadir tests de seguridad (autorización cruzada, SQL injection) al pipeline CI.

Performance e Arquitectura (HI-002, ME-001, ME-002). Planificar refactor de streaming y separación de responsabilidades en el siguiente milestone. La N+1 en consultas puede resolverse en paralelo como quick win.

Accesibilidad (ME-003, ME-006, LO-003). El ExportModal tiene múltiples issues WCAG AA. Recomendable reemplazar implementación custom por @radix-ui/react-dialog que provee accesibilidad out-of-the-box, resolviendo ME-003 y ME-006 simultáneamente.