1 Commits
Author SHA1 Message Date
qpismont e7b8048992 add default image docker fallback
ci/woodpecker/push/tests Pipeline was successful
2026-09-22 20:57:21 +00:00
11 changed files with 266 additions and 125 deletions
Generated
+1
View File
@@ -485,6 +485,7 @@ dependencies = [
"thiserror",
"tokio",
"tokio-stream",
"tracing",
]
[[package]]
+10 -8
View File
@@ -46,7 +46,8 @@ Herald reviews pull requests inside an ephemeral
[Dev Container](https://containers.dev/). For each review it:
1. clones the pull request head into a temporary directory,
2. builds and starts the repository's devcontainer (`devcontainer-rs`),
2. builds and starts the repository's devcontainer (`devcontainer-rs`) — or, when
the repository has none, a default one based on `debian:stable-slim`,
3. reads the pull request diff and file list from the Gitea API with
`GITEA_TOKEN` (so private repositories work), tells the model which files and
lines changed — additions and deletions, with the line numbers of the new and
@@ -67,8 +68,10 @@ what the pull request does well.
Herald drives the container daemon through its socket: `DOCKER_HOST` (default
`unix:///var/run/docker.sock`), which covers both docker and podman's
Docker-compatible socket. The repository must contain a
`.devcontainer/devcontainer.json`.
Docker-compatible socket. When the repository contains a
`.devcontainer/devcontainer.json` (or `.devcontainer.json`), Herald uses it;
otherwise it falls back to a default devcontainer that pulls `debian:stable-slim`
and runs the review in `/workspace`.
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
@@ -82,11 +85,10 @@ podman run --env-file=.env -p 3001:3001 \
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
defeat the network cut described below. A devcontainer that relies on them
(`--gpus all`, `--cap-add`, `--shm-size`…) will not get them.
The `runArgs` of that file are ignored: they come from an untrusted pull request,
and one of them (`--network host`) would attach the container to another network
and quietly defeat the network cut described below. A devcontainer that relies on
them (`--gpus all`, `--cap-add`, `--shm-size`…) will not get them.
Each sandbox is isolated: it gets its own image tag, container and network. The
container starts with network access so the `postCreateCommand` /
+1
View File
@@ -10,6 +10,7 @@ futures-util = { workspace = true }
tar = "0.4"
tokio = { workspace = true }
tokio-stream = { workspace = true }
tracing = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
anyhow = { workspace = true }
+2
View File
@@ -22,3 +22,5 @@ pub(crate) const CONTEXT_CHUNKS: usize = 4;
/// Lectures successives du code de sortie d'un exec, et attente entre elles.
pub(crate) const EXIT_CODE_ATTEMPTS: usize = 10;
pub(crate) const EXIT_CODE_DELAY: Duration = Duration::from_millis(20);
pub(crate) const DEFAULT_IMAGE_NAME: &str = "debian:stable-slim";
+6 -5
View File
@@ -15,18 +15,19 @@ pub struct Container {
workspace_folder: String,
remote_user: Option<String>,
network: Option<String>,
image: Option<String>,
owned_image: Option<String>,
}
impl Container {
/// Assemble un container démarré, avec son réseau et son image à nettoyer.
/// Assemble un container démarré, avec son réseau et, le cas échéant, l'image
/// construite pour lui et à nettoyer.
pub(crate) fn new(
runtime: ContainerRuntime,
name: String,
workspace_folder: String,
remote_user: Option<String>,
network: Option<String>,
image: Option<String>,
owned_image: Option<String>,
) -> Self {
Self {
runtime,
@@ -34,7 +35,7 @@ impl Container {
workspace_folder,
remote_user,
network,
image,
owned_image,
}
}
@@ -84,7 +85,7 @@ impl Container {
let _ = self.runtime.remove_network(network).await;
}
if let Some(image) = &self.image {
if let Some(image) = &self.owned_image {
let _ = self.runtime.remove_image(image).await;
}
+86 -60
View File
@@ -8,12 +8,17 @@ use std::{
};
use bollard::models::{ContainerCreateBody, HostConfig};
use tracing::info;
use crate::{container::Container, errors::ContainerError, runtime::ContainerRuntime};
use crate::{
consts::DEFAULT_IMAGE_NAME, container::Container, errors::ContainerError,
runtime::ContainerRuntime,
};
#[derive(Debug)]
pub struct DevContainer {
pub container_file_path: PathBuf,
pub image: Option<String>,
pub container_file_path: Option<PathBuf>,
pub name: Option<String>,
pub build_args: HashMap<String, String>,
pub container_env: HashMap<String, String>,
@@ -24,20 +29,6 @@ pub struct DevContainer {
}
impl DevContainer {
/// Nom de l'image de base dérivé du nom du devcontainer.
pub fn image_name(&self) -> String {
let base = self.name.as_deref().unwrap_or("devcontainer");
format!("devcontainer-rs/{}", sanitize(base))
}
/// Tag d'image unique pour une exécution de sandbox donnée.
///
/// L'unicité est importante : deux sandboxes concurrentes (éventuellement pour des
/// dépôts différents partageant un nom de devcontainer) ne doivent pas se disputer le même tag.
pub fn image_tag(&self) -> String {
format!("{}:{}", self.image_name(), unique_suffix())
}
/// Dossier de workspace dans le container, par défaut `/workspaces/workspace`.
pub fn workspace_folder(&self) -> String {
self.workspace_folder
@@ -45,26 +36,35 @@ impl DevContainer {
.unwrap_or_else(|| "/workspaces/workspace".to_string())
}
/// Nom de container unique pour cette exécution.
pub fn container_name(&self) -> String {
let base = self.name.as_deref().unwrap_or("devcontainer");
format!("devcontainer-rs-{}-{}", sanitize(base), unique_suffix())
/// Tag de l'image construite localement à partir du Dockerfile.
fn local_image_tag(&self, id: &str) -> String {
format!("devcontainer-rs/{}:{}", self.sanitized_name(), id)
}
/// Nom du container de cette sandbox.
fn container_name(&self, id: &str) -> String {
format!("devcontainer-rs-{}-{}", self.sanitized_name(), id)
}
/// Nom du devcontainer nettoyé pour servir de nom d'image/container.
fn sanitized_name(&self) -> String {
sanitize(self.name.as_deref().unwrap_or("devcontainer"))
}
/// Construit l'image devcontainer sous `image_tag`.
pub async fn build(
async fn build(
&self,
runtime: &ContainerRuntime,
container_file_path: &Path,
image_tag: &str,
) -> Result<(), ContainerError> {
let context = self.container_file_path.parent().ok_or_else(|| {
let context = container_file_path.parent().ok_or_else(|| {
ContainerError::Unexpected(String::from(
"the devcontainer file path has no parent directory",
))
})?;
let dockerfile = self
.container_file_path
let dockerfile = container_file_path
.file_name()
.and_then(|name| name.to_str())
.ok_or_else(|| {
@@ -78,30 +78,45 @@ impl DevContainer {
.await
}
/// Construit l'image, démarre le container, exécute les hooks puis coupe le réseau.
///
/// En cas d'échec, le container, le réseau et l'image sont nettoyés avant de
/// renvoyer l'erreur, afin qu'aucune ressource ne soit laissée en place.
pub async fn up(
&self,
runtime: &ContainerRuntime,
workspace_dir: &Path,
) -> Result<Container, ContainerError> {
let image_tag = self.image_tag();
self.build(runtime, &image_tag).await?;
let id = unique_id();
let name = self.container_name();
let image = self
.image
.clone()
.unwrap_or_else(|| self.local_image_tag(&id));
let owns_image = self.image.is_none();
info!(image = %image, "Starting sandbox container");
match owns_image {
true => {
let container_file_path = self.container_file_path.as_ref().ok_or_else(|| {
ContainerError::Unexpected(String::from(
"the devcontainer has neither an image nor a container file",
))
})?;
self.build(runtime, container_file_path, &image).await?;
}
false => {
runtime.pull_image(&image).await?;
}
}
let name = self.container_name(&id);
let network = format!("{name}-net");
// Réseau dédié afin de pouvoir couper la connectivité après les hooks.
if let Err(err) = runtime.create_network(&network).await {
let _ = runtime.remove_image(&image_tag).await;
return Err(err);
}
runtime.create_network(&network).await?;
let workspace_folder = self.workspace_folder();
let body = ContainerCreateBody {
image: Some(image_tag.clone()),
image: Some(image.clone()),
cmd: Some(vec![String::from("sleep"), String::from("infinity")]),
user: self.remote_user.clone(),
env: Some(
@@ -120,28 +135,21 @@ impl DevContainer {
if let Err(err) = runtime.create_container(&name, body).await {
let _ = runtime.remove_network(&network).await;
let _ = runtime.remove_image(&image_tag).await;
return Err(err);
}
if let Err(err) = runtime.start_container(&name).await {
let _ = runtime.remove_container(&name).await;
let _ = runtime.remove_network(&network).await;
let _ = runtime.remove_image(&image_tag).await;
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);
}
@@ -151,7 +159,7 @@ impl DevContainer {
self.workspace_folder(),
self.remote_user.clone(),
Some(network.clone()),
Some(image_tag),
owns_image.then_some(image),
);
// Les hooks s'exécutent avec accès au réseau (installation de dépendances, etc.).
@@ -195,6 +203,22 @@ impl DevContainer {
}
}
impl Default for DevContainer {
fn default() -> Self {
Self {
name: Some("generic-devcontainer".to_string()),
image: Some(DEFAULT_IMAGE_NAME.to_string()),
post_create_command: None,
post_start_command: None,
build_args: HashMap::new(),
container_env: HashMap::new(),
container_file_path: None,
remote_user: None,
workspace_folder: None,
}
}
}
/// Nettoie une chaîne pour qu'elle puisse servir de nom d'image/container docker.
fn sanitize(input: &str) -> String {
let sanitized: String = input
@@ -216,8 +240,8 @@ fn sanitize(input: &str) -> String {
}
}
/// Suffixe unique à une exécution de sandbox, combinant l'identifiant de processus et un horodatage.
fn unique_suffix() -> String {
/// Identifiant unique à une exécution de sandbox, combinant l'identifiant de processus et un horodatage.
fn unique_id() -> String {
let nanos = SystemTime::now()
.duration_since(UNIX_EPOCH)
.map(|duration| duration.as_nanos())
@@ -256,27 +280,29 @@ mod tests {
}
#[test]
fn image_name_is_sanitized() {
fn local_image_tag_is_sanitized() {
let dir = tempfile::tempdir().unwrap();
let dc = devcontainer(dir.path());
assert_eq!(dc.image_name(), "devcontainer-rs/my-project");
}
#[test]
fn image_tags_are_unique() {
let dir = tempfile::tempdir().unwrap();
let dc = devcontainer(dir.path());
assert_ne!(dc.image_tag(), dc.image_tag());
assert_eq!(dc.local_image_tag("abc"), "devcontainer-rs/my-project:abc");
}
#[test]
fn container_name_is_sanitized() {
let dir = tempfile::tempdir().unwrap();
let dc = devcontainer(dir.path());
assert_eq!(dc.container_name("abc"), "devcontainer-rs-my-project-abc");
}
assert!(
dc.container_name()
.starts_with("devcontainer-rs-my-project-")
);
#[test]
fn unique_ids_differ() {
assert_ne!(unique_id(), unique_id());
}
#[test]
fn default_devcontainer_pulls_a_base_image() {
let dc = DevContainer::default();
assert_eq!(dc.image.as_deref(), Some("debian:stable-slim"));
assert_eq!(dc.container_file_path, None);
}
}
+3
View File
@@ -47,6 +47,9 @@ pub enum ParseError {
#[error("container file `{0}` does not exist or is not a regular file")]
ContainerFileNotFound(PathBuf),
#[error("devcontainer file `{0}` declares neither `image` nor `build`")]
MissingImageOrBuild(PathBuf),
#[error("the devcontainer file path has no parent directory: `{0}`")]
InvalidDevContainerPath(PathBuf),
}
+7 -7
View File
@@ -1,7 +1,8 @@
//! 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 et y copier le workspace, exécuter les
//! Cette crate pilote l'API du daemon de containers pour préparer l'image
//! devcontainer (construite depuis un Dockerfile, ou tirée d'un registre),
//! 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.
//!
@@ -19,10 +20,9 @@
//! (par ex. `npm install`) ; une fois les hooks exécutés, le container est déconnecté
//! du réseau pour le reste de sa durée de vie. Chaque commande est bornée par un timeout.
//!
//! Les `runArgs` du `devcontainer.json` sont lus mais pas transmis au daemon : ils
//! viennent du dépôt, donc d'une pull request non fiable, et pourraient rattacher le
//! container à un autre réseau ou lui donner des privilèges qui annuleraient cette
//! isolation.
//! Les `runArgs` du `devcontainer.json` sont ignorés : ils viennent du dépôt, donc
//! d'une pull request non fiable, et pourraient rattacher le container à un autre
//! réseau ou lui donner des privilèges qui annuleraient cette isolation.
mod consts;
mod container;
@@ -40,4 +40,4 @@ pub use errors::{ContainerError, ParseError};
pub use exec::ExecOutput;
pub use path::normalize;
pub use runtime::ContainerRuntime;
pub use schema::{DevContainerBuildSchema, DevContainerSchema, parse};
pub use schema::parse;
+31 -2
View File
@@ -9,11 +9,13 @@ use bollard::{
exec::{CreateExecOptions, StartExecOptions, StartExecResults},
models::{BuildInfo, ContainerCreateBody, NetworkCreateRequest, NetworkDisconnectRequest},
query_parameters::{
BuildImageOptions, CreateContainerOptions, RemoveContainerOptions, RemoveImageOptions,
StartContainerOptions, StopContainerOptions, UploadToContainerOptions,
BuildImageOptions, CreateContainerOptions, CreateImageOptionsBuilder,
RemoveContainerOptions, RemoveImageOptions, StartContainerOptions, StopContainerOptions,
UploadToContainerOptions,
},
};
use futures_util::StreamExt;
use tracing::info;
use crate::{
consts::{DEFAULT_COMMAND_TIMEOUT, DEFAULT_ENDPOINT, EXIT_CODE_ATTEMPTS, EXIT_CODE_DELAY},
@@ -133,6 +135,33 @@ impl ContainerRuntime {
}
}
pub(crate) async fn pull_image(&self, image: &str) -> Result<(), ContainerError> {
info!("Pulling image `{image}`");
let options = CreateImageOptionsBuilder::default()
.from_image(image)
.build();
let mut stream = self.docker.create_image(Some(options), None, None);
let consume = async {
while let Some(info) = stream.next().await {
info?;
}
Ok::<_, BollardError>(())
};
match tokio::time::timeout(self.timeout, consume).await {
Ok(Ok(())) => Ok(()),
Ok(Err(source)) => Err(ContainerError::Request { source }),
Err(_) => Err(ContainerError::Timeout {
operation: format!("pull image `{image}`"),
timeout: self.timeout,
}),
}
}
/// Crée un container, sans le démarrer.
pub(crate) async fn create_container(
&self,
+110 -38
View File
@@ -11,42 +11,39 @@ use serde::Deserialize;
use crate::{devcontainer::DevContainer, errors::ParseError};
#[derive(Debug, Deserialize)]
pub struct DevContainerBuildSchema {
struct DevContainerBuildSchema {
#[serde(default)]
pub dockerfile: String,
dockerfile: String,
#[serde(default)]
pub args: HashMap<String, String>,
args: HashMap<String, String>,
}
#[derive(Debug, Deserialize)]
pub struct DevContainerSchema {
struct DevContainerSchema {
#[serde(default)]
pub name: Option<String>,
pub build: DevContainerBuildSchema,
name: Option<String>,
/// Image de base tirée du registre, alternative à `build`.
#[serde(default)]
image: Option<String>,
#[serde(default)]
build: Option<DevContainerBuildSchema>,
#[serde(rename = "workspaceFolder", default)]
pub workspace_folder: Option<String>,
workspace_folder: Option<String>,
#[serde(rename = "containerEnv", default)]
pub container_env: HashMap<String, String>,
container_env: HashMap<String, String>,
#[serde(rename = "postCreateCommand", default)]
pub post_create_command: Option<String>,
post_create_command: Option<String>,
#[serde(rename = "postStartCommand", default)]
pub post_start_command: Option<String>,
post_start_command: Option<String>,
#[serde(rename = "remoteUser", default)]
pub remote_user: Option<String>,
/// Arguments passés à `docker run`.
///
/// Lus pour rester fidèle au format `devcontainer.json`, mais
/// **délibérément pas transmis** au runtime : ils viennent d'une pull
/// request non fiable et pourraient casser l'isolation de la sandbox, décrite
/// dans le doc de la crate.
#[serde(rename = "runArgs", default)]
pub run_args: Vec<String>,
remote_user: Option<String>,
}
impl TryFrom<(DevContainerSchema, PathBuf)> for DevContainer {
@@ -59,34 +56,52 @@ impl TryFrom<(DevContainerSchema, PathBuf)> for DevContainer {
.parent()
.ok_or_else(|| ParseError::InvalidDevContainerPath(devcontainer_path.clone()))?;
let container_file_path = base_dir.join(schema.build.dockerfile);
let DevContainerSchema {
name,
image,
build,
workspace_folder,
container_env,
post_create_command,
post_start_command,
remote_user,
} = schema;
if !container_file_path.is_file() {
return Err(ParseError::ContainerFileNotFound(container_file_path));
}
let (image, container_file_path, build_args) = match (image, build) {
(Some(image), _) => (Some(substitute_local_env(&image)), None, HashMap::new()),
(None, Some(build)) => {
let container_file_path = base_dir.join(&build.dockerfile);
let build_args = schema
.build
.args
.into_iter()
.map(|(k, v)| (k, substitute_local_env(&v)))
.collect();
if !container_file_path.is_file() {
return Err(ParseError::ContainerFileNotFound(container_file_path));
}
let container_env = schema
.container_env
let build_args = build
.args
.into_iter()
.map(|(k, v)| (k, substitute_local_env(&v)))
.collect();
(None, Some(container_file_path), build_args)
}
(None, None) => return Err(ParseError::MissingImageOrBuild(devcontainer_path)),
};
let container_env = container_env
.into_iter()
.map(|(k, v)| (k, substitute_local_env(&v)))
.collect();
Ok(Self {
image,
container_file_path,
name: schema.name,
name,
build_args,
container_env,
workspace_folder: schema.workspace_folder,
post_create_command: schema.post_create_command,
post_start_command: schema.post_start_command,
remote_user: schema.remote_user,
workspace_folder,
post_create_command,
post_start_command,
remote_user,
})
}
}
@@ -188,13 +203,70 @@ mod tests {
let config = parse(&devcontainer_path).await.unwrap();
assert_eq!(config.name.as_deref(), Some("test"));
assert_eq!(config.container_file_path, dockerfile_path);
assert_eq!(config.image, None);
assert_eq!(config.container_file_path, Some(dockerfile_path));
assert_eq!(config.build_args.get("VERSION").unwrap(), "1");
assert_eq!(config.container_env.get("RUST_LOG").unwrap(), "debug");
assert_eq!(config.workspace_folder.as_deref(), Some("/workspace"));
assert_eq!(config.remote_user.as_deref(), Some("dev"));
}
#[tokio::test]
async fn parses_image_based_devcontainer() {
let dir = tempfile::tempdir().unwrap();
let devcontainer_path = dir.path().join("devcontainer.json");
fs::write(
&devcontainer_path,
r#"{
"name": "image-based",
"image": "mcr.microsoft.com/devcontainers/base:debian"
}"#,
)
.unwrap();
let config = parse(&devcontainer_path).await.unwrap();
assert_eq!(
config.image.as_deref(),
Some("mcr.microsoft.com/devcontainers/base:debian")
);
assert_eq!(config.container_file_path, None);
}
#[tokio::test]
async fn ignores_run_args() {
let dir = tempfile::tempdir().unwrap();
let devcontainer_path = dir.path().join("devcontainer.json");
// `runArgs` vient d'une pull request non fiable : il doit être accepté par
// l'analyse mais jamais repris dans le devcontainer.
fs::write(
&devcontainer_path,
r#"{
"image": "alpine",
"runArgs": ["--network", "host"]
}"#,
)
.unwrap();
let config = parse(&devcontainer_path).await.unwrap();
assert_eq!(config.image.as_deref(), Some("alpine"));
}
#[tokio::test]
async fn rejects_devcontainer_without_image_or_build() {
let dir = tempfile::tempdir().unwrap();
let devcontainer_path = dir.path().join("devcontainer.json");
fs::write(&devcontainer_path, r#"{ "name": "empty" }"#).unwrap();
let err = parse(&devcontainer_path).await.unwrap_err();
assert!(matches!(err, ParseError::MissingImageOrBuild(_)));
}
#[test]
fn local_env_references_are_never_read_from_the_environment() {
unsafe { std::env::set_var("DEVCONTAINER_TEST_SECRET", "s3cret") };
+9 -5
View File
@@ -50,12 +50,16 @@ impl Sandbox {
clone_pull_request(clone_url, token, pull_request_number, &repo_dir).await?;
make_readable(&repo_dir).await?;
let devcontainer_path = find_devcontainer(&repo_dir)
.with_context(|| format!("no devcontainer found in `{clone_url}`"))?;
let devcontainer = match find_devcontainer(&repo_dir) {
Some(path) => devcontainer_rs::parse(&path).await?,
None => {
info!(
"no devcontainer found in the pull request, falling back to the default image"
);
devcontainer_rs::DevContainer::default()
}
};
let devcontainer = devcontainer_rs::parse(&devcontainer_path).await?;
info!(image = %devcontainer.image_tag(), "Building and starting sandbox container");
let container = devcontainer.up(runtime, &repo_dir).await?;
let sandbox = Self {