Processus lancés dans le sandbox : vérifier qu'un timeout est appliqué, que le processus (et son groupe) est tué en cas de dépassement ou d'annulation, et que les pipes stdout/stderr sont consommés pour éviter un blocage du processus si le buffer se remplit. Prévoir aussi une limite de taille de sortie pour ne pas saturer la mémoire avec un output verbeux ou malveillant.
L'option --network est insérée avant self.run_args, qui sont concaténés ensuite (ligne 340) : dans docker run, la dernière option gagne, donc un runArgs du dépôt peut remplacer le réseau dédié et contourner la coupure réseau effectuée en fin de up(). Documenter/verrouiller cet ordre si l'isolation réseau doit être garantie.
Les runArgs proviennent du devcontainer.json du dépôt analysé, donc d'une PR potentiellement issue d'un fork : ils sont concaténés tels quels aux arguments de docker run. Un runArgs du type ["--privileged", "-v", "/:/host"] (ou --pid=host, --cap-add=SYS_ADMIN) monte l'hôte dans le conteneur et annule totalement l'isolation annoncée. Comme ils sont ajoutés après le --network posé ligne 326, ils peuvent aussi le surcharger (--network host). Prévoir une liste blanche d'options (ou ignorer runArgs/containerEnv pour les dépôts non fiables) avant de démarrer le conteneur.
L'affirmation « Each sandbox is isolated » est plus forte que la réalité tant que les runArgs/containerEnv du dépôt analysé sont transmis tels quels à docker run (cf. container.rs). Soit durcir le code, soit nuancer la section en listant les limites de l'isolation.
Le job container-build compile l'image mais ne l'exécute jamais : il ne détectera donc pas l'absence de git/runtime conteneur dans l'image finale. Ajouter un smoke test (lancer l'image et vérifier la présence de git et docker/podman), et noter que le pipeline ne se déclenche que sur push, pas sur les pull requests.
La suppression du .dockerignore réintègre target/ et .env dans le contexte envoyé au builder (buildah bud ... .), ce qui alourdit fortement le build et expose potentiellement des secrets. Conserver un .dockerignore minimal (target, .env, .git).
L'absence de runtime conteneur ne produit qu'un warn : le service démarre normalement alors que toutes les revues échoueront ensuite. Le contrôle étant déjà fait au démarrage, un échec rapide (fail-fast) ou un mode dégradé explicite serait plus sûr et plus lisible.
SANDBOX_MAX_ITERATIONS retombe silencieusement sur 8 si la valeur est invalide et accepte 0 : avec 0, la boucle de l'agent (1..=max_iterations) ne s'exécute pas et la revue échoue immédiatement. Valider la valeur (>= 1) et journaliser en cas de repli sur la valeur par défaut.
Indentation par tabulation incohérente avec le reste du fichier ; --security-opt label=disable désactive l'étiquetage SELinux et --userns=keep-id est spécifique à Podman. À nettoyer/justifier, d'autant que ce devcontainer sert aussi de contexte d'exécution à la sandbox.
tempfile = "3" est déclaré en dur alors que toutes les autres dépendances passent par [workspace.dependencies] : le placer dans le workspace pour garder une version unique et cohérente (idem pour le dev-dependency de devcontainer-rs).
messages.clone() et tool_definitions.clone() à chaque itération recopient toute la conversation à chaque tour (coût O(n²)) alors qu'un passage par référence (&[Message]) suffirait. Par ailleurs rien ne borne la taille du contenu renvoyé par les outils : ajouter un plafond par appel pour maîtriser le contexte et le coût.
read_file exécute cat sans aucune borne : un gros fichier (ou binaire) injecte tout son contenu dans la conversation et peut saturer le contexte et faire exploser le coût. Plafonner le nombre de lignes/octets renvoyés avec une note de troncature (idem pour grep et find, potentiellement très verbeux).
L'étage final debian:trixie-slim ne contient que le binaire, or herald-server a besoin de git (clone de la PR) et d'un binaire de runtime conteneur (docker/podman) pour exécuter la sandbox. Avec cette image, chaque revue échouera à l'exécution. Ajouter ces paquets (plus ca-certificates) ou documenter explicitement le montage du socket/runtime requis.
Incohérence de toolchain : rust:1.97-trixie ici, alors que .devcontainer/Dockerfile et .woodpecker/tests.yml utilisent rust:1.98. Aligner les versions et écrire AS en majuscules (convention Dockerfile).
base_dir.join(schema.build.dockerfile) : un dockerfile absolu (/etc/passwd) ou contenant .. sort du dossier du devcontainer et devient le -f de docker build (lecture de fichier arbitraire côté hôte). Valider que le chemin reste bien sous base_dir (via normalize par exemple) avant de l'accepter.
Le token d’authentification est passé via GIT_CONFIG_VALUE_0. C’est correct car il n’apparaît pas dans les arguments. Toutefois, en cas d’erreur, git peut afficher la ligne de commande dans les logs, ce qui pourrait exposer le token indirectement via l’URL de clonage. Envisager d’utiliser un credential helper ou de filtrer les messages d’erreur.