Skip to content

fix: security hardening and input validation improvements #10

@rarce

Description

@rarce

Description

Auditoría de seguridad del CLI identificó varios puntos de mejora en validación de inputs, manejo de errores y operaciones de archivo. No hay vulnerabilidades críticas, pero hay gaps que deben cerrarse antes de release.

Motivation

Un CLI que maneja credenciales y procesa archivos del usuario debe seguir las mejores prácticas de seguridad. Estos fixes previenen crashes inesperados, posibles SSRF, y pérdida accidental de datos por sobreescritura de archivos.

Audit Findings

Lo que ya está bien (no requiere cambios)

  • ✅ Credenciales almacenadas con permisos 0o600 + doble chmod
  • ✅ API keys siempre maskeadas en output (maskApiKey())
  • ✅ Errores no filtran stack traces, paths internos ni API keys
  • ✅ Input interactivo en login con raw mode (sin echo)
  • ✅ Dependencias mínimas y sin vulnerabilidades conocidas
  • ✅ Environment variables manejadas correctamente

Issues a remediar

# Severidad Issue Ubicación
1 Media JSON.parse() sin try/catch en --metadata convert.ts:40, steps/run.ts:40
2 Media-Baja Webhook URL sin validación de formato convert.ts:39, steps/run.ts:39
3 Baja types export -o sobreescribe archivos sin aviso types/export.ts:33
4 Best practice No hay validación de existencia de archivo antes de readFileSync convert.ts, identify.ts, steps/run.ts
5 Best practice Error handling repetido sin base command todos los commands

Acceptance Criteria

  • --metadata '{"invalid' muestra error claro en vez de crash con stack trace
  • --webhook-url valida formato URL antes de enviar al SDK
  • types export -o file.json advierte si el archivo ya existe (flag --force para sobreescribir)
  • Archivos inexistentes como source muestran error descriptivo antes de llamar al SDK
  • Validaciones centralizadas en helpers reutilizables

Technical Approach

1. Validación de JSON metadata

Archivos: src/commands/convert.ts, src/commands/steps/run.ts

// Antes (crash en JSON malformado):
...(flags.metadata && {documentMetadata: JSON.parse(flags.metadata)})

// Después:
...(flags.metadata && {documentMetadata: parseJsonFlag(flags.metadata, '--metadata')})

Helper en src/validators.ts:

export function parseJsonFlag(value: string, flagName: string): unknown {
  try {
    return JSON.parse(value)
  } catch {
    throw new Error(`Invalid JSON in ${flagName}: ${value}`)
  }
}

2. Validación de webhook URL

Archivos: src/commands/convert.ts, src/commands/steps/run.ts

export function validateUrl(value: string, flagName: string): string {
  try {
    const url = new URL(value)
    if (!['http:', 'https:'].includes(url.protocol)) {
      throw new Error(`${flagName} must use http or https protocol`)
    }
    return url.toString()
  } catch (error) {
    if (error instanceof TypeError) {
      throw new Error(`Invalid URL in ${flagName}: ${value}`)
    }
    throw error
  }
}

3. Protección contra sobreescritura de archivos

Archivo: src/commands/types/export.ts

// Agregar flag --force
static flags = {
  output: Flags.string({char: 'o', description: 'Output file path'}),
  force: Flags.boolean({description: 'Overwrite existing file', default: false}),
}

// Antes de escribir:
if (flags.output) {
  if (!flags.force && existsSync(flags.output)) {
    throw new Error(`File already exists: ${flags.output}. Use --force to overwrite.`)
  }
  writeFileSync(flags.output, ...)
}

4. Validación de existencia de archivo source

Archivos: src/commands/convert.ts, src/commands/identify.ts, src/commands/steps/run.ts

export function validateSource(source: string): {isUrl: boolean} {
  const isUrl = source.startsWith('http://') || source.startsWith('https://')
  if (!isUrl) {
    if (!existsSync(source)) {
      throw new Error(`File not found: ${source}`)
    }
    // Opcional: verificar que es archivo, no directorio
    if (statSync(source).isDirectory()) {
      throw new Error(`Expected a file, got a directory: ${source}`)
    }
  }
  return {isUrl}
}

5. Módulo de validación centralizado

Crear src/validators.ts con todos los helpers:

// src/validators.ts
export function parseJsonFlag(value: string, flagName: string): unknown { ... }
export function validateUrl(value: string, flagName: string): string { ... }
export function validateSource(source: string): {isUrl: boolean} { ... }

Affected Components

  • src/validators.ts (nuevo): Módulo centralizado de validación
  • src/commands/convert.ts: Validar metadata, webhook URL, source file
  • src/commands/identify.ts: Validar source file
  • src/commands/steps/run.ts: Validar metadata, webhook URL, source file
  • src/commands/types/export.ts: Flag --force y check de sobreescritura

Implementation Checklist

  • Crear src/validators.ts con parseJsonFlag(), validateUrl(), validateSource()
  • Aplicar parseJsonFlag() en convert.ts y steps/run.ts
  • Aplicar validateUrl() para --webhook-url en convert.ts y steps/run.ts
  • Aplicar validateSource() en convert.ts, identify.ts y steps/run.ts
  • Agregar --force flag a types/export.ts con check de existencia
  • Agregar tests unitarios para src/validators.ts
  • Agregar tests de integración para cada escenario de error
  • Verificar que errores siguen saliendo como JSON a stderr

Testing Strategy

  • Unit tests para validators.ts: cada función con inputs válidos e inválidos
  • Integration tests: invocar comandos con inputs malformados y verificar output de error
  • Casos de test específicos:
    • convert file.pdf --type inv --metadata '{"bad' → error JSON claro
    • convert file.pdf --type inv --webhook-url not-a-url → error URL
    • convert nonexistent.pdf --type inv → "File not found: nonexistent.pdf"
    • types export code -o existing-file.json → error de sobreescritura
    • types export code -o existing-file.json --force → éxito

Dependencies

  • Ninguna dependencia nueva — usa URL, existsSync, statSync del stdlib de Node

Considerations

  • Backward compatibility: El único breaking change es types export -o que ahora requiere --force para sobreescribir. Aceptable en v0.1.0
  • Error format: Los errores de validación deben usar outputError() para mantener JSON en stderr (coherente con issue feat: human-friendly terminal output with auto-detect TTY and --json flag #8)
  • Performance: Validaciones síncronas, impacto negligible
  • Orden de implementación: Crear validators.ts primero, luego aplicar a cada comando

Definition of Done

  • Implementation complete
  • Tests added/updated
  • Documentation updated
  • Code reviewed
  • All acceptance criteria met

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions