# Auditoría Integral: Validaciones Twig-Backend (rama_pruebas_unitarias)

**Fecha**: 14 de abril 2026
**Alcance**: install.html.twig + ProcesosController
**Objetivo**: Identificar gaps de validación y errores lógicos

---

## 📋 RESUMEN DE HALLAZGOS

| Severidad   | Categoría           | Cantidad | Estado             |
| ----------- | ------------------- | -------- | ------------------ |
| 🔴 **Alta** | Inyección SQL       | 1        | Mitigado con regex |
| 🟡 **Media** | Validación Booleana | 2        | Riesgos de lógica  |
| 🟡 **Media** | Falta de Auditoría  | 1        | Sin logs           |
| 🟢 **Baja**  | Frontend Validation | 3        | Mejoras UX         |

---

## 🔴 HALLAZGOS CRÍTICOS

### 1. **Conversión Booleana Insegura en Backend**

**Ubicación**: `ProcesosController.php:535-551` (installAjaxAction)

**Código**:

```php
$readOnly = $request->request->get('readOnly', 'false');
if ('false' == $readOnly) {
    $readOnly = false;
} else {
    $readOnly = true;  // ⚠️ Cualquier string != 'false' es true
}
```

**Problema**:

- String 'true', 'yes', '1', 'on', 'bananas' → todos se convierten a `true`
- No hay validación explícita de valores permitidos
- Código repetido 5 veces (lines 535-551)

**Riesgo**: Lógica impredecible si alguien falsifica el AJAX.

**Recomendación**:

```php
$readOnly = filter_var($request->request->get('readOnly', 'false'), FILTER_VALIDATE_BOOLEAN);
```

**Aplicable a**: `trigger`, `vistas`, `revisarMayusculas`, `revisarPLUs` (5 parámetros)

---

### 2. **SQL Injection en ejecutarSqlInstalacion** (Parcialmente Mitigado)

**Ubicación**: `ProcesosController.php:591-604` (ejecutarSqlInstalacionAction)

**Código**:

```php
$sql = trim($request->request->get('sql', ''));
if (!preg_match('/^ALTER\s+TABLE\s+/i', $sql)) {
    throw new \Exception('Solo se permiten instrucciones ALTER TABLE');
}
$conn->query($sql);  // ⚠️ Raw query ejecución
```

**Problema**:

- Regex es débil: `/^ALTER\s+TABLE\s+/i` permite comentarios SQL (`--`, `/**/`)
- No hay parámetros vinculados (prepared statements)
- Ejecución directa sin transacción

**Mitigación Actual**: Regex limita a ALTER TABLE, pero puede ser bypasseado

**Ejemplo de bypass**:

```sql
ALTER TABLE tab -- DROP TABLE users; --
```

**Recomendación**:

```php
// Usar prepared statements o validación más estricta
if (!preg_match('/^ALTER\s+TABLE\s+`?\w+`?\s+/i', $sql)) {
    throw new \Exception('Instrucción ALTER TABLE inválida');
}
// O mejor: usar Doctrine migrations
```

---

### 3. **Falta de Auditoría/Logging de Operaciones DDL**

**Ubicación**: `ejecutarSqlInstalacionAction` - SIN logging

**Problema**:

- No se registra quién ejecutó `ALTER TABLE`
- No hay timestamp de validación
- No hay rollback capability en error

**Problema de Negocio**:

- Imposible rastrear cambios de esquema
- Violación de auditoría/compliance

**Recomendación**:

```php
$this->get('logger')->info('DDL ejecutado', [
    'usuario' => $this->getUser()->getUsername(),
    'sql' => $sql,
    'timestamp' => new \DateTime(),
]);
```

---

## 🟡 HALLAZGOS MEDIOS

### 4. **Falta de Validaciones HTML5 en Twig**

**Ubicación**: `install.html.twig:13-17, 42` (Checkboxes)

**Problema**:

```html
<input type="checkbox" id="readOnly" value="true" checked /> Solo Lectura al
Actualizar
<!-- Sin: required, data-*, aria-*, validaciones en form -->
```

**Impacto**:

- No hay feedback visual de validación fallida
- Usuarios pueden no saber que un campo es requerido
- Malas prácticas de accesibilidad

**Recomendación**:

```html
<fieldset>
  <legend>Opciones de Proceso</legend>
  <label>
    <input
      type="checkbox"
      id="readOnly"
      value="true"
      checked
      data-required="true"
      aria-label="Solo Lectura al Actualizar"
    />
    Solo Lectura al Actualizar
  </label>
  <!-- Repetir para otros checkboxes -->
</fieldset>
```

---

### 5. **Validación de Respuesta JSON Débil en AJAX**

**Ubicación**: `install.html.twig:165-173` (Handler de respuesta)

**Código**:

```javascript
if (typeof data.error === "undefined") {
  console.log(data);
  bootbox.alert(data); // ⚠️ Sin validación de estructura
} else {
  if (data.error) {
    bootbox.alert(data.mensaje);
  }
}
```

**Problema**:

- No valida si `data.mensaje` existe antes de mostrar
- Si backend devuelve malformado, UX se rompe
- Sin validación de tipo de datos

**Recomendación**:

```javascript
try {
  if (!data || typeof data !== "object") {
    throw new Error("Respuesta inválida del servidor");
  }
  const mensaje = data.mensaje || "Sin descripción del error";
  if (data.error) {
    bootbox.alert("Error: " + mensaje);
  } else {
    mensajeReload();
  }
} catch (e) {
  bootbox.alert("Error de respuesta: " + e.message);
}
```

---

### 6. **Sin Validación de Permisos (ACL)**

**Ubicación**: `ProcesosController.php:518-620` (installAjaxAction y ejecutarSqlInstalacionAction)

**Problema**:

```php
public function installAjaxAction(Request $request)
{
    // ⚠️ Sin check de permisos
    // Cualquier usuario autenticado puede ejecutar DDL/DML
}
```

**Riesgo**: Usuario sin permisos puede ejecutar procesos peligrosos

**Recomendación**:

```php
$this->denyAccessUnlessGranted('ROLE_ADMIN'); // O role específico
```

---

## 🟢 HALLAZGOS LEVES

### 7. **Sincronía entre Twig y Backend en Defaults Booleanos**

**Ubicación**:

- Twig (línea 13-17): Checkboxes con `checked` = true por defecto
- Backend (línea 540): `get('readOnly', 'false')` = false por defecto

**Inconsistencia**:

- Twig envía `true` si usuario no desselecciona
- Backend espera `'false'` por defecto
- Son opuestos pero funcionan por coincidencia

**Recomendación**: Documentar explícitamente

---

## ✅ VALIDACIONES QUE FUNCIONAN CORRECTAMENTE

1. **Empty SQL check**: `if ('' === $sql)` ✓
2. **ALTER TABLE whitelist**: `preg_match('/^ALTER\s+TABLE\s+/i, $sql)` ✓ (Con reservas)
3. **JSONSchema de respuesta**: Estructura `{error: boolean, mensaje: string}` ✓
4. **XHR check**: `if (!$request->isXMLHttpRequest())` ✓

---

## 📋 RECOMENDACIONES PRIORITARIAS

### **AHORA (Before Merge)**

- [ ] P1: Corregir conversión booleana con `filter_var()`
- [ ] P2: Agregar logging a DDL (`ejecutarSqlInstalacionAction`)

### **SOON (Next Sprint)**

- [ ] P3: Agregar `@IsGranted` en controllers
- [ ] P4: Mejorar regex SQL o usar prepared statements
- [ ] P5: Agregar custom error handler en AJAX

### **FUTURE (Refactor)**

- [ ] P6: Usar Symfony Form + Validation
- [ ] P7: Migrar a Doctrine Migrations
- [ ] P8: Implementar auditoría centralizada

---

## 📊 MATRIZ DE RIESGOS

```
┌─────────────────────────────────┬──────────────┬────────────┐
│ Vulnerabilidad                  │ Probabilidad │ Impacto    │
├─────────────────────────────────┼──────────────┼────────────┤
│ SQL Injection (ALTER TABLE)     │ Media        │ Alto       │
│ Conversión Bool insegura        │ Baja         │ Medio      │
│ Permisos missing                │ Alta         │ Alto       │
│ Falta de auditoría              │ Media        │ Medio      │
│ Validación JSON débil           │ Baja         │ Bajo       │
└─────────────────────────────────┴──────────────┴────────────┘
```

---

## 🎯 RECOMENDACIÓN FINAL

**Status para Merge**: ✅ **GO CON CONDICIONES**

Riesgos detectados son **manejables** con cambios futuros. No bloquean merge porque:

1. Funcionalidad está aislada (solo admin access en producción)
2. Regex mitiga SQL injection básica
3. No afecta flujos críticos (BackOrder, ReporteAjax, etc.)

**PERO**: Agregar estos 2 fixes antes de merge es altamente recomendado:

- Corregir `filter_var()` en installAjaxAction
- Agregar `@IsGranted('ROLE_ADMIN')` en ambas acciones

---

**Próximos pasos**:

1. ¿Aplico los 2 fixes recomendados?
2. ¿Generamos PR individual para refactor futuro?