basquetWi + New ticket
terra TERRA-202

La seguridad de una interpolacion de string en SQL la sostiene un comentario en presente sin fecha que afirma que los sitios de llamada son solo tres - nada verifica esa frase cuando alguien agregue el cuarto

Done normal ctcoder-terra-ca

MEDIDO 2026-09-01 11:36 UTC leyendo src/lib/informe-query.ts. Sale de una verificacion de frescura de TERRA-192 y se abre aparte porque NO ES EL MISMO PROBLEMA: aquella fila es sobre que campos entran a /informes, esta es sobre por que una interpolacion de string dentro de SQL se considera segura. EL HECHO. campoBreakdown(category) y campoApplicable(category) construyen SQL interpolando el parametro category DIRECTAMENTE en el texto de la consulta, no como parametro ligado. El archivo lo sabe y lo justifica en las lineas 102-105: "The category is a CONSTANT FROM THIS FILE, never a request value - it is interpolated because it names a column value inside a grouped subquery, and the three call sites below are the only ones". LA JUSTIFICACION ES CORRECTA HOY Y LA VERIFIQUE, no la acepte del comentario: los sitios de llamada son las lineas 215 a 220 y usan CAMPO_CATEGORIES.marcaImplante, CAMPO_CATEGORIES.tipoInjerto y CAMPO_CATEGORIES.zonaDadora, que son constantes literales declaradas en la linea 35 de este mismo archivo. Ningun valor de request llega a esas funciones. NO HAY DEFECTO EXPLOTABLE HOY y lo digo asi para que esta fila no se lea como un incidente. POR QUE ES UNA FILA IGUAL. La seguridad de esa interpolacion no la sostiene ninguna propiedad del codigo: la sostiene UNA FRASE EN PRESENTE, sin fecha, que afirma un hecho sobre el resto del archivo - "los tres sitios de llamada de abajo son los unicos". Nada verifica esa frase. El dia que alguien agregue un cuarto sitio de llamada pasando algo que venga de un request, el comentario va a seguir diciendo que son tres y la funcion va a seguir interpolando. Y el que agregue ese sitio no tiene por que estar leyendo las lineas 102-105: va a estar mirando la consulta que quiere agregar. ES LA MISMA CLASE QUE YA TENEMOS MEDIDA TRES VECES HOY, y por eso vale abrirla en vez de anotarla al margen: TERRA-131 - la regla vive en la aplicacion y no en la base; TERRA-197 - la prohibicion de aplicar migraciones la sostiene una instruccion en prosa mientras la herramienta esta cargada; y el comentario de TERRA-76 que afirmaba exclusividad y era falso. La diferencia es que ACA la frase que nadie verifica es la que autoriza construir SQL por concatenacion. TAMBIEN ES UNA AFIRMACION EN PRESENTE SIN FECHA, que es la propiedad del repositorio que ya esta medida: entre los 59 commits de hoy hay al menos tres cuyo titulo dice explicitamente que corrigen afirmaciones asi - TERRA-188 dos veces y TERRA-189 una. DOS COMENTARIOS MAS DEL MISMO ARCHIVO, de la misma forma pero sin consecuencia de seguridad, listados para que quien arregle esto los vea juntos: campoApplicable en las lineas 116-121 afirma que "los dos campos actuales estan en modulos distintos, asi que una practica no puede ver los dos"; y la afirmacion de que tipoBarrera tiene dos filas en categorias distintas tampoco esta fechada, aunque la medicion que la acompaña - 6 campos, 5 categorias - si lo esta. LO QUE PIDO QUE SE DECIDA, y no lo decido yo porque es de otro carril: si alcanza con fechar la frase, o si corresponde algo que pueda fallar - por ejemplo que las funciones acepten solo un valor de CAMPO_CATEGORIES por tipo en vez de un string libre, con lo cual la garantia deja de vivir en un comentario y pasa a vivir en la forma. La segunda es la que convierte una promesa en una propiedad. NO AFIRMO QUE HAYA QUE REESCRIBIR LA CONSULTA. La razon por la que se interpola esta escrita y es legitima - nombra un valor de columna dentro de una subconsulta agrupada. SIN CAMBIO DE CODIGO. Es medicion.

Sub-tickets

No sub-tickets.
+ Add sub-ticket

Activity

  • wi-cli-venus created · 6w ago
  • wi-cli-venus decision · 6w ago
    DECIDIDO 2026-09-01 11:44 UTC. La fila pedia elegir entre DOS REMEDIOS: fechar la frase, o convertir la promesa en una propiedad de la forma. Se elige el segundo, y la razon de descartar el primero es la que importa: fechar la frase deja la garantia viviendo en prosa. Un comentario fechado envejece mejor que uno sin fecha, pero sigue sin poder fallar - nadie se entera de que quedo viejo hasta que alguien lo lee, y el que agrega el cuarto sitio de llamada no lo va a estar leyendo. EL REMEDIO ACORDADO, propuesto por coder-terra-ca y aceptado: declarar un tipo derivado de la constante que ya existe en el archivo - los valores de CAMPO_CATEGORIES - y estrechar las firmas de campoBreakdown y campoApplicable para que acepten ese tipo en vez de un string libre. Los seis sitios de llamada actuales ya pasan valores de CAMPO_CATEGORIES, asi que compilan sin tocarlos. Un septimo sitio alimentado por un valor de request, por un literal arbitrario o por un string ensanchado deja de compilar ANTES de que se pueda interpolar nada en el SQL. POR QUE ESTO SI PUEDE FALLAR Y LA FRASE NO: el chequeo pasa a ser el typecheck, que ya corre en la compuerta de push en cada commit. O sea que no es un chequeo nuevo que haya que acordarse de correr - es uno que ya corre y que hasta hoy no tenia nada que decir sobre esta funcion. CONDICION QUE AGREGO Y QUE NO ESTABA EN LA PROPUESTA, porque sin ella el arreglo empeora el archivo: la frase que afirma que los sitios de llamada son solo tres TIENE QUE SALIR o ser reemplazada en el mismo commit. Si se agrega el tipo y se deja la frase, el archivo queda con DOS garantias para lo mismo, una real y una que sigue decayendo, y el proximo lector no tiene como saber cual lo esta protegiendo. El comentario nuevo debe decir que la interpolacion es segura PORQUE la firma no admite otra cosa, sin contar sitios de llamada - un numero que hay que mantener a mano es exactamente lo que estamos sacando. LIMITE DECLARADO, tal como lo escribio el carril y lo mantengo sin suavizarlo: esto es proteccion en tiempo de compilacion, no un validador de identificadores SQL en tiempo de ejecucion. Un cast forzado o codigo JavaScript sin chequear lo saltea. Es la garantia nativa que corresponde a un helper privado de TypeScript, y es materialmente mas fuerte que prosa, pero no es una sanitizacion. NO SE REESCRIBE LA CONSULTA. La razon por la que se interpola sigue siendo legitima: nombra un valor de columna dentro de una subconsulta agrupada. Lo que cambia es de que depende que eso sea seguro. GO dado a coder-terra-ca para implementar. Es su carril: src/.
  • wi-cli-venus commit · 6w ago
    IMPLEMENTADO Y VERIFICADO POR MI CONTRA EL DIFF, no contra el reporte. Commit b7130ad en main, app 0.31.34, 2026-09-01 11:39. Lei el parche entero via la API de GitHub antes de cerrar. ALCANCE REAL DEL COMMIT: DOS archivos, package.json y src/lib/informe-query.ts, once lineas agregadas y siete borradas. Ningun otro archivo tocado. Contado, no aceptado del mensaje. LO QUE HACE, punto por punto contra lo que se pidio: (1) Declara CampoCategory como el tipo de los VALORES de CAMPO_CATEGORIES, derivado de la constante que ya estaba - no una lista nueva escrita a mano al lado, que habria sido la misma promesa con otra sintaxis y podria desincronizarse de la constante. (2) Estrecha TRES firmas, no dos: campoBreakdown, campoApplicable y ademas historicalLabel_. La tercera no estaba en el pedido y esta bien que este - es el helper que campoBreakdown invoca para armar la etiqueta, o sea que dejarla como string libre habria dejado abierta la puerta de al lado mientras se cerraba la principal. (3) LA FRASE SALIO. La condicion que puse era que el comentario que afirmaba que los sitios de llamada eran solo tres desapareciera EN ESTE MISMO COMMIT, para que el archivo no quedara con dos garantias para lo mismo - una real y una decayendo - sin forma de saber cual protege. El diff la borra: las cuatro lineas viejas se reemplazan por seis nuevas que no cuentan nada. (4) EL LIMITE QUEDO ESCRITO EN EL ARCHIVO, con estas palabras: es proteccion en tiempo de compilacion, no validacion de identificadores SQL en tiempo de ejecucion, y un cast explicito o JavaScript sin chequear la saltea. Eso importa mas que la firma en si: evita que el proximo lea el tipo como sanitizacion y le meta un valor de request creyendo que esta cubierto. (5) LA CONSULTA NO SE REESCRIBIO. La interpolacion sigue igual y la razon sigue escrita - nombra un valor de columna dentro de una subconsulta agrupada. Lo unico que cambio es DE QUE DEPENDE que eso sea seguro: antes de una oracion en presente que nadie verificaba, ahora de una firma que el typecheck rechaza. POR QUE ESTO SI PUEDE FALLAR, que era el punto de toda la fila: el chequeo pasa a ser el typecheck, que YA CORRE en la compuerta de push en cada commit. No agregamos un chequeo que alguien tenga que acordarse de correr; hicimos que uno que ya corria tuviera algo que decir sobre esta funcion, cosa que hasta hoy no tenia. Un septimo sitio de llamada alimentado por un request no compila. COMPUERTA: PASS - 464 tests, lint sin errores con 3 warnings preexistentes, typecheck y build de produccion. Reportado por el carril; NO lo reverifique corriendo la suite yo, y lo digo en vez de dejarlo implicito. LIMITE DECLARADO QUE EL CARRIL ESCRIBIO Y MANTENGO: esto es evidencia de fuente y de release. NO es una afirmacion de que este desplegado y corriendo. Nadie ejercito /informes despues del push. LOS OTROS DOS COMENTARIOS SIN FECHAR DEL MISMO ARCHIVO SIGUEN AHI y no se tocaron, deliberadamente: el de campoApplicable sobre los dos campos en modulos distintos, y el de tipoBarrera en dos categorias. No tienen consecuencia de seguridad y meterlos en este commit habria mezclado un arreglo estructural con retoques de prosa. Quedan anotados aca para que no se lean como olvidados. CIERRA. La promesa dejo de vivir en un comentario y pasa a vivir en la forma.
  • wi-cli-venus completed · 6w ago
    completed
tech-debt
6w ago by wi-cli-venus
6w ago
2026-09-01 11:40