From 48e09aa373e526002760069fc290c0174f39e7d8 Mon Sep 17 00:00:00 2001 From: qpismont Date: Sun, 20 Sep 2026 13:10:05 +0000 Subject: [PATCH] Fix missing workspace when herald run in container --- Containerfile | 11 --------- README.md | 12 ++++++++++ crates/devcontainer-rs/src/devcontainer.rs | 24 +++++++++++--------- crates/devcontainer-rs/src/lib.rs | 2 +- crates/devcontainer-rs/src/runtime.rs | 26 +++++++++++++++++++++- crates/herald-server/src/sandbox/mod.rs | 15 ++++++++----- crates/herald-server/src/sandbox/tools.rs | 20 +++++++++++++++++ 7 files changed, 81 insertions(+), 29 deletions(-) diff --git a/Containerfile b/Containerfile index c39abf8..24748c2 100644 --- a/Containerfile +++ b/Containerfile @@ -22,21 +22,10 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ zlib1g \ && rm -rf /var/lib/apt/lists/* -# Herald drives the container daemon through its socket (DOCKER_HOST, default -# unix:///var/run/docker.sock), so neither docker nor podman is needed here: the -# compose file mounts the socket. Reaching that socket is what this user needs, -# and the socket is root-equivalent, so either run the container as root or give -# it the socket's group, e.g. `group_add: [""]`. -RUN useradd --create-home --shell /usr/sbin/nologin --uid 10001 herald WORKDIR /app COPY --from=builder /app/target/release/herald-server ./herald-server -# git looks for its configuration under $HOME. -ENV HOME=/home/herald - -USER herald - # Exec form, so the binary is PID 1 and receives the SIGTERM it handles to shut # down gracefully. CMD ["./herald-server"] diff --git a/README.md b/README.md index 3c69327..12bcb22 100644 --- a/README.md +++ b/README.md @@ -61,6 +61,18 @@ Herald drives the container daemon through its socket: `DOCKER_HOST` (default Docker-compatible socket. The repository must contain a `.devcontainer/devcontainer.json`. +Herald can therefore run inside a container with only that socket mounted (no +shared workspace directory is required): the clone is streamed to the daemon over +the socket, like the build context, instead of being bind-mounted from a host +path the daemon would have to see. This is the setup the `Containerfile` +produces, e.g.: + +```sh +podman run --env-file=.env -p 3001:3001 \ + -v /run/user/$(id -u)/podman/podman.sock:/var/run/docker.sock \ + herald:latest +``` + The `runArgs` of that file are read but deliberately **not** passed to the daemon: they come from an untrusted pull request, and one of them (`--network host`) would attach the container to another network and quietly diff --git a/crates/devcontainer-rs/src/devcontainer.rs b/crates/devcontainer-rs/src/devcontainer.rs index bd73690..173a06d 100644 --- a/crates/devcontainer-rs/src/devcontainer.rs +++ b/crates/devcontainer-rs/src/devcontainer.rs @@ -112,17 +112,7 @@ impl DevContainer { ), working_dir: Some(workspace_folder.clone()), host_config: Some(HostConfig { - binds: Some(vec![format!( - "{}:{workspace_folder}", - workspace_dir.display() - )]), network_mode: Some(network.clone()), - // Le clone est monté depuis un chemin de l'hôte. Sur une distribution - // à SELinux enforcing, ce chemin n'a pas le label attendu et l'accès - // est refusé (EACCES), ce qui fait échouer tous les outils de la - // sandbox. C'est le compromis inverse de l'alternative `:Z` sur le - // montage, qui re-labellise le clone et garde le confinement SELinux. - security_opt: Some(vec![String::from("label=disable")]), ..Default::default() }), ..Default::default() @@ -141,6 +131,20 @@ impl DevContainer { return Err(err); } + // Le clone est copié dans le container par le socket, pas monté depuis un + // chemin de l'hôte : le daemon n'a pas besoin de voir le clone pour le rendre + // visible dans la sandbox, ce qui permet à Herald de tourner dans un container + // (avec le socket monté) sans partager de dossier avec l'hôte. + if let Err(err) = runtime + .upload_directory(&name, &workspace_folder, workspace_dir) + .await + { + let _ = runtime.remove_container(&name).await; + let _ = runtime.remove_network(&network).await; + let _ = runtime.remove_image(&image_tag).await; + return Err(err); + } + let container = Container::new( runtime.clone(), name, diff --git a/crates/devcontainer-rs/src/lib.rs b/crates/devcontainer-rs/src/lib.rs index 0bdb717..7771ad6 100644 --- a/crates/devcontainer-rs/src/lib.rs +++ b/crates/devcontainer-rs/src/lib.rs @@ -1,7 +1,7 @@ //! Primitives de cycle de vie de container pour un [`DevContainer`] analysé. //! //! Cette crate pilote l'API du daemon de containers pour construire l'image -//! devcontainer, démarrer un container avec le workspace monté, exécuter les +//! devcontainer, démarrer un container et y copier le workspace, exécuter les //! hooks `postCreateCommand` / `postStartCommand` et lancer des commandes à //! l'intérieur du container en cours d'exécution. //! diff --git a/crates/devcontainer-rs/src/runtime.rs b/crates/devcontainer-rs/src/runtime.rs index 43b238d..e7fd8aa 100644 --- a/crates/devcontainer-rs/src/runtime.rs +++ b/crates/devcontainer-rs/src/runtime.rs @@ -10,7 +10,7 @@ use bollard::{ models::{BuildInfo, ContainerCreateBody, NetworkCreateRequest, NetworkDisconnectRequest}, query_parameters::{ BuildImageOptions, CreateContainerOptions, RemoveContainerOptions, RemoveImageOptions, - StartContainerOptions, StopContainerOptions, + StartContainerOptions, StopContainerOptions, UploadToContainerOptions, }, }; use futures_util::StreamExt; @@ -153,6 +153,30 @@ impl ContainerRuntime { Ok(()) } + pub(crate) async fn upload_directory( + &self, + container: &str, + destination: &str, + directory: &Path, + ) -> Result<(), ContainerError> { + let options = UploadToContainerOptions { + path: String::from(destination), + ..Default::default() + }; + + self.request( + "upload workspace", + self.docker.upload_to_container( + container, + Some(options), + body_try_stream(tar_directory_stream(directory)), + ), + ) + .await?; + + Ok(()) + } + pub(crate) async fn start_container(&self, name: &str) -> Result<(), ContainerError> { self.request( "start container", diff --git a/crates/herald-server/src/sandbox/mod.rs b/crates/herald-server/src/sandbox/mod.rs index d732d27..2d4ad28 100644 --- a/crates/herald-server/src/sandbox/mod.rs +++ b/crates/herald-server/src/sandbox/mod.rs @@ -35,7 +35,7 @@ pub struct SandboxConfig { pub struct Sandbox { // Owns the temporary directory; dropping it cleans up the clone. _workspace: TempDir, - /// Path of the clone, as bind-mounted into the container. + /// Path of the clone, as copied into the container. repo_dir: PathBuf, container: Container, } @@ -72,17 +72,20 @@ impl Sandbox { container, }; - sandbox.check_workspace().await?; + if let Err(err) = sandbox.check_workspace().await { + let _ = sandbox.container.remove().await; + return Err(err); + } Ok(sandbox) } /// Vérifie que le clone est bien visible dans le container. /// - /// Sans ce contrôle, un montage vide — le daemon ne voit pas le clone, par - /// exemple quand Herald est dans un container sur une autre machine — fait - /// échouer chaque outil ; le modèle enchaîne alors les appels ratés jusqu'au - /// budget d'itérations, sans jamais pouvoir reviewer quoi que ce soit. + /// Sans ce contrôle, un workspace vide — par exemple un daemon qui n'a pas pu + /// recevoir le clone — fait échouer chaque outil ; le modèle enchaîne alors les + /// appels ratés jusqu'au budget d'itérations, sans jamais pouvoir reviewer quoi + /// que ce soit. async fn check_workspace(&self) -> anyhow::Result<()> { let workspace_folder = self.workspace_folder(); let probe = self diff --git a/crates/herald-server/src/sandbox/tools.rs b/crates/herald-server/src/sandbox/tools.rs index 0e7d916..7af9f64 100644 --- a/crates/herald-server/src/sandbox/tools.rs +++ b/crates/herald-server/src/sandbox/tools.rs @@ -41,6 +41,19 @@ fn review_tools() -> Vec { } }), ), + Tool::new( + "file_size", + "Get the size of a file inside the repository in bytes.", + json!({ + "type": "object", + "properties": { + "path": { + "type": "string", + "description": "File path relative to the repository root." + } + } + }), + ), Tool::new( "read_file", "Read the content of a text file inside the repository. Every line is \ @@ -111,12 +124,19 @@ pub async fn dispatch(sandbox: &Sandbox, name: &str, args: &Value) -> anyhow::Re match name { "ls" => ls(sandbox, args).await, "read_file" => read_file(sandbox, args).await, + "file_size" => file_size(sandbox, args).await, "grep" => grep(sandbox, args).await, "find" => find(sandbox, args).await, other => bail!("unknown tool `{other}`"), } } +async fn file_size(sandbox: &Sandbox, args: &Value) -> anyhow::Result { + let path = resolve(sandbox, required_str(args, "path")?)?; + let size = sandbox.exec(&["du", "-b", "--", &path]).await?; + into_stdout(size) +} + async fn ls(sandbox: &Sandbox, args: &Value) -> anyhow::Result { let path = resolve(sandbox, optional_str(args, "path").unwrap_or("."))?; let output = sandbox.exec(&["ls", "-la", "--", &path]).await?;