Skip to content

post-DynDOLOD: reproducir hallazgos no verificados de la auditoría antes de modificarlos #576

Description

@FacundoSu1986

Contexto

Este issue conserva los hallazgos de la auditoría que no están demostrados contra el main actual y que merecen una reproducción dirigida antes de abrir PRs.

Backlog deliberadamente posterior a terminar DynDOLOD. No debe bloquear PR-2 / staging externo / rig / refactor final.

Baseline revisado: main @ 6dff01171378bbc525b52c8584fbbb8666fc2a8c (2026-09-11).

Regla para este issue: no implementar fixes hasta conseguir una reproducción o una invariante rota concreta.

1. CoreEventBus._drain_abandoned_queue() — impacto real de latencia

La función drena sincrónicamente con get_nowait() + task_done(). El código existe, pero no está demostrado que el coste sea suficientemente alto como para justificar un rediseño ni mucho menos un P0.

Verificación

  • Benchmark/event-loop probe con cola al límite operativo relevante.
  • Medir latencia máxima y duración total del drain.
  • Solo modificar si se incumple un presupuesto explícito.

2. FileSnapshotManager.create_snapshot() — mutación externa del source durante snapshot

Existe una carrera filesystem genérica entre pre-checks y operaciones posteriores, pero mover exists()/is_file() dentro de un asyncio.Lock no resuelve carreras contra otros procesos.

El código actual convierte OSError del cuerpo en JournalSnapshotError; no está demostrado un fallo de integridad o excepción sin encapsular.

Verificación

  • Test que borre/reemplace/trunque el source durante checksum/stat/copy.
  • Comprobar que no queda snapshot parcial presentado como válido.
  • Comprobar limpieza de sidecar y artefactos temporales ante error.
  • Solo proponer locking adicional si protege una invariante real.

3. NetworkGateway — DNS rebinding end-to-end

El gateway actual valida IPs, usa SafeResolver y comparte DNS pin cache entre connectors. Ya existe cobertura de shared pinning, por lo que la hipótesis original de una carrera DNS no está demostrada.

Verificación

  • Test end-to-end/socket-level que fuerce respuestas DNS distintas sucesivas.
  • Confirmar que el peer efectivo usado por aiohttp coincide con una IP validada/pineada.
  • Incluir IPv4/IPv6 y redirects si forman parte del camino real.
  • No tocar el resolver si la propiedad ya queda demostrada.

4. /api/chat — rate limiting productivo

La autenticación sí está implementada y testeada; ese supuesto P0 queda descartado. Lo que no quedó verificado en esta pasada es si existe un rate limit efectivo para /api/chat en otra capa productiva o si el endpoint depende únicamente de controles externos.

Verificación

  • Buscar rate limiting en WebApp, composition root, reverse proxy/empaquetado y entrypoints productivos.
  • Definir threat model local/desktop real antes de añadir middleware.
  • Si falta y se justifica, test de burst + ventana + respuesta estable.

5. TextInspector compartido a nivel módulo

router.py mantiene _HISTORY_INSPECTOR y _TOOL_RESULT_INSPECTOR como instancias compartidas y el comentario afirma que son stateless. En esta pasada no se verificó exhaustivamente la implementación interna de TextInspector.

Verificación

  • Confirmar que no posee estado mutable por request/session.
  • Test concurrente de dos chats con inputs distintos si existe estado interno.
  • Si es stateless, documentar la propiedad y cerrar este punto sin refactor.

6. Gaps de auditoría que requieren pasada dedicada antes de declarar release-ready

La auditoría original reconoció que no cubrió a fondo áreas de alta centralidad. Conservar como checklist futura:

  • app_context.py / composition root.
  • config.py / defaults, env vars y secrets.
  • logging_config.py / redacción y audit trail.
  • __main__.py / dispatch de modos.
  • sky_claw.spec / empaquetado PyInstaller y ausencia de secretos embebidos.
  • requirements.lock / auditoría de CVEs con herramienta apropiada.
  • tests/bdd/ y tests/agent/ / cobertura real.

Hallazgos que NO deben reabrirse salvo evidencia nueva

Estos puntos del informe original contradicen el main actual o ya están resueltos:

  • XML tool parser con U+200B: el parser actual usa <tool_call> normal.
  • Credential vault sin O_CREAT | O_EXCL: actualmente usa O_CREAT | O_EXCL y O_NOFOLLOW cuando está disponible.
  • /api/chat y /ws/ui sin auth: existe AuthTokenManager y comportamiento fail-closed.
  • Gateway Node sin auth: exige WS_AUTH_TOKEN, requireAuth() y timingSafeEqual.
  • Guardrail agent y orchestrator separados: son scopes distintos y la separación actual es deliberada.

Criterio de cierre

Cada subhallazgo se cierra con una de estas salidas:

  1. REPRODUCIDO → abrir issue/PR específico con test rojo y severidad basada en impacto.
  2. NO REPRODUCIDO / propiedad demostrada → documentar evidencia y marcar cerrado sin cambio productivo.
  3. NO APLICA al camino productivo → documentar por qué y cerrar.

No convertir "vale verificar", "parece" o "no lo audité" en P0 sin reproducción.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions