Skip to content

fix: trivyignore 3 CRITICAL CVEs in Debian base image - #36

Merged
Kaiohz merged 2 commits into
mainfrom
fix/trivyignore-debian-cve
Jul 23, 2026
Merged

fix: trivyignore 3 CRITICAL CVEs in Debian base image#36
Kaiohz merged 2 commits into
mainfrom
fix/trivyignore-debian-cve

Conversation

@Kaiohz

@Kaiohz Kaiohz commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Changes

Add 3 CRITICAL CVEs to .trivyignore that have no fix available in Debian bookworm:

  • CVE-2026-6653 (libxml2: DoS via crafted XML — fix_deferred)
  • CVE-2026-13221 (perl-base: incorrect regex matching — affected, no fix)
  • CVE-2026-57433 (perl-base: Storable integer overflow — affected, no fix)

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.

Kaiohz added 2 commits July 23, 2026 13:22
- 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 Kaiohz left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review — PR #36

Score global : 1/10 ⚠️

⚠️ Cette review n'est pas une review de qualité code — c'est un blocage critique sur le scope de la PR. Les changements eux-mêmes semblent cohérents, mais le packaging de la PR est complètement cassé et ne peut pas être mergé en l'état.


🚨 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 dans postgres_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) et stream_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 (messagestrace_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 :

  1. 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.
  2. 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 du feat! ou au minimum feat:.
  3. Si la CI trivyignore est cassée sur main, on ne pourra pas reverter le commit trivyignore sans revert aussi la refonte, et inversement.
  4. Le commit feat: trace events as single source of truth + flatten agents folder n'a probablement même pas été review (le diff montre un refacto de +371 lignes sur adapter.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-23 cohé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.

    1. fix: trivyignore 3 CRITICAL CVEs — 1 fichier, 19 lignes. Merge rapide.
    2. 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 migration messagestrace_events (breaking, downtime, dual-write window, etc.).
  • Option B : Renommer cette PR en feat!: refactor trace events + flatten agents + trivyignore base image et 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.

@Kaiohz
Kaiohz merged commit 3c7a956 into main Jul 23, 2026
1 check passed
@Kaiohz
Kaiohz deleted the fix/trivyignore-debian-cve branch July 23, 2026 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant