A research_and_code mission failed with: turn executor failed: turn timed out turn executor failed: turn timed out and that is the entire record. Investigating it found: the run produced zero steps and zero output, it died at exactly 700s (TURN_TIMEOUT), the node→claw aliases were bound correctly, and the same zeroclaw team path passes in the `multirole` scenario. So the platform path is fine and the agent simply never finished a turn — but the one place the reason lived, the per-mission runtime container, is torn down after the phase and takes its log with it. By the time anyone looks, all that survives is the string. The timeout now reads the last 40 lines out of that container while it still exists, and reports which agent alias and which gateway it was driving. Best-effort by construction: it runs on a path that is ALREADY failing, so a docker error there degrades to a note rather than replacing the real failure with a second one. `container_name` derives the container from the gateway URL and returns None rather than guessing, because this feeds a diagnostic — a wrong name would put a different container's log under a failure and send the reader somewhere else entirely. Co-Authored-By: Claude Opus 5 <[email protected]>
406 lines
16 KiB
Rust
406 lines
16 KiB
Rust
//! Running a command inside a container, over the Docker API.
|
|
//!
|
|
//! Three call sites needed this and each had shelled out to the `docker` CLI:
|
|
//! the evaluator's verification sandbox, the security scanner, and the
|
|
//! benchmark runner. **The server image does not ship a `docker` binary**
|
|
//! (`images/server.Dockerfile` installs `git ca-certificates chromium
|
|
//! fonts-liberation` and nothing else), so every one of those calls failed
|
|
//! with a spawn error at runtime.
|
|
//!
|
|
//! The failure was invisible in the worst way. `evaluator_tools::Sandbox::run`
|
|
//! turns any execution failure into evidence text rather than an error —
|
|
//! deliberately, so a judge reasons about "that command did not run" instead
|
|
//! of the pass collapsing. With no `docker` binary every verification command
|
|
//! returned `COULD NOT RUN`, the judge correctly concluded it could not verify,
|
|
//! and fail-closed returned "not met". The verdicts were right; the
|
|
//! verification never happened.
|
|
//!
|
|
//! `bollard` was already a dependency and already reaches the daemon through
|
|
//! the socket proxy (`DOCKER_HOST=tcp://socket-proxy:2375`) for every
|
|
//! container operation in `mission_runtime`. This routes command execution the
|
|
//! same way.
|
|
//!
|
|
//! The argv contract is unchanged: a command is a vector, never a shell
|
|
//! string, so the allow-list in `evaluator_tools::check_argv` keeps meaning
|
|
//! what it says.
|
|
|
|
use bollard::exec::{CreateExecOptions, StartExecResults};
|
|
use bollard::Docker;
|
|
use futures::StreamExt;
|
|
use std::time::Duration;
|
|
|
|
/// What a command did. Both streams are captured separately because callers
|
|
/// need them for different things — the evaluator shows the judge stdout *and*
|
|
/// stderr, while the scanners parse JSON from stdout alone and would choke on
|
|
/// interleaved progress output.
|
|
#[derive(Debug, Clone)]
|
|
pub struct ExecOutput {
|
|
/// `None` when the daemon reported no status (a still-running exec, which
|
|
/// we treat as unknown rather than success).
|
|
pub exit_code: Option<i64>,
|
|
pub stdout: String,
|
|
pub stderr: String,
|
|
}
|
|
|
|
impl ExecOutput {
|
|
/// Exit status 0. An absent status is **not** success — an exec whose
|
|
/// status could not be read must not be reported as a passing test run.
|
|
pub fn success(&self) -> bool {
|
|
self.exit_code == Some(0)
|
|
}
|
|
|
|
/// Both streams in the order a human reads them. Used where the consumer
|
|
/// is a model rather than a parser.
|
|
pub fn combined(&self) -> String {
|
|
let mut out = String::new();
|
|
if !self.stdout.trim().is_empty() {
|
|
out.push_str(&self.stdout);
|
|
}
|
|
if !self.stderr.trim().is_empty() {
|
|
if !out.is_empty() {
|
|
out.push('\n');
|
|
}
|
|
out.push_str(&self.stderr);
|
|
}
|
|
out
|
|
}
|
|
}
|
|
|
|
/// Connect to the Docker daemon the same way `mission_runtime` does: honour
|
|
/// `DOCKER_HOST` when set (the socket proxy in production), else the local
|
|
/// socket.
|
|
pub fn connect() -> Result<Docker, String> {
|
|
if std::env::var("DOCKER_HOST").is_ok() {
|
|
Docker::connect_with_defaults().map_err(|e| format!("docker connect (DOCKER_HOST): {e}"))
|
|
} else {
|
|
Docker::connect_with_local_defaults().map_err(|e| format!("docker connect (local): {e}"))
|
|
}
|
|
}
|
|
|
|
/// The uid every mission artefact must belong to.
|
|
///
|
|
/// The runtime container's own processes already run as this; only `docker
|
|
/// exec` defaulted to root, because `CreateExecOptions::user` was never set.
|
|
/// That one omission is the origin of four separate patches: root-owned
|
|
/// `target/` directories appearing inside a checkout that uid 65532 then could
|
|
/// not delete, `root_copy` existing at all, and a cleanup path that had to
|
|
/// re-enter the container as root to undo what it had just done.
|
|
pub(crate) const MISSION_UID: &str = "65532:65532";
|
|
|
|
/// Environment a non-root exec needs, because the image gives uid 65532 no
|
|
/// writable `HOME` and no writable `CARGO_HOME`.
|
|
///
|
|
/// Measured in the deployed image: `/zeroclaw-data` (its `HOME`) and
|
|
/// `/usr/local/cargo` are both root-owned and unwritable, so switching execs to
|
|
/// 65532 without this would break every `cargo` invocation — the benchmark
|
|
/// runner, the judge's verification sandbox, and the delivery test gate — in a
|
|
/// new and much quieter way than the problem it fixes.
|
|
///
|
|
/// The missions root is bind-mounted into the runtime container at the same
|
|
/// path and IS writable by 65532, so the cargo cache lives there and is shared
|
|
/// across missions rather than re-downloaded per mission. Verified end to end:
|
|
/// a clean `cargo build` as 65532 with these three variables produces output
|
|
/// owned entirely by 65532.
|
|
fn mission_env() -> Vec<String> {
|
|
let root = crate::mission_workspace::missions_root();
|
|
vec![
|
|
format!("HOME={}", root.join("_home").display()),
|
|
format!("CARGO_HOME={}", root.join("_cargo").display()),
|
|
"TMPDIR=/tmp".to_string(),
|
|
]
|
|
}
|
|
|
|
/// Whether a workdir is inside the tree missions own.
|
|
///
|
|
/// The rule is positional rather than per-caller on purpose. Twelve call sites
|
|
/// each remembering to pass a uid is twelve chances to forget, and the one that
|
|
/// forgets leaves debris the others cannot clean up — which is exactly the
|
|
/// history here.
|
|
fn is_mission_path(workdir: Option<&str>) -> bool {
|
|
let Some(dir) = workdir else { return false };
|
|
let root = crate::mission_workspace::missions_root();
|
|
std::path::Path::new(dir).starts_with(&root)
|
|
}
|
|
|
|
/// Run `argv` in `container`, optionally in `workdir`, and capture both
|
|
/// streams plus the exit status.
|
|
///
|
|
/// `timeout` bounds the whole exec. On expiry the error says so explicitly:
|
|
/// the exec may still be running inside the container, and a caller that
|
|
/// retries needs to know it is not looking at a clean slate.
|
|
pub async fn exec(
|
|
docker: &Docker,
|
|
container: &str,
|
|
workdir: Option<&str>,
|
|
argv: &[String],
|
|
timeout: Duration,
|
|
) -> Result<ExecOutput, String> {
|
|
exec_with_env(docker, container, workdir, argv, &[], timeout).await
|
|
}
|
|
|
|
/// Run `argv` as **root**, deliberately.
|
|
///
|
|
/// The one legitimate use is clearing debris that earlier root-run execs left
|
|
/// behind: uid 65532 cannot delete a root-owned `target/`, so the cleanup has
|
|
/// to out-rank it. Every other caller goes through [`exec`], which runs mission
|
|
/// work as 65532 so no new debris is created.
|
|
pub async fn exec_as_root(
|
|
docker: &Docker,
|
|
container: &str,
|
|
workdir: Option<&str>,
|
|
argv: &[String],
|
|
timeout: Duration,
|
|
) -> Result<ExecOutput, String> {
|
|
let fut = exec_inner(docker, container, workdir, argv, &[], None);
|
|
match tokio::time::timeout(timeout, fut).await {
|
|
Err(_) => Err(format!(
|
|
"timed out after {}s (the command may still be running in {container})",
|
|
timeout.as_secs()
|
|
)),
|
|
Ok(res) => res,
|
|
}
|
|
}
|
|
|
|
/// As [`exec`], with extra environment for the command.
|
|
pub async fn exec_with_env(
|
|
docker: &Docker,
|
|
container: &str,
|
|
workdir: Option<&str>,
|
|
argv: &[String],
|
|
env: &[String],
|
|
timeout: Duration,
|
|
) -> Result<ExecOutput, String> {
|
|
// Mission work runs as 65532 with a writable HOME/CARGO_HOME; anything
|
|
// outside the missions tree (runtime preflight probes, image checks) keeps
|
|
// the daemon's default so this cannot break unrelated call sites.
|
|
let (user, mut full_env) = if is_mission_path(workdir) {
|
|
(Some(MISSION_UID), mission_env())
|
|
} else {
|
|
(None, Vec::new())
|
|
};
|
|
full_env.extend_from_slice(env);
|
|
let fut = exec_inner(docker, container, workdir, argv, &full_env, user);
|
|
match tokio::time::timeout(timeout, fut).await {
|
|
Err(_) => Err(format!(
|
|
"timed out after {}s (the command may still be running in {container})",
|
|
timeout.as_secs()
|
|
)),
|
|
Ok(res) => res,
|
|
}
|
|
}
|
|
|
|
async fn exec_inner(
|
|
docker: &Docker,
|
|
container: &str,
|
|
workdir: Option<&str>,
|
|
argv: &[String],
|
|
env: &[String],
|
|
user: Option<&str>,
|
|
) -> Result<ExecOutput, String> {
|
|
let created = docker
|
|
.create_exec(
|
|
container,
|
|
CreateExecOptions {
|
|
cmd: Some(argv.to_vec()),
|
|
working_dir: workdir.map(str::to_string),
|
|
env: if env.is_empty() {
|
|
None
|
|
} else {
|
|
Some(env.to_vec())
|
|
},
|
|
user: user.map(str::to_string),
|
|
attach_stdout: Some(true),
|
|
attach_stderr: Some(true),
|
|
..Default::default()
|
|
},
|
|
)
|
|
.await
|
|
.map_err(|e| format!("create_exec on {container}: {e}"))?;
|
|
|
|
let started = docker
|
|
.start_exec(&created.id, None)
|
|
.await
|
|
.map_err(|e| format!("start_exec on {container}: {e}"))?;
|
|
let StartExecResults::Attached { mut output, .. } = started else {
|
|
return Err(format!("exec on {container} returned a detached result"));
|
|
};
|
|
|
|
// Keep the streams apart. `LogOutput`'s Display merges them, which is what
|
|
// the previous helper used and why nothing downstream could tell a JSON
|
|
// payload from a progress bar.
|
|
let mut stdout = String::new();
|
|
let mut stderr = String::new();
|
|
while let Some(chunk) = output.next().await {
|
|
match chunk {
|
|
Ok(bollard::container::LogOutput::StdOut { message }) => {
|
|
stdout.push_str(&String::from_utf8_lossy(&message));
|
|
}
|
|
Ok(bollard::container::LogOutput::StdErr { message }) => {
|
|
stderr.push_str(&String::from_utf8_lossy(&message));
|
|
}
|
|
// A container without a TTY still emits Console/StdIn frames in
|
|
// some daemon versions; treat them as stdout rather than dropping
|
|
// output on the floor.
|
|
Ok(other) => stdout.push_str(&other.to_string()),
|
|
Err(e) => return Err(format!("exec output stream on {container}: {e}")),
|
|
}
|
|
}
|
|
|
|
// The status is only available after the stream drains.
|
|
let inspected = docker
|
|
.inspect_exec(&created.id)
|
|
.await
|
|
.map_err(|e| format!("inspect_exec on {container}: {e}"))?;
|
|
|
|
Ok(ExecOutput {
|
|
exit_code: inspected.exit_code,
|
|
stdout,
|
|
stderr,
|
|
})
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
|
|
fn out(code: Option<i64>, stdout: &str, stderr: &str) -> ExecOutput {
|
|
ExecOutput {
|
|
exit_code: code,
|
|
stdout: stdout.into(),
|
|
stderr: stderr.into(),
|
|
}
|
|
}
|
|
|
|
/// An exec whose status could not be read must not pass for success —
|
|
/// `commit_policy = "on_green_tests"` gates on exactly this, and treating
|
|
/// "unknown" as "green" would push untested work.
|
|
#[test]
|
|
fn an_unknown_exit_status_is_not_success() {
|
|
assert!(out(Some(0), "ok", "").success());
|
|
assert!(!out(Some(1), "", "boom").success());
|
|
assert!(!out(None, "ok", "").success());
|
|
}
|
|
|
|
#[test]
|
|
fn combined_keeps_both_streams_and_skips_empty_ones() {
|
|
assert_eq!(out(Some(0), "hello", "").combined(), "hello");
|
|
assert_eq!(out(Some(1), "", "bad").combined(), "bad");
|
|
assert_eq!(out(Some(1), "a", "b").combined(), "a\nb");
|
|
assert_eq!(out(Some(0), " ", "\n").combined(), "");
|
|
}
|
|
|
|
/// Mission work is 65532; everything else keeps the daemon's default.
|
|
///
|
|
/// The rule is positional so that no caller has to remember it. Twelve call
|
|
/// sites each passing a uid is twelve chances to forget, and the one that
|
|
/// forgets leaves debris the other eleven cannot delete — which is the
|
|
/// actual history: root-owned `target/` directories inside a checkout owned
|
|
/// by 65532, `root_copy` written to work around them, and a cleanup that had
|
|
/// to re-enter the container as root to undo its own mess.
|
|
#[test]
|
|
fn only_work_inside_the_missions_tree_drops_to_the_mission_uid() {
|
|
let root = crate::mission_workspace::missions_root();
|
|
let inside = root.join("019fe785-0f82-7780-8d58-da79fb4c31bc/repo");
|
|
assert!(is_mission_path(Some(&inside.display().to_string())));
|
|
assert!(is_mission_path(Some(&root.display().to_string())));
|
|
|
|
// Probes and image checks run with no workdir at all, and must not be
|
|
// forced to a uid the image may not have set up for them.
|
|
assert!(!is_mission_path(None));
|
|
assert!(!is_mission_path(Some("/")));
|
|
assert!(!is_mission_path(Some("/usr/local/cargo")));
|
|
// A path that merely SHARES A PREFIX is not inside the tree.
|
|
// `starts_with` on `Path` compares components, so this is already true;
|
|
// the assertion is here so a switch to string matching cannot pass.
|
|
let sibling = format!("{}-evil/repo", root.display());
|
|
assert!(!is_mission_path(Some(&sibling)));
|
|
}
|
|
|
|
/// The non-root exec carries the three variables the image does not give it.
|
|
///
|
|
/// Measured in the deployed image: uid 65532's `HOME` (`/zeroclaw-data`)
|
|
/// and `/usr/local/cargo` are both root-owned and unwritable. Without these
|
|
/// overrides, dropping execs to 65532 would break every cargo invocation —
|
|
/// the benchmark runner, the judge's sandbox, the delivery test gate — far
|
|
/// more quietly than the leak it fixes.
|
|
#[test]
|
|
fn the_mission_env_replaces_the_paths_the_image_leaves_unwritable() {
|
|
let env = mission_env();
|
|
let root = crate::mission_workspace::missions_root();
|
|
assert!(env.iter().any(|v| v == &format!("HOME={}/_home", root.display())));
|
|
assert!(env.iter().any(|v| v == &format!("CARGO_HOME={}/_cargo", root.display())));
|
|
assert!(env.iter().any(|v| v == "TMPDIR=/tmp"));
|
|
for v in &env {
|
|
assert!(
|
|
!v.contains("/usr/local/cargo") && !v.contains("/zeroclaw-data"),
|
|
"{v} points back at a root-owned path"
|
|
);
|
|
}
|
|
}
|
|
|
|
/// One place builds an exec, so one place decides its uid.
|
|
///
|
|
/// The original bug was not a wrong value — it was an ABSENT one:
|
|
/// `CreateExecOptions` never set `user`, so the daemon defaulted to root
|
|
/// and twelve callers inherited that without any of them choosing it. A
|
|
/// second construction site is how that comes back, so the guard is on the
|
|
/// number of sites rather than on any particular uid.
|
|
#[test]
|
|
fn exactly_one_place_builds_an_exec() {
|
|
let src = include_str!("container_exec.rs");
|
|
// Split so this needle does not match itself in this very file.
|
|
let needle = concat!("CreateExec", "Options {");
|
|
let sites = src.matches(needle).count();
|
|
assert_eq!(
|
|
sites, 1,
|
|
"exec options must be built in one place; found {sites}"
|
|
);
|
|
assert!(
|
|
src.contains(concat!("user: ", "user.map(str::to_string)")),
|
|
"that one place must set `user` — leaving it unset is the bug"
|
|
);
|
|
}
|
|
}
|
|
|
|
/// The tail of a container's log, for putting in an error message.
|
|
///
|
|
/// A turn that times out destroys the only place the reason lived: the
|
|
/// per-mission runtime container is torn down after the phase, taking its logs
|
|
/// with it, and the operator is left with the string "turn timed out". This
|
|
/// copies the last few lines out while the container still exists.
|
|
///
|
|
/// Best-effort by construction — it runs on a path that is ALREADY failing, so
|
|
/// every error here degrades to a note rather than replacing the real failure
|
|
/// with a docker one.
|
|
pub async fn tail_logs(container: &str, lines: usize) -> String {
|
|
use futures::StreamExt as _;
|
|
|
|
let Ok(docker) = connect() else {
|
|
return "(docker unreachable, so no container log)".into();
|
|
};
|
|
let opts = bollard::query_parameters::LogsOptionsBuilder::default()
|
|
.stdout(true)
|
|
.stderr(true)
|
|
.tail(&lines.to_string())
|
|
.build();
|
|
let mut stream = docker.logs(container, Some(opts));
|
|
let mut out = String::new();
|
|
while let Some(chunk) = stream.next().await {
|
|
match chunk {
|
|
Ok(c) => out.push_str(&c.to_string()),
|
|
Err(e) => {
|
|
if out.is_empty() {
|
|
return format!("(could not read {container} logs: {e})");
|
|
}
|
|
break;
|
|
}
|
|
}
|
|
}
|
|
let out = out.trim();
|
|
if out.is_empty() {
|
|
format!("({container} logged nothing)")
|
|
} else {
|
|
out.to_string()
|
|
}
|
|
}
|