Revisión de Código F#

La consulta de búsqueda de transacciones construye SQL concatenando directamente el parámetro merchantId proporcionado por el usuario en el cuerpo de la petición. Un atacante puede inyectar SQL arbitrario para exfiltrar o destruir datos.

let getTransactionsByMerchant (db: DbContext) (merchantId: string) =
    let sql = $"SELECT * FROM transactions WHERE merchant_id = '{merchantId}'"
    db.Database.ExecuteSqlRaw(sql)  // ⚠ SQL injection directo

let getTransactionsByMerchant (db: DbContext) (merchantId: MerchantId) =
    db.Transactions
        .Where(fun t -> t.MerchantId = merchantId.Value)
        .AsNoTracking()
        .ToListAsync()  // Consulta parametrizada vía LINQ/EF Core

El handler de descarga de recibos acepta el nombre de fichero desde el query string y lo concatena directamente al directorio base sin validar que el path resultante esté dentro del directorio permitido. Permite GET /receipts?file=../../appsettings.json.

let baseDir = "/var/cultivapay/receipts"
let filePath = Path.Combine(baseDir, fileName) // sin validar
let content = File.ReadAllBytes(filePath)

let safeReadReceipt baseDir fileName =
    let fullPath = Path.GetFullPath(Path.Combine(baseDir, fileName))
    if not (fullPath.StartsWith(Path.GetFullPath(baseDir) + Path.DirectorySeparatorChar.ToString())) then
        Error "Ruta de fichero no permitida"
    else
        Ok (File.ReadAllBytes(fullPath))

La API key del proveedor de pagos externo (Stripe) está hardcoded como literal de cadena en el módulo de servicio. Cualquier persona con acceso al repositorio puede obtenerla, incluyendo colaboradores externos o bots de GitHub.

let stripeApiKey = "sk_live_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"

let getStripeKey (config: IConfiguration) =
    config.["Stripe:ApiKey"]
    |> Option.ofObj
    |> Result.ofOption "Stripe:ApiKey no configurada"

El handler Giraffe usa .Result para esperar la tarea asíncrona de guardado de transacción, bloqueando el hilo del ThreadPool y pudiendo causar deadlock bajo carga.

let result = saveTransactionAsync db tx .Result  // bloqueo síncrono

let processPaymentHandler: HttpHandler =
    fun next ctx ->
        task {
            let! body = ctx.BindJsonAsync<PaymentRequest>()
            let! result = saveTransactionAsync db body.ToTransaction()
            return! json result next ctx
        }

transactionId, merchantId y customerId son simples string, lo que permite pasarlos en el orden incorrecto sin que el compilador lo detecte. En una pasarela de pagos, confundir IDs tiene consecuencias graves.

let processPayment (txId: string) (merchantId: string) (customerId: string) = ...

type TransactionId = TransactionId of string
type MerchantId    = MerchantId    of string
type CustomerId    = CustomerId    of string

let processPayment (txId: TransactionId) (merchantId: MerchantId) (customerId: CustomerId) = ...

El bloque de captura de error del procesamiento de pago captura con with _ -> None, haciendo que cualquier fallo (timeout de red, error de base de datos) sea silencioso e indistinguible de un pago correctamente completado.

let chargeCard amount cardToken =
    try stripeClient.Charge(amount, cardToken) |> Some
    with _ -> None  // ⚠ excepción completamente silenciada

let chargeCard amount cardToken =
    try
        stripeClient.Charge(amount, cardToken) |> Ok
    with
    | :? StripeException as ex -> Error (StripeError ex.Message)
    | :? TimeoutException    -> Error NetworkTimeout
    | ex                     -> Error (Unexpected ex.Message)

La transformación de DTO a entidad de dominio usa un for con acumulador mutable en lugar de las funciones de colección estándar de F#, añadiendo complejidad innecesaria.

let mutable results = []
for dto in dtos do
    results <- results @ [mapToTransaction dto]  // O(n²) y mutable

let results = dtos |> List.map mapToTransaction

La secuencia de validaciones se define como Seq.map perezoso y se itera dos veces (primero para detectar errores, luego para contar). Cada iteración re-evalúa las validaciones. Usar |> List.ofSeq para materializar una sola vez.

Los tipos de dominio transactionStatus, paymentMethod están declarados en camelCase. En F#, tipos, módulos y casos de DU usan PascalCase.

El handler processPaymentHandler tiene 58 líneas mezclando validación, llamada a servicio, mapeo de respuesta y logging. Difícil de testear unitariamente.