fix: trivyignore 3 CRITICAL CVEs in Debian base image - #36
Conversation
- Move agent YAML configs from agents/bricks/ to agents/ root level - Trace events replace messages as the single source of truth - Message entity reduced to a computed projection from trace events - Agent runner rewritten to emit TraceEvents (HUMAN_MESSAGE, THINKING, CONTENT, TOOL_CALL, TOOL_RESULT, AI_MESSAGE) - New routes, use cases, repositories and error message catalogue - Alembic migrations 005-007 for trace_events table
Kaiohz
left a comment
There was a problem hiding this comment.
Code review — PR #36
Score global : 1/10
🚨 Problème bloquant #1 — Scope complètement不符 avec le titre
Le titre annonce :
fix: trivyignore 3 CRITICAL CVEs in Debian base image
On s'attend à un seul fichier touché : .trivyignore, +3 lignes.
Réalité : 79 fichiers modifiés, +4238 / -1357 lignes, avec en autres :
- 3 nouvelles migrations Alembic (
005_create_trace_events_table.py,006_migrate_messages_to_trace_events.py,007_drop_messages_table.py) - Un nouveau domaine
trace_event(entity, port, repo PostgreSQL, 189 lignes danspostgres_trace/adapter.py) - Refonte complète de
src/infrastructure/deepagent/adapter.py(+371/-173) src/infrastructure/deepagent/schema_utils.py(+84/-42)src/infrastructure/deepagent/factory.py(+48/-84)- Refonte de
src/application/use_cases/send_message.py(+74/-63) etstream_message.py(+42/-64) README.md(+260/-40)- 4 nouveaux YAML d'agents, restructuration de
agents/(flatten + bricks supprimés) - 4 nouveaux fichiers de tests, plusieurs tests existants refactorés
Le 1er commit (219bd428d9) est :
feat: trace events as single source of truth + flatten agents folder
C'est une refonte majeure de l'architecture du service, avec en particulier un drop de table (messages → trace_events). C'est exactement le genre de PR qui doit aller sur main séparément, dans sa propre PR dédiée, avec sa propre review approfondie (breaking change au niveau schéma DB, impact sur les consumers de la websocket, etc.).
Pourquoi c'est grave :
- La review trivyignore n'a aucun sens si on merge en même temps que la migration
007_drop_messages_table.py— personne ne pourra reverter l'un sans l'autre. - Le titre
fix:(semver patch) sous-entend un changement non-breaking. En réalité on a un drop de table et un refacto d'adapter : c'est dufeat!ou au minimumfeat:. - Si la CI trivyignore est cassée sur
main, on ne pourra pas reverter le commit trivyignore sans revert aussi la refonte, et inversement. - Le commit
feat: trace events as single source of truth + flatten agents foldern'a probablement même pas été review (le diff montre un refacto de +371 lignes suradapter.py).
✅ Ce qui est OK dans la PR (le seul vrai contenu)
Le .trivyignore lui-même est propre et bien documenté :
- 3 CVE au format attendu (CVE-YYYY-NNNN).
- Chaque entrée a un commentaire avec : composant affecté, résumé, raison du
trivyignore(fix_deferred/affected), date de review. - Justification de réduction du risque ("The service does not parse untrusted XML input directly", "The service does not invoke Perl or Storable") — c'est ce qu'on attend.
- Date
Review: 2026-07-23cohérente avec la date du commit.
Le changement trivialignore pourrait être mergé tel quel dans une PR isolée, score 9/10 sur le fond.
Mais : aucun ticket / issue lié n'est mentionné dans la PR body. Pour un trivyignore on devrait idéalement avoir un lien vers l'issue de tracking (expiration du ignore, qui le revoit, quand).
📋 Actions attendues avant merge
Obligatoire — choisir l'une des deux :
-
Option A (préférée) : Séparer en 2 PRs.
fix: trivyignore 3 CRITICAL CVEs— 1 fichier, 19 lignes. Merge rapide.feat!: refactor trace events as single source of truth + flatten agents folder— la vraie refonte, avec sa propre review de fond, son CHANGELOG, et probablement une discussion sur la migrationmessages→trace_events(breaking, downtime, dual-write window, etc.).
-
Option B : Renommer cette PR en
feat!: refactor trace events + flatten agents + trivyignore base imageet la traiter comme une PR majeure avec une review complète. Mais honnêtement la taille justifie déjà une PR dédiée, donc je ne recommande pas.
Recommandé en plus :
- Ajouter un lien vers une issue / un ticket qui justifie le
trivyignore(avec date de revue prévue — ex. "à re-checker dans 30j quand Debian unstable publie le fix backporté"). - Vérifier que les dates des 3 CVE sont bien réelles et pas des placeholder (
CVE-2026-6653,CVE-2026-13221,CVE-2026-57433— je n'ai pas vérifié en base, mais le naming pattern est cohérent). - Ajouter les CVE au rapport de scan interne (si vous en avez un) pour ne pas les oublier.
📊 Détail du score
| Critère | Note | Commentaire |
|---|---|---|
| Scope / packaging | 1/10 | Titre ne reflète pas du tout le contenu. Refonte majeure embarquée. |
.trivyignore lui-même |
9/10 | Propre, bien commenté, justifications présentes. |
| Qualité du diff global (trace events) | non évalué | Hors scope de cette review — demande une review dédiée. |
| CI | ✅ | test job passed. |
| Lisibilité PR body | 6/10 | Clair sur le trivyignore, mais ne mentionne pas la refonte. |
🤖 Review postée par SoluBot suite au webhook CI terminé. Si vous voulez que je re-review la PR feat!: trace events après split, ping-moi.
Changes
Add 3 CRITICAL CVEs to .trivyignore that have no fix available in Debian bookworm:
These are OS-level vulnerabilities in the Docker base image, not in our application code. The service does not parse untrusted XML or invoke Perl.