§10 Análisis de errores y riesgos

10.1 🔍 Metodología

Auditoría estática (revisión de código sin ejecutar) de los 63 archivos productivos + análisis del dump SQL real (u120688891_chess.sql).

Categorización

Prioridades

• Crítico — Bloquea producción o expone datos sensibles. Resolver YA.
• Alto — Causa errores recurrentes o riesgo grave. Resolver en sprint actual.
• Medio — Mejora significativa pero no bloqueante. Resolver en próximo trimestre.
• Bajo — Pulido, refactor, mejoras nice-to-have.

10.2 🔒 Seguridad (SEC)

SEC-01 · Endpoints administrativos sin autenticación

Descripción: Los archivos reset_db.php, sync.php, sync_bulk.php, save.php, save_raw.php, reinit_import.php, delete-sync.php de los 5 módulos tienen session_start() + verificación de auth comentados.

Evidencia

// reportes/caja/reset_db.php (línea 2-3)
// session_start();
// if (!isset($_SESSION['usuario'])) { header('Location: /login.php'); exit; }

// reportes/index.php (línea 2-3)
// session_start();
// if (!isset($_SESSION['usuario'])) { header('Location: /login.php'); exit; }

Impacto

• Cualquier persona con la URL puede:
  • Llamar DROP TABLE en las 10 tablas de caja.
  • Borrar todos los syncs de ventas (reinit_import.php?action=reset).
  • Inyectar filas falsas en caja_gastos via save.php.
  • Triggerar TRUNCATE articulos_raw.

Fix sugerido

// Reactivar la auth en cada endpoint admin:
session_start();
if (!isset($_SESSION['usuario'])) {
    http_response_code(401);
    header('Location: /login.php');
    exit;
}

// O mejor: crear includes/auth.php que se incluya con 1 línea:
require_once __DIR__ . '/../_shared/auth.php';
require_role(['admin', 'finanzas']);

SEC-02 · Credenciales hardcoded en 5 archivos

Descripción: Las constantes DB_PASS, CHESS_API_PASS están en plaintext en 5 copias de config/database.php.

Archivos afectados

1. reportes/venta/config/database.php
2. reportes/caja/config/database.php
3. reportes/cashflow/config/database.php
4. reportes/articulos/config/database.php
5. reportes/listas/config/database.php

Evidencia

// Las 5 copias contienen:
define('DB_PASS',         't2#h*wvQ2./wZaS');
define('CHESS_API_USER',  'api_zonas');
define('CHESS_API_PASS',  'z0n4saridas');

Impacto

• Si se publica el repo en GitHub → toda la BD comprometida.
• Si se filtra un solo archivo (backup mal hecho, descarga errónea) → idem.
• Rotar passwords requiere editar 5 archivos.
• Backups del filesystem (que muchos hostings hacen) incluyen las credenciales.

Fix sugerido

// Crear un único punto de carga fuera del doc root:
// /home/u120688891/.env (fuera de public_html)
DB_HOST=localhost
DB_NAME=u120688891_chess
DB_USER=u120688891_chess
DB_PASS=t2#h*wvQ2./wZaS
CHESS_API_USER=api_zonas
CHESS_API_PASS=z0n4saridas

// /reportes/_shared/config.php
$env = parse_ini_file('/home/u120688891/.env');
define('DB_PASS',        $env['DB_PASS']);
// ... etc

// Los 5 config/database.php se reemplazan por:
require_once __DIR__ . '/../../_shared/config.php';

SEC-03 · Dump SQL accesible vía HTTP

Descripción: El archivo u120688891_chess.sql (765 MB) vive en el doc root y es descargable por cualquiera con la URL exacta.

URL accesible

https://zonasaridas.com.ar/reportes/u120688891_chess.sql

Impacto

• Exfiltración total de la BD (515K ventas, ~330K filas caja, todos los clientes, credenciales no — pero datos comerciales sí).
• Apache lista el archivo en el directorio (Options +Indexes) si alguna config lo permite.
• Búsquedas Google con filetype:sql site:zonasaridas.com.ar pueden indexarlo.

Fix sugerido

# Opción 1: mover fuera del doc root
mv public_html/reportes/u120688891_chess.sql /home/u120688891/backups/

# Opción 2: bloquear con .htaccess en /reportes/
<FilesMatch "\.(sql|md|bak|env|log|swp|orig)$">
    Order allow,deny
    Deny from all
</FilesMatch>

# Opción 3: explícito
<Files "u120688891_chess.sql">
    Order allow,deny
    Deny from all
</Files>

SEC-04 · Interpolación SQL en lugar de prepared statements

Descripción: Algunas queries usan interpolación de variables PHP en el SQL. La mayoría tienen un cast a int previo, pero es mala práctica.

Evidencia

// reportes/caja/api/save.php:63
$total = $pdo->query("SELECT COUNT(*) FROM caja_gastos WHERE sync_id=$syncId")->fetchColumn();
//                                                              ↑ interpolación

// reportes/caja/reset_db.php:46-48
foreach ($TABLAS_SYNC as $t) {
    $pdo->prepare("DELETE FROM $t WHERE sync_id=?")->execute([$sid]);
    //                  ↑ $t interpolado (pero validado en array $TABLAS_SYNC)
}

// data_api.php usa string interpolation con $sucWhere construido vía $db->quote()
$sucWhere = $sucursal !== '' ? "AND dsSucursal = " . $db->quote($sucursal) : '';
// ↑ Seguro pero mejor un prepared statement

Impacto

• Riesgo real: bajo (variables casteadas o desde whitelist).
• Riesgo simbólico: alto — mala práctica que puede propagarse.
• Si alguien futuro copia el patrón sin entender el contexto → potencial SQLi.

Fix sugerido

// Reemplazar todas las interpolaciones por prepared statements:
$stmt = $pdo->prepare("SELECT COUNT(*) FROM caja_gastos WHERE sync_id = ?");
$stmt->execute([$syncId]);
$total = $stmt->fetchColumn();

// Para nombres de tabla (no parametrizables) → whitelist explícita
if (!in_array($t, ALLOWED_TABLES, true)) throw new Exception("Tabla inválida: $t");
$stmt = $pdo->prepare("DELETE FROM `$t` WHERE sync_id = ?");
$stmt->execute([$sid]);

SEC-05 · Sin rate limiting en endpoints públicos

Los endpoints filter.php y api/data.php aceptan requests ilimitadas. Si alguien hace un loop podría:

• Saturar la BD (queries pesadas con LEFT JOIN).
• Generar tráfico que cuente contra la cuota de Hostinger.
• Mining de datos comerciales.

Fix sugerido

// /reportes/_shared/rate_limit.php
function rate_limit(string $key, int $max = 60, int $window = 60): void {
    $cache = __DIR__ . '/cache/' . md5($key) . '.json';
    $data = file_exists($cache) ? json_decode(file_get_contents($cache), true) : ['count'=>0, 'reset'=>time()+$window];
    if (time() > $data['reset']) $data = ['count'=>0, 'reset'=>time()+$window];
    $data['count']++;
    if ($data['count'] > $max) {
        http_response_code(429);
        header('Retry-After: ' . ($data['reset'] - time()));
        exit(json_encode(['error'=>'Too many requests']));
    }
    file_put_contents($cache, json_encode($data));
}

// En el endpoint:
rate_limit($_SERVER['REMOTE_ADDR'] . ':filter', 60, 60);

10.3 🏗️ Arquitectura (ARQ)

ARQ-01 · Constantes de BD redefinidas en 5 archivos

Las constantes DB_* y CHESS_API_* están duplicadas en 5 config/database.php. Ya divergen:

• venta/config/database.php agrega SITE_VERSION, UPLOAD_MAX_MB, TIMEZONE, SITE_URL.
• caja/config/database.php sólo tiene DB_* + date_default_timezone_set.
• listas/config/database.php agrega LISTAS_DISPONIBLES.
• Otros mezclan estilos.

Fix: ver SEC-02 (mismo refactor).

ARQ-02 · Código legado en producción

Archivos que ya no se usan pero quedaron en el doc root:

ARQ-03 · CAJERO_PROV hardcoded y duplicado

El mapeo cajero → provincia está en JS hardcoded duplicado en sync.php (~línea 150) y sync_bulk.php (~línea 310). Cada cajero nuevo requiere editar 2 archivos.

Fix: tabla DB editable (ver §4 P-CAJ-1).

ARQ-04 · Una sola FK en toda la BD

De 14 relaciones lógicas entre tablas, sólo 1 está declarada como FK (ventas.sync_id → sincronizaciones.id ON DELETE CASCADE). El resto se mantiene con limpieza explícita en código (más frágil).

Fix: ver §2.10 FKs propuestas.

ARQ-05 · Constantes de fórmula duplicadas

Las constantes del cálculo de margen (SUM_VENTAS_REALES, SUM_COSTO_NETO, EXPR_CONTRIBUCION, EXPR_MARGEN_PCT) están duplicadas entre venta/includes/data_api.php y venta/api/filter.php.

Si la fórmula cambia, hay que editar 2 archivos. Ver §3.10 P-VEN-2.

ARQ-06 · Pipeline 3-tier corre en cliente (JS)

El pipeline de clasificación de caja corre en JavaScript dentro de sync.php y sync_bulk.php. Esto:

• Difícil de testear unitariamente.
• Bug histórico (col 5 vs 6) estuvo ~años sin detectarse.
• Pesa el browser (parsea 6 Excel grandes).

Fix: mover al backend (ver §4 P-CAJ-2).

10.4 ⚡ Performance (PERF)

PERF-01 · Landing con N+1 queries

El landing /reportes/index.php hace 12 prepares secuenciales para totales por mes de caja:

// reportes/index.php (líneas 51-58)
$stmt = $pdo->prepare("SELECT SUM(monto) FROM caja_gastos WHERE mes=? AND anio=?");
$stmt->execute([(int)$cLatest['mes'], (int)$cLatest['anio']]);
$cTotalUltimo = (float)$stmt->fetchColumn();

foreach (array_reverse($cRows) as $r) {
    $stmt2 = $pdo->prepare("SELECT SUM(monto) FROM caja_gastos WHERE mes=? AND anio=?");
    $stmt2->execute([(int)$r['mes'], (int)$r['anio']]);
    $cMeses[] = ['label' => ..., 'v' => (float)$stmt2->fetchColumn()];
}

Fix

// 1 sola query con GROUP BY
$cTotales = $pdo->query("
    SELECT mes, anio, SUM(monto) AS total
    FROM caja_gastos
    WHERE (anio, mes) IN (
        SELECT anio, mes FROM caja_sincronizaciones
        WHERE estado='completado'
        ORDER BY anio DESC, mes DESC LIMIT 12
    )
    GROUP BY anio, mes
    ORDER BY anio DESC, mes DESC
")->fetchAll(PDO::FETCH_ASSOC);

// Indexar resultado para acceso rápido
$totalesByPeriodo = [];
foreach ($cTotales as $r) {
    $totalesByPeriodo[$r['anio'].'-'.$r['mes']] = (float)$r['total'];
}

Reducción de 12+ round-trips a 1.

PERF-02 · Falta índice compuesto en ventas

Todas las queries del dashboard filtran por sync_id AND anulado='NO', pero no hay índice compuesto. MariaDB usa idx_sync y luego filter por anulado con table scan dentro del rango.

Fix

ALTER TABLE ventas ADD INDEX idx_sync_anulado_articulo
    (sync_id, anulado, dsArticulo(60));

-- Cubre las queries más comunes del dashboard
-- Estimación: 12x menos páginas leídas en una query típica

PERF-03 · CAST inhibe uso de índice

El JOIN ON a.codigo = CAST(idArticulo AS UNSIGNED) hace que el optimizer no use el índice PK de articulos_raw.codigo.

En el dump real, ventas.idArticulo es INT, no VARCHAR como dice el doc → el CAST es innecesario.

Fix

-- 1. Verificar que no haya valores no numéricos
SELECT idArticulo FROM ventas WHERE idArticulo IS NOT NULL
  AND idArticulo != CAST(idArticulo AS UNSIGNED) LIMIT 5;

-- 2. Si todo OK, simplificar el JOIN:
LEFT JOIN articulos_raw a ON a.codigo = idArticulo  -- sin CAST

PERF-04 · Sin cache HTTP en endpoints JSON

Los endpoints JSON (filter.php, api/data.php de cashflow) envían Cache-Control: no-store aunque los datos cambian sólo cuando se hace un sync nuevo.

Fix

// Generar ETag basado en last_sync_at + filtros
$etag = md5(json_encode([$syncId, $sucursal, $lastSyncAt]));
header("ETag: \"$etag\"");

if (isset($_SERVER['HTTP_IF_NONE_MATCH']) &&
    trim($_SERVER['HTTP_IF_NONE_MATCH'], '"') === $etag) {
    http_response_code(304);
    exit;
}

// Cache de 5 min, pero permite revalidación
header("Cache-Control: private, max-age=300, must-revalidate");

PERF-05 · Backfill UPDATE en cada hit de listas/index.php

Ver §7.6 LIS-03 — el _schema.php de listas ejecuta un UPDATE masivo en CADA hit a la página. Mover a cron o trigger.

PERF-06 · dashboard.php de venta carga TODO en HTML inicial

El dashboard inyecta los datos de los 6 tabs Rentabilidad como variables JS al renderizar el HTML. Con períodos grandes (Cliente ~1000 filas, Artículo ~1200) el HTML inicial puede pesar > 2 MB.

Fix: lazy-load por tab via fetch al hacer click. Acelera el primer paint en ~50%.

10.5 🎨 UX

UX-01 · Sin empty state consistente

Cuando un período no tiene datos, los dashboards muestran "0,00" o tablas vacías sin mensaje explicativo.

Fix: componente <EmptyState> reutilizable con ícono + mensaje + CTA "Cargar datos del período".

UX-02 · Sin loading states en drill-downs

filter.php responde en ~500ms pero el usuario no ve indicador.

Fix: spinner en el modal mientras espera.

UX-03 · Sin shortcut de teclado

No hay shortcuts. Para usuarios power (uso diario) sería útil:

• / → focus en buscador global
• g v → ir a venta
• g c → ir a caja
• ? → mostrar lista de shortcuts
• 1-6 → cambiar entre tabs de Rentabilidad

UX-04 · Mobile poco amigable

Los dashboards tienen breakpoints básicos pero las tablas con 11 columnas no se adaptan bien a pantallas <760px. Considerar:

• Vista "cards" para móvil (1 card por fila con todas las métricas).
• Botón "Modo cards" / "Modo tabla" en cada sección.

10.6 📚 Documentación (DOC)

DOC-01 · Funciones sin docstring

Las 20+ funciones de data_api.php tienen nombre auto-explicativo pero faltan PHPDoc con tipos y ejemplos.

Esfuerzo: 6 horas para cubrir todas.

DOC-02 · README al nivel raíz de /reportes/

Existe DOCUMENTACION.md pero no un README.md corto que sea lo primero que vea alguien clonando el repo.

Contener: getting started, requisitos, cómo levantar local, links a docs.

DOC-03 · Sin documentar las heurísticas

Las heurísticas de rubros (cruce_ventas.php) y categorías de riesgo (cashflow/api/data.php) están en arrays PHP sin comment explicando por qué esos umbrales.

10.7 🛡️ Resiliencia (RES)

RES-01 · Sin retry en cliente ChessERP

Si la API ChessERP da timeout o 5xx, el sync falla y queda en estado procesando. El usuario debe ir a monitor.php + reinit_import.php?resume=1.

Fix sugerido

// chess_api_client.php · agregar retry con exponential backoff
public function fetchVentasLote($desde, $hasta, $lote, $maxRetries = 3): ?array {
    $delay = 2;
    for ($i = 0; $i <= $maxRetries; $i++) {
        try {
            $resp = $this->httpGet("/ventas?fechaDesde=$desde&fechaHasta=$hasta&nroLote=$lote");
            return json_decode($resp, true);
        } catch (Exception $e) {
            if ($i === $maxRetries) throw $e;
            sleep($delay);
            $delay *= 2;
        }
    }
}

RES-02 · Sin logging estructurado

Los errores se devuelven via {ok: false, error: $e->getMessage()} pero no quedan en log persistente (salvo caja_audit_log que solo registra eventos de sync).

Fix

// /reportes/_shared/logger.php
function log_error(string $context, Exception $e, array $extra = []): void {
    $entry = [
        'timestamp'   => date('c'),
        'context'     => $context,
        'error'       => $e->getMessage(),
        'trace'       => $e->getTraceAsString(),
        'request_uri' => $_SERVER['REQUEST_URI'] ?? '',
        'user_agent'  => $_SERVER['HTTP_USER_AGENT'] ?? '',
        'extra'       => $extra,
    ];
    file_put_contents(
        __DIR__ . '/logs/' . date('Y-m') . '.jsonl',
        json_encode($entry) . "\n",
        FILE_APPEND
    );
}

RES-03 · Sin healthcheck endpoint

No hay /reportes/health.php que verifique BD + última sync + storage.

Fix

// /reportes/health.php
$checks = [
    'db' => false,
    'last_sync_venta'   => null,
    'last_sync_caja'    => null,
    'disk_free_mb'      => round(disk_free_space('/') / 1024 / 1024),
];
try {
    $pdo = getApiDB();
    $checks['db'] = true;
    $checks['last_sync_venta'] = $pdo->query("SELECT MAX(fecha_fin) FROM sincronizaciones WHERE estado='completado'")->fetchColumn();
    $checks['last_sync_caja']  = $pdo->query("SELECT MAX(finalizado_at) FROM caja_sincronizaciones WHERE estado='completado'")->fetchColumn();
} catch (Exception $e) {
    http_response_code(503);
}
header('Content-Type: application/json');
echo json_encode($checks, JSON_PRETTY_PRINT);

10.8 📊 Matriz riesgo × impacto

   IMPACTO
   ↑
   │   ALTO (compromete operación / datos sensibles)
   │   ┌──────────────────────────────────────────────┐
   │   │ SEC-03 dump.sql público                      │
   │   │ SEC-01 endpoints sin auth   ◄ FASE 0         │
   │   │ SEC-02 credenciales hardcoded                │
   │   ├──────────────────────────────────────────────┤
   │   │ ARQ-02 código legado en prod                 │
   │   │ SEC-04 SQL interpolation    ◄ FASE 1         │
   │   │ ARQ-01 5 copias de config                    │
   │   │ RES-01 sin retry ChessERP                    │
   │   ├──────────────────────────────────────────────┤
   │   │ PERF-01 N+1 en landing                       │
   │   │ PERF-02 falta índice compuesto               │
   │   │ ARQ-03 CAJERO_PROV duplicado ◄ FASE 2        │
   │   │ ARQ-04 sólo 1 FK declarada                   │
   │   │ ARQ-05 fórmula duplicada                     │
   │   │ ARQ-06 pipeline en JS                        │
   │   │ UX-01  empty states                          │
   │   │ CAJ-08 IVA débito genérico                   │
   │   ├──────────────────────────────────────────────┤
   │   │ PERF-05 backfill en cada hit                 │
   │   │ DOC-01 docstrings faltantes  ◄ Backlog       │
   │   │ UX-03  shortcuts teclado                     │
   │   │ UX-04  mobile poco friendly                  │
   │   └──────────────────────────────────────────────┘
   │   BAJO
   └──────────────────────────────────────────────────►
       BAJA            MEDIA            ALTA          PROBABILIDAD
                                                      (exposición)

10.9 ✅ Bugs históricos ya resueltos

Esto NO son hallazgos nuevos — son bugs que el equipo ya fixeó y dejaron learnings importantes documentados: