fix: three gaps the P0 validation runs exposed
Validating P0 against production found one bug in each of the three pieces, none of which any test would have caught. **The scanners were installed but not allow-listed.** Mission 019fc058's condition asked for a gitleaks result; `gitleaks detect` came back `ran=false`, and the judge said it could not verify. P0.3 put the binaries in the image and never added them to `evaluator_tools::ALLOWED_PROGRAMS`, so the judge could not invoke the tools installed for it. Adds gitleaks, trivy, semgrep and `which`. **Every `continue` after a fire claim leaked the claim.** Introduced by the scheduler fix itself: the orphan-agent and empty-action paths skipped `complete_fire`, so the row stayed `claimed` — which reads as a crash mid-fire, meaning the routine is re-claimed forever and the table grows one stuck row per occurrence. Observed in production: five `claimed` rows, no dispatch, no `routine_runs`. Both paths now settle with a reason, and log it. **The agent writes its own identity files into the user's repository.** `workspace.path` is pinned to the repo root, so the runtime drops AGENTS.md, HEARTBEAT.md, IDENTITY.md, MEMORY.md, SOUL.md, TOOLS.md and USER.md into the checkout — SOUL.md opens "Who You Are / You're not a chatbot." Two consequences: every mission's tree is permanently dirty, so a `done_when` about a clean tree can never pass; and P1's `git add -A` would have committed the agent's SOUL.md into someone's repository and pushed it. The P1 deny-list covered build artifacts and would not have caught this. Fixed by writing the names to `.git/info/exclude` after clone — local to the checkout, never itself a change, and it suppresses only *untracked* files, so a repo that genuinely tracks its own AGENTS.md still reports modifications to it. Idempotent, and preserves any pre-existing exclude. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
491449f3ce
commit
d90a42b759
Generated
+1
@@ -969,6 +969,7 @@ dependencies = [
|
|||||||
"serde_yaml",
|
"serde_yaml",
|
||||||
"sha2",
|
"sha2",
|
||||||
"sqlx",
|
"sqlx",
|
||||||
|
"tempfile",
|
||||||
"thiserror 2.0.18",
|
"thiserror 2.0.18",
|
||||||
"time",
|
"time",
|
||||||
"tokio",
|
"tokio",
|
||||||
|
|||||||
@@ -51,6 +51,7 @@ uuid = { workspace = true }
|
|||||||
|
|
||||||
[dev-dependencies]
|
[dev-dependencies]
|
||||||
axum = { version = "0.8", features = ["ws"] }
|
axum = { version = "0.8", features = ["ws"] }
|
||||||
|
tempfile = "3"
|
||||||
jsonwebtoken = "9"
|
jsonwebtoken = "9"
|
||||||
eventsource-stream = "0.2"
|
eventsource-stream = "0.2"
|
||||||
reqwest = { version = "0.12", default-features = false, features = [
|
reqwest = { version = "0.12", default-features = false, features = [
|
||||||
|
|||||||
@@ -64,7 +64,16 @@ const ALLOWED_PROGRAMS: &[&str] = &[
|
|||||||
"cargo", "npm", "pnpm", "yarn", "node", "python", "python3", "pytest", "make", "just", "go",
|
"cargo", "npm", "pnpm", "yarn", "node", "python", "python3", "pytest", "make", "just", "go",
|
||||||
"pnpx", "npx", "bun", "dotnet", "mvn", "gradle", "ruff", "mypy", "eslint", "tsc", "jest",
|
"pnpx", "npx", "bun", "dotnet", "mvn", "gradle", "ruff", "mypy", "eslint", "tsc", "jest",
|
||||||
"vitest", "phpunit", "rspec", "bundle", "poetry", "uv", "tox",
|
"vitest", "phpunit", "rspec", "bundle", "poetry", "uv", "tox",
|
||||||
// Version control, narrowed by subcommand.
|
// Security scanners. These ship in the runtime image specifically so a
|
||||||
|
// `done_when` can be written about them ("gitleaks reports no secrets"),
|
||||||
|
// and a judge that cannot invoke them has to fall back to asking the
|
||||||
|
// agents — which is the failure this module exists to prevent. Installing
|
||||||
|
// them without allow-listing them left exactly that gap.
|
||||||
|
"gitleaks", "trivy", "semgrep",
|
||||||
|
// Locate a tool before running it. Cheap, read-only, and it saves the
|
||||||
|
// judge from concluding a tool is missing when the real answer is that it
|
||||||
|
// guessed the wrong name.
|
||||||
|
"which", // Version control, narrowed by subcommand.
|
||||||
"git",
|
"git",
|
||||||
];
|
];
|
||||||
|
|
||||||
@@ -399,6 +408,27 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The scanners exist in the runtime image so conditions can be written
|
||||||
|
/// about them. Shipping the binaries without allow-listing them left the
|
||||||
|
/// judge unable to run the very tools installed for it — observed on
|
||||||
|
/// mission 019fc058, where `gitleaks detect` came back `ran=false` and the
|
||||||
|
/// judge had to say it could not verify.
|
||||||
|
#[test]
|
||||||
|
fn security_scanners_are_runnable() {
|
||||||
|
for cmd in [
|
||||||
|
vec!["gitleaks", "detect", "--no-git"],
|
||||||
|
vec!["trivy", "fs", "."],
|
||||||
|
vec!["semgrep", "--config=auto"],
|
||||||
|
vec!["cargo", "audit"],
|
||||||
|
vec!["which", "gitleaks"],
|
||||||
|
] {
|
||||||
|
assert!(
|
||||||
|
check_argv(&argv(&cmd)).is_ok(),
|
||||||
|
"{cmd:?} must be runnable — it is installed in the runtime image"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn refuses_programs_off_the_list() {
|
fn refuses_programs_off_the_list() {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
|
|||||||
@@ -107,9 +107,64 @@ async fn clone(path: &std::path::Path, url: &str) -> Result<(), String> {
|
|||||||
.collect::<String>()
|
.collect::<String>()
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
ignore_agent_scaffolding(path);
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Files the agent runtime writes into its own workspace, which is pinned to
|
||||||
|
/// the repository root (`MissionRuntimeProvisioner::pin_agent_workspaces`).
|
||||||
|
///
|
||||||
|
/// They are the agent's identity scaffolding, not the user's code — `SOUL.md`
|
||||||
|
/// opens "Who You Are / You're not a chatbot." Observed on mission 019fc058,
|
||||||
|
/// where all seven appeared as untracked files in a freshly cloned repo.
|
||||||
|
const AGENT_SCAFFOLDING: &[&str] = &[
|
||||||
|
"AGENTS.md",
|
||||||
|
"HEARTBEAT.md",
|
||||||
|
"IDENTITY.md",
|
||||||
|
"MEMORY.md",
|
||||||
|
"SOUL.md",
|
||||||
|
"TOOLS.md",
|
||||||
|
"USER.md",
|
||||||
|
];
|
||||||
|
|
||||||
|
/// Keep the agent's own scaffolding out of the user's repository.
|
||||||
|
///
|
||||||
|
/// Two things went wrong without this. Every mission's tree was permanently
|
||||||
|
/// dirty, so a `done_when` written about a clean tree could never pass. And
|
||||||
|
/// once mission delivery starts committing, `git add -A` would have put the
|
||||||
|
/// agent's `SOUL.md` and `MEMORY.md` into someone's repository and pushed
|
||||||
|
/// them.
|
||||||
|
///
|
||||||
|
/// Written to `.git/info/exclude` rather than `.gitignore`: the exclude file
|
||||||
|
/// is local to this checkout and never itself appears as a change, so the
|
||||||
|
/// repository the user gets back is untouched. Crucially it only suppresses
|
||||||
|
/// *untracked* files — a repo that genuinely tracks its own `AGENTS.md` still
|
||||||
|
/// reports modifications to it, which is the behaviour we want.
|
||||||
|
///
|
||||||
|
/// Best-effort: a checkout that cannot be annotated is noisier, not broken.
|
||||||
|
fn ignore_agent_scaffolding(path: &std::path::Path) {
|
||||||
|
let exclude = path.join(".git/info/exclude");
|
||||||
|
let mut body = std::fs::read_to_string(&exclude).unwrap_or_default();
|
||||||
|
if body.contains("clawmates: agent scaffolding") {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
body.push_str("\n# clawmates: agent scaffolding — written by the runtime into its\n");
|
||||||
|
body.push_str("# pinned workspace, never part of the repository.\n");
|
||||||
|
for name in AGENT_SCAFFOLDING {
|
||||||
|
body.push_str(&format!("/{name}\n"));
|
||||||
|
}
|
||||||
|
if let Some(dir) = exclude.parent() {
|
||||||
|
let _ = std::fs::create_dir_all(dir);
|
||||||
|
}
|
||||||
|
if let Err(e) = std::fs::write(&exclude, body) {
|
||||||
|
eprintln!(
|
||||||
|
"mission_workspace: could not write {} ({e}) — agent scaffolding will show as \
|
||||||
|
untracked in this checkout",
|
||||||
|
exclude.display()
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fn redact_token(s: &str) -> String {
|
fn redact_token(s: &str) -> String {
|
||||||
// Strip any "oauth2:<token>@" segment that git may echo back on
|
// Strip any "oauth2:<token>@" segment that git may echo back on
|
||||||
// failures. Belt-and-braces: also nuke any raw token env value.
|
// failures. Belt-and-braces: also nuke any raw token env value.
|
||||||
@@ -174,3 +229,45 @@ async fn fetch_and_reset(path: &std::path::Path, branch: &str) -> Result<(), Str
|
|||||||
}
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
/// The exclude must be idempotent — `ensure_checkout` re-runs on every
|
||||||
|
/// phase, and appending the same block each time would grow the file
|
||||||
|
/// without bound.
|
||||||
|
#[test]
|
||||||
|
fn scaffolding_exclusion_is_written_once() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
std::fs::create_dir_all(dir.path().join(".git/info")).unwrap();
|
||||||
|
|
||||||
|
ignore_agent_scaffolding(dir.path());
|
||||||
|
let first = std::fs::read_to_string(dir.path().join(".git/info/exclude")).unwrap();
|
||||||
|
assert!(
|
||||||
|
first.contains("/SOUL.md"),
|
||||||
|
"the agent's identity file is excluded"
|
||||||
|
);
|
||||||
|
assert!(first.contains("/MEMORY.md"));
|
||||||
|
|
||||||
|
ignore_agent_scaffolding(dir.path());
|
||||||
|
let second = std::fs::read_to_string(dir.path().join(".git/info/exclude")).unwrap();
|
||||||
|
assert_eq!(first, second, "re-running must not append a second block");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// An existing exclude file belongs to the repository; keep it.
|
||||||
|
#[test]
|
||||||
|
fn an_existing_exclude_is_preserved() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
std::fs::create_dir_all(dir.path().join(".git/info")).unwrap();
|
||||||
|
std::fs::write(dir.path().join(".git/info/exclude"), "/local-scratch\n").unwrap();
|
||||||
|
|
||||||
|
ignore_agent_scaffolding(dir.path());
|
||||||
|
let body = std::fs::read_to_string(dir.path().join(".git/info/exclude")).unwrap();
|
||||||
|
assert!(
|
||||||
|
body.contains("/local-scratch"),
|
||||||
|
"pre-existing rules survive"
|
||||||
|
);
|
||||||
|
assert!(body.contains("/AGENTS.md"));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -103,8 +103,28 @@ impl Scheduler {
|
|||||||
routines::set_next_run(&self.pool, routine.id, next).await?;
|
routines::set_next_run(&self.pool, routine.id, next).await?;
|
||||||
|
|
||||||
let agent_id = cm_domain::AgentId::from(routine.agent_id);
|
let agent_id = cm_domain::AgentId::from(routine.agent_id);
|
||||||
let Ok(agent) = agents::get(&self.pool, agent_id).await else {
|
let agent = match agents::get(&self.pool, agent_id).await {
|
||||||
continue; // deleted agent: routine is orphaned
|
Ok(a) => a,
|
||||||
|
Err(e) => {
|
||||||
|
// Orphaned routine (deleted agent, or a row we cannot
|
||||||
|
// read). Settle the slot rather than leaving it `claimed`:
|
||||||
|
// an unsettled claim looks like a crash mid-fire, so every
|
||||||
|
// tick would re-claim the same routine forever and the
|
||||||
|
// table would grow one stuck row per occurrence.
|
||||||
|
eprintln!(
|
||||||
|
"scheduler: routine {} references agent {agent_id} which could not be \
|
||||||
|
read ({e}) — settling the occurrence as failed",
|
||||||
|
routine.id
|
||||||
|
);
|
||||||
|
let _ = routines::complete_fire(
|
||||||
|
&self.pool,
|
||||||
|
routine.id,
|
||||||
|
slot,
|
||||||
|
Some(&format!("agent {agent_id} unreadable: {e}")),
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
// Topology routine: fire the whole team's stored topology as one
|
// Topology routine: fire the whole team's stored topology as one
|
||||||
@@ -149,6 +169,19 @@ impl Scheduler {
|
|||||||
|
|
||||||
let message = routine.action["message"].as_str().unwrap_or_default();
|
let message = routine.action["message"].as_str().unwrap_or_default();
|
||||||
if message.is_empty() {
|
if message.is_empty() {
|
||||||
|
// Neither a topology nor a message action: there is nothing to
|
||||||
|
// dispatch. Settle it so the slot is not mistaken for a crash.
|
||||||
|
eprintln!(
|
||||||
|
"scheduler: routine {} has no `topology` or `message` action — nothing to fire",
|
||||||
|
routine.id
|
||||||
|
);
|
||||||
|
let _ = routines::complete_fire(
|
||||||
|
&self.pool,
|
||||||
|
routine.id,
|
||||||
|
slot,
|
||||||
|
Some("routine action has neither `topology` nor `message`"),
|
||||||
|
)
|
||||||
|
.await;
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user