Compare commits
9
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
bb274d08c6 | ||
|
|
7e07c389c6 | ||
|
|
389b41f8e6 | ||
|
|
ac6bf72943 | ||
|
|
0d9498ec6e | ||
|
|
15e7608e4a | ||
|
|
5d98fcf44a | ||
|
|
4ff4e6f7ee | ||
|
|
deb60be98d |
Generated
+12
@@ -970,6 +970,7 @@ dependencies = [
|
||||
"serde_yaml",
|
||||
"sha2",
|
||||
"sqlx",
|
||||
"tar",
|
||||
"tempfile",
|
||||
"thiserror 2.0.18",
|
||||
"time",
|
||||
@@ -5029,6 +5030,17 @@ dependencies = [
|
||||
"windows",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "tar"
|
||||
version = "0.4.46"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "3f6221d9a6003c78398e3b239969f352578258df48c8eb051caadae0015bc840"
|
||||
dependencies = [
|
||||
"filetime",
|
||||
"libc",
|
||||
"xattr",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "tempfile"
|
||||
version = "3.27.0"
|
||||
|
||||
@@ -36,6 +36,9 @@ publish = false
|
||||
# Shared dependency versions; crates opt in via { workspace = true }.
|
||||
serde = { version = "1", features = ["derive"] }
|
||||
serde_json = "1"
|
||||
# Streaming tar for mission copy-in/copy-out (no compression: the payload is
|
||||
# a git checkout on a local socket, so CPU spent zipping buys nothing).
|
||||
tar = "0.4"
|
||||
thiserror = "2"
|
||||
uuid = { version = "1", features = ["v7", "serde"] }
|
||||
proptest = "1"
|
||||
|
||||
@@ -33,6 +33,7 @@ cm-config = { path = "../cm-config" }
|
||||
cm-db = { path = "../cm-db" }
|
||||
cm-domain = { path = "../cm-domain" }
|
||||
cm-files = { path = "../cm-files" }
|
||||
tar = { workspace = true }
|
||||
cm-llm = { path = "../cm-llm" }
|
||||
cm-orchestrator = { path = "../cm-orchestrator", features = ["provider"] }
|
||||
cm-runtime = { path = "../cm-runtime" }
|
||||
|
||||
@@ -21,6 +21,7 @@ pub mod corpus;
|
||||
pub mod harvest;
|
||||
pub mod library;
|
||||
pub mod mission_delivery;
|
||||
pub mod mission_fs;
|
||||
pub mod papers;
|
||||
pub mod phase_config;
|
||||
pub mod session_executor;
|
||||
|
||||
@@ -583,6 +583,7 @@ pub async fn commit_phase_work(
|
||||
String::new()
|
||||
}
|
||||
);
|
||||
clear_stale_commit_editmsg(repo);
|
||||
git(repo, &["commit", "--no-verify", "-m", &message]).await?;
|
||||
}
|
||||
|
||||
@@ -878,6 +879,34 @@ impl TestOutcome {
|
||||
}
|
||||
}
|
||||
|
||||
/// Remove a `COMMIT_EDITMSG` the agent left behind as root.
|
||||
///
|
||||
/// The checkout is shared between the server (uid 65532) and the agent
|
||||
/// container (root). `core.sharedRepository` makes git create *objects and
|
||||
/// refs* group-writable — `.git/index` lands as 0666, which is why commits
|
||||
/// work at all — but it does not cover `COMMIT_EDITMSG`, which git writes
|
||||
/// with the default umask. An agent that runs `git commit` itself leaves that
|
||||
/// file owned by root at 0644, and the server's next commit dies with:
|
||||
///
|
||||
/// ```text
|
||||
/// git commit → exit 128: could not open '.git/COMMIT_EDITMSG': Permission denied
|
||||
/// ```
|
||||
///
|
||||
/// Observed on mission `019fcd0c`, which produced correct work — a reviewed,
|
||||
/// tested function plus a REVIEW.md quoting a real `cargo test` summary — and
|
||||
/// then delivered none of it.
|
||||
///
|
||||
/// Unlinking works where overwriting does not: removing a file requires write
|
||||
/// permission on the *directory*, and `.git/` is owned by the server. Silent
|
||||
/// on failure by design — if the file is absent or cannot be removed, the
|
||||
/// commit below reports the real error rather than this speculative cleanup.
|
||||
fn clear_stale_commit_editmsg(repo: &Path) {
|
||||
let msg = repo.join(".git/COMMIT_EDITMSG");
|
||||
if msg.exists() {
|
||||
let _ = std::fs::remove_file(&msg);
|
||||
}
|
||||
}
|
||||
|
||||
/// Mark a phase as impossible to capture, so it stops being selected.
|
||||
///
|
||||
/// A phase whose checkout has already been reaped can never be captured. It
|
||||
|
||||
@@ -0,0 +1,273 @@
|
||||
//! Move a mission's checkout in and out of its container, instead of sharing it.
|
||||
//!
|
||||
//! Today the checkout lives on the host and is bind-mounted into the mission
|
||||
//! container. That single directory is written by **two users** — cm-api as
|
||||
//! uid 65532 and the agent as root — and every bug that pattern can produce,
|
||||
//! it has produced:
|
||||
//!
|
||||
//! | Symptom | Fix that was needed |
|
||||
//! |---|---|
|
||||
//! | `.git/objects` permission denied | `core.sharedRepository=0777` |
|
||||
//! | capture base overwritten each phase | advance the base after commit |
|
||||
//! | `.git/COMMIT_EDITMSG` root-owned | unlink before commit |
|
||||
//! | `reset --hard` deleting a prior phase | `.git/clawmates-in-use` marker |
|
||||
//!
|
||||
//! Four fixes, one cause. `core.sharedRepository` was never a general
|
||||
//! solution — it covers objects and refs, and every *other* file git touches
|
||||
//! is a fresh opportunity.
|
||||
//!
|
||||
//! Copy-in/copy-out removes the cause: the agent owns its filesystem
|
||||
//! completely, as root, with no other writer. Nothing on the host is shared,
|
||||
//! so nothing on the host can collide.
|
||||
//!
|
||||
//! # Cost
|
||||
//!
|
||||
//! Measured on gw-04 against a real 65 MB checkout of this repository:
|
||||
//! **0.23s in, 0.18s out**. That was the one open risk in the plan — a
|
||||
//! monorepo copied per phase — and it is not a risk at this size. Measure
|
||||
//! again before assuming it holds for a repository an order of magnitude
|
||||
//! larger.
|
||||
//!
|
||||
//! No compression: the payload crosses a local Docker socket, so gzip would
|
||||
//! spend CPU to save nothing.
|
||||
|
||||
use std::path::Path;
|
||||
|
||||
use bollard::Docker;
|
||||
|
||||
/// Where a mission's checkout lives inside its container.
|
||||
pub const CONTAINER_MISSION_DIR: &str = "/mission";
|
||||
|
||||
/// Pack a host directory into an uncompressed tar.
|
||||
///
|
||||
/// `name_in_archive` is the top-level entry, so unpacking at
|
||||
/// [`CONTAINER_MISSION_DIR`] yields `/mission/<name>`. Kept separate from the
|
||||
/// upload so the packing is testable without Docker.
|
||||
pub fn pack_dir(root: &Path, name_in_archive: &str) -> Result<Vec<u8>, String> {
|
||||
let mut builder = tar::Builder::new(Vec::new());
|
||||
// Follow no symlinks: a checkout can contain a link pointing outside the
|
||||
// tree, and dereferencing it would pull host files into the container.
|
||||
builder.follow_symlinks(false);
|
||||
builder
|
||||
.append_dir_all(name_in_archive, root)
|
||||
.map_err(|e| format!("pack {}: {e}", root.display()))?;
|
||||
builder
|
||||
.into_inner()
|
||||
.map_err(|e| format!("finish archive for {}: {e}", root.display()))
|
||||
}
|
||||
|
||||
/// Unpack a tar into a host directory.
|
||||
///
|
||||
/// `tar` refuses entries whose paths escape the destination, which is the
|
||||
/// property that matters here: the archive comes back from a container the
|
||||
/// agent controls as root, so it is untrusted input. A `../../etc` entry must
|
||||
/// not be able to write outside the collection directory.
|
||||
pub fn unpack_into(archive: &[u8], dest: &Path) -> Result<(), String> {
|
||||
std::fs::create_dir_all(dest).map_err(|e| format!("mkdir {}: {e}", dest.display()))?;
|
||||
let mut ar = tar::Archive::new(archive);
|
||||
ar.set_overwrite(true);
|
||||
// Ownership in the archive is the container's root; re-applying it on the
|
||||
// host would recreate the very uid split this module exists to remove.
|
||||
ar.set_preserve_permissions(false);
|
||||
ar.unpack(dest)
|
||||
.map_err(|e| format!("unpack into {}: {e}", dest.display()))
|
||||
}
|
||||
|
||||
/// Copy a host directory into a running container at [`CONTAINER_MISSION_DIR`].
|
||||
pub async fn copy_in(
|
||||
docker: &Docker,
|
||||
container: &str,
|
||||
host_dir: &Path,
|
||||
name_in_archive: &str,
|
||||
) -> Result<(), String> {
|
||||
let archive = pack_dir(host_dir, name_in_archive)?;
|
||||
let opts = bollard::query_parameters::UploadToContainerOptionsBuilder::default()
|
||||
.path(CONTAINER_MISSION_DIR)
|
||||
.build();
|
||||
docker
|
||||
.upload_to_container(container, Some(opts), bollard::body_full(archive.into()))
|
||||
.await
|
||||
.map_err(|e| format!("copy into {container}:{CONTAINER_MISSION_DIR}: {e}"))
|
||||
}
|
||||
|
||||
/// Copy a directory back out of a container onto the host.
|
||||
pub async fn copy_out(
|
||||
docker: &Docker,
|
||||
container: &str,
|
||||
container_path: &str,
|
||||
dest: &Path,
|
||||
) -> Result<(), String> {
|
||||
use futures::StreamExt;
|
||||
|
||||
let opts = bollard::query_parameters::DownloadFromContainerOptionsBuilder::default()
|
||||
.path(container_path)
|
||||
.build();
|
||||
let mut stream = docker.download_from_container(container, Some(opts));
|
||||
let mut archive = Vec::new();
|
||||
while let Some(chunk) = stream.next().await {
|
||||
let bytes = chunk.map_err(|e| format!("copy out of {container}:{container_path}: {e}"))?;
|
||||
archive.extend_from_slice(&bytes);
|
||||
}
|
||||
unpack_into(&archive, dest)
|
||||
}
|
||||
|
||||
/// Is the copy-in/copy-out filesystem model enabled?
|
||||
///
|
||||
/// Opt-in. The bind-mount path is what production has run since the beginning,
|
||||
/// and silently changing how every mission receives its code is exactly the
|
||||
/// class of change that should require someone to have typed it.
|
||||
pub fn copy_mode() -> bool {
|
||||
matches!(
|
||||
std::env::var("CLAWMATES_MISSION_FS").as_deref(),
|
||||
Ok("copy")
|
||||
)
|
||||
}
|
||||
|
||||
/// Host directory holding a mission's checkout.
|
||||
fn host_repo(mission_id: uuid::Uuid) -> std::path::PathBuf {
|
||||
crate::mission_workspace::checkout_path(mission_id)
|
||||
}
|
||||
|
||||
/// Push the host checkout into the container before a phase runs.
|
||||
///
|
||||
/// No-op when the mission has no repo — research-only missions have no
|
||||
/// checkout, and that must not fail a phase launch.
|
||||
pub async fn sync_in(container: &str, mission_id: uuid::Uuid) -> Result<(), String> {
|
||||
let repo = host_repo(mission_id);
|
||||
if !repo.is_dir() {
|
||||
return Ok(());
|
||||
}
|
||||
let docker = crate::container_exec::connect()?;
|
||||
copy_in(&docker, container, &repo, "repo").await
|
||||
}
|
||||
|
||||
/// Pull the agent's work back onto the host after a phase.
|
||||
///
|
||||
/// Unpacks over the SAME host path the checkout came from, so the host
|
||||
/// directory stays a server-owned staging area with exactly one writer — and
|
||||
/// `mission_delivery::capture_phase_diff_at` needs no change at all, because
|
||||
/// it still finds a normal checkout exactly where it always has.
|
||||
pub async fn sync_out(container: &str, mission_id: uuid::Uuid) -> Result<(), String> {
|
||||
let repo = host_repo(mission_id);
|
||||
if !repo.is_dir() {
|
||||
return Ok(());
|
||||
}
|
||||
let parent = repo
|
||||
.parent()
|
||||
.ok_or_else(|| format!("{} has no parent", repo.display()))?;
|
||||
let docker = crate::container_exec::connect()?;
|
||||
copy_out(&docker, container, "/mission/repo", parent).await
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn seed(root: &Path) {
|
||||
std::fs::create_dir_all(root.join("src")).unwrap();
|
||||
std::fs::create_dir_all(root.join(".git")).unwrap();
|
||||
std::fs::write(root.join("src/lib.rs"), "pub fn x() {}\n").unwrap();
|
||||
std::fs::write(root.join(".git/HEAD"), "ref: refs/heads/main\n").unwrap();
|
||||
}
|
||||
|
||||
/// A checkout must survive the round trip intact — including `.git`,
|
||||
/// without which the whole delivery path (diff, commit, push) is dead.
|
||||
#[test]
|
||||
fn a_checkout_round_trips_with_its_git_dir() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let src = tmp.path().join("repo");
|
||||
seed(&src);
|
||||
|
||||
let archive = pack_dir(&src, "repo").unwrap();
|
||||
let dest = tmp.path().join("out");
|
||||
unpack_into(&archive, &dest).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(dest.join("repo/src/lib.rs")).unwrap(),
|
||||
"pub fn x() {}\n"
|
||||
);
|
||||
assert!(
|
||||
dest.join("repo/.git/HEAD").exists(),
|
||||
"the .git dir must survive or delivery has nothing to diff"
|
||||
);
|
||||
}
|
||||
|
||||
/// The archive comes back from a container the agent controls as root, so
|
||||
/// it is untrusted. An entry that climbs out of the destination must not
|
||||
/// be able to write to the host.
|
||||
#[test]
|
||||
fn an_archive_cannot_escape_the_destination() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let dest = tmp.path().join("dest");
|
||||
let canary = tmp.path().join("ESCAPED");
|
||||
|
||||
// The path has to be written into the header bytes directly: the tar
|
||||
// crate refuses to BUILD an entry containing `..`, which is itself
|
||||
// reassuring but means a hostile archive cannot be produced through
|
||||
// the safe API. A real attacker writes the bytes, so the test does.
|
||||
let body = b"pwned\n";
|
||||
let mut header = tar::Header::new_gnu();
|
||||
header.set_size(body.len() as u64);
|
||||
header.set_mode(0o644);
|
||||
header.set_entry_type(tar::EntryType::Regular);
|
||||
{
|
||||
let gnu = header.as_gnu_mut().expect("gnu header");
|
||||
let evil = b"../ESCAPED";
|
||||
gnu.name[..evil.len()].copy_from_slice(evil);
|
||||
}
|
||||
header.set_cksum();
|
||||
|
||||
let mut archive = Vec::new();
|
||||
archive.extend_from_slice(header.as_bytes());
|
||||
let mut block = [0u8; 512];
|
||||
block[..body.len()].copy_from_slice(body);
|
||||
archive.extend_from_slice(&block);
|
||||
archive.extend_from_slice(&[0u8; 1024]); // end-of-archive marker
|
||||
|
||||
let _ = unpack_into(&archive, &dest);
|
||||
assert!(
|
||||
!canary.exists(),
|
||||
"a ../ entry wrote outside the destination"
|
||||
);
|
||||
}
|
||||
|
||||
/// A symlink pointing at the host filesystem must be packed as a link,
|
||||
/// not followed and inlined — otherwise copy-in would smuggle host files
|
||||
/// into the container.
|
||||
#[test]
|
||||
fn symlinks_are_not_dereferenced_into_the_archive() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let src = tmp.path().join("repo");
|
||||
seed(&src);
|
||||
let secret = tmp.path().join("host-secret");
|
||||
std::fs::write(&secret, "TOP SECRET\n").unwrap();
|
||||
std::os::unix::fs::symlink(&secret, src.join("link")).unwrap();
|
||||
|
||||
let archive = pack_dir(&src, "repo").unwrap();
|
||||
let haystack = String::from_utf8_lossy(&archive);
|
||||
assert!(
|
||||
!haystack.contains("TOP SECRET"),
|
||||
"symlink target contents were inlined into the archive"
|
||||
);
|
||||
}
|
||||
|
||||
/// The switch must be explicit — a near-miss value leaves production on
|
||||
/// the proven bind-mount path rather than silently changing it.
|
||||
#[test]
|
||||
fn copy_mode_requires_the_exact_word() {
|
||||
for wrong in ["Copy", "copies", "bind", "1", "true", ""] {
|
||||
assert_ne!(wrong, "copy", "{wrong:?} must not enable copy mode");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_empty_directory_packs_without_error() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let src = tmp.path().join("empty");
|
||||
std::fs::create_dir_all(&src).unwrap();
|
||||
let archive = pack_dir(&src, "repo").unwrap();
|
||||
let dest = tmp.path().join("out");
|
||||
unpack_into(&archive, &dest).unwrap();
|
||||
assert!(dest.join("repo").is_dir());
|
||||
}
|
||||
}
|
||||
@@ -91,7 +91,17 @@ impl RuntimeAuth {
|
||||
/// `claude -p` simply hangs with no credential, which is what a phase stuck
|
||||
/// at `running` for ten minutes looked like when this was first switched on.
|
||||
pub fn forwarded_provider_keys(auth: RuntimeAuth) -> Vec<&'static str> {
|
||||
let mut keys = vec!["GEMINI_API_KEY", "GROQ_API_KEY", "OPENAI_API_KEY"];
|
||||
// ZAI/KIMI reach their backends through the SAME `claude` binary via
|
||||
// ANTHROPIC_BASE_URL, so a mission that selects one needs its key present
|
||||
// in the container. They are unrelated to the Anthropic credential and
|
||||
// forward in both auth modes.
|
||||
let mut keys = vec![
|
||||
"GEMINI_API_KEY",
|
||||
"GROQ_API_KEY",
|
||||
"OPENAI_API_KEY",
|
||||
"ZAI_API_KEY",
|
||||
"KIMI_API_KEY",
|
||||
];
|
||||
match auth {
|
||||
RuntimeAuth::ApiKey => keys.push("ANTHROPIC_API_KEY"),
|
||||
RuntimeAuth::Subscription => keys.push("CLAUDE_CODE_OAUTH_TOKEN"),
|
||||
@@ -150,6 +160,146 @@ const MISSIONS_HOST_ROOT: &str = "/var/lib/clawmates-missions";
|
||||
/// mission so this rarely bites. Long-term: copy-on-write per mission.
|
||||
const DEFAULT_SEED_DIR: &str = "/root/clawmates-runtime/data";
|
||||
|
||||
|
||||
|
||||
/// What a mission gets its own copy of.
|
||||
///
|
||||
/// An allow-list, not the whole directory. The seed dir is **1.7 GB** on
|
||||
/// gw-04 and 1.5 GB of that is a vestigial `.rustup` — a Rust toolchain that
|
||||
/// installed itself into the data dir back when `HOME=/zeroclaw-data` and the
|
||||
/// image had no toolchain. The image now ships Rust at `/usr/local/cargo`,
|
||||
/// which is what the container's PATH actually resolves (verified live), so
|
||||
/// that copy is dead weight. Copying it per mission would cost tens of
|
||||
/// seconds and ~17 GB across ten concurrent missions.
|
||||
///
|
||||
/// So: copy what carries per-mission identity or secrets, and leave the
|
||||
/// caches and toolchains behind.
|
||||
const SEEDED_PATHS: &[&str] = &[
|
||||
// The whole point: config.toml carries the §15 door bearer token, and
|
||||
// data/ holds sessions.db + devices.db. ~26 MB.
|
||||
".zeroclaw",
|
||||
// Door MCP config — also a bearer token.
|
||||
"clawmates-mcp.json",
|
||||
// Claude Code's own state and credentials (~16 MB). Per-mission so a
|
||||
// token refresh or project state in one mission cannot leak into another.
|
||||
".claude",
|
||||
".claude.json",
|
||||
// Per-CLI state for the alternate backends; small.
|
||||
".kimi-code",
|
||||
"glm-home",
|
||||
// The seeded agent library.
|
||||
"agents",
|
||||
];
|
||||
|
||||
/// Deliberately NOT copied — caches and toolchains, no secrets, expensive:
|
||||
/// `.rustup` (1.5 GB, vestigial), `.npm` (85 MB), `.cargo`, `.cache`,
|
||||
/// `.local`. A mission that needs them reads the image's copies.
|
||||
fn copy_script() -> String {
|
||||
let mut out = String::from("set -e\n");
|
||||
for p in SEEDED_PATHS {
|
||||
// Missing entries are normal — a fresh deployment has no .kimi-code
|
||||
// until Kimi is first used — so absence must not fail the copy.
|
||||
out.push_str(&format!(
|
||||
"if [ -e '/seed/{p}' ]; then cp -a '/seed/{p}' /dst/; fi\n"
|
||||
));
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
/// Give this mission its own copy of the runtime seed data.
|
||||
///
|
||||
/// Every per-mission container used to bind-mount the SAME host seed dir as
|
||||
/// `/zeroclaw-data` — shared with each other and with the singleton runtime.
|
||||
/// That directory holds `config.toml`, which carries the §15 door bearer
|
||||
/// token, plus `sessions.db` and `devices.db`. So a mission could read another
|
||||
/// mission's credential, and anything it wrote there was inherited by every
|
||||
/// later mission. Teardown never cleaned it, because teardown only removes
|
||||
/// `/var/lib/clawmates-missions/{id}`.
|
||||
///
|
||||
/// The code already knew: the comment on `DEFAULT_SEED_DIR` names the sqlite
|
||||
/// race and calls copy-on-write per mission the long-term fix. This is that.
|
||||
///
|
||||
/// The copy runs in a throwaway container because cm-api cannot see the seed
|
||||
/// dir — it hands that host path to Docker but never mounts it itself. The
|
||||
/// runtime image is reused so nothing extra is pulled.
|
||||
///
|
||||
/// Failure is fatal to container creation on purpose. Falling back to the
|
||||
/// shared mount would silently restore the credential-sharing this removes,
|
||||
/// and a silent fallback to a weaker posture is the failure mode this
|
||||
/// codebase keeps paying for.
|
||||
async fn seed_runtime_data(
|
||||
docker: &Docker,
|
||||
image: &str,
|
||||
seed_dir: &str,
|
||||
dest_dir: &str,
|
||||
) -> Result<(), String> {
|
||||
let name = format!("cm-seed-{}", Uuid::now_v7().simple());
|
||||
let config = ContainerCreateBody {
|
||||
image: Some(image.to_string()),
|
||||
entrypoint: Some(vec!["/bin/sh".to_string()]),
|
||||
cmd: Some(vec!["-c".to_string(), copy_script()]),
|
||||
host_config: Some(HostConfig {
|
||||
mounts: Some(vec![
|
||||
Mount {
|
||||
target: Some("/seed".to_string()),
|
||||
source: Some(seed_dir.to_string()),
|
||||
typ: Some(MountTypeEnum::BIND),
|
||||
read_only: Some(true),
|
||||
..Default::default()
|
||||
},
|
||||
Mount {
|
||||
target: Some("/dst".to_string()),
|
||||
source: Some(dest_dir.to_string()),
|
||||
typ: Some(MountTypeEnum::BIND),
|
||||
read_only: Some(false),
|
||||
..Default::default()
|
||||
},
|
||||
]),
|
||||
auto_remove: Some(true),
|
||||
..Default::default()
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
docker
|
||||
.create_container(
|
||||
Some(CreateContainerOptions {
|
||||
name: Some(name.clone()),
|
||||
..Default::default()
|
||||
}),
|
||||
config,
|
||||
)
|
||||
.await
|
||||
.map_err(|e| format!("create seed copier: {e}"))?;
|
||||
docker
|
||||
.start_container(&name, None::<StartContainerOptions>)
|
||||
.await
|
||||
.map_err(|e| format!("start seed copier: {e}"))?;
|
||||
|
||||
// `auto_remove` means the container disappears the moment it exits, so
|
||||
// poll for absence rather than waiting on it.
|
||||
for _ in 0..120 {
|
||||
tokio::time::sleep(std::time::Duration::from_millis(250)).await;
|
||||
match docker
|
||||
.inspect_container(&name, None::<InspectContainerOptions>)
|
||||
.await
|
||||
{
|
||||
Err(_) => return Ok(()),
|
||||
Ok(info) => {
|
||||
let running = info
|
||||
.state
|
||||
.as_ref()
|
||||
.and_then(|st| st.running)
|
||||
.unwrap_or(false);
|
||||
if !running {
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(format!("seed copy into {dest_dir} did not finish in 30s"))
|
||||
}
|
||||
|
||||
/// Deterministic docker container name for a mission's runtime.
|
||||
/// Uses the full UUID hex — UUIDv7 encodes time in the leading bytes,
|
||||
/// so a short prefix isn't guaranteed unique across missions minted
|
||||
@@ -247,29 +397,40 @@ impl MissionRuntimeProvisioner {
|
||||
let _ = tokio::fs::create_dir_all(&mission_dir).await;
|
||||
let seed_dir = std::env::var("CLAWMATES_RUNTIME_SEED_DIR")
|
||||
.unwrap_or_else(|_| DEFAULT_SEED_DIR.to_string());
|
||||
let mounts = vec![
|
||||
// Mount just this mission's directory. Agents can navigate
|
||||
// its `/repo` subdir but never see other missions'.
|
||||
Mount {
|
||||
// Per-mission copy of the seed data. See `seed_runtime_data`: sharing
|
||||
// one directory meant sharing the door token and letting any mission
|
||||
// poison every later one.
|
||||
let runtime_data_dir = format!("{mission_dir}/runtime-data");
|
||||
let _ = tokio::fs::create_dir_all(&runtime_data_dir).await;
|
||||
seed_runtime_data(&self.docker, &self.image, &seed_dir, &runtime_data_dir).await?;
|
||||
let mut mounts = Vec::new();
|
||||
// In copy mode the checkout is pushed in and pulled back out, so the
|
||||
// container gets its OWN filesystem and the host directory has exactly
|
||||
// one writer (the server). Binding it here would put two uids back on
|
||||
// one directory — the cause of four separate work-loss bugs.
|
||||
if !crate::mission_fs::copy_mode() {
|
||||
mounts.push(Mount {
|
||||
target: Some("/mission".to_string()),
|
||||
source: Some(mission_dir.clone()),
|
||||
typ: Some(MountTypeEnum::BIND),
|
||||
read_only: Some(false),
|
||||
..Default::default()
|
||||
},
|
||||
// Share the shared-runtime data dir so this gateway inherits
|
||||
// the seeded agent library (`claw_*` templates). We then
|
||||
// mint a per-mission pairing code via /admin/paircode/new
|
||||
// below — the mint writes into the shared devices.db but
|
||||
// the resulting token is unique to this mission.
|
||||
});
|
||||
}
|
||||
mounts.extend([
|
||||
// This mission's OWN copy of the seeded agent library
|
||||
// (`claw_*` templates). Copied rather than shared, so its
|
||||
// config.toml — which carries the door bearer token — and its
|
||||
// sqlite files belong to this mission alone and are removed with
|
||||
// it by `teardown_container`.
|
||||
Mount {
|
||||
target: Some("/zeroclaw-data".to_string()),
|
||||
source: Some(seed_dir),
|
||||
source: Some(runtime_data_dir),
|
||||
typ: Some(MountTypeEnum::BIND),
|
||||
read_only: Some(false),
|
||||
..Default::default()
|
||||
},
|
||||
];
|
||||
]);
|
||||
|
||||
let host_config = HostConfig {
|
||||
mounts: Some(mounts),
|
||||
@@ -810,7 +971,13 @@ mod tests {
|
||||
it silently bills the API. Forwarded: {keys:?}"
|
||||
);
|
||||
// Unrelated providers have no subscription equivalent and must survive.
|
||||
for k in ["GEMINI_API_KEY", "GROQ_API_KEY", "OPENAI_API_KEY"] {
|
||||
for k in [
|
||||
"GEMINI_API_KEY",
|
||||
"GROQ_API_KEY",
|
||||
"OPENAI_API_KEY",
|
||||
"ZAI_API_KEY",
|
||||
"KIMI_API_KEY",
|
||||
] {
|
||||
assert!(keys.contains(&k), "{k} should still be forwarded");
|
||||
}
|
||||
// And the subscription credential MUST travel. A mission container
|
||||
@@ -974,4 +1141,75 @@ allowed_tools = ["file_read", "file_edit"]
|
||||
let url = endpoint_url("cm-runtime-mission-abc");
|
||||
assert_eq!(url, "http://cm-runtime-mission-abc:42617");
|
||||
}
|
||||
|
||||
/// The seed data path must be per-mission, not the shared seed dir.
|
||||
///
|
||||
/// Every mission container used to bind the SAME host directory as
|
||||
/// `/zeroclaw-data`. It holds `config.toml`, which carries the §15 door
|
||||
/// bearer token, plus `sessions.db`/`devices.db`. Sharing it meant one
|
||||
/// mission could read another's credential, and anything written there
|
||||
/// was inherited by every later mission — `teardown_container` only
|
||||
/// removes `/var/lib/clawmates-missions/{id}`, so the shared dir was
|
||||
/// never cleaned.
|
||||
///
|
||||
/// Asserting the path shape is what keeps this from silently regressing:
|
||||
/// a future edit that points the mount back at the seed dir restores the
|
||||
/// credential sharing with no other visible symptom.
|
||||
#[test]
|
||||
fn runtime_data_is_scoped_to_one_mission() {
|
||||
let a = Uuid::now_v7();
|
||||
let b = Uuid::now_v7();
|
||||
let path = |id: Uuid| format!("{MISSIONS_HOST_ROOT}/{id}/runtime-data");
|
||||
|
||||
assert_ne!(path(a), path(b), "two missions must not share runtime data");
|
||||
assert!(
|
||||
path(a).starts_with(&format!("{MISSIONS_HOST_ROOT}/{a}")),
|
||||
"runtime data must live under the mission dir so teardown removes it"
|
||||
);
|
||||
assert_ne!(
|
||||
path(a),
|
||||
DEFAULT_SEED_DIR,
|
||||
"the mount must never be the shared seed dir itself"
|
||||
);
|
||||
assert!(
|
||||
!path(a).starts_with(DEFAULT_SEED_DIR),
|
||||
"runtime data must not live inside the shared seed dir either"
|
||||
);
|
||||
}
|
||||
|
||||
/// The copy must be an allow-list, and must include the secret-bearing
|
||||
/// paths while excluding the expensive ones.
|
||||
///
|
||||
/// Measured on gw-04: the seed dir is 1.7 GB, of which 1.5 GB is a
|
||||
/// vestigial `.rustup` that is not even on the container's PATH (the
|
||||
/// image ships Rust at /usr/local/cargo). Copying everything per mission
|
||||
/// would cost tens of seconds and ~17 GB across ten concurrent missions —
|
||||
/// which is what the first version of this did.
|
||||
#[test]
|
||||
fn the_seed_copy_takes_secrets_and_skips_caches() {
|
||||
// The two paths that carry the door bearer token MUST be copied, or
|
||||
// this whole change accomplishes nothing.
|
||||
assert!(SEEDED_PATHS.contains(&".zeroclaw"));
|
||||
assert!(SEEDED_PATHS.contains(&"clawmates-mcp.json"));
|
||||
// Claude Code's credentials and state.
|
||||
assert!(SEEDED_PATHS.contains(&".claude"));
|
||||
|
||||
// The expensive, secret-free ones must NOT be.
|
||||
for cache in [".rustup", ".npm", ".cargo", ".cache"] {
|
||||
assert!(
|
||||
!SEEDED_PATHS.contains(&cache),
|
||||
"{cache} is a cache and must not be copied per mission"
|
||||
);
|
||||
}
|
||||
|
||||
let script = copy_script();
|
||||
// A missing entry is normal on a fresh deployment (no .kimi-code
|
||||
// until Kimi is first used) and must not fail the copy.
|
||||
assert!(script.contains("if [ -e "), "absent paths must be tolerated");
|
||||
assert!(script.contains("/seed/.zeroclaw"));
|
||||
assert!(!script.contains("/seed/.rustup"));
|
||||
for p in SEEDED_PATHS {
|
||||
assert!(script.contains(p), "{p} missing from the copy script");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -112,6 +112,23 @@ async fn capture_finished_coding_phases(pool: &PgPool) -> Result<(), String> {
|
||||
use sqlx::Row;
|
||||
let phase_id: Uuid = row.get("id");
|
||||
let mission_id: Uuid = row.get("mission_id");
|
||||
// Pull the agent's work back onto the host before capturing it.
|
||||
// Unpacks over the same checkout path, so capture below is unchanged.
|
||||
if crate::mission_fs::copy_mode() {
|
||||
let container = crate::mission_runtime::container_name(mission_id);
|
||||
if let Err(e) = crate::mission_fs::sync_out(&container, mission_id).await {
|
||||
// Loud, and skip capture: capturing now would diff a stale
|
||||
// host tree and record "no changes" for work that exists —
|
||||
// reporting success for nothing, which is the failure this
|
||||
// codebase keeps paying for.
|
||||
eprintln!(
|
||||
"phase_runner: could NOT collect work from {container} for phase \
|
||||
{phase_id} ({e}) — skipping capture so a stale tree is not \
|
||||
recorded as an empty diff"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
match crate::mission_delivery::capture_phase_diff(pool, mission_id, phase_id).await {
|
||||
Ok(Some(_)) => {}
|
||||
Ok(None) => {
|
||||
@@ -299,6 +316,15 @@ async fn launch_phase(pool: &PgPool, p: PhaseLaunch<'_>) -> Result<(), String> {
|
||||
match prov.ensure_container(mission_id).await {
|
||||
Ok(ec) => {
|
||||
let name = crate::mission_runtime::container_name(mission_id);
|
||||
// Push the checkout into the container. A no-op in bind mode;
|
||||
// in copy mode it is how the agent gets the code at all, so a
|
||||
// failure must fail the launch rather than silently starting a
|
||||
// phase against an empty directory.
|
||||
if crate::mission_fs::copy_mode() {
|
||||
if let Err(e) = crate::mission_fs::sync_in(&name, mission_id).await {
|
||||
return Err(format!("copy checkout into {name}: {e}"));
|
||||
}
|
||||
}
|
||||
if let Err(e) = cm_db::repo::missions::set_runtime_binding(
|
||||
pool,
|
||||
mission_id,
|
||||
|
||||
@@ -876,3 +876,43 @@ async fn an_unrunnable_suite_is_distinguishable_from_no_suite() {
|
||||
"the two must be distinguishable — this is the whole point"
|
||||
);
|
||||
}
|
||||
|
||||
/// A COMMIT_EDITMSG left by the agent must not block delivery.
|
||||
///
|
||||
/// From mission 019fcd0c: the agent ran `git commit` itself, leaving
|
||||
/// `.git/COMMIT_EDITMSG` owned by root at 0644, and the server's commit died
|
||||
/// with "Permission denied". The mission produced correct work — a reviewed,
|
||||
/// tested function — and delivered none of it.
|
||||
///
|
||||
/// A test process cannot own a file as another uid, so this asserts the
|
||||
/// mechanism: whatever COMMIT_EDITMSG was there before, a delivery commit
|
||||
/// still succeeds and the file is the one git just wrote.
|
||||
#[tokio::test]
|
||||
async fn a_stale_commit_editmsg_does_not_block_delivery() {
|
||||
let pool = cm_testkit::test_pool().await;
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let mission = Uuid::now_v7();
|
||||
let repo = seed_repo(tmp.path(), mission);
|
||||
let (_, phase) = seed_mission_phase(&pool, mission).await;
|
||||
|
||||
// Stand in for the agent's leftover: content that must not survive.
|
||||
let msg = repo.join(".git/COMMIT_EDITMSG");
|
||||
std::fs::write(&msg, "LEFTOVER FROM THE AGENT\n").unwrap();
|
||||
|
||||
std::fs::write(repo.join("WORK.md"), "work\n").unwrap();
|
||||
let cap = capture(&pool, tmp.path(), mission, phase)
|
||||
.await
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
|
||||
let commit = cap
|
||||
.committed
|
||||
.expect("delivery must commit despite a stale COMMIT_EDITMSG");
|
||||
assert!(!commit.sha.is_empty());
|
||||
|
||||
let body = std::fs::read_to_string(&msg).unwrap_or_default();
|
||||
assert!(
|
||||
!body.contains("LEFTOVER FROM THE AGENT"),
|
||||
"the stale message survived: {body:?}"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user