# Bug Report — mythicbyte-facturacion-electronica

**Fecha:** 2026-03-23
**Branch:** develop

---

## Críticos

### 1. Migration `down()` borra la tabla equivocada

**Archivo:** `database/migrations/2026_02_23_000001_create_mythicbyte_sent_receipts_table.php`

El método `down()` ejecuta `Schema::dropIfExists('mythicbyte_receipts')` en lugar de `Schema::dropIfExists('mythicbyte_sent_receipts')`. Un rollback de esta migración destruiría la tabla principal de recibos, causando pérdida masiva de datos.

```php
// Actual (incorrecto)
public function down(): void
{
    Schema::dropIfExists('mythicbyte_receipts');
}

// Correcto
public function down(): void
{
    Schema::dropIfExists('mythicbyte_sent_receipts');
}
```

---

### 2. Race condition en generación de consecutivos

**Archivo:** `src/Traits/ConsecutiveNumberTrait.php` — líneas 12-20

`lockForUpdate()` protege la lectura del registro existente, pero si no existe, la creación del registro ocurre fuera del lock. Dos requests concurrentes pueden ambos crear un registro nuevo y generar consecutivos duplicados.

```php
// Actual (vulnerable)
$consecutive = ReceiptConsecutive::where('type', $type)->lockForUpdate()->first();
if (!$consecutive) {
    $consecutive = ReceiptConsecutive::create(['type' => $type, 'consecutive' => 0]);
}

// Sugerido: usar firstOrCreate dentro de la transacción, o INSERT ... ON DUPLICATE KEY
```

---

### 3. Error de precedencia de operadores lógicos

**Archivo:** `src/Services/Validators/DetailLinesValidator.php` — línea 57

Por la precedencia de `&&` sobre `||`, la condición siempre evalúa `true` porque la última parte `|| $codeReference != '10'` es casi siempre verdadera.

```php
// Actual (incorrecto)
if ($documentType == '02' || $documentType == '03' && $codeReference != '09' || $codeReference != '10')

// Evaluación real:
// ($documentType == '02') || ($documentType == '03' && $codeReference != '09') || ($codeReference != '10')

// Correcto (intención probable)
if (($documentType == '02' || $documentType == '03') && ($codeReference != '09' && $codeReference != '10'))
```

---

## Altos

### 4. Acceso a array incorrecto en validación de líneas de detalle

**Archivo:** `src/Services/Validators/DetailLinesValidator.php` — línea 173

Se itera `$lines['Impuesto']` pero `$lines` es el array completo de líneas de detalle, no una línea individual. La key `Impuesto` nunca existe a ese nivel.

```php
// Actual (incorrecto)
foreach ($lines['Impuesto'] ?? [] as $index => $line) {

// Correcto
foreach ($lines as $index => $line) {
    foreach ($line['Impuesto'] ?? [] as $taxIndex => $tax) {
```

---

### 5. Variable `$tax` usada antes de definirse

**Archivo:** `src/Services/Validators/DetailLinesValidator.php` — línea 177

`$tax['Monto']` se referencia antes del `foreach` que la define. Siempre suma 0 al total de impuestos.

```php
// Actual (incorrecto)
$taxTotal = 0;
$taxTotal += $tax['Monto'] ?? 0;  // $tax no existe aún
foreach ($line['Impuesto'] ?? [] as $taxIndex => $tax) {
```

---

### 6. `continue` mal ubicado en TaxesValidator

**Archivo:** `src/Services/Validators/TaxesValidator.php` — línea 218

El `continue` está fuera del bloque `if`, alterando el flujo de control y saltando validaciones que deberían ejecutarse.

```php
// Actual (incorrecto)
if(isset($line['Impuesto'])) {
    foreach ($line['Impuesto'] as $taxIndex => $tax) {
        // ...
    }
}continue ;  // ← fuera del if, siempre salta
```

---

### 7. Nesting extra en CyberfuelTransformer

**Archivo:** `src/Providers/Cyberfuel/Transformer/CyberfuelTransformer.php` — líneas 388-403

Cada cargo en `OtrosCargos` se envuelve en un nivel de array extra, creando `[[...]]` en vez de `[...]`. Esto rompe el formato esperado por la API de Cyberfuel.

```php
// Actual (incorrecto)
$result[] = [
    [
        'tipodocumento' => $charge['TipoDocumentoOC'] ?? "",
        // ...
    ],
];

// Correcto
$result[] = [
    'tipodocumento' => $charge['TipoDocumentoOC'] ?? "",
    // ...
];
```

---

### 8. Rutas de test sin protección

**Archivo:** `routes/api.php` — líneas 26-36

Los endpoints `/test/sign-xml`, `/test/hacienda/send` y `/test/hacienda/health` no tienen middleware de autenticación. Si se despliegan a producción, cualquier persona podría firmar XMLs o enviar documentos a Hacienda.

```php
// Sugerido: proteger con middleware o limitar por entorno
Route::middleware(['auth:api'])->group(function () {
    Route::post('/test/sign-xml', [XmlSignerTestController::class, 'sign']);
    // ...
});

// O limitar por entorno
if (app()->environment('local', 'testing')) {
    // rutas de test
}
```

---

### 9. `exchange_rate` almacenado como string en vez de decimal

**Archivo:** `database/migrations/2026_01_28_000001_create_mythicbyte_receipts_table.php` — línea 35

El tipo de cambio se define como `string` pero debería ser `decimal` para operaciones aritméticas y precisión numérica.

```php
// Actual
$table->string('exchange_rate');

// Correcto
$table->decimal('exchange_rate', 10, 5);
```

---

## Medios

### 10. Validación dentro del loop reporta errores múltiples

**Archivo:** `src/Services/Validators/DetailLinesValidator.php` — líneas 149-167

La verificación de `MontoTotal` está dentro del `foreach` de líneas de detalle. Una vez detectada la diferencia, el mismo error se agrega en cada iteración restante. El check debe estar después del loop.

---

### 11. `SendSentReceiptToProviderJob` no maneja errores no-retriable

**Archivo:** `src/Jobs/SendSentReceiptToProviderJob.php` — líneas 57-66

A diferencia de `SendReceiptToProviderJob`, este job no verifica el flag `retriable` en la respuesta del provider. Errores que no deberían reintentarse se reintentan hasta agotar los intentos.

---

### 12. Comparación loose `== false` en Jobs

**Archivos:**
- `src/Jobs/SendReceiptToProviderJob.php` — línea 62
- `src/Jobs/SendSentReceiptToProviderJob.php` — línea 57

Usan `== false` (loose) en vez de `=== false` (strict). Valores como `"0"`, `null`, o `""` evaluarían incorrectamente.

```php
// Actual
if ($response['success'] == false)

// Correcto
if ($response['success'] === false)
```

---

### 13. Null check faltante en payload del Job

**Archivo:** `src/Jobs/SendReceiptToProviderJob.php` — línea 55

Accede `$this->receipt->payload->payload_json` sin verificar que la relación `payload` no sea null. Si el payload fue eliminado, genera un error fatal.

---

### 14. Token cache TTL sin margen de seguridad

**Archivo:** `src/Services/Hacienda/Idp/HaciendaIdpService.php` — líneas 137-143

El TTL del cache usa `refreshExpiresAt - time()` pero no descuenta los 30 segundos de margen que aplica `isRefreshExpired()`. Un token marcado como expirado puede aún existir en cache.

```php
// Actual
$ttl = $this->tokenData->refreshExpiresAt - time();

// Correcto
$ttl = max(0, $this->tokenData->refreshExpiresAt - time() - 30);
```

---

### 15. Modelos sin casts para campos decimales

**Archivos:**
- `src/Models/Receipt.php` — líneas 55-58
- `src/Models/SentReceipt.php` — líneas 57-60

Campos monetarios (`total_amount`, `tax_amount`, `total_discount`, `total_voucher`) no tienen cast a `decimal`. Se retornan como strings desde la base de datos.

```php
// Agregar en $casts
'total_amount' => 'decimal:3',
'tax_amount' => 'decimal:3',
'total_discount' => 'decimal:3',
'total_voucher' => 'decimal:3',
'exchange_rate' => 'decimal:5',
```

---

### 16. Campo referenciado incorrecto en CyberfuelTransformer

**Archivo:** `src/Providers/Cyberfuel/Transformer/CyberfuelTransformer.php` — línea 510

Para documentos enviados, usa `$payload['Clave']['SituacionComprobante']` pero debería ser `$payload['Encabezado']['SituacionComprobante']` (consistente con `transformDocument`).

---

### 17. Falta índice en columna `receipt_status`

**Archivos:**
- `database/migrations/2026_01_28_000001_create_mythicbyte_receipts_table.php`
- `database/migrations/2026_02_23_000001_create_mythicbyte_sent_receipts_table.php`

La columna `receipt_status` se consulta en `DbService` pero no tiene índice. Con volumen alto de datos, estas queries serán lentas.

```php
$table->index('receipt_status');
```

---

## XmlGeneratorService

### 18. XML injection por falta de escape en `nodeValue`

**Archivo:** `src/Services/XmlGenerator/XmlGeneratorService.php` — línea 92

**Severidad:** Alta

`addElement()` asigna el valor directamente a `nodeValue`, lo cual no escapa caracteres especiales XML (`&`, `<`, `>`). Si un campo contiene por ejemplo "A & B S.A.", el XML generado será inválido y Hacienda lo rechazará.

```php
// Actual (incorrecto)
$element->nodeValue = (string) $value;

// Correcto
$element->appendChild($this->dom->createTextNode((string) $value));
```

---

### 19. Tipo comprobante `07` (REP) falta en el mapa de root elements

**Archivo:** `src/Services/XmlGenerator/XmlGeneratorService.php` — líneas 73-80

**Severidad:** Alta

`initRootElement` mapea los tipos `01`, `02`, `03`, `04`, `08`, `09` pero no incluye `07` (Recibo Electrónico de Pago). Si se genera un REP, cae al fallback `FacturaElectronica` con el namespace equivocado, produciendo un XML con estructura incorrecta que Hacienda rechazará.

```php
// Falta agregar en $map:
'07' => ['ReciboElectronicoPago', 'https://cdn.comprobanteselectronicos.go.cr/xml-schemas/v4.4/reciboElectronicoPago'],
```

---

### 20. `addOtroTexto` crea un solo nodo para múltiples items

**Archivo:** `src/Services/XmlGenerator/XmlGeneratorService.php` — líneas 678-691

**Severidad:** Alta

Se crea un solo elemento `<OtroTexto>` y luego se iteran todos los items dentro de él, duplicando tags `codigo` y `valor` en el mismo nodo. Esto produce XML inválido.

```php
// Actual (incorrecto) — un solo <OtroTexto> con múltiples codigo/valor
$otroTexto = $this->dom->createElement('OtroTexto');
$parent->appendChild($otroTexto);
foreach ($data as $textoData) {
    // agrega codigo y valor al mismo $otroTexto
}

// Correcto — un <OtroTexto> por cada item
foreach ($data as $textoData) {
    $otroTexto = $this->dom->createElement('OtroTexto');
    $parent->appendChild($otroTexto);
    // ...
}
```

---

### 21. `CorreoElectronico` del Receptor no itera como array

**Archivo:** `src/Services/XmlGenerator/XmlGeneratorService.php` — línea 193

**Severidad:** Media

En `addEmisorData` (línea 157), `CorreoElectronico` se itera como array para soportar múltiples correos. En `addReceptorData` (línea 193), se pasa como string directamente. Si el Receptor puede tener múltiples correos, solo se enviará el primer elemento (o fallará si es un array).

```php
// Emisor (correcto — itera)
foreach ($data['CorreoElectronico'] as $email) {
    $this->addElement($emisor, 'CorreoElectronico', $email);
}

// Receptor (inconsistente — no itera)
$this->addElement($receptor, 'CorreoElectronico', $data['CorreoElectronico']);
```

---

### 22. Sin validación de `Emisor` y `Receptor` en `generateXml`

**Archivo:** `src/Services/XmlGenerator/XmlGeneratorService.php` — líneas 30-31

**Severidad:** Media

`generateXml()` accede directamente a `$data['Emisor']` y `$data['Receptor']` sin verificar que existan. Si faltan, genera un `Undefined index` en vez de un mensaje descriptivo.

---

### 23. Exception wrapping pierde el stack trace

**Archivo:** `src/Services/XmlGenerator/XmlGeneratorService.php` — línea 54

**Severidad:** Baja

Se crea una nueva excepción sin pasar la original como `$previous`, perdiendo el stack trace completo.

```php
// Actual
throw new Exception("Error generating XML: " . $e->getMessage());

// Correcto
throw new Exception("Error generating XML: " . $e->getMessage(), 0, $e);
```

---

### 24. Docblock huérfano

**Archivo:** `src/Services/XmlGenerator/XmlGeneratorService.php` — líneas 59-66

**Severidad:** Baja

Hay un bloque `/** ... */` que describe `addElement` pero está ubicado antes del docblock de `initRootElement` (línea 68). Debería moverse justo antes de `addElement` en la línea 89.