# Revisión de código · 2026-08-02

Revisión integral del monorepo hecha sobre el commit `b1c8202` (import inicial
del repo `github.com/neracosu/gustitoexpress`). No es una auditoría de seguridad
en caliente contra la base viva como la del 30-jul; es una lectura del código
fuente, área por área, para encontrar errores de corrección antes de que el
proyecto siga creciendo.

## Cómo se hizo (para poder confiar en la lista)

1. **Cinco revisores en paralelo**, uno por área: tienda del cliente + `shared`,
   panel del restaurante, repartidor + Edge Functions, Estación Fiscal +
   cerebrito, y validador + worker + migraciones SQL.
2. Cada hallazgo pasó por un **verificador independiente** que releyó el código
   citado y lo puntuó de 0 a 100 según qué tan seguro estaba de que es un
   problema real que se topa en la práctica.
3. Se conservan como **confirmados** solo los que puntuaron 80 o más. Los que
   quedaron en 75 se listan aparte: son reales, pero el daño lo amortigua otra
   capa (casi siempre la base de datos, donde vive la regla de negocio) o el
   vector es acotado.

Total: 26 hallazgos candidatos → **7 confirmados** (todos con puntaje máximo) +
**12 de segundo nivel**.

## Resumen

| Área | Confirmados | Segundo nivel |
|---|---|---|
| Tienda del cliente | 1 | 3 |
| Panel del restaurante | 0 | 3 |
| Repartidor / Edge Functions | 2 | 3 |
| Estación Fiscal / cerebrito | 2 | 3 |
| Validador / migraciones | 2 | 0 |

**Lo importante:** ningún hallazgo es una fuga de datos ni un secreto expuesto
(eso se barrió por separado y salió limpio: no hay `.env` ni tokens en el
árbol). Son errores de corrección. Cuatro de los siete confirmados tocan dinero
o lo fiscal, que es donde más duele en este negocio.

> **Estado (2026-08-02): los 7 confirmados están corregidos.** Verificado con
> typecheck + build y la suite fiscal offline (incluida una prueba funcional del
> guard del respaldo). Registrado en el changelog como **v1.5.4**
> (`packages/shared/src/changelog.ts`). Los de segundo nivel siguen abiertos.

---

## Confirmados (puntaje 100) — corregidos el 2026-08-02

### 1 · "Agregar otro para personalizarlo aparte" cobra distinto de lo que muestra
**Dónde:** `apps/cliente/src/store/carrito.ts:69-79` (función `duplicar`).
**Categoría:** dinero.

`duplicar()` copia `producto_id`, `nombre`, `precio_usd` y pone `cantidad: 1`,
pero **no** copia `opcionesElegidas`, `opcionesSel` ni `opcionesTexto`. Como el
servidor recalcula el extra desde `opciones_sel` y no confía en el `precio_usd`
que manda el cliente, la línea duplicada de un producto con opciones se factura
sin el extra, mientras el checkout muestra el precio que sí lo incluye: el total
que ve el cliente no es el que se cobra. Si el grupo de opciones era
obligatorio, `crear_pedido` puede fallar y perderse el pedido. La cocina tampoco
ve el sabor/tamaño de esa línea.

### 2 · La función de importar menú deja entrar al mesero
**Dónde:** `supabase/functions/importar-menu-ia/index.ts:55`.
**Categoría:** seguridad (escalada de rol).

Valida contra `['dueno', 'staff', 'admin']`, incluyendo `staff` (el mesero, que
"solo opera su sala"). Las dos funciones hermanas —`cerebro-factura:45` y
`cerebro-receta:47`— usan `['dueno', 'encargado', 'admin']`, sin `staff` y con
`encargado`. El propio mensaje de error de la línea 56 dice *"Solo el dueno
puede importar el menu"*, lo que confirma que incluir a `staff` fue un desliz.
Un mesero autenticado puede sobrescribir o agregar productos al catálogo, fuera
de su rol.

### 3 · Crear repartidor/mesero responde "listo" aunque no se haya fijado la clave
**Dónde:** `supabase/functions/crear-repartidor/index.ts:76-80` y
`supabase/functions/gestionar-mesero/index.ts:152-156`.
**Categoría:** bug.

Tras invitar al usuario se hace `PUT .../admin/users/{id}` con la contraseña
elegida, **sin comprobar `.ok`**, y se responde `{ ok: true }`. Si ese PUT falla,
el dueño cree que el repartidor/mesero ya puede entrar con la clave que definió,
cuando en realidad solo podría hacerlo por el enlace de invitación por correo.
El contraste lo confirma: el flujo hermano `resetear_clave`
(`gestionar-mesero:114-119`) **sí** valida `!rRes.ok` y devuelve error.

### 4 · El respaldo cifrado puede volverse irrecuperable por una escritura no atómica
**Dónde:** `apps/fiscal/src/respaldo.ts:51-62` (`guardarFrase`) y `:64-77`
(`fraseDeRecuperacion`).
**Categoría:** fiscal (pérdida de datos).

`guardarFrase()` escribe con `openSync(ruta, 'w')`, que trunca el archivo sin el
patrón tmp+rename que sí usa `reloj.ts:77-87`. Si se corta la luz durante la
escritura, `respaldo-frase.json` queda vacío o dañado; entonces
`fraseDeRecuperacion()` genera y persiste una **frase nueva sin verificar si ya
hay asientos cifrados con la anterior**. Los asientos viejos quedan cifrados con
una clave que ya no existe: irrecuperables para siempre, sin ningún aviso, hasta
que alguien intente restaurar. En un libro fiscal esto es lo más grave que puede
pasar en silencio.

### 5 · El IGTF se calcula y se guarda, pero no sale en el reporte del contador
**Dónde:** `apps/fiscal/src/auditor.ts` (interfaz `FilaReporte:19-37`, `reporte()`
`:97-121`, `totales():143-177`).
**Categoría:** fiscal / dinero.

Ninguna de esas piezas referencia `igtf_usd`, aunque `armador.ts` lo calcula y
`libro.ts` lo persiste íntegro en cada asiento. El CSV y el panel `/fiscal` que
se lleva el contador muestran base, IVA y total, pero omiten el IGTF del período
—el impuesto del 3% sobre pagos en divisas—, con riesgo de dejarlo por fuera de
la declaración.

### 6 · El POS del cerebrito no frena el doble envío
**Dónde:** `apps/cerebrito/src/salon.ts` (`tomarPedidoMesa:194-205`,
`ventaDirecta:291-309`), servido por `apps/cerebrito/src/pos.ts`.
**Categoría:** dinero.

Ni `/pos/pedido` ni `/pos/venta` tienen clave de idempotencia: cada llamada
genera un `randomUUID()` nuevo. Un doble-tap en la tablet o un reintento por wifi
lento crea dos pedidos/ventas con IDs distintos —doble comanda a cocina y/o doble
cobro—, y ambos suben a la nube como operaciones legítimas separadas.

### 7 · `pagar_parte_cliente` (abierta a `anon`) no exige referencia en pagos electrónicos
**Dónde:** `supabase/migrations/20260720200000_pago_por_persona.sql:57-70` y
`supabase/migrations/20260724013000_comprobante_pago_mesa.sql:66-136`
(`cobrar_parte_mesa`).
**Categoría:** fiscal / dinero.

La migración `20260730050000_todo_pago_electronico_lleva_referencia.sql` creó
`fn_exigir_referencia` diciendo textualmente que la regla *"no puede ser
opcional… es parte del registro fiscal"* y que *"una validación que solo está en
el navegador se la salta cualquiera llamando la RPC directo"*. Pero solo la
cableó en `cobrar_y_cerrar_mesa`. Sus dos hermanas nunca se tocaron: siguen
insertando en `pagos_mesa` con `nullif(trim(coalesce(p_referencia,'')),'')`, sin
validar nada. Como `pagar_parte_cliente` está otorgada a `anon`, cualquiera con
la apikey pública (va en el bundle del cliente) puede llamarla con
`p_metodo:'pago_movil', p_referencia:''` y registrar un pago electrónico sin
referencia: exactamente el hueco que esa migración dijo cerrar.

---

## Segundo nivel (reales, puntaje 75 — el daño lo amortigua otra capa)

No cruzaron el umbral, pero varios valen la pena.

**Panel del restaurante**
- **"Falta por cobrar" no resta lo marcado como pérdida.** `Mesas.tsx:595` usa
  `total_usd - pagado_usd`, mientras `saldoDe` (`:229`) sí resta `perdida_usd`.
  El backend cobra el monto correcto, pero el cajero ve un "Falta $X" mayor y
  puede pedirle de más en efectivo al cliente.
- **El modal "Cobrar todo" trabaja con una foto congelada; el backend cobra en
  vivo.** `Mesas.tsx:470,579-651`. Si otro dispositivo agrega un pedido mientras
  el cajero llena el formulario, el sistema cierra la mesa por el total nuevo y
  la caja física recibió el viejo, sin aviso.
- **Al cerrar la jornada se borran los comprobantes antes de confirmar el
  cierre.** `Panel.tsx:202-237`: se hace `comprobantes.remove(...)` y recién
  después el RPC `cerrar_negocio`; si el RPC falla, los archivos ya no están y
  el `catch` no los repone.

**Tienda del cliente**
- **La validación de referencia de pago no usa la regla compartida.**
  `SeccionPago.tsx:149` deja enviar sin referencia si hay foto; `Cuenta.tsx`
  acepta 1-3 dígitos. Ninguno importa `faltaReferencia` de `shared/pagos.ts`.
  (Relacionado con el confirmado #7, pero del lado UI.)
- **`fn_medir` viaja con la anon key en el query string** (`medicion.ts:53`),
  que queda en logs y referrers; mejor en cabecera `apikey`.
- **`teniaCuentaRef` no se reinicia entre mesas** (`Tienda.tsx:169,219-238` sin
  `key={slug}` en `App.tsx`): puede mostrar "La Mesa X ya se cerró" en una mesa
  recién escaneada.

**Repartidor / Edge Functions**
- **`guardarPosicion()` no maneja el error del RPC** (`Home.tsx:252-258`): si
  falla, la ubicación deja de guardarse en silencio y el cliente deja de verla.
- **Push silenciados sin rastro.** `enviar-push/index.ts:90-93` descarta todo
  fallo que no sea 404/410, sin `console.error`.
- **`limpiar-datos-viejos` promete en un comentario "se loguea abajo" pero no
  hay log** (`:44-48`), en una función que existe por auditoría legal de
  retención.

**Estación Fiscal / cerebrito**
- **`extra_usd` no se revalida en el POS local** (`salon.ts:119`): un POST
  manipulado con extra negativo rebaja la venta. Vector de red local, no de
  internet.
- **Documentos "rechazados" para siempre por un regex demasiado amplio.**
  `cola.ts:56-59`: `/invalid/i` matchea un `TypeError: Invalid URL` transitorio
  y marca el asiento como rechazado sin retorno; queda sellado en el libro pero
  sin número de control.
- **La factura de talonario manual no tiene campo de IGTF** (`armador.ts:149-179`
  recibe `igtf_activo` y no lo usa): si el operador anota el total con IGTF
  incluido, el IVA declarado sale inflado.

**Validador / worker**
- **Comparaciones de secretos no constantes en tiempo** (`validador/.../admin.ts:56`
  y `respaldo-imprenta/src/index.mjs:60`), en un código que usa `timingSafeEqual`
  en otros lados. Explotabilidad remota baja.
- **`subir-imagen` valida el tipo solo por el `Content-Type` declarado**, no por
  los bytes, y lo reutiliza al guardar en R2.

---

## Orden sugerido para arreglar

Los confirmados, de menor esfuerzo a mayor:

1. **#2** (rol `staff`): quitar `staff`, poner `encargado`. Una línea.
2. **#3** (PUT sin verificar): agregar el chequeo de `.ok` como en `resetear_clave`.
3. **#4** (escritura atómica de la frase): usar tmp+rename como `reloj.ts`. Es el
   de mayor daño potencial pese a ser chico.
4. **#7** (referencia obligatoria): cablear `fn_exigir_referencia` en
   `pagar_parte_cliente` y `cobrar_parte_mesa`.
5. **#1** (duplicar con opciones): copiar las opciones, o forzar re-selección.
6. **#6** (idempotencia del POS): clave de idempotencia estable por operación.
7. **#5** (IGTF en el reporte): sumar la columna a `auditor.ts`.

Del segundo nivel, los tres de dinero del panel de mesas y el borrado de
comprobantes antes del cierre son los que conviene mirar aunque no hayan
cruzado el umbral.