feat(missions): copy-in/copy-out primitive for the mission checkout
The first half of removing the shared bind mount. Not wired yet — this adds the mechanism and its tests. One cause, four fixes so far: .git/objects permission denied (core.sharedRepository), the capture base being overwritten each phase, COMMIT_EDITMSG root-owned, and reset --hard deleting a prior phase's work (.git/clawmates-in-use). 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 instead: the agent owns its filesystem with no second writer. Measured before building, because the plan named copy cost as the open risk: a real 65 MB checkout of this repo copies in 0.23s and out 0.18s on gw-04. Not a risk at this size; re-measure an order of magnitude larger. No compression — the payload crosses a local socket, so gzip would spend CPU to save nothing. Two safety properties, both tested: - The archive comes back from a container the agent controls as ROOT, so it is untrusted input. A `../ESCAPED` entry must not write outside the destination. The test writes the tar header bytes by hand because the tar crate refuses to BUILD such an entry through its safe API — which is reassuring, but means the hostile case has to be constructed the way an attacker would. - Symlinks are packed as links, never dereferenced. Following them on copy-IN would smuggle host files into the container; the test plants a host secret behind a symlink and asserts its contents never appear in the archive. Ownership is deliberately not preserved on unpack: the archive's uids are the container's root, and re-applying them on the host would recreate the exact uid split this exists to remove. 413 tests, clippy clean. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
ac6bf72943
commit
389b41f8e6
@@ -0,0 +1,216 @@
|
||||
//! 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)
|
||||
}
|
||||
|
||||
#[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"
|
||||
);
|
||||
}
|
||||
|
||||
#[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());
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user