-
Notifications
You must be signed in to change notification settings - Fork 0
Chore/security/ci code scan #329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
1 similar comment
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
|
1 similar comment
|
ec1d1b5 to
2b0da57
Compare
|
|
|
|
|
|
Clemsazert
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L'approche avec un job générique oblige à faire pas mal de configuration sur les briques qui ne nécessitent pas certaines étapes (check Dockerfile pour csv_parser & nomenclature_parser). Elle rend aussi le fichier du job peu lisible avec les nombreux conditionnels qui s'y trouvent. Aussi il n'est pas possible de controller quand est ce qu'un job doit être joué en fonction de changements qui affecteraient uniquement certains fichiers specifiques (ex : je ne veux pas lancer l'analyse du csv_parser si j'ai juste fait des modifications dans le converter). On est déjà arrivé certains mois en limite du nombre de minutes de CI autorisée par Github, j'ai peur qu'on atteigne encore plus facilement cette limite.
| SCAN_PATH: ${{ inputs.scan-path }} | ||
| run: | | ||
| if [ ! -f "$SARIF_FILE" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pourquoi le script diffère de l'action équivalente sur le répo Santé ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Juste la partie qui remplace les paths, qui permettais de convertir les chemins absolus en chemins relatifs.
Et après vérification, les SARIF générés n’utilisent jamais de chemins absolus, j’ai donc supprimé cette complexité et conservé une logique jq simple du répo Santé.
| severity: 'CRITICAL,HIGH,MEDIUM' | ||
|
|
||
| - name: Upload Trivy image scan results to GitHub Security tab | ||
| if: steps.extract_info.outputs.project == 'converter' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pas besoin de fixer les path dans les SARIF files ici ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non pas besoin, dans les releases, on ne fait que des scans d'images, et les images n'ont pas de paths
|
| with: | ||
| working-directory: '.' | ||
| component-name: 'library' | ||
| dependency-scan: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pourquoi le dependency scan n'est pas joué ici ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dependency-scan permet juste de lancer un uv-security pour python. Finalement j'ai complètement enlever cette option car pas encore de projet en uv sur ce repo
|
Je ne vois pas particulièrement de retour à faire sur ton code, mais je me demande si on peut le merge en l'état Sur le repo Santé où ce travail a déjà été fait, toutes les CI de scan sont au rouge et je ne suis pas très clair sur ce qu'on veut en faire Ce n'est pas pour remettre en cause ton travail, mais plus le besoin en amont, maintenant qu'on a un peu plus de recul avec le travail qui a déjà été fait sur le repo Santé On pourra voir ça en daily avec l'équipe, mais dispo pour en discuter d'ici là si tu veux ! |
|
🔎 Détails
Cette pull request ajoute la configuration d’analyse de code (code scan) dans le pipeline CI.
📄 Documentation
📸 Captures d'écran
🔗 Ticket associé
Asana