Decisión de seguridad: hardening del confirm Visio (s220) — 3 fixes

Contexto

Auditoría de seguridad (s220) en POST /api/racks/visio/confirm — endpoint que toma la lista de masters (formas convertidas o subidas) y las persiste en la librería de stencils del tenant. Se identificaron 3 vulnerabilidades:

1. Permiso faltante: confirm no comprobaba permisos; analyze sí. Un usuario autenticado sin permisos admin podía importar stencils (violación de RBAC).
2. Path-traversal: parámetro image_path con ../ podía resolver fuera de MEDIA_ROOT y mover un fichero arbitrario del servidor a la librería del tenant (LFI → mutación).
3. XSS persistente: SVG subidos o generados podían contener malware embebido (<script>, handlers, javascript: URLs) sin sanitización.

Decisiones de remediación

Fix 1: Gate de permisos (racks:admin)

Cambio: Añadir require_perm(request, "racks", "admin") en confirm, idéntico a analyze.

Ratificación:

• Importar stencils es acción administrativa (afecta a toda la librería del tenant)
• analyzer ya exigía admin; confirm debe mantener paridad
• Migración de test: test_racks_visio_confirm.py fixture ahora admin_user en lugar de operator_user

Ubicación: racks/api/library_files.py, línea ~130

Fix 2: Guard anti-path-traversal

Cambio original (s220): Validar image_path con realpath() + prefijo de MEDIA_ROOT antes de mover fichero.

real = os.path.realpath(temp_full_path)
if not real.startswith(os.path.realpath(settings.MEDIA_ROOT) + os.sep):
    continue  # Skip este master, no se persiste

Ratificación (s220):

• realpath() resuelve todos los ../ y symlinks
• Comparación con prefijo de MEDIA_ROOT evita escala
• Fallback seguro: master se ignora silenciosamente (no error 400, evita fingerprinting)

⚠️ Superado por v1.92.0 (commit c2df243, #478): confinar a MEDIA_ROOT entero resultó insuficiente — un image_path apuntando a signage/<orgB>/… o backups/… (fuera del temp_dir de la sesión pero dentro de MEDIA_ROOT) seguía resolviendo dentro del prefijo válido, y como el propio código usa shutil.move, un master de OTRA organización se movía (y destruía) hacia la librería del tenant atacante. El guard correcto confina al temp_dir de la sesión actual, no a MEDIA_ROOT:

real = os.path.realpath(temp_full_path)
if not is_within_directory(temp_dir, real):
    continue

Usa el helper racks.utils.safe_fs.is_within_directory (véase [[entity—racks—service—safe-fs]]) en vez de comparar prefijos a mano. Mismo patrón aplicado en confirm_restore_backup y racks/api/export/restore_domains.py::_ensure_media_file en el mismo commit.

Ubicación: racks/api/library_files.py, línea ~212 (s220) → confinamiento estrechado a temp_dir en v1.92.0

Fix 3: Defensa en profundidad — sanitización en confirm

Cambio: Aplicar sanitize_svg_file() en el gate final de confirm, antes de mover a librería.

if filename.lower().endswith(".svg"):
    try:
        sanitize_svg_file(temp_full_path)
    except ValueError:
        continue  # Skip este master

Ratificación:

• SVGs ya se sanitizan aguas arriba (en analyze y en task Huey), pero confirm es la puerta final
• Si la lógica anterior fallara o sesión se manipulara, esto atrapa residuo
• Second pass es bajo costo (fichero ya en disco, parse rápido)

Ubicación: racks/api/library_files.py, línea ~220

Impacto en API contrato

• ✅ No cambios en request payload
• ✅ No cambios en response payload (confirm sigue devolviendo 200 + imported list)
• ⚠️ Código 403 ahora posible (estaba implícito en require_perm, ahora explícito en schema)

Testing

Nueva suite: tests/api/test_visio_import_v2.py

• test_operator_cannot_confirm: operator→403 (fix 1)
• test_confirm_blocks_path_traversal: intento de ../ se ignora silenciosamente, fichero externo no se mueve (fix 2)
• test_confirm_sanitizes_svg_before_persisting: SVG malicioso llega limpio a disco (fix 3)

v1.92.0 añade a tests/racks/test_tanda2_file_confinement.py el caso de un image_path que resuelve dentro de MEDIA_ROOT pero fuera del temp_dir de la sesión (asset de otra org) — antes pasaba el guard de s220, ahora se descarta.

Migración de test existente:

• tests/api/test_racks_visio_confirm.py::TestConfirmVisioImportEmpty::test_empty_masters_is_noop_200
  • Fue: operator_user → 200 noop
  • Ahora: admin_user → 200 noop (alineado con nuevo contrato)
  • Nuevo caso en v2: operator_user → 403

Precedentes

• RLS en confirm de otros imports: racks/api/import_*.py endpoints de confirmación ya validaban permisos
• Sanitización SVG: standard para user-supplied geometry (OWASP XSS Prevention Cheat Sheet)
• Realpath guards: best practice en operaciones de filesystem con paths de usuario — refinado en v1.92.0 a confinar al directorio de trabajo de la sesión, no a la raíz de medios completa

Impacto observacional

• Usuarios admin: ninguno — confirm sigue funcionando igual
• Usuarios operator/viewer: ahora reciben 403 en confirm, eran 200 noop antes (nunca lograban crear stencils porque estaban vacíos, pero no era por permisos)
• Security posture: cierre de 3 vías de ataque identificadas en auditoría (s220) + 1 vía residual cerrada en v1.92.0

Véase también

• [[feature—racks—import-visio-v2]]
• [[entity—racks—service—svg-sanitizer]]
• [[entity—racks—endpoint—visio-confirm]]
• [[entity—racks—endpoint—visio-analyze]]
• [[entity—racks—service—safe-fs]]
• [[concept—saas—security]]