From 99d1c2feefe4503b290b532e79d230a53a9845ec Mon Sep 17 00:00:00 2001 From: qpismont Date: Thu, 17 Sep 2026 13:16:15 +0000 Subject: [PATCH] Add sandbox --- .env.example | 6 +- Cargo.lock | 1 + README.md | 24 + crates/devcontainer-rs/src/container.rs | 649 ++++++++++++++++++ crates/devcontainer-rs/src/lib.rs | 102 ++- crates/herald-server/Cargo.toml | 3 +- crates/herald-server/src/bot.rs | 8 + .../herald-server/src/bot_actions/review.rs | 81 ++- crates/herald-server/src/consts.rs | 10 + crates/herald-server/src/env.rs | 15 + crates/herald-server/src/gitea.rs | 16 + crates/herald-server/src/main.rs | 16 + crates/herald-server/src/open_router.rs | 44 +- crates/herald-server/src/sandbox/agent.rs | 129 ++++ crates/herald-server/src/sandbox/mod.rs | 197 ++++++ crates/herald-server/src/sandbox/tools.rs | 248 +++++++ 16 files changed, 1537 insertions(+), 12 deletions(-) create mode 100644 crates/devcontainer-rs/src/container.rs create mode 100644 crates/herald-server/src/sandbox/agent.rs create mode 100644 crates/herald-server/src/sandbox/mod.rs create mode 100644 crates/herald-server/src/sandbox/tools.rs diff --git a/.env.example b/.env.example index 06a8c62..cabc5a0 100644 --- a/.env.example +++ b/.env.example @@ -18,5 +18,9 @@ SENTRY_DSN= RUST_LOG=info RUST_BACKTRACE=1 +METRICS_BIND_ADDR= -METRICS_BIND_ADDR= \ No newline at end of file +# Sandboxed tool execution (optional) +SANDBOX_ENABLED=false +CONTAINER_RUNTIME=docker +SANDBOX_MAX_ITERATIONS=8 \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index 4989bb3..02d3b25 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -847,6 +847,7 @@ dependencies = [ "sentry-anyhow", "serde", "serde_json", + "tempfile", "thiserror 2.0.20", "tokio", "tokio-stream", diff --git a/README.md b/README.md index 1c1de1e..a749dfd 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,30 @@ Herald reads its configuration from environment variables (a `.env` file is supp | `METRICS_BIND_ADDR` | *(optional)* Bind address for the Prometheus metrics endpoint (e.g. `0.0.0.0:9100`). If unset, the metrics exporter is disabled. | | `SENTRY_DSN` | *(optional)* Sentry DSN for error tracking | | `RUST_LOG` | *(optional)* Log level, defaults to `info` | +| `SANDBOX_ENABLED` | *(optional)* Run reviews inside a devcontainer sandbox so the model can explore the repository with tools. Defaults to `false` | +| `CONTAINER_RUNTIME` | *(optional)* Container runtime binary used for the sandbox (`docker` or `podman`). Defaults to `docker` | +| `SANDBOX_MAX_ITERATIONS` | *(optional)* Maximum number of tool-calling iterations per sandboxed review. Defaults to `8` | + +## Sandboxed reviews + +When `SANDBOX_ENABLED=true`, 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`), +3. lets the model explore the repository with read-only tools (`ls`, `read_file`, + `grep`, `find`) executed inside the container, +4. posts the review and removes the container and the temporary clone. + +The container runtime is selected with `CONTAINER_RUNTIME` (`docker` or +`podman`). The repository must contain a `.devcontainer/devcontainer.json`. + +Each sandbox is isolated: it gets its own image tag, container and network. The +container starts with network access so the `postCreateCommand` / +`postStartCommand` hooks can install dependencies (e.g. `npm install`); once the +hooks have run, the container is disconnected from the network for the rest of +the review. Every container command is bounded by a timeout, and the container, +network and image are removed when the review ends (including on failure). ## Development diff --git a/crates/devcontainer-rs/src/container.rs b/crates/devcontainer-rs/src/container.rs new file mode 100644 index 0000000..a2ce142 --- /dev/null +++ b/crates/devcontainer-rs/src/container.rs @@ -0,0 +1,649 @@ +//! Container lifecycle primitives for a parsed [`DevContainer`]. +//! +//! This module shells out to a container runtime (`docker` or `podman`) to build +//! the devcontainer image, start a container with the workspace mounted, run the +//! `postCreateCommand` / `postStartCommand` hooks and execute commands inside the +//! running container. +//! +//! It is intentionally runtime-agnostic: any binary exposing the `docker` CLI +//! surface (including `podman`) can be used via [`ContainerRuntime::new`]. +//! +//! # Isolation +//! +//! Each sandbox gets its own image tag, its own container and its own network. +//! The container starts attached to that network so the `postCreateCommand` / +//! `postStartCommand` hooks can fetch dependencies (e.g. `npm install`); once the +//! hooks have run, the container is disconnected from the network for the rest of +//! its lifetime. Every command is bounded by a timeout. + +use std::{ + path::{Path, PathBuf}, + process::Stdio, + time::{Duration, SystemTime, UNIX_EPOCH}, +}; + +use tokio::process::Command; + +use crate::DevContainer; + +/// Timeout applied to build/run/stop/remove operations. +const DEFAULT_COMMAND_TIMEOUT: Duration = Duration::from_secs(600); + +/// Timeout applied to commands executed inside a running container. +const DEFAULT_EXEC_TIMEOUT: Duration = Duration::from_secs(60); + +/// Result of a command executed inside a container. +#[derive(Debug, Clone)] +pub struct ExecOutput { + /// Exit code, or `-1` if the process was terminated by a signal. + pub status: i32, + pub stdout: String, + pub stderr: String, +} + +impl ExecOutput { + pub fn success(&self) -> bool { + self.status == 0 + } + + /// Turns a non-zero exit code into a [`ContainerError::Command`]. + pub fn ensure_success(self, program: &str, args: &[String]) -> Result { + if self.success() { + return Ok(self); + } + + Err(ContainerError::Command { + program: program.to_string(), + args: args.join(" "), + status: self.status, + stderr: self.stderr.trim().to_string(), + }) + } +} + +#[derive(Debug, thiserror::Error)] +pub enum ContainerError { + #[error("failed to run `{program}`: {source}")] + Spawn { + program: String, + source: std::io::Error, + }, + + #[error("`{program} {args}` failed with status {status}: {stderr}")] + Command { + program: String, + args: String, + status: i32, + stderr: String, + }, + + #[error("`{program} {args}` timed out after {timeout:?}")] + Timeout { + program: String, + args: String, + timeout: Duration, + }, +} + +/// A container runtime binary exposing the `docker` CLI surface. +#[derive(Debug, Clone)] +pub struct ContainerRuntime { + program: String, + timeout: Duration, +} + +impl ContainerRuntime { + pub fn new(program: impl Into) -> Self { + Self { + program: program.into(), + timeout: DEFAULT_COMMAND_TIMEOUT, + } + } + + pub fn docker() -> Self { + Self::new("docker") + } + + pub fn podman() -> Self { + Self::new("podman") + } + + /// Overrides the timeout applied to build/run/stop/remove operations. + pub fn with_timeout(mut self, timeout: Duration) -> Self { + self.timeout = timeout; + self + } + + pub fn program(&self) -> &str { + &self.program + } + + pub fn timeout(&self) -> Duration { + self.timeout + } + + /// Checks that the runtime binary is present and responsive. + pub async fn available(&self) -> bool { + Command::new(&self.program) + .arg("version") + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .await + .map(|status| status.success()) + .unwrap_or(false) + } + + /// Runs the runtime with the given arguments, capturing stdout/stderr. + /// + /// Only spawn failures and timeouts are reported as errors; a non-zero exit + /// code is returned in the [`ExecOutput`] so callers can decide how to react. + pub async fn run(&self, args: &[String]) -> Result { + self.run_with_timeout(args, self.timeout).await + } + + /// Like [`run`](Self::run) with an explicit timeout. + pub async fn run_with_timeout( + &self, + args: &[String], + timeout: Duration, + ) -> Result { + let output = Command::new(&self.program) + .args(args) + .stdin(Stdio::null()) + .kill_on_drop(true) + .output(); + + match tokio::time::timeout(timeout, output).await { + Ok(Ok(output)) => Ok(ExecOutput { + status: output.status.code().unwrap_or(-1), + stdout: String::from_utf8_lossy(&output.stdout).into_owned(), + stderr: String::from_utf8_lossy(&output.stderr).into_owned(), + }), + Ok(Err(source)) => Err(ContainerError::Spawn { + program: self.program.clone(), + source, + }), + Err(_) => Err(ContainerError::Timeout { + program: self.program.clone(), + args: args.join(" "), + timeout, + }), + } + } +} + +/// A running devcontainer. +#[derive(Debug, Clone)] +pub struct Container { + runtime: ContainerRuntime, + name: String, + workspace_folder: String, + remote_user: Option, + network: Option, + image: Option, +} + +impl Container { + pub fn name(&self) -> &str { + &self.name + } + + pub fn workspace_folder(&self) -> &str { + &self.workspace_folder + } + + /// Executes a command inside the container, returning its output. + /// + /// The command is passed as an argv vector (no shell), so no quoting or + /// interpolation is performed. + pub async fn exec(&self, cmd: &[&str]) -> Result { + self.exec_with_timeout(cmd, DEFAULT_EXEC_TIMEOUT).await + } + + async fn exec_with_timeout( + &self, + cmd: &[&str], + timeout: Duration, + ) -> Result { + let mut args = vec!["exec".to_string()]; + + if let Some(user) = &self.remote_user { + args.push("--user".to_string()); + args.push(user.clone()); + } + + args.push(self.name.clone()); + args.extend(cmd.iter().map(|arg| arg.to_string())); + + self.runtime.run_with_timeout(&args, timeout).await + } + + /// Executes a shell script inside the container via `sh -c`. + pub async fn exec_shell(&self, script: &str) -> Result { + self.exec(&["sh", "-c", script]).await + } + + /// Stops the container. + pub async fn stop(&self) -> Result<(), ContainerError> { + let args = vec!["stop".to_string(), self.name.clone()]; + self.runtime + .run(&args) + .await? + .ensure_success(self.runtime.program(), &args)?; + Ok(()) + } + + /// Removes the container, its network and its image. + /// + /// Network and image removal are best-effort: they may already be gone. + pub async fn remove(&self) -> Result<(), ContainerError> { + let args = vec![ + "rm".to_string(), + "-f".to_string(), + "-v".to_string(), + self.name.clone(), + ]; + self.runtime + .run(&args) + .await? + .ensure_success(self.runtime.program(), &args)?; + + if let Some(network) = &self.network { + let args = vec!["network".to_string(), "rm".to_string(), network.clone()]; + let _ = self.runtime.run(&args).await; + } + + if let Some(image) = &self.image { + let args = vec!["rmi".to_string(), image.clone()]; + let _ = self.runtime.run(&args).await; + } + + Ok(()) + } +} + +impl DevContainer { + /// Base image name derived from the devcontainer name. + pub fn image_name(&self) -> String { + let base = self.name.as_deref().unwrap_or("devcontainer"); + format!("devcontainer-rs/{}", sanitize(base)) + } + + /// Unique image tag for a single sandbox run. + /// + /// Uniqueness matters: two concurrent sandboxes (possibly for different + /// repositories sharing a devcontainer name) must not race on a shared tag. + pub fn image_tag(&self) -> String { + format!("{}:{}", self.image_name(), unique_suffix()) + } + + /// Arguments passed to `docker build` (everything after the `build` verb). + pub fn build_args(&self, image_tag: &str) -> Vec { + let context = self + .container_file_path + .parent() + .unwrap_or_else(|| Path::new(".")); + + let mut args = vec![ + "-f".to_string(), + self.container_file_path.display().to_string(), + "-t".to_string(), + image_tag.to_string(), + ]; + + for (key, value) in &self.build_args { + args.push("--build-arg".to_string()); + args.push(format!("{key}={value}")); + } + + args.push(context.display().to_string()); + args + } + + /// Arguments passed to `docker run` (everything after the `run` verb). + pub fn run_args( + &self, + workspace_dir: &Path, + container_name: &str, + image_tag: &str, + network: Option<&str>, + ) -> Vec { + let workspace_folder = self.workspace_folder(); + + let mut args = vec![ + "-d".to_string(), + "--name".to_string(), + container_name.to_string(), + "-v".to_string(), + format!("{}:{}", workspace_dir.display(), workspace_folder), + "-w".to_string(), + workspace_folder.to_string(), + ]; + + if let Some(network) = network { + args.push("--network".to_string()); + args.push(network.to_string()); + } + + if let Some(user) = &self.remote_user { + args.push("--user".to_string()); + args.push(user.clone()); + } + + for (key, value) in &self.container_env { + args.push("-e".to_string()); + args.push(format!("{key}={value}")); + } + + args.extend(self.run_args.iter().cloned()); + + args.push(image_tag.to_string()); + // Keep the container alive so we can `exec` into it. + args.push("sleep".to_string()); + args.push("infinity".to_string()); + + args + } + + /// Workspace folder inside the container, defaulting to `/workspaces/workspace`. + pub fn workspace_folder(&self) -> String { + self.workspace_folder + .clone() + .unwrap_or_else(|| "/workspaces/workspace".to_string()) + } + + /// Unique container name for this run. + pub fn container_name(&self) -> String { + let base = self.name.as_deref().unwrap_or("devcontainer"); + format!("devcontainer-rs-{}-{}", sanitize(base), unique_suffix()) + } + + /// Builds the devcontainer image under `image_tag`. + pub async fn build( + &self, + runtime: &ContainerRuntime, + image_tag: &str, + ) -> Result<(), ContainerError> { + let mut args = vec!["build".to_string()]; + args.extend(self.build_args(image_tag)); + + runtime + .run(&args) + .await? + .ensure_success(runtime.program(), &args)?; + + Ok(()) + } + + /// Builds the image, starts the container with the workspace mounted, runs + /// the `postCreateCommand` / `postStartCommand` hooks with network access, + /// then disconnects the container from the network. + /// + /// On any failure the container, network and image are cleaned up before + /// returning, so no resource is leaked. + pub async fn up( + &self, + runtime: &ContainerRuntime, + workspace_dir: &Path, + ) -> Result { + let image_tag = self.image_tag(); + self.build(runtime, &image_tag).await?; + + let name = self.container_name(); + let network = format!("{name}-net"); + + // Dedicated network so connectivity can be cut after the hooks. + let args = vec!["network".to_string(), "create".to_string(), network.clone()]; + if let Err(err) = runtime + .run(&args) + .await + .and_then(|out| out.ensure_success(runtime.program(), &args)) + { + let _ = runtime.run(&["rmi".to_string(), image_tag]).await; + return Err(err); + } + + let mut args = vec!["run".to_string()]; + args.extend(self.run_args(workspace_dir, &name, &image_tag, Some(&network))); + + if let Err(err) = runtime + .run(&args) + .await + .and_then(|out| out.ensure_success(runtime.program(), &args)) + { + let _ = runtime + .run(&["network".to_string(), "rm".to_string(), network]) + .await; + let _ = runtime.run(&["rmi".to_string(), image_tag]).await; + return Err(err); + } + + let container = Container { + runtime: runtime.clone(), + name, + workspace_folder: self.workspace_folder(), + remote_user: self.remote_user.clone(), + network: Some(network.clone()), + image: Some(image_tag), + }; + + // Hooks run with network access (dependency installation, etc.). + if let Err(err) = self.run_hooks(&container).await { + let _ = container.remove().await; + return Err(err); + } + + // Cut network access for the rest of the sandbox lifetime. + let args = vec![ + "network".to_string(), + "disconnect".to_string(), + network, + container.name.clone(), + ]; + if let Err(err) = runtime + .run(&args) + .await + .and_then(|out| out.ensure_success(runtime.program(), &args)) + { + let _ = container.remove().await; + return Err(err); + } + + Ok(container) + } + + async fn run_hooks(&self, container: &Container) -> Result<(), ContainerError> { + // Hooks may install dependencies, so they get the long command timeout + // rather than the short one used for tool execution. + let timeout = container.runtime.timeout(); + + for command in [&self.post_create_command, &self.post_start_command] + .into_iter() + .flatten() + { + let output = container + .exec_with_timeout(&["sh", "-c", command], timeout) + .await?; + output.ensure_success( + container.runtime.program(), + &["exec".to_string(), command.clone()], + )?; + } + + Ok(()) + } +} + +/// Sanitizes a string so it can be used as a docker image/container name. +fn sanitize(input: &str) -> String { + let sanitized: String = input + .chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == '.' { + c.to_ascii_lowercase() + } else { + '-' + } + }) + .collect(); + + let trimmed = sanitized.trim_matches(|c| c == '-' || c == '.' || c == '_'); + if trimmed.is_empty() { + "devcontainer".to_string() + } else { + trimmed.to_string() + } +} + +/// Suffix unique to a sandbox run, combining the process id and a timestamp. +fn unique_suffix() -> String { + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|duration| duration.as_nanos()) + .unwrap_or(0); + + format!("{}-{}", std::process::id(), nanos) +} + +/// Lexically normalizes a path, resolving `.` and `..` without touching the +/// filesystem. Returns `None` if the path escapes its root. +pub fn normalize(path: &Path) -> Option { + use std::path::Component; + + let mut out = PathBuf::new(); + for component in path.components() { + match component { + Component::RootDir => out.push("/"), + Component::CurDir => {} + Component::ParentDir => { + if !out.pop() { + return None; + } + } + Component::Normal(part) => out.push(part), + Component::Prefix(_) => return None, + } + } + + Some(out) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + + fn devcontainer(dir: &Path) -> DevContainer { + let devcontainer_path = dir.join("devcontainer.json"); + let dockerfile_path = dir.join("Dockerfile"); + + fs::write(&dockerfile_path, "FROM alpine\n").unwrap(); + fs::write( + &devcontainer_path, + r#"{ + "name": "My Project", + "build": { + "dockerfile": "Dockerfile", + "args": { "VERSION": "1" } + }, + "workspaceFolder": "/workspaces/my-project", + "containerEnv": { "RUST_LOG": "debug" }, + "remoteUser": "dev", + "runArgs": ["--userns=keep-id"] + }"#, + ) + .unwrap(); + + let runtime = tokio::runtime::Runtime::new().unwrap(); + runtime.block_on(crate::parse(&devcontainer_path)).unwrap() + } + + #[test] + fn image_name_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()); + } + + #[test] + fn build_args_include_dockerfile_tag_build_args_and_context() { + let dir = tempfile::tempdir().unwrap(); + let dc = devcontainer(dir.path()); + let args = dc.build_args("devcontainer-rs/my-project:test"); + + assert_eq!(args[0], "-f"); + assert!(args[1].ends_with("Dockerfile")); + assert_eq!(args[2], "-t"); + assert_eq!(args[3], "devcontainer-rs/my-project:test"); + assert!(args.contains(&"--build-arg".to_string())); + assert!(args.contains(&"VERSION=1".to_string())); + assert_eq!(args.last().unwrap(), &dir.path().display().to_string()); + } + + #[test] + fn run_args_mount_workspace_and_keep_alive() { + let dir = tempfile::tempdir().unwrap(); + let dc = devcontainer(dir.path()); + let workspace = Path::new("/tmp/clone"); + let args = dc.run_args( + workspace, + "devcontainer-rs-my-project-42", + "devcontainer-rs/my-project:test", + Some("sandbox-net"), + ); + + assert!(args.contains(&"-d".to_string())); + assert!(args.contains(&"--name".to_string())); + assert!(args.contains(&"devcontainer-rs-my-project-42".to_string())); + assert!(args.contains(&"/tmp/clone:/workspaces/my-project".to_string())); + assert!(args.contains(&"--network".to_string())); + assert!(args.contains(&"sandbox-net".to_string())); + assert!(args.contains(&"--user".to_string())); + assert!(args.contains(&"dev".to_string())); + assert!(args.contains(&"RUST_LOG=debug".to_string())); + assert!(args.contains(&"--userns=keep-id".to_string())); + assert!(args.contains(&"devcontainer-rs/my-project:test".to_string())); + assert_eq!( + &args[args.len() - 2..], + &["sleep".to_string(), "infinity".to_string()] + ); + } + + #[test] + fn normalize_rejects_escaping_paths() { + assert_eq!( + normalize(Path::new("/workspaces/project/src/../main.rs")), + Some(PathBuf::from("/workspaces/project/main.rs")) + ); + assert_eq!(normalize(Path::new("/workspaces/../../etc/passwd")), None); + } + + #[tokio::test] + async fn run_captures_output() { + let runtime = ContainerRuntime::new("echo"); + let output = runtime.run(&["hello".to_string()]).await.unwrap(); + + assert!(output.success()); + assert_eq!(output.stdout.trim(), "hello"); + } + + #[tokio::test] + async fn run_times_out_and_kills_the_process() { + let runtime = ContainerRuntime::new("sleep"); + let err = runtime + .run_with_timeout(&["10".to_string()], Duration::from_millis(50)) + .await + .unwrap_err(); + + assert!(matches!(err, ContainerError::Timeout { .. })); + } +} diff --git a/crates/devcontainer-rs/src/lib.rs b/crates/devcontainer-rs/src/lib.rs index 81960de..8743f0a 100644 --- a/crates/devcontainer-rs/src/lib.rs +++ b/crates/devcontainer-rs/src/lib.rs @@ -5,6 +5,10 @@ use std::{ use serde::Deserialize; +mod container; + +pub use container::{Container, ContainerError, ContainerRuntime, ExecOutput, normalize}; + #[derive(Debug, Deserialize)] pub struct DevContainerBuildSchema { #[serde(default)] @@ -30,6 +34,12 @@ pub struct DevContainerSchema { #[serde(rename = "postStartCommand", default)] pub post_start_command: Option, + + #[serde(rename = "remoteUser", default)] + pub remote_user: Option, + + #[serde(rename = "runArgs", default)] + pub run_args: Vec, } #[derive(Debug)] @@ -41,6 +51,8 @@ pub struct DevContainer { pub workspace_folder: Option, pub post_create_command: Option, pub post_start_command: Option, + pub remote_user: Option, + pub run_args: Vec, } #[derive(Debug, thiserror::Error)] @@ -80,18 +92,72 @@ impl TryFrom<(DevContainerSchema, PathBuf)> for DevContainer { return Err(ParseError::ContainerFileNotFound(container_file_path)); } + let build_args = schema + .build + .args + .into_iter() + .map(|(k, v)| (k, substitute_local_env(&v))) + .collect(); + + let container_env = schema + .container_env + .into_iter() + .map(|(k, v)| (k, substitute_local_env(&v))) + .collect(); + Ok(Self { container_file_path, name: schema.name, - build_args: schema.build.args, - container_env: schema.container_env, + 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, + run_args: schema.run_args, }) } } +/// Resolves `${localEnv:VAR}` and `${localEnv:VAR:default}` references using the +/// current process environment, as described by the devcontainer specification. +/// Unresolved variables without a default expand to an empty string. +fn substitute_local_env(input: &str) -> String { + const PREFIX: &str = "${localEnv:"; + + let mut out = String::with_capacity(input.len()); + let mut rest = input; + + while let Some(start) = rest.find(PREFIX) { + out.push_str(&rest[..start]); + let after = &rest[start + PREFIX.len()..]; + + match after.find('}') { + Some(end) => { + let inner = &after[..end]; + let (key, default) = match inner.split_once(':') { + Some((key, default)) => (key, Some(default)), + None => (inner, None), + }; + + match std::env::var(key) { + Ok(value) => out.push_str(&value), + Err(_) => out.push_str(default.unwrap_or("")), + } + + rest = &after[end + 1..]; + } + None => { + out.push_str(PREFIX); + rest = after; + } + } + } + + out.push_str(rest); + out +} + pub async fn parse(path: impl AsRef) -> Result { let path = path.as_ref().to_path_buf(); let contents = tokio::fs::read_to_string(&path) @@ -136,7 +202,9 @@ mod tests { "workspaceFolder": "/workspace", "containerEnv": { "RUST_LOG": "debug" - } + }, + "remoteUser": "dev", + "runArgs": ["--userns=keep-id"] }"#, ) .unwrap(); @@ -148,5 +216,33 @@ mod tests { 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")); + assert_eq!(config.run_args, vec!["--userns=keep-id"]); + } + + #[test] + fn substitutes_local_env_with_default() { + unsafe { std::env::set_var("DEVCONTAINER_TEST_UID", "1000") }; + + assert_eq!( + substitute_local_env("${localEnv:DEVCONTAINER_TEST_UID}"), + "1000" + ); + assert_eq!( + substitute_local_env("uid=${localEnv:DEVCONTAINER_TEST_UID}"), + "uid=1000" + ); + assert_eq!( + substitute_local_env("${localEnv:DEVCONTAINER_TEST_MISSING:fallback}"), + "fallback" + ); + assert_eq!( + substitute_local_env("${localEnv:DEVCONTAINER_TEST_MISSING}"), + "" + ); + assert_eq!( + substitute_local_env("no variables here"), + "no variables here" + ); } } diff --git a/crates/herald-server/Cargo.toml b/crates/herald-server/Cargo.toml index 8e015d9..9635007 100644 --- a/crates/herald-server/Cargo.toml +++ b/crates/herald-server/Cargo.toml @@ -27,4 +27,5 @@ hex = { workspace = true } bytes = { workspace = true } metrics = { workspace = true } metrics-exporter-prometheus = { workspace = true } -devcontainer-rs = { path = "../devcontainer-rs" } \ No newline at end of file +devcontainer-rs = { path = "../devcontainer-rs" } +tempfile = "3" \ No newline at end of file diff --git a/crates/herald-server/src/bot.rs b/crates/herald-server/src/bot.rs index 0a4482c..ae1029e 100644 --- a/crates/herald-server/src/bot.rs +++ b/crates/herald-server/src/bot.rs @@ -2,6 +2,7 @@ use crate::{ gitea::{GiteaAPI, WebhookType}, metrics, open_router::OpenRouterClient, + sandbox::SandboxConfig, }; use serde::Deserialize; use std::{collections::HashSet, sync::Arc}; @@ -31,6 +32,7 @@ pub struct Bot { http_client: reqwest::Client, max_concurrent: usize, open_router_model: String, + sandbox: SandboxConfig, actions_handled: Arc>>, } @@ -42,6 +44,7 @@ impl Bot { http_client: reqwest::Client, max_concurrent: usize, open_router_model: String, + sandbox: SandboxConfig, ) -> Self { Self { bot_name, @@ -50,6 +53,7 @@ impl Bot { http_client, max_concurrent, open_router_model, + sandbox, actions_handled: Arc::new(Mutex::new(HashSet::new())), } } @@ -113,12 +117,16 @@ impl Bot { } }; + let tools = crate::sandbox::tools::for_webhook(&webhook); + let exec_result = match webhook { WebhookType::Review(review_payload) => crate::bot_actions::review::exec_review( &self.gitea_api, &self.open_router_client, &self.http_client, &self.open_router_model, + &self.sandbox, + tools, review_payload, ), } diff --git a/crates/herald-server/src/bot_actions/review.rs b/crates/herald-server/src/bot_actions/review.rs index 8ca234a..fdb39bb 100644 --- a/crates/herald-server/src/bot_actions/review.rs +++ b/crates/herald-server/src/bot_actions/review.rs @@ -1,22 +1,33 @@ use futures_util::stream::TryStreamExt; +use openrouter_rs::types::Tool; use tokio::io::AsyncReadExt; use tokio_util::io::StreamReader; -use tracing::instrument; +use tracing::{info, instrument, warn}; use crate::{ bot::ReviewResult, - consts::{BOT_PROCESS_MSG, MAX_DIFF_SIZE, REVIEW_PROMPT}, + consts::{BOT_PROCESS_MSG, MAX_DIFF_SIZE, REVIEW_PROMPT, SANDBOX_SYSTEM_PROMPT}, gitea::{GiteaAPI, ReviewPayload}, metrics, open_router::OpenRouterClient, + sandbox::{Sandbox, SandboxConfig, agent}, }; -#[instrument(skip(gitea_api, open_router_client, http_client, review_payload))] +#[instrument(skip( + gitea_api, + open_router_client, + http_client, + sandbox_config, + tools, + review_payload +))] pub async fn exec_review( gitea_api: &GiteaAPI, open_router_client: &OpenRouterClient, http_client: &reqwest::Client, model: &str, + sandbox_config: &SandboxConfig, + tools: Vec, review_payload: ReviewPayload, ) -> anyhow::Result<()> { tracing::info!( @@ -45,10 +56,24 @@ pub async fn exec_review( .replace("{comment}", &review_payload.comment.body) .replace("{diff}", &diff_for_llm); - let chat_result = open_router_client.chat(&bot_request).await?; - let mut review_result = serde_json::from_str::(&chat_result.message)?; + let (message, cost) = if sandbox_config.enabled { + run_sandboxed_review( + gitea_api, + open_router_client, + sandbox_config, + tools, + &review_payload, + &bot_request, + ) + .await? + } else { + let chat_result = open_router_client.chat(&bot_request).await?; + (chat_result.message, chat_result.cost) + }; - review_result.cost = chat_result.cost; + let mut review_result = serde_json::from_str::(&message)?; + + review_result.cost = cost; if let Some(cost) = review_result.cost { metrics::openrouter_cost_usd(cost); } @@ -86,6 +111,50 @@ pub async fn exec_review( } } +/// Runs the review inside a sandbox container, letting the model explore the +/// repository with tools before answering. +async fn run_sandboxed_review( + gitea_api: &GiteaAPI, + open_router_client: &OpenRouterClient, + sandbox_config: &SandboxConfig, + tools: Vec, + review_payload: &ReviewPayload, + bot_request: &str, +) -> anyhow::Result<(String, Option)> { + let repo_url = gitea_api.repo_clone_url(&review_payload.repository.full_name); + + let sandbox = Sandbox::create( + &sandbox_config.runtime, + &repo_url, + gitea_api.token(), + review_payload.pull_request.number, + ) + .await?; + + let result = agent::run( + open_router_client, + &sandbox, + tools, + SANDBOX_SYSTEM_PROMPT, + bot_request, + sandbox_config.max_iterations, + ) + .await; + + if let Err(err) = sandbox.cleanup().await { + warn!(%err, "Failed to clean up sandbox container"); + } + + let result = result?; + info!( + iterations = result.iterations, + cost = ?result.cost, + "Sandboxed review finished" + ); + + Ok((result.message, result.cost)) +} + fn review_result_to_markdown(review_result: &ReviewResult) -> String { if review_result.reviews.is_empty() { return String::from("No issues found. ✅"); diff --git a/crates/herald-server/src/consts.rs b/crates/herald-server/src/consts.rs index 9a38974..7dc6547 100644 --- a/crates/herald-server/src/consts.rs +++ b/crates/herald-server/src/consts.rs @@ -8,6 +8,16 @@ pub const BOT_PROCESS_MSG: &str = " Review in progress with the model \"{model}\"... "; +pub const SANDBOX_SYSTEM_PROMPT: &str = " + You are a senior software engineer reviewing a pull request. + + The repository is checked out in your working directory. Use the provided + tools (ls, read_file, grep, find) to explore the code and gather the context + you need before answering. Paths are relative to the repository root. + + When you have enough information, answer with the requested JSON only. +"; + pub const REVIEW_PROMPT: &str = " You are a senior software engineer reviewing code changes. diff --git a/crates/herald-server/src/env.rs b/crates/herald-server/src/env.rs index 8aed750..6fa8ced 100644 --- a/crates/herald-server/src/env.rs +++ b/crates/herald-server/src/env.rs @@ -12,6 +12,9 @@ pub struct EnvConfig { pub gitea_token: String, pub gitea_timeout: u64, pub metrics_bind_addr: Option, + pub container_runtime: String, + pub sandbox_enabled: bool, + pub sandbox_max_iterations: usize, } pub fn load_config() -> anyhow::Result { @@ -25,6 +28,15 @@ pub fn load_config() -> anyhow::Result { let gitea_token = try_get_env("GITEA_TOKEN")?; let gitea_timeout = try_get_env("GITEA_TIMEOUT")?.parse()?; let metrics_bind_addr = std::env::var("METRICS_BIND_ADDR").ok(); + let container_runtime = + std::env::var("CONTAINER_RUNTIME").unwrap_or_else(|_| "docker".to_string()); + let sandbox_enabled = std::env::var("SANDBOX_ENABLED") + .map(|value| matches!(value.as_str(), "1" | "true" | "yes")) + .unwrap_or(false); + let sandbox_max_iterations = std::env::var("SANDBOX_MAX_ITERATIONS") + .ok() + .and_then(|value| value.parse().ok()) + .unwrap_or(8); Ok(EnvConfig { http_port, @@ -37,6 +49,9 @@ pub fn load_config() -> anyhow::Result { gitea_token, gitea_timeout, metrics_bind_addr, + container_runtime, + sandbox_enabled, + sandbox_max_iterations, }) } diff --git a/crates/herald-server/src/gitea.rs b/crates/herald-server/src/gitea.rs index b6b48fb..a86123c 100644 --- a/crates/herald-server/src/gitea.rs +++ b/crates/herald-server/src/gitea.rs @@ -9,6 +9,7 @@ use crate::{bot::ReviewResult, errors::AppError}; #[derive(Clone)] pub struct GiteaAPI { base_url: String, + token: String, client: reqwest::Client, } @@ -22,6 +23,7 @@ impl GiteaAPI { Ok(Self { base_url: String::from(base_url), + token: String::from(token), client: reqwest::Client::builder() .timeout(Duration::from_secs(timeout)) .default_headers(default_headers) @@ -29,6 +31,20 @@ impl GiteaAPI { }) } + /// API token used to authenticate against Gitea. + pub fn token(&self) -> &str { + &self.token + } + + /// HTTPS clone URL for a repository, suitable for `git clone`. + pub fn repo_clone_url(&self, full_name: &str) -> String { + format!( + "{}/{}.git", + self.base_url.trim_end_matches('/'), + full_name.trim_start_matches('/') + ) + } + #[instrument(skip(self))] pub async fn get_authorized_user(&self) -> anyhow::Result { let url = format!("{}/api/v1/user", self.base_url); diff --git a/crates/herald-server/src/main.rs b/crates/herald-server/src/main.rs index 366eab1..2c313e4 100644 --- a/crates/herald-server/src/main.rs +++ b/crates/herald-server/src/main.rs @@ -2,6 +2,7 @@ use crate::{ bot::Bot, gitea::{GiteaAPI, WebhookType}, open_router::OpenRouterClient, + sandbox::SandboxConfig, state::AppState, }; @@ -20,6 +21,7 @@ mod errors; mod gitea; mod metrics; mod open_router; +mod sandbox; mod state; fn main() -> anyhow::Result<()> { @@ -76,6 +78,19 @@ async fn run() -> anyhow::Result<()> { let shutdown = CancellationToken::new(); + let sandbox = SandboxConfig { + enabled: config.sandbox_enabled, + runtime: devcontainer_rs::ContainerRuntime::new(config.container_runtime.clone()), + max_iterations: config.sandbox_max_iterations, + }; + + if sandbox.enabled && !sandbox.runtime.available().await { + warn!( + runtime = sandbox.runtime.program(), + "Sandbox is enabled but the container runtime is not available" + ); + } + let bot = Bot::new( gitea_user.login, gitea_api, @@ -83,6 +98,7 @@ async fn run() -> anyhow::Result<()> { reqwest::Client::new(), config.bot_max_concurrent, config.open_router_model.clone(), + sandbox, ); let (tx, rx) = tokio::sync::mpsc::channel::(config.bot_max_concurrent * 2); diff --git a/crates/herald-server/src/open_router.rs b/crates/herald-server/src/open_router.rs index 4320511..dd093ac 100644 --- a/crates/herald-server/src/open_router.rs +++ b/crates/herald-server/src/open_router.rs @@ -1,6 +1,10 @@ use std::time::Duration; -use openrouter_rs::{Message, api::chat::ChatCompletionRequest}; +use openrouter_rs::{ + Message, + api::chat::ChatCompletionRequest, + types::{Tool, ToolCall}, +}; use tracing::instrument; pub struct ChatResult { @@ -8,6 +12,12 @@ pub struct ChatResult { pub cost: Option, } +pub struct ToolChatResult { + pub message: Option, + pub tool_calls: Vec, + pub cost: Option, +} + #[derive(Clone)] pub struct OpenRouterClient { client: openrouter_rs::OpenRouterClient, @@ -47,4 +57,36 @@ impl OpenRouterClient { cost: response.usage.and_then(|u| u.cost), }) } + + /// Sends a conversation with tool definitions and returns either a final + /// message or the tool calls requested by the model. + #[instrument(skip(self, messages, tools), err)] + pub async fn chat_with_tools( + &self, + messages: Vec, + tools: Vec, + ) -> anyhow::Result { + let request = ChatCompletionRequest::builder() + .model(&self.model) + .enable_reasoning() + .messages(messages) + .tools(tools) + .tool_choice_auto() + .build()?; + + let response = self.client.chat().create(&request).await?; + let choice = response + .choices + .first() + .ok_or_else(|| anyhow::anyhow!("No choices in response"))?; + + Ok(ToolChatResult { + message: choice.content().map(String::from), + tool_calls: choice + .tool_calls() + .map(<[ToolCall]>::to_vec) + .unwrap_or_default(), + cost: response.usage.and_then(|u| u.cost), + }) + } } diff --git a/crates/herald-server/src/sandbox/agent.rs b/crates/herald-server/src/sandbox/agent.rs new file mode 100644 index 0000000..35d53c4 --- /dev/null +++ b/crates/herald-server/src/sandbox/agent.rs @@ -0,0 +1,129 @@ +//! Tool-calling loop driving the model against a [`Sandbox`]. +//! +//! The loop sends the conversation and the available tools to OpenRouter, +//! executes any requested tool call inside the sandbox and feeds the results +//! back, until the model produces a final message or the iteration budget is +//! exhausted. + +use anyhow::Context; +use openrouter_rs::{ + Message, + types::{Role, Tool, ToolCall}, +}; +use serde_json::Value; +use tracing::{debug, warn}; + +use crate::{ + open_router::OpenRouterClient, + sandbox::{Sandbox, tools}, +}; + +/// Final output of an agent run. +pub struct AgentResult { + pub message: String, + pub cost: Option, + pub iterations: usize, +} + +/// Runs the tool-calling loop until the model answers or `max_iterations` is +/// reached. +/// +/// `tool_definitions` is the set of tools the model may call; it is selected by +/// the caller based on the webhook action (see [`tools::for_webhook`]). +pub async fn run( + open_router: &OpenRouterClient, + sandbox: &Sandbox, + tool_definitions: Vec, + system_prompt: &str, + user_prompt: &str, + max_iterations: usize, +) -> anyhow::Result { + let mut messages = vec![ + Message::new(Role::System, system_prompt), + Message::new(Role::User, user_prompt), + ]; + + let mut total_cost = 0.0_f64; + let mut has_cost = false; + + for iteration in 1..=max_iterations { + let response = open_router + .chat_with_tools(messages.clone(), tool_definitions.clone()) + .await?; + + if let Some(cost) = response.cost { + total_cost += cost; + has_cost = true; + } + + if response.tool_calls.is_empty() { + return Ok(AgentResult { + message: response.message.unwrap_or_default(), + cost: has_cost.then_some(total_cost), + iterations: iteration, + }); + } + + messages.push(Message::assistant_with_tool_calls( + response.message.unwrap_or_default(), + response.tool_calls.clone(), + )); + + for call in &response.tool_calls { + debug!(tool = call.name(), "Executing tool call"); + let content = execute(sandbox, call).await; + messages.push(Message::tool_response(call.id(), content)); + } + } + + warn!(max_iterations, "Agent reached the iteration limit"); + anyhow::bail!("agent exceeded the maximum number of iterations ({max_iterations})") +} + +async fn execute(sandbox: &Sandbox, call: &ToolCall) -> String { + let args = match parse_args(call) { + Ok(args) => args, + Err(err) => return format!("error: {err}"), + }; + + match tools::dispatch(sandbox, call.name(), &args).await { + Ok(output) => output, + Err(err) => format!("error: {err}"), + } +} + +fn parse_args(call: &ToolCall) -> anyhow::Result { + let raw = call.arguments_json().trim(); + if raw.is_empty() { + return Ok(Value::Object(serde_json::Map::new())); + } + + serde_json::from_str(raw) + .with_context(|| format!("invalid arguments for tool `{}`", call.name())) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_args_accepts_empty_arguments() { + let call = ToolCall::new("id", "ls", ""); + assert_eq!( + parse_args(&call).unwrap(), + Value::Object(serde_json::Map::new()) + ); + } + + #[test] + fn parse_args_parses_json_object() { + let call = ToolCall::new("id", "ls", r#"{"path":"src"}"#); + assert_eq!(parse_args(&call).unwrap()["path"], "src"); + } + + #[test] + fn parse_args_rejects_invalid_json() { + let call = ToolCall::new("id", "ls", "not json"); + assert!(parse_args(&call).is_err()); + } +} diff --git a/crates/herald-server/src/sandbox/mod.rs b/crates/herald-server/src/sandbox/mod.rs new file mode 100644 index 0000000..f31a507 --- /dev/null +++ b/crates/herald-server/src/sandbox/mod.rs @@ -0,0 +1,197 @@ +//! Sandboxed tool execution for the AI bot. +//! +//! A [`Sandbox`] clones a pull request into a temporary directory, builds and +//! starts its devcontainer (via `devcontainer-rs`) and exposes command +//! execution inside the resulting container. The [`tools`] module maps model +//! tool calls to commands run in that container, and [`agent`] drives the +//! tool-calling loop against OpenRouter. + +pub mod agent; +pub mod tools; + +use std::{ + path::{Path, PathBuf}, + process::Stdio, +}; + +use anyhow::Context; +use devcontainer_rs::{Container, ContainerRuntime, ExecOutput}; +use tempfile::TempDir; +use tracing::{info, instrument}; + +/// Devcontainer locations recognized within a repository, in priority order. +const DEVCONTAINER_PATHS: [&str; 2] = [".devcontainer/devcontainer.json", ".devcontainer.json"]; + +/// Sandbox-related runtime configuration. +#[derive(Clone)] +pub struct SandboxConfig { + /// Whether the bot should run its tools inside a sandbox container. + pub enabled: bool, + /// Container runtime binary to drive (e.g. `docker`, `podman`). + pub runtime: ContainerRuntime, + /// Maximum number of tool-calling iterations per agent run. + pub max_iterations: usize, +} + +/// A cloned repository running inside an ephemeral devcontainer. +pub struct Sandbox { + // Owns the temporary directory; dropping it cleans up the clone. + _workspace: TempDir, + container: Container, +} + +impl Sandbox { + /// Clones the pull request head, builds the devcontainer and starts it. + /// + /// The clone is PR-aware: it fetches `refs/pull//head`, which works + /// for both same-repository and forked pull requests. + #[instrument(skip(runtime, token), fields(pr = pull_request_number))] + pub async fn create( + runtime: &ContainerRuntime, + repo_url: &str, + token: &str, + pull_request_number: u64, + ) -> anyhow::Result { + let workspace = tempfile::tempdir().context("failed to create sandbox workspace")?; + let repo_dir = workspace.path().join("repo"); + + clone_pull_request(repo_url, token, pull_request_number, &repo_dir).await?; + + let devcontainer_path = find_devcontainer(&repo_dir) + .with_context(|| format!("no devcontainer found in `{repo_url}`"))?; + + 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?; + + Ok(Self { + _workspace: workspace, + container, + }) + } + + /// Executes a command in the container as an argv vector (no shell). + pub async fn exec(&self, cmd: &[&str]) -> anyhow::Result { + Ok(self.container.exec(cmd).await?) + } + + /// Path of the repository inside the container. + pub fn workspace_folder(&self) -> &str { + self.container.workspace_folder() + } + + /// Stops and removes the container. The temporary clone is removed on drop. + pub async fn cleanup(self) -> anyhow::Result<()> { + self.container.remove().await?; + Ok(()) + } +} + +fn find_devcontainer(repo_dir: &Path) -> Option { + DEVCONTAINER_PATHS + .iter() + .map(|relative| repo_dir.join(relative)) + .find(|candidate| candidate.is_file()) +} + +async fn clone_pull_request( + repo_url: &str, + token: &str, + pull_request_number: u64, + dest: &Path, +) -> anyhow::Result<()> { + let dest = dest.display().to_string(); + + run_git( + token, + &[ + "clone".to_string(), + "--depth".to_string(), + "1".to_string(), + repo_url.to_string(), + dest.clone(), + ], + ) + .await?; + + run_git( + token, + &[ + "-C".to_string(), + dest.clone(), + "fetch".to_string(), + "--depth".to_string(), + "1".to_string(), + "origin".to_string(), + format!("refs/pull/{pull_request_number}/head"), + ], + ) + .await?; + + run_git( + token, + &[ + "-C".to_string(), + dest, + "checkout".to_string(), + "FETCH_HEAD".to_string(), + ], + ) + .await?; + + Ok(()) +} + +/// Runs git with the token injected through `http.extraHeader`, keeping the +/// secret out of the process arguments. +async fn run_git(token: &str, args: &[String]) -> anyhow::Result<()> { + let output = tokio::process::Command::new("git") + .args(args) + .env("GIT_CONFIG_COUNT", "1") + .env("GIT_CONFIG_KEY_0", "http.extraHeader") + .env( + "GIT_CONFIG_VALUE_0", + format!("Authorization: token {token}"), + ) + .env("GIT_TERMINAL_PROMPT", "0") + .stdin(Stdio::null()) + .output() + .await + .context("failed to spawn git")?; + + if !output.status.success() { + anyhow::bail!( + "git {} failed: {}", + args.join(" "), + String::from_utf8_lossy(&output.stderr).trim() + ); + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn find_devcontainer_prefers_dot_devcontainer_dir() { + let dir = tempfile::tempdir().unwrap(); + let nested = dir.path().join(".devcontainer"); + std::fs::create_dir(&nested).unwrap(); + std::fs::write(nested.join("devcontainer.json"), "{}").unwrap(); + std::fs::write(dir.path().join(".devcontainer.json"), "{}").unwrap(); + + assert_eq!( + find_devcontainer(dir.path()), + Some(nested.join("devcontainer.json")) + ); + } + + #[test] + fn find_devcontainer_returns_none_when_absent() { + let dir = tempfile::tempdir().unwrap(); + assert_eq!(find_devcontainer(dir.path()), None); + } +} diff --git a/crates/herald-server/src/sandbox/tools.rs b/crates/herald-server/src/sandbox/tools.rs new file mode 100644 index 0000000..a595984 --- /dev/null +++ b/crates/herald-server/src/sandbox/tools.rs @@ -0,0 +1,248 @@ +//! Tool definitions exposed to the model and their execution inside a +//! [`Sandbox`]. +//! +//! Every tool is read-only and confined to the repository workspace: paths are +//! resolved relative to the container workspace folder and rejected if they +//! escape it. Commands are executed as argv vectors (never through a shell), +//! so tool arguments cannot be used for shell injection. + +use std::path::Path; + +use anyhow::{Context, bail}; +use devcontainer_rs::{ExecOutput, normalize}; +use openrouter_rs::types::Tool; +use serde_json::{Value, json}; + +use super::Sandbox; +use crate::gitea::WebhookType; + +/// Tools available to the model for a given webhook action. +/// +/// The match is exhaustive on [`WebhookType`], so adding a new action forces a +/// decision here about which tools that action may use. +pub fn for_webhook(webhook: &WebhookType) -> Vec { + match webhook { + WebhookType::Review(_) => review_tools(), + } +} + +/// Read-only tools used to explore a repository during a review. +fn review_tools() -> Vec { + vec![ + Tool::new( + "ls", + "List the entries of a directory inside the repository.", + json!({ + "type": "object", + "properties": { + "path": { + "type": "string", + "description": "Directory path relative to the repository root. Defaults to the repository root." + } + } + }), + ), + Tool::new( + "read_file", + "Read the content of a text file inside the repository.", + json!({ + "type": "object", + "properties": { + "path": { + "type": "string", + "description": "File path relative to the repository root." + }, + "start_line": { + "type": "integer", + "description": "First line to read (1-based, inclusive). Defaults to the first line." + }, + "end_line": { + "type": "integer", + "description": "Last line to read (1-based, inclusive). Defaults to the last line." + } + }, + "required": ["path"] + }), + ), + Tool::new( + "grep", + "Search for a regular expression across the repository files.", + json!({ + "type": "object", + "properties": { + "pattern": { + "type": "string", + "description": "Extended regular expression to search for." + }, + "path": { + "type": "string", + "description": "File or directory to search in, relative to the repository root. Defaults to the repository root." + } + }, + "required": ["pattern"] + }), + ), + Tool::new( + "find", + "Find files by name pattern inside the repository.", + json!({ + "type": "object", + "properties": { + "pattern": { + "type": "string", + "description": "Glob pattern matched against file names, e.g. `*.rs`." + }, + "path": { + "type": "string", + "description": "Directory to search in, relative to the repository root. Defaults to the repository root." + } + }, + "required": ["pattern"] + }), + ), + ] +} + +/// Executes a tool call and returns its textual result. +/// +/// Errors are returned as `Err` so the caller can decide whether to surface +/// them to the model or abort. +pub async fn dispatch(sandbox: &Sandbox, name: &str, args: &Value) -> anyhow::Result { + match name { + "ls" => ls(sandbox, args).await, + "read_file" => read_file(sandbox, args).await, + "grep" => grep(sandbox, args).await, + "find" => find(sandbox, args).await, + other => bail!("unknown tool `{other}`"), + } +} + +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?; + into_stdout(output) +} + +async fn read_file(sandbox: &Sandbox, args: &Value) -> anyhow::Result { + let path = resolve(sandbox, required_str(args, "path")?)?; + + let start = args.get("start_line").and_then(Value::as_u64); + let end = args.get("end_line").and_then(Value::as_u64); + + let output = if start.is_none() && end.is_none() { + sandbox.exec(&["cat", "--", &path]).await? + } else { + let start = start.unwrap_or(1); + let end = end + .map(|line| line.to_string()) + .unwrap_or_else(|| "$".to_string()); + let range = format!("{start},{end}p"); + sandbox.exec(&["sed", "-n", &range, "--", &path]).await? + }; + + into_stdout(output) +} + +async fn grep(sandbox: &Sandbox, args: &Value) -> anyhow::Result { + let pattern = required_str(args, "pattern")?; + let path = resolve(sandbox, optional_str(args, "path").unwrap_or("."))?; + + let output = sandbox + .exec(&["grep", "-rn", "-E", "--", pattern, &path]) + .await?; + + // grep exits with 1 when there is no match, which is not an error. + match output.status { + 0 | 1 => Ok(output.stdout), + _ => bail!("grep failed: {}", output.stderr.trim()), + } +} + +async fn find(sandbox: &Sandbox, args: &Value) -> anyhow::Result { + let pattern = required_str(args, "pattern")?; + let path = resolve(sandbox, optional_str(args, "path").unwrap_or("."))?; + + let output = sandbox + .exec(&["find", &path, "-type", "f", "-name", pattern]) + .await?; + + into_stdout(output) +} + +/// Resolves a tool path against the container workspace folder, rejecting any +/// path that escapes it. +fn resolve(sandbox: &Sandbox, path: &str) -> anyhow::Result { + let workspace = sandbox.workspace_folder(); + let candidate = Path::new(workspace).join(path); + + let normalized = + normalize(&candidate).with_context(|| format!("path `{path}` escapes the workspace"))?; + + if !normalized.starts_with(workspace) { + bail!("path `{path}` escapes the workspace"); + } + + Ok(normalized.display().to_string()) +} + +fn required_str<'a>(args: &'a Value, key: &str) -> anyhow::Result<&'a str> { + args.get(key) + .and_then(Value::as_str) + .with_context(|| format!("`{key}` is required")) +} + +fn optional_str<'a>(args: &'a Value, key: &str) -> Option<&'a str> { + args.get(key).and_then(Value::as_str) +} + +fn into_stdout(output: ExecOutput) -> anyhow::Result { + if output.success() { + Ok(output.stdout) + } else { + bail!( + "command failed (status {}): {}", + output.status, + output.stderr.trim() + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::gitea::{Comment, PullRequest, Repository, ReviewPayload}; + + fn review_webhook() -> WebhookType { + WebhookType::Review(ReviewPayload { + action: "created".to_string(), + pull_request: PullRequest { + diff_url: "https://example.com/diff".to_string(), + number: 1, + title: "My PR".to_string(), + }, + repository: Repository { + full_name: "owner/repo".to_string(), + }, + comment: Comment { + id: 1, + body: "@bot review".to_string(), + }, + }) + } + + #[test] + fn review_webhook_exposes_read_only_tools() { + let names: Vec = for_webhook(&review_webhook()) + .into_iter() + .map(|tool| tool.function.name) + .collect(); + + assert_eq!(names, vec!["ls", "read_file", "grep", "find"]); + } + + #[test] + fn required_str_reports_missing_key() { + let err = required_str(&json!({}), "path").unwrap_err(); + assert!(err.to_string().contains("path")); + } +}