La méthode quitte immédiatement si remove_container échoue, alors que sa documentation indique que le réseau et l'image doivent également être supprimés en best-effort. Dans ce cas, l'image et le réseau restent orphelins, et Sandbox::cleanup ne peut pas récupérer ces ressources. Il faut tenter les suppressions même après l'échec du container, puis retourner une erreur agrégée ou la première erreur rencontrée.
label=disable désactive explicitement le confinement SELinux du container. Pour une sandbox qui exécute du code provenant d'une PR non fiable, cela élargit fortement l'impact d'une évasion ou d'une vulnérabilité du runtime. Il faudrait privilégier un montage correctement relabellisé (:Z/équivalent) ou rendre cette option explicite et désactivée par défaut, avec une justification claire du niveau de risque.
La normalisation est uniquement lexicale et ne résout pas les liens symboliques. Un dépôt contrôlé par l'auteur de la PR peut donc contenir un lien tel que secret -> /proc/self/environ ou secret -> /etc/...; read_file transmettra ensuite ce chemin à cat et permettra de lire des fichiers hors du workspace. Il faut résoudre et valider la cible réelle dans le container, ou refuser les liens symboliques avant toute lecture.
[Élevé] Si check_workspace() échoue, le ? abandonne Sandbox après le démarrage du conteneur, mais Container n'a pas de Drop capable de supprimer les ressources asynchrones. Le conteneur, le réseau et l'image restent donc potentiellement sur le daemon à chaque échec de vérification. Nettoyer explicitement le conteneur avant de retourner l'erreur, avec conservation de l'erreur initiale même si le nettoyage échoue.
[Élevé] Le conteneur sandbox n'a aucune limite de ressources (mémoire, CPU, processus ou taille disque). Le Dockerfile, les hooks et les commandes demandées par le modèle sont contrôlés indirectement par une PR non fiable et peuvent provoquer une fork bomb, remplir le disque ou monopoliser l'hôte. Ajouter des limites explicites dans HostConfig et prévoir aussi un nettoyage fiable en cas de dépassement.
[Élevé] label=disable désactive explicitement le confinement SELinux pour un conteneur exécutant du code contrôlé par une pull request. Cela élargit fortement l'impact d'une compromission du conteneur et contredit l'objectif de sandboxing. Préférer un montage correctement relabellisé (:Z/équivalent selon le runtime) ou rendre ce comportement optionnel et documenter clairement la perte d'isolation.
[Bloquant] Le chemin du Dockerfile est construit directement à partir d'une valeur contrôlée par la pull request. Un chemin absolu ou contenant suffisamment de .. peut faire sortir le contexte de build du clone ; build() empaquettera alors le répertoire parent arbitraire et l'enverra au daemon, ce qui peut exposer des fichiers de l'hôte au build non fiable. Il faut rejeter les chemins absolus et vérifier lexicalement que le Dockerfile et son contexte restent sous la racine du dépôt cloné.
Documenter le nouveau fonctionnement (workspace multi-crates + sandboxing) : prérequis (Podman/Docker), variables d'environnement nécessaires et limites connues de l'isolation.
Nouveau point d'entrée du crate herald-server : vérifier que les modules déplacés (sandbox, open_router, consts, bot_actions) sont bien déclarés et que plus aucune référence au chemin racine src/... ne subsiste.
Boucle d'agent exécutant des outils : prévoir un nombre maximal d'itérations et une limite de temps globale, sinon une boucle d'appels d'outils peut consommer indéfiniment des tokens/CPU.
Le module de sandboxing devrait exposer des limites explicites (mémoire, CPU, timeout, réseau désactivé par défaut) et retourner des erreurs typées plutôt que des String, afin que l'appelant puisse décider de la politique de repli.
Fichier supprimé : s'assurer que toutes les constantes sont reprises dans crates/herald-server/src/consts.rs et qu'aucune constante n'a été perdue lors du déplacement.
Ancien fichier supprimé : vérifier que la logique a bien été entièrement migrée vers crates/herald-server/src/bot_actions/review.rs et qu'aucun test ne référence encore l'ancien chemin.
Fichier déplacé depuis src/open_router.rs : s'assurer qu'il n'y a pas de duplication avec l'ancien fichier supprimé et que les nouveaux appels (utilisés par l'agent de sandbox) gèrent les erreurs HTTP et les timeouts.
Nouvelles variables d'environnement : documenter chacune dans .env.example et fournir une valeur par défaut sûre ; une variable manquante ne devrait pas désactiver silencieusement le sandboxing.