Feature/CWE recommendations related#137
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughSe han realizado cambios significativos en la aplicación FastAPI y en varios componentes del frontend. El archivo Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
🧹 Outside diff range comments (1)
cwe_api/main.py (1)
Line range hint
1-45: Resumen de cambios y recomendaciones finalesLos cambios principales implican la sustitución de
classifierporinferencer. Aunque esto parece ser parte de una refactorización más amplia, hay varias áreas que requieren atención:
- Falta información sobre el módulo
inferencer.- No hay manejo de errores en las llamadas a
inferencer.- La documentación es insuficiente.
- Se utiliza un ejemplo hardcodeado en la función
read_root.Recomendaciones:
- Proporcione documentación detallada sobre el módulo
inferencer.- Implemente un manejo de errores robusto en todas las llamadas a
inferencer.- Añada docstrings y comentarios explicativos donde sea necesario.
- Considere mover el ejemplo a un archivo de configuración.
Estas mejoras aumentarán significativamente la mantenibilidad y fiabilidad del código.
🧰 Tools
🪛 Ruff
22-22: Missing return type annotation for public function
classify_vulnerability(ANN201)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (9)
- cwe_api/main.py (3 hunks)
- frontend/src/files/parent_to_child.json (1 hunks)
- frontend/src/i18n/en-US/index.ts (2 hunks)
- frontend/src/routes/vulnerabilities/add/addVulnerability.tsx (6 hunks)
- frontend/src/routes/vulnerabilities/components/MultiCheckboxButton.tsx (1 hunks)
- frontend/src/routes/vulnerabilities/edit/editVulnerability.tsx (6 hunks)
- frontend/src/routes/vulnerabilities/hooks/useVulnState.ts (3 hunks)
- frontend/src/services/getCWEName.ts (1 hunks)
- frontend/src/services/vulnerabilities.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (8)
cwe_api/main.py (1)
6-6: Se requiere más información sobre el móduloinferencer.La importación del módulo
inferenceres nueva y no está claro de dónde proviene este módulo. Es crucial proporcionar más detalles sobre su origen, estructura e interfaz.Para verificar la existencia y ubicación del módulo
inferencer, ejecute el siguiente script:frontend/src/services/vulnerabilities.ts (1)
74-78: Cambio aprobado, pero verifica su impacto en el resto del código.La modificación del tipo
CWEDataes coherente con los objetivos del PR de mejorar las recomendaciones de CWE. Sin embargo, este cambio podría afectar a otras partes del código que utilizan este tipo.Ejecuta el siguiente script para verificar el uso de
CWEDataen el resto del código:frontend/src/i18n/en-US/index.ts (1)
677-680: Las nuevas entradas de localización son correctas y están alineadas con los objetivos del PR.Las adiciones de 'recommendedCwe', 'recommendCwe', 'showAncestors' y 'hideAncestors' son apropiadas y respaldan directamente la nueva funcionalidad de recomendación de CWE descrita en los objetivos del PR. La implementación es clara y sigue el formato existente en el archivo.
frontend/src/routes/vulnerabilities/hooks/useVulnState.ts (2)
3-7: Nueva definición del tipoCWERelatedimplementada correctamenteLa definición del tipo
CWERelatedestá bien estructurada y utiliza correctamente las propiedades opcionales con?.
111-116: Estados adicionales expuestos correctamenteLos nuevos estados se han añadido adecuadamente al objeto de retorno de
useVulnState, permitiendo su uso en otros componentes.frontend/src/services/getCWEName.ts (1)
37-37: Verificar actualizaciones de llamadas aGetCWENameByLanguageLa firma del método
GetCWENameByLanguageha cambiado para aceptarcweDataArray: CWENumbers[]en lugar decweIdToFind: string. Asegúrate de que todas las llamadas a este método estén actualizadas para evitar errores en tiempo de ejecución.Ejecuta el siguiente script para localizar llamadas al método y verificar su uso:
✅ Verification successful
Verificación completada: No se encontraron llamadas a
GetCWENameByLanguagecon la firma antigua.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Descripción: Busca llamadas a 'GetCWENameByLanguage' con la antigua firma. # Comando: Busca en archivos TypeScript y JavaScript. rg --type ts --type js -P "GetCWENameByLanguage\s*\(\s*\w+\s*,\s*[^)]*\)"Length of output: 75
frontend/src/routes/vulnerabilities/components/MultiCheckboxButton.tsx (1)
57-62: Optimizar la generación de la lista de CWE para mejorar rendimientoEn la generación de
allStrings, estás aplanando el array de CWEs y filtrando valores nulos o indefinidos. SicwesRecommendedes una lista grande, esta operación podría afectar al rendimiento.Considera construir
allStringsdurante el proceso de mapeo inicial o revisar si es posible gestionar el estado sin necesidad de aplanar y filtrar, reduciendo así la complejidad computacional.[performance]
frontend/src/routes/vulnerabilities/add/addVulnerability.tsx (1)
28-29: Considere verificar que 'cweParent' y 'cweGrandParent' no sean nulos antes de usarlos.En la definición del tipo
CWERelated,cweParentycweGrandParentson opcionales. Asegúrese de manejar adecuadamente los casos en que estos valores puedan serundefinedpara evitar posibles errores en tiempo de ejecución.
There was a problem hiding this comment.
Actionable comments posted: 11
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- cwe_api/inferencer.py (1 hunks)
🧰 Additional context used
🪛 Ruff
cwe_api/inferencer.py
14-14: Missing return type annotation for public function
postprocess(ANN201)
14-14: Missing type annotation for
selfin method(ANN101)
14-14: Missing type annotation for function argument
model_outputs(ANN001)
16-16: Unnecessary assignment to
best_classbeforereturnstatementRemove unnecessary assignment
(RET504)
18-18: Missing return type annotation for public function
inferencer(ANN201)
18-18: Missing type annotation for function argument
vuln(ANN001)
47-47: Unnecessary
startargument inrangeRemove
startargument(PIE808)
49-49: Unnecessary assignment to
predictsbeforereturnstatementRemove unnecessary assignment
(RET504)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This reverts commit b178ca8.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (3)
- cwe_api/inferencer.py (1 hunks)
- frontend/src/i18n/en-US/index.ts (2 hunks)
- frontend/src/routes/vulnerabilities/add/addVulnerability.tsx (6 hunks)
🧰 Additional context used
🪛 Ruff
cwe_api/inferencer.py
14-14: Missing return type annotation for public function
postprocess(ANN201)
14-14: Missing type annotation for
selfin method(ANN101)
14-14: Missing type annotation for function argument
model_outputs(ANN001)
16-16: Unnecessary assignment to
best_classbeforereturnstatementRemove unnecessary assignment
(RET504)
18-18: Missing return type annotation for public function
inferencer(ANN201)
18-18: Missing type annotation for function argument
vuln(ANN001)
43-43: Unnecessary
startargument inrangeRemove
startargument(PIE808)
🔇 Additional comments (9)
frontend/src/i18n/en-US/index.ts (4)
686-686: ¡Bien hecho! La adición de 'recommendedCwe' es apropiada.La inclusión de esta cadena de texto para CWE recomendados es coherente con los objetivos del PR y mejorará la funcionalidad relacionada con las recomendaciones de CWE.
687-687: La adición de 'recommendCwe' es acertada y necesaria.Esta cadena de texto proporciona una acción clara para que los usuarios soliciten recomendaciones de CWE, lo cual es fundamental para la nueva funcionalidad que se está implementando.
688-688: La inclusión de 'showAncestors' es crucial para la nueva funcionalidad.Esta cadena permitirá a los usuarios ver las relaciones jerárquicas de los CWE, lo cual es una parte importante de la mejora propuesta en este PR. Es una adición necesaria y bien pensada.
686-689: Las adiciones a las cadenas de localización son apropiadas y bien implementadas.Todas las nuevas cadenas añadidas son coherentes con los objetivos del PR y proporcionarán una interfaz de usuario clara para la nueva funcionalidad de recomendación de CWE. La única sugerencia menor es asegurar la consistencia en el uso de singular o plural para 'Ancestor(s)'.
frontend/src/routes/vulnerabilities/add/addVulnerability.tsx (5)
20-24: Definición adecuada del tipo 'CWEData'La definición del tipo
CWEDataes correcta y sigue las convenciones de TypeScript.
26-30: Definición adecuada del tipo 'CWERelated'La estructura del tipo
CWERelatedestá bien diseñada, y las propiedades opcionales se manejan apropiadamente.
97-101: Inicialización correcta de estados para recomendaciones CWELa declaración de estados con
useStatepara gestionar las recomendaciones de CWE es apropiada y sigue las buenas prácticas de React.
176-177: Reseteo de selecciones y recomendaciones de CWE al cambiar de idiomaEs correcto limpiar las selecciones y recomendaciones de CWE al cambiar el lenguaje, evitando inconsistencias en los datos mostrados al usuario.
232-240: Manejo adecuado del estado de carga durante la solicitud de recomendaciones CWEEl estado
cweLoadingse establece correctamente antes y después de la solicitud, y se gestiona adecuadamente en caso de error, evitando loaders infinitos.
jllanosg
requested review from
Icksir,
caverav,
jllanosg,
massi-ponce and
sealra
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- frontend/src/routes/vulnerabilities/add/addVulnerability.tsx (6 hunks)
- frontend/src/routes/vulnerabilities/edit/editVulnerability.tsx (6 hunks)
🧰 Additional context used
🔇 Additional comments (9)
frontend/src/routes/vulnerabilities/add/addVulnerability.tsx (9)
20-24: Definición correcta del tipoCWEData.La estructura definida para
CWEDataes adecuada y permitirá manejar correctamente los datos relacionados con las CWEs.
26-30: Definición correcta del tipoCWERelated.La declaración del tipo
CWERelatedestá bien construida y soporta opcionalmente los campos para ancestros de las CWEs.
97-101: Gestión adecuada del estado para las recomendaciones CWE.La inicialización de los estados
cweLoading,cweRecommendationSelectedycweRecommendedes correcta y sigue buenas prácticas de React.
176-177: Reset del estado al cambiar el lenguaje.Al cambiar el lenguaje, se está limpiando correctamente las recomendaciones y las selecciones de CWEs, lo cual previene inconsistencias.
243-244: Correcto manejo del estado de carga en el bloquefinally.Al mover
setCweLoading(false);al bloquefinally, asegura que el indicador de carga se detenga incluso si ocurre una excepción.
469-470: 🧹 Nitpick (assertive)Utilice un componente de carga estándar para mejorar la accesibilidad.
El uso de
ArrowPathIconcomo indicador de carga es poco convencional. Sería más apropiado utilizar un componente de spinner estándar para mejorar la experiencia del usuario y la accesibilidad.Considere este cambio:
<ArrowPathIcon className="h-8 w-8 animate-spin text-blue-500" /> +<Spinner className="h-8 w-8 text-blue-500" />No olvide importar el componente
Spinnercorrespondiente.Likely invalid or redundant comment.
464-465:⚠️ Potential issueCorrija la ortografía de 'handleCWERecomendation' a 'handleCWERecommendation'.
El nombre de la función está mal escrito. Debería ser
handleCWERecommendationpara mantener la consistencia y claridad en el código.Aplique este cambio:
<PrimaryButton onClick={() => handleCWERecomendation()}> +<PrimaryButton onClick={() => handleCWERecommendation()}>Asegúrese de actualizar todas las referencias a esta función en su código.
Likely invalid or redundant comment.
251-251:⚠️ Potential issueEvite duplicados al actualizar la lista de CWEs.
Al combinar
cweRecommendationSelectedyprevDetail.cwes, podrían surgir duplicados en la lista de CWEs. Es importante mantener una lista única para evitar redundancias.Puede modificar el código para eliminar duplicados:
cwes: [...cweRecommendationSelected, ...prevDetail.cwes], + cwes: [...new Set([...prevDetail.cwes, ...cweRecommendationSelected])],Esto garantizará que la lista de CWEs sea única y no contenga elementos repetidos.
Likely invalid or redundant comment.
481-487: 🧹 Nitpick (assertive)Añada atributos ARIA para mejorar la accesibilidad del botón de confirmación.
Para incrementar la accesibilidad, es recomendable añadir un atributo
aria-labelotitleal botón de confirmación.Puede hacerlo de la siguiente manera:
<PrimaryButton onClick={() => { handlerSubmitCWE(); }} + aria-label={t('btn.confirmSelection')} >Likely invalid or redundant comment.
3 participants
Descripción
Se agrega el lógica y los cambios necesario para la recomendación de CWE. Ahora se hacen 3 (más probables) y sus ancestros (padre y abuelo) en caso de existir. Se agrega la funcionalidad en añadir y editar vulnerabilidades. Además, se utiliza el modelo de recomendación mejorado. Made by @Icksir
Importante: Puede que se repitan recomendaciones entre un ancestro y las otras recomendaciones, se puede modificar para quitar ese caso, pero por temas de tiempo considero que debería quedar como issue.
Motivación y Contexto
Este cambio es necesario para la funcionalidad de recomendación, ya que antes se limitaba a una sola recomendación.
¿Cómo ha sido probado?
-) Se va la vista de agregar o editar una vulnerabilidad, se introduce la descripción de una vulnerabilidad y se apreta el botón de recomendar CWE.
-) Una vez se tiene la recomendacióno, se cambia de idioma en el dropdown que se encuentra arriba a la derecha y se ve que se vuelve al estado original.
-) Se seleccionan recomendaciones, se apreta el botón de añadir opciones y confirmar que se agregaron los CWEs al inicio del TextArea y desaparece la recomendación.
Capturas de pantalla (si es apropiado):
Tipos de cambios
Lista de verificación:
Summary by CodeRabbit
Nuevas Funciones
AddVulnerabilitypara gestionar recomendaciones de CWE.EditVulnerabilitypara manejar el estado de carga y recomendaciones de CWE.MultiCheckboxButtonpara seleccionar múltiples entradas de CWE.Mejoras
CWEDatapara incluir un campo deprioridaden los resultados.