From e5b42e56279f9fce1d56bd190e1367a0c70ef171 Mon Sep 17 00:00:00 2001 From: Omar Sobh Date: Wed, 23 Sep 2026 06:32:00 -0500 Subject: [PATCH] feat(judge): install npm dependencies offline from the lockfile before judging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The judge verifies a copy that excludes node_modules (on purpose: it must not run agent-built binaries) in a container with no registry route, so every npm project failed any "tests pass" condition — the frontend team's first run was correct (10/10 re-run by hand) and failed twice on `vitest: not found`. When the copy has package-lock.json, the harness copies the mission's npm cache (already on the host: /zeroclaw-data is bound from /runtime-data) into the verify root and runs `npm ci --offline` against the copy, so the judge stays offline, every tarball is checked against the lockfile's hashes, and nothing root-owned lands in the mission's tree. The judge is told whether the install worked, so a missing install never reads as a failing suite. Exit status is npm's own (no `| tail` laundering) — tested with a fake npm in both directions; the real path was run by hand on the delivered branch: offline install of 173 packages, then 10/10. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/cm-api/src/evaluator.rs | 15 +++ crates/cm-api/src/evaluator_tools.rs | 186 +++++++++++++++++++++++++++ 2 files changed, 201 insertions(+) diff --git a/crates/cm-api/src/evaluator.rs b/crates/cm-api/src/evaluator.rs index 920effe..f74c48c 100644 --- a/crates/cm-api/src/evaluator.rs +++ b/crates/cm-api/src/evaluator.rs @@ -603,6 +603,21 @@ pub async fn evaluate( evidence: &str, ) -> Verdict { let sandbox = crate::evaluator_tools::Sandbox::for_mission(mission_id); + // A JavaScript project arrives without node_modules (excluded from the + // copy on purpose) in a container with no registry. Install offline from + // the lockfile first, and tell the judge how that went. + let deps_note = match &sandbox { + Some(sb) => sb.prepare_dependencies(mission_id).await, + None => None, + }; + let evidence_with_deps; + let evidence = match deps_note { + Some(note) => { + evidence_with_deps = format!("{evidence}\n\n{note}"); + evidence_with_deps.as_str() + } + None => evidence, + }; // Purged explicitly at every exit below: `Drop` runs as uid 65532 and cannot // delete the root-owned `target/` the judge's own `cargo test` leaves behind. // Wrapped so the purge below runs on EVERY exit: this function returns diff --git a/crates/cm-api/src/evaluator_tools.rs b/crates/cm-api/src/evaluator_tools.rs index 9790d96..3ae16b9 100644 --- a/crates/cm-api/src/evaluator_tools.rs +++ b/crates/cm-api/src/evaluator_tools.rs @@ -403,7 +403,117 @@ fn git_ownership_env(workdir: &str) -> Vec { ] } +/// Where a mission agent's npm cache lands on the host. +/// +/// The mission container's `HOME` is `/zeroclaw-data`, bind-mounted from +/// `/runtime-data`, so an agent's `npm install` has already filled +/// `/runtime-data/.npm` on the host — in a path the judge's container +/// can see. No copy out of the mission container is needed. +pub fn mission_npm_cache(mission_id: Uuid) -> PathBuf { + crate::mission_workspace::missions_root() + .join(mission_id.to_string()) + .join("runtime-data") + .join(".npm") +} + +/// How long an offline install may take. Longer than a check: a cold `npm ci` +/// of a Vite app unpacks a few hundred packages. +const INSTALL_TIMEOUT: Duration = Duration::from_secs(300); + +/// The script that gives the judge a `node_modules` it built itself. +/// +/// The verify copy excludes `node_modules` on purpose (the transport packer's +/// list: the judge must not run agent-built binaries), and the judge's +/// container has no route to a registry (`clawmates_core` has no gateway), so +/// `npm test` in a copy used to fail with `vitest: not found` however good the +/// work was. Measured on the frontend team's first run: correct component, +/// 10/10 tests when re-run by hand, failed twice by a judge that could not +/// install. +/// +/// Not piped into `tail`: a pipe exits with its LAST command's status, so +/// `npm ci | tail` reports success over a failed install. The log is written, +/// its tail printed, and npm's own status returned. +/// +/// `npm ci --offline` rebuilds `node_modules` from the lockfile using only the +/// cache, and checks every tarball against the lockfile's integrity hash. The +/// cache is COPIED into the verify root first: `npm ci` writes to its cache, +/// the judge runs as root, and pointing it at the mission's own cache would +/// leave root-owned files in a tree uid 65532 owns — the single-writer breach +/// the verify copy exists to prevent. The copy goes with the verify root when +/// the sandbox is purged. +pub fn npm_offline_script(cache: &Path, verify_root: &Path) -> String { + let local = verify_root.join("npm-cache"); + format!( + "cp -a {cache} {local} || exit 3\n\ + npm ci --offline --no-audit --no-fund --cache {local} > {log} 2>&1\n\ + rc=$?\n\ + tail -25 {log}\n\ + exit $rc\n", + cache = crate::vm_tool_tap::shell_quote(&cache.display().to_string()), + local = crate::vm_tool_tap::shell_quote(&local.display().to_string()), + log = crate::vm_tool_tap::shell_quote(&verify_root.join("npm-ci.log").display().to_string()), + ) +} + impl Sandbox { + /// Install a JavaScript project's dependencies for the judge, offline, + /// when the copy has a `package-lock.json`. Returns a note for the judge's + /// evidence, or `None` when there is nothing to install. + /// + /// Server-driven, not a judge tool call: `npm ci` is not on the judge's + /// allow-list and should not be — installing is the harness's job, and the + /// judge only needs to know whether it worked, so that "could not install" + /// never reads as "the tests fail". + pub async fn prepare_dependencies(&self, mission_id: Uuid) -> Option { + if !self.workdir.join("package-lock.json").is_file() { + return None; + } + let cache = mission_npm_cache(mission_id); + if !cache.join("_cacache").is_dir() { + return Some(format!( + "DEPENDENCIES: not installed. This is an npm project, but the mission \ + left no npm cache at {} to install from offline. A test command that \ + cannot find its runner is a missing install, not a failing suite.", + cache.display() + )); + } + let root = self.workdir.parent()?.to_path_buf(); + let docker = crate::container_exec::connect().ok()?; + let argv = vec![ + "sh".to_string(), + "-lc".to_string(), + npm_offline_script(&cache, &root), + ]; + let workdir = self.workdir.display().to_string(); + let out = crate::container_exec::exec_with_env( + &docker, + &self.container, + Some(&workdir), + &argv, + &git_ownership_env(&workdir), + INSTALL_TIMEOUT, + ) + .await; + let note = match out { + Ok(o) if o.exit_code == Some(0) && self.workdir.join("node_modules/.bin").is_dir() => { + "DEPENDENCIES: installed by the harness with `npm ci --offline` from \ + package-lock.json, every package checked against the lockfile's \ + integrity hashes. node_modules is present; run the project's test \ + command directly." + .to_string() + } + Ok(o) => format!( + "DEPENDENCIES: offline install FAILED (exit {:?}). A test command that \ + cannot find its runner is a missing install, not a failing suite.\n{}", + o.exit_code, + clamp_output(&format!("{}{}", o.stdout, o.stderr)), + ), + Err(e) => format!("DEPENDENCIES: offline install could not run: {e}"), + }; + eprintln!("evaluator_tools: mission {mission_id} — {}", note.lines().next().unwrap_or("")); + Some(note) + } + /// Remove the copy, from inside the container that wrote it. /// /// `Drop` cannot do this. The judge runs `cargo test` in a container as @@ -777,3 +887,79 @@ mod tests { let _ = clamp_output(&long); } } + +#[cfg(test)] +mod npm_offline_tests { + use super::*; + + /// Run the generated script with a fake `npm` that records its arguments + /// and exits `npm_rc`. Returns (exit code, recorded npm argv). + fn run_with_fake_npm(npm_rc: i32, with_cache: bool) -> (Option, String) { + let tmp = tempfile::tempdir().unwrap(); + let bin = tmp.path().join("bin"); + std::fs::create_dir_all(&bin).unwrap(); + let argv_log = tmp.path().join("npm-argv"); + let fake = bin.join("npm"); + std::fs::write( + &fake, + format!("#!/bin/sh\necho \"$@\" > {}\necho npm said hello\nexit {npm_rc}\n", argv_log.display()), + ) + .unwrap(); + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&fake, std::fs::Permissions::from_mode(0o755)).unwrap(); + + let cache = tmp.path().join("mission-cache"); + if with_cache { + std::fs::create_dir_all(cache.join("_cacache")).unwrap(); + } + let root = tmp.path().join("verify"); + std::fs::create_dir_all(&root).unwrap(); + + let out = std::process::Command::new("sh") + .arg("-c") + .arg(npm_offline_script(&cache, &root)) + .env("PATH", format!("{}:{}", bin.display(), std::env::var("PATH").unwrap_or_default())) + .output() + .unwrap(); + let recorded = std::fs::read_to_string(&argv_log).unwrap_or_default(); + if with_cache { + assert!(root.join("npm-cache/_cacache").is_dir(), "the cache must be copied into the verify root"); + } + (out.status.code(), recorded) + } + + /// npm's own failure must survive: `npm ci | tail` exits 0 over a failed + /// install, and the judge would then be told dependencies were ready. + #[test] + fn a_failed_install_is_reported_as_failed() { + let (rc, _) = run_with_fake_npm(7, true); + assert_eq!(rc, Some(7)); + } + + #[test] + fn a_clean_install_exits_zero_offline_against_the_copy() { + let (rc, argv) = run_with_fake_npm(0, true); + assert_eq!(rc, Some(0)); + assert!(argv.starts_with("ci --offline"), "{argv}"); + // Pointed at the COPY in the verify root, never the mission's cache: + // npm writes to its cache and the judge runs as root. + assert!(argv.contains("/verify/npm-cache"), "{argv}"); + assert!(!argv.contains("mission-cache"), "{argv}"); + } + + /// No cache to copy stops before npm runs, with its own status. + #[test] + fn a_missing_cache_never_reaches_npm() { + let (rc, argv) = run_with_fake_npm(0, false); + assert_eq!(rc, Some(3)); + assert!(argv.is_empty(), "npm ran without a cache: {argv}"); + } + + /// The cache path is where the mission container's HOME is bound. + #[test] + fn the_npm_cache_is_under_the_bound_home() { + let id = Uuid::nil(); + let p = mission_npm_cache(id); + assert!(p.ends_with(format!("{id}/runtime-data/.npm")), "{}", p.display()); + } +}