basquetWi + New ticket
movilba MOVILBA-538

El estado del colectivo se escribe sin chequear que pertenezca al catalogo de estados: la FK a opciones deja pasar el id de cualquier otro campo

Done normal cmcoder03-mba-l-cc

src/repositories/vehiculos.ts acepta cualquier entero como estado del colectivo, en los DOS caminos de escritura: createColectivo linea ~803 hace parseInt(colectivoData.estadoId) sin radix, sin guard de NaN y sin chequear pertenencia al catalogo; updateColectivoCompleto linea ~1220 agrega el guard de NaN (MOVILBA-344) pero tampoco chequea pertenencia. La FK estado_id -> opciones.id NO alcanza como control: prueba que el id existe en opciones, no que sea un estado de vehiculo, asi que un id de tramite o de carroceria pasa la FK. Escenario de falla: un POST con estadoId de otro campo del catalogo entra; esBaja (linea ~1272, comparacion contra el id de BAJA) da false, y se appendea un periodo ABIERTO con un estado que no es ni ALTA ni BAJA - invisible para v_vehiculos_list_alta y v_vehiculos_list_baja, que es la misma forma de MOVILBA-316 (fichas con estado_id contradiciendo al historial). El cliente ya solo ofrece ALTA y BAJA (comentario en linea ~277), asi que el agujero es puramente de servidor. FIX: un solo helper que resuelva el estado crudo contra el catalogo vigente y tire ApiException(VALIDATION_FAILED), reemplazando los dos bloques inline, no conviviendo con ellos. La lista de estados se resuelve en runtime del catalogo, NUNCA una lista literal de ids (regla de Elazar sobre ids). POSTCONDICION A MEDIR POR EL CARRIL DE BASE: contar en DEV y PROD las filas de vehiculos_datos_colectivos con estado_id fuera del catalogo de estado de vehiculo; si da > 0 el agujero ya se ejercito.

Sub-tickets

No sub-tickets.
+ Add sub-ticket

Activity

  • wi-cli-venus created · 3w ago
  • wi-cli-venus note · 3w ago
    STARTED MOVILBA-538 at 2026-09-22T16:21:17-03:00
  • wi-cli-venus commit · 3w ago
    d246b009 v5.6.16 en dev: helper resolverEstadoColectivo(ejecutor, estadoCrudo) unico, reemplaza los dos bloques inline (createColectivo ~803 y updateColectivoCompleto ~1220), no convive con ellos. Parsea con radix, guard de NaN, y pertenencia leida en runtime de v_opciones por (grupo_nombre='vehiculo', campo_nombre='estado') - sin lista literal de ids. Entrada invalida -> ApiException(VALIDATION_FAILED); catalogo vacio -> Error, porque es rotura de configuracion, no entrada del usuario. Lint y typecheck pasaron en el deploy; health de dev01 sirviendo patch 16.
  • wi-cli-venus statusChanged · 3w ago
    Codigo shipeado en DEV. Falta: (1) el control NEGATIVO no esta medido - nadie mando un estadoId de otro campo contra dev01, asi que el camino de rechazo se leyo pero no se vio fallar; (2) la postcondicion de datos la debe el carril de base (conteo de estado_id fuera del catalogo en DEV y PROD). Hasta que esas dos existan esto es codigo revisado, no control probado.
  • wi-cli-venus verified · 3w ago
    Postcondicion medida por db03-mba-l-cc 2026-09-22 16:24 ART, SELECT only. vehiculos_datos_colectivos con estado_id fuera de (grupo='vehiculo', campo='estado'): 0 de 2422 en DEV y 0 de 2413 en PROD, cero NULL en las dos. Catalogo vivo identico: 116=ALTA, 117=BAJA. El agujero NO fue ejercitado en la tabla viva. En vehiculos_datos_colectivos_archive de DEV hay 68 filas con estado_id 118 y 119, pero NO son evidencia de este bug: eran opciones legitimas del MISMO grupo y campo cuando se escribieron (PROD conserva 118 = valor_opcion 'TRANSFERENCIA' con borrado_en 2025-10-13; 119 era MANTENIMIENTO hasta el hard delete de MOVILBA-534, hoy). Este bug escribe un id de OTRO campo; aquello es un id del campo correcto que dejo de existir. LO QUE SIGUE SIN MEDIRSE: el camino de RECHAZO. Cero filas malas es igual de consistente con 'el helper anda' que con 'nadie mando nunca un estadoId ajeno', asi que esto todavia es una linea verde, no un control. Solo un intento real con un id de otro campo contra dev01 lo convierte en control, y eso es una escritura por pantalla, no una medicion.
  • wi-cli-venus note · 3w ago
    Una cobertura que la medicion de db03 hace pertinente y que SI queda cerrada: el helper resuelve contra v_opciones, que filtra borrado_en IS NULL (.claude/rules/catalogos.md:13), asi que un estado RETIRADO del campo correcto - el 118 de PROD, soft-deleted - tambien se rechaza, no solo un id de otro campo. No bloquea a nadie hoy: cero filas vivas en DEV y PROD llevan 118 o 119, y el camino de edicion solo valida cuando el cliente manda estadoId explicito.
  • wi-cli-venus completed · 3w ago
    Cerrado con auditoria de audit-mba-l-ca: APPROVED sobre 98cc58c5, mas el seguimiento no bloqueante ya shipeado en b688a84f. Shipeado en tres commits: d246b009 (v5.6.16, el control inicial), 98cc58c5 (v5.6.17, los dos blockers de audit) y b688a84f (v5.6.18, sintaxis decimal + doc). Que quedo: resolverEstadoColectivo en src/repositories/vehiculos.ts es el unico punto por el que pasan los DOS caminos de escritura de vehiculos_datos_colectivos.estado_id (createColectivo y applyColectivoEditWithHistory; audit confirmo que no hay un tercero en src/). Resuelve la fila exacta contra v_opciones - vista comun, no materializada - filtrando (grupo_nombre='vehiculo', campo_nombre='estado') Y valor_opcion IN ('ALTA','BAJA'), y devuelve {id, valorOpcion} para que el camino de edicion derive esBaja de la misma lectura. La resolucion del alta se movio ADENTRO de la transaccion. Sin fila: se re-consulta el catalogo para distinguir 'id invalido' (VALIDATION_FAILED, culpa del usuario) de 'catalogo vacio' (Error, rotura de configuracion). Los dos defectos reales que encontro audit sobre el primer commit, los dos de la familia 'el control se comia justo lo que venia a rechazar': 1. parseInt aceptaba '116junk' como 116. Peor, sanitizeInt en src/actions/vehiculos.ts convertia lo malformado en null, y null significa CAMPO AUSENTE aguas abajo, asi que el rechazo se tragaba en silencio en vez de rechazar. Ahora tira ApiException(VALIDATION_FAILED), y cubre todos los campos enteros de ese sanitizer. 2. La pertenencia al campo es MAS DEBIL que el invariante: el ciclo es binario porque v_vehiculos_list_alta y v_vehiculos_list_baja son sus dos unicos consumidores, asi que cualquier tercera opcion que se repusiera reabria la forma de MOVILBA-316 sin tocar una linea de codigo. PROD todavia guarda un TRANSFERENCIA borrado logico (118, 2025-10-13). Ahora los VALORES canonicos son la regla y los ids se leen en runtime. Y uno tercero, no bloqueante: Number('0x74'), Number('0b1110100') y Number('1.16e2') dan los tres 116 y pasaban Number.isInteger. Se valida sintaxis decimal como TEXTO antes de convertir. El control se VIO RECHAZAR, no solo pasar: tools/tests/colectivo-estado-catalogo.test.mjs, 7 tests contra la base DEV real (SELECT-only: el rechazo dispara antes de cualquier INSERT), con control positivo al lado de cada negativo y sin un solo id literal - se leen del catalogo vivo. Es el primer test del repo que toca una base de verdad; hasta ahora todos eran mocks, uno incluso afirma 'Unexpected DB access'. Revertir SOLO el parseo a parseInt pone roja la asercion de cola de basura, y revertirlo a Number() pone roja la de las otras bases: las dos se vieron fallar contra el comportamiento anterior y verdes despues. test:tools pasa a correr bajo tsx para levantarlo. Documentado en docs/reference/opciones-catalogo.md, seccion 'Estados de vehiculo'. Fuera de alcance, ya filtrado a su propio ticket: MOVILBA-541 (test:tools en 85/89, 4 fallas que preceden a d246b009).
bug
3w ago by wi-cli-venus
3w ago
2026-09-23 07:55