From 11e6f2150d24a42e066d34023b637a1855f4705c Mon Sep 17 00:00:00 2001 From: Larry Klosowski Date: Thu, 15 Oct 2026 11:34:41 -0700 Subject: [PATCH 01/12] feat(sidecar): untrusted-content taint escalates to HIC approval (HGW-1) Owner decision 2026-10-15 (HGW (2)). Before this change any shell_run output, even an approved `node -v`, tainted the session, and every later command and sidecar-hosted write was declined for the rest of the conversation ("which this tool's host cannot ask for"), with no way to clear it. Now, in a session opened with hicAware: true: - a tainted effectful sidecar call (fs_*, file_write, sheet_write, toolchain tools, capsules) waits on a new per-session action card (GET /sessions/:id/actions/pending, POST .../actions/decide {id, allow, subject}); the card shows the tool, the exact canonical arguments, a summary, the hicReason and every taint source; - shell_run keeps its own card and still asks after taint; the card now carries hicReason and taintSources; - an approval releases that one call; the session stays tainted, so the next action asks again (never global, never automatic); - decisions are recorded (kind sidecar.tool_call, write-ahead). Without hicAware (headless, scheduled) nobody can be asked: the call is declined with citrate_agent_loop::cannot_ask_reason, which names the taint source and says why. Tests: agent-loop taint_tests (+1), action_approvals unit (+3), action_approvals_tests session/route tests (+6), shell_run_tests (+2, one rewritten), files_tests taint case rewritten to the card flow. Workspace: 2241 -> 2253 passed, 0 failed. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_018wWZZ8GRVU9USh3kKQHsFe --- agent-loop/src/lib.rs | 19 +- agent-loop/tests/taint_tests.rs | 42 +++ agent-sidecar/src/action_approvals.rs | 357 ++++++++++++++++++++ agent-sidecar/src/action_approvals_tests.rs | 350 +++++++++++++++++++ agent-sidecar/src/files.rs | 5 +- agent-sidecar/src/files_tests.rs | 22 +- agent-sidecar/src/hgw_support.rs | 286 ++++++++++++++++ agent-sidecar/src/lib.rs | 119 +++++++ agent-sidecar/src/sessions.rs | 102 +++++- agent-sidecar/src/shell_run.rs | 76 ++++- agent-sidecar/src/shell_run_tests.rs | 133 ++++++-- 11 files changed, 1452 insertions(+), 59 deletions(-) create mode 100644 agent-sidecar/src/action_approvals.rs create mode 100644 agent-sidecar/src/action_approvals_tests.rs create mode 100644 agent-sidecar/src/hgw_support.rs diff --git a/agent-loop/src/lib.rs b/agent-loop/src/lib.rs index 0f02c56..a9f209f 100644 --- a/agent-loop/src/lib.rs +++ b/agent-loop/src/lib.rs @@ -231,6 +231,18 @@ pub trait ToolHost: Send + Sync { } } +/// HGW-1: why a tainted session's effectful call is declined when no member can be asked: the +/// host is not one that shows approval cards (a headless or scheduled run, or a client that did +/// not promise to route `hic: "required"` to a person). Hosts that decline for the same reason +/// use this text too, so the model and the log read one message. `source` names what tainted the +/// session, when known. +pub fn cannot_ask_reason(source: Option<&str>) -> String { + let from = source.map(|s| format!(" (from {s})")).unwrap_or_default(); + format!( + "this session read untrusted content{from}, so this action needs a member's explicit approval, and no member can be asked in this session (it was not opened by an app that shows approval cards, for example a headless or scheduled run)" + ) +} + /// The record of what tainted a session. #[derive(Debug, Clone, PartialEq, Eq)] pub struct TaintRecord { @@ -912,10 +924,9 @@ pub fn run_turn_with( (None, None) if declined.is_some() => { ToolOutcome::Denied(declined.clone().unwrap_or_default()) } - (None, None) if refused_hic => ToolOutcome::Denied( - "this session read untrusted content and this action needs a member's explicit approval, which this tool's host cannot ask for" - .into(), - ), + (None, None) if refused_hic => ToolOutcome::Denied(cannot_ask_reason( + tools.taint().record().as_ref().map(|r| r.source.as_str()), + )), (None, None) => { ToolOutcome::Error(format!("'{}' is not available in this session", call.name)) } diff --git a/agent-loop/tests/taint_tests.rs b/agent-loop/tests/taint_tests.rs index 4e74ecf..b6739b4 100644 --- a/agent-loop/tests/taint_tests.rs +++ b/agent-loop/tests/taint_tests.rs @@ -344,6 +344,48 @@ fn a_host_that_cannot_ask_a_member_refuses_instead_of_running() { assert!(msg.content.starts_with("declined:"), "{}", msg.content); } +/// HGW-1: when nobody can be asked (a headless or scheduled session), the refusal says what +/// tainted the session and why it cannot ask, so the model (and a person reading the log) can +/// tell a member's "no" from "this run has no member to ask". +#[test] +fn a_headless_refusal_names_the_source_and_says_nobody_can_be_asked() { + let llm = ScriptLlm::new(vec![ + AssistantTurn::tools(vec![call("c1", "web_fetch"), call("c2", "write_file")]), + AssistantTurn::text("done"), + ]); + let host = Arc::new(RecHost::new(false, ToolOutcome::Ok("ok".into()))); + let tools = ToolRegistry::new(catalog()).with_host(HostKind::Core, host.clone()); + let sink = Sink::default(); + let mut history = vec![]; + run_turn( + &cfg(), + &llm, + &tools, + &sink, + &StopFlag::default(), + &mut history, + "go", + ); + let msg = history + .iter() + .find(|m| m.tool_call_id.as_deref() == Some("c2")) + .unwrap(); + assert!(msg.content.contains("from web_fetch"), "{}", msg.content); + assert!(msg.content.contains("explicit approval"), "{}", msg.content); + assert!( + msg.content.contains("no member can be asked"), + "{}", + msg.content + ); + assert_eq!( + msg.content, + format!( + "declined: {}. Nothing was done.", + cannot_ask_reason(Some("web_fetch")) + ) + ); +} + #[test] fn the_taint_persists_across_steps_and_turns() { let llm = ScriptLlm::new(vec![ diff --git a/agent-sidecar/src/action_approvals.rs b/agent-sidecar/src/action_approvals.rs new file mode 100644 index 0000000..82b20c8 --- /dev/null +++ b/agent-sidecar/src/action_approvals.rs @@ -0,0 +1,357 @@ +//! HGW-1 — the member's decisions on the sidecar's own actions after taint, per session. +//! +//! Owner decision 2026-10-15 (HGW (2)): once a session has read untrusted content (a command's +//! output, a web page, an MCP result), an effectful sidecar-hosted call (`fs_*`, `file_write`, +//! `sheet_write`, the toolchain tools, `project_create`, capsules) is not refused for the rest of +//! the conversation. It waits here as an approval card that citrate-core shows to the member +//! (`GET /sessions/:id/actions/pending`, decided with `POST /sessions/:id/actions/decide`): +//! +//! - the card shows the tool, the exact arguments that will be used (`subject`, canonical JSON), +//! a one-line summary, why it is gated (`hicReason`) and every source that tainted the session +//! (`taintSources`); +//! - a decision must carry the `subject` that was shown; anything else is refused and changes +//! nothing; +//! - an allow releases **that one call**, with those arguments. The session stays tainted, so +//! the next effectful call asks again (no approval is ever global); +//! - a decline, a session stop, or no decision within [`ACTION_APPROVAL_TIMEOUT`] declines it. +//! +//! The queue exists only for a session opened with `hicAware: true` (core's promise that a +//! `hic: "required"` card always reaches a person, with no automatic path). Without it the +//! sidecar declines tainted effectful calls with [`citrate_agent_loop::cannot_ask_reason`]. +//! `shell_run`, the browser and MCP keep their own cards (`/shell`, `/browser/actions`, `/mcp`), +//! which carry the same taint reason. Nothing here signs or holds a key (Rule 3). + +use citrate_agent_loop::{StopFlag, ToolCall}; +use serde::Serialize; +use std::sync::{Condvar, Mutex}; +use std::time::{Duration, Instant}; + +/// How long a card waits for the member before it is declined. +pub const ACTION_APPROVAL_TIMEOUT: Duration = Duration::from_secs(300); +/// At most this many cards wait at once per session. +pub const MAX_WAITING: usize = 8; +/// Longest summary line on a card. +pub const SUMMARY_MAX_CHARS: usize = 240; + +/// What the approval card shows. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct ActionPending { + /// The approval id (`act-`), which the decision names. + pub id: String, + /// The loop's tool call id. + pub call_id: String, + pub tool: String, + /// Always `required`. + pub hic: String, + /// The exact arguments that will be used (canonical JSON); a decision must carry it back. + pub subject: String, + /// One line for a person: the tool and its main argument. + pub summary: String, + /// Why the member is asked (the loop's `hic_reason`). + pub hic_reason: String, + /// Every tool that brought untrusted content into the session since it was last clean. + pub taint_sources: Vec, + /// Seconds left before it is declined for want of a decision. + pub expires_in_secs: u64, +} + +impl ActionPending { + /// The card for `call` (its `id` is assigned when it is queued). + pub fn for_call(call: &ToolCall, reason: &str, taint_sources: Vec) -> Self { + let subject = canonical_arguments(&call.arguments); + ActionPending { + id: String::new(), + call_id: call.id.clone(), + tool: call.name.clone(), + hic: "required".into(), + summary: summary_line(&call.name, &subject), + subject, + hic_reason: reason.to_string(), + taint_sources, + expires_in_secs: 0, + } + } +} + +/// The arguments as canonical JSON (sorted keys, no whitespace); the raw text when it is not +/// JSON (the call is then refused by its tool after release, never run differently). +pub fn canonical_arguments(raw: &str) -> String { + let raw = if raw.trim().is_empty() { "{}" } else { raw }; + match serde_json::from_str::(raw) { + Ok(v) => sorted(v).to_string(), + Err(_) => raw.to_string(), + } +} + +/// Keys sorted at every level (whatever serde_json's map order is in this build). +fn sorted(v: serde_json::Value) -> serde_json::Value { + match v { + serde_json::Value::Object(m) => { + let mut entries: Vec<(String, serde_json::Value)> = m.into_iter().collect(); + entries.sort_by(|a, b| a.0.cmp(&b.0)); + serde_json::Value::Object(entries.into_iter().map(|(k, v)| (k, sorted(v))).collect()) + } + serde_json::Value::Array(items) => { + serde_json::Value::Array(items.into_iter().map(sorted).collect()) + } + other => other, + } +} + +/// `tool path` (or the first string argument), clipped to [`SUMMARY_MAX_CHARS`]. +fn summary_line(tool: &str, subject: &str) -> String { + let main = serde_json::from_str::(subject) + .ok() + .and_then(|v| { + let o = v.as_object()?.clone(); + ["path", "from", "project", "name", "cwd"] + .iter() + .find_map(|k| o.get(*k).and_then(|x| x.as_str()).map(str::to_string)) + .or_else(|| o.values().find_map(|x| x.as_str().map(str::to_string))) + }) + .unwrap_or_default(); + let line = if main.is_empty() { + tool.to_string() + } else { + format!("{tool} {main}") + }; + if line.chars().count() > SUMMARY_MAX_CHARS { + let mut s: String = line.chars().take(SUMMARY_MAX_CHARS).collect(); + s.push_str("..."); + s + } else { + line + } +} + +struct Waiting { + view: ActionPending, + deadline: Instant, + answer: Option, +} + +#[derive(Default)] +struct Inner { + next: u64, + waiting: Vec, +} + +/// One session's action cards. +pub struct ActionApprovals { + inner: Mutex, + cv: Condvar, + timeout: Duration, +} + +impl Default for ActionApprovals { + fn default() -> Self { + Self::new(ACTION_APPROVAL_TIMEOUT) + } +} + +impl ActionApprovals { + pub fn new(timeout: Duration) -> Self { + ActionApprovals { + inner: Mutex::new(Inner::default()), + cv: Condvar::new(), + timeout, + } + } + + fn lock(&self) -> std::sync::MutexGuard<'_, Inner> { + self.inner.lock().unwrap_or_else(|p| p.into_inner()) + } + + /// The cards waiting now, oldest first. + pub fn pending(&self) -> Vec { + let now = Instant::now(); + self.lock() + .waiting + .iter() + .filter(|w| w.answer.is_none()) + .map(|w| { + let mut v = w.view.clone(); + v.expires_in_secs = w.deadline.saturating_duration_since(now).as_secs(); + v + }) + .collect() + } + + /// The card with this id, if it is still waiting (for the decision record). + pub fn waiting(&self, id: &str) -> Option { + self.lock() + .waiting + .iter() + .find(|w| w.view.id == id && w.answer.is_none()) + .map(|w| w.view.clone()) + } + + /// The member's decision on card `id`. It must carry the subject that was shown; otherwise + /// (or when nothing with that id is waiting) it is refused and changes nothing. + pub fn decide(&self, id: &str, allow: bool, subject: &str) -> Result<(), String> { + let mut g = self.lock(); + let w = g + .waiting + .iter_mut() + .find(|w| w.view.id == id && w.answer.is_none()) + .ok_or_else(|| "that action is no longer waiting for a decision".to_string())?; + if w.view.subject != subject { + return Err( + "the decision does not match the action that is waiting; nothing was decided" + .to_string(), + ); + } + w.answer = Some(allow); + drop(g); + self.cv.notify_all(); + Ok(()) + } + + /// Park `view` until the member decides, it expires, or the session stops. `Ok(())` only for + /// an explicit allow. + pub fn request(&self, mut view: ActionPending, stop: &StopFlag) -> Result<(), String> { + let deadline = Instant::now() + self.timeout; + let mut g = self.lock(); + if g.waiting.iter().filter(|w| w.answer.is_none()).count() >= MAX_WAITING { + return Err("too many actions are already waiting for the member".to_string()); + } + g.next += 1; + let id = format!("act-{}", g.next); + view.id = id.clone(); + view.expires_in_secs = self.timeout.as_secs(); + g.waiting.push(Waiting { + view, + deadline, + answer: None, + }); + let outcome = loop { + let answer = g + .waiting + .iter() + .find(|w| w.view.id == id) + .and_then(|w| w.answer); + if let Some(a) = answer { + break if a { + Ok(()) + } else { + Err("the member declined this action".to_string()) + }; + } + if stop.is_stopped() { + break Err("the session was stopped before the member decided".to_string()); + } + let now = Instant::now(); + if now >= deadline { + break Err(format!( + "no decision within {}s, so the action was declined", + self.timeout.as_secs() + )); + } + let wait = (deadline - now).min(Duration::from_millis(100)); + g = match self.cv.wait_timeout(g, wait) { + Ok((guard, _)) => guard, + Err(p) => p.into_inner().0, + }; + }; + g.waiting.retain(|w| w.view.id != id); + outcome + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Arc; + + fn call() -> ToolCall { + ToolCall { + id: "c1".into(), + name: "fs_write".into(), + arguments: r#"{ "path": "/p/a.txt", "content": "x" }"#.into(), + } + } + + fn wait_for_card(a: &ActionApprovals) -> ActionPending { + let deadline = Instant::now() + Duration::from_secs(3); + loop { + if let Some(p) = a.pending().into_iter().next() { + return p; + } + assert!(Instant::now() < deadline, "no card appeared"); + std::thread::sleep(Duration::from_millis(5)); + } + } + + #[test] + fn the_card_shows_canonical_arguments_a_summary_and_the_reason() { + let v = ActionPending::for_call(&call(), "because", vec!["shell_run".into()]); + assert_eq!(v.subject, r#"{"content":"x","path":"/p/a.txt"}"#); + assert_eq!(v.summary, "fs_write /p/a.txt"); + assert_eq!(v.hic, "required"); + assert_eq!(v.hic_reason, "because"); + let j = serde_json::to_value(&v).expect("json"); + for k in ["callId", "hicReason", "taintSources", "expiresInSecs"] { + assert!(j.get(k).is_some(), "camelCase field {k} missing: {j}"); + } + let long = ToolCall { + arguments: serde_json::json!({ "path": "x".repeat(500) }).to_string(), + ..call() + }; + let v = ActionPending::for_call(&long, "r", vec![]); + assert!(v.summary.chars().count() <= SUMMARY_MAX_CHARS + 3); + } + + #[test] + fn an_allow_needs_the_subject_that_was_shown() { + let a = Arc::new(ActionApprovals::default()); + let a2 = a.clone(); + let t = std::thread::spawn(move || { + a2.request( + ActionPending::for_call(&call(), "r", vec![]), + &StopFlag::default(), + ) + }); + let card = wait_for_card(&a); + assert!(card.id.starts_with("act-")); + assert!(a.decide(&card.id, true, "{}").is_err()); + assert!(a.decide("act-999", true, &card.subject).is_err()); + assert_eq!(a.waiting(&card.id).map(|c| c.call_id), Some("c1".into())); + assert!(a.decide(&card.id, true, &card.subject).is_ok()); + assert_eq!(t.join().expect("join"), Ok(())); + assert!(a.pending().is_empty()); + } + + #[test] + fn a_decline_a_stop_and_a_timeout_all_decline() { + let a = Arc::new(ActionApprovals::default()); + let a2 = a.clone(); + let t = std::thread::spawn(move || { + a2.request( + ActionPending::for_call(&call(), "r", vec![]), + &StopFlag::default(), + ) + }); + let card = wait_for_card(&a); + a.decide(&card.id, false, &card.subject).expect("decide"); + assert!(t.join().expect("join").is_err()); + + let stop = StopFlag::default(); + let s2 = stop.clone(); + let a3 = a.clone(); + let t = std::thread::spawn(move || { + a3.request(ActionPending::for_call(&call(), "r", vec![]), &s2) + }); + wait_for_card(&a); + stop.stop(); + assert!(t.join().expect("join").is_err()); + + let quick = ActionApprovals::new(Duration::from_millis(150)); + assert!(quick + .request( + ActionPending::for_call(&call(), "r", vec![]), + &StopFlag::default() + ) + .is_err()); + } +} diff --git a/agent-sidecar/src/action_approvals_tests.rs b/agent-sidecar/src/action_approvals_tests.rs new file mode 100644 index 0000000..f1cb77a --- /dev/null +++ b/agent-sidecar/src/action_approvals_tests.rs @@ -0,0 +1,350 @@ +//! HGW-1 — untrusted-content taint escalates to HIC approval instead of ending the conversation. +//! +//! Owner decision 2026-10-15 (HGW (2)): once a session has read untrusted content, every +//! side-effecting action goes to the member as an approval card that shows what is about to run +//! and why it is gated. An approval releases that one action; the session stays tainted, so the +//! next action asks again. Where nobody can be asked (a session not opened by an HIC-aware app, +//! for example a headless run), the action is declined with a message that says so. Taint still +//! never allows a silent or automatic action. +//! +//! BDD map: +//! - AT-1.1 (three or more consecutive `shell_run`s, file writes after taint): +//! `after_taint_commands_and_writes_go_to_a_card_and_run_when_approved`. +//! - Per-action, not global: `an_approval_releases_one_action_and_the_next_asks_again`. +//! - Exact subject: `a_decision_must_carry_the_arguments_that_were_shown`. +//! - Headless: `without_an_hic_aware_app_tainted_actions_are_declined_and_say_why`. +//! - Routes: `the_action_routes_need_the_bearer_and_a_known_session`. +//! - Records: `action_decisions_are_recorded_for_the_anchor`. + +#![cfg(unix)] + +use super::hgw_support::*; +use super::*; +use axum::http::StatusCode; +use citrate_agent_loop::AssistantTurn; + +fn reason_of(card: &serde_json::Value) -> String { + card["hicReason"].as_str().unwrap_or_default().to_string() +} + +#[tokio::test(flavor = "multi_thread")] +async fn after_taint_commands_and_writes_go_to_a_card_and_run_when_approved() { + let fx = Fx::new("taint-cards"); + let proj = fx.proj(); + let written = proj.join("src/index.js"); + let turns = vec![ + AssistantTurn::tools(vec![shell("c1", &["echo", "one"], &proj)]), + AssistantTurn::tools(vec![fs_write("c2", &written, "console.log('hi')\n")]), + AssistantTurn::tools(vec![shell("c3", &["echo", "two"], &proj)]), + AssistantTurn::tools(vec![shell("c4", &["echo", "three"], &proj)]), + AssistantTurn::text("Built."), + ]; + let mgr = Arc::new(manager_with(&fx, turns, Some(shell_open()))); + let id = mgr.create(create_req(fx.rw_doc(), true)).expect("session"); + let s = mgr.get(&id).expect("session"); + let app = app_for(mgr.clone()); + mgr.send(&id, "build it".into(), None).expect("send"); + let answered = member_answers(&app, &s, &id, |_, _| true).await; + + // Every call ran: one untainted command, then three tainted actions, each released by a card. + for c in ["c1", "c2", "c3", "c4"] { + assert_eq!(result_of(&s, c).await["status"], "ok", "{c}"); + } + assert_eq!( + std::fs::read_to_string(&written).expect("written"), + "console.log('hi')\n" + ); + let routes: Vec<&str> = answered.iter().map(|a| a.route).collect(); + assert_eq!(routes, vec!["shell", "actions", "shell", "shell"]); + // The first command was asked before any taint: no taint reason on its card. + assert!( + answered[0].card.get("hicReason").is_none(), + "{}", + answered[0].card + ); + // The write's card shows what will run and why it is gated. + let w = &answered[1].card; + assert_eq!(w["tool"], "fs_write"); + assert_eq!(w["hic"], "required"); + assert!( + w["id"].as_str().unwrap_or_default().starts_with("act-"), + "{w}" + ); + assert!(reason_of(w).contains("shell_run"), "{w}"); + assert_eq!(w["taintSources"], serde_json::json!(["shell_run"])); + assert!( + w["subject"] + .as_str() + .unwrap_or_default() + .contains("index.js"), + "the card shows the exact arguments: {w}" + ); + assert!( + w["summary"] + .as_str() + .unwrap_or_default() + .contains("fs_write"), + "{w}" + ); + // Later commands' cards carry the same reason. + for a in &answered[2..] { + assert!(reason_of(&a.card).contains("shell_run"), "{}", a.card); + assert_eq!(a.card["taintSources"], serde_json::json!(["shell_run"])); + } + // No "host cannot ask" decline anywhere in the session. + assert!(!events(&s).iter().any(|e| e["content"] + .as_str() + .unwrap_or_default() + .contains("no member can be asked"))); + // The session is still tainted: approvals never clear it. + assert!(s.taint().is_tainted()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn an_approval_releases_one_action_and_the_next_asks_again() { + let fx = Fx::new("taint-per-action"); + let proj = fx.proj(); + let a = proj.join("a.txt"); + let b = proj.join("b.txt"); + let turns = vec![ + AssistantTurn::tools(vec![shell("c1", &["echo", "x"], &proj)]), + AssistantTurn::tools(vec![fs_write("c2", &a, "A")]), + AssistantTurn::tools(vec![fs_write("c3", &b, "B")]), + AssistantTurn::text("ok"), + ]; + let mgr = Arc::new(manager_with(&fx, turns, Some(shell_open()))); + let id = mgr.create(create_req(fx.rw_doc(), true)).expect("session"); + let s = mgr.get(&id).expect("session"); + let app = app_for(mgr.clone()); + mgr.send(&id, "go".into(), None).expect("send"); + let b_shown = b.clone(); + let answered = member_answers(&app, &s, &id, move |route, card| { + // Nothing is written before the member decides. + if route == "actions" { + assert!(!b_shown.exists()); + } + // Allow the command and the first write; decline the second write. + !(route == "actions" + && card["subject"] + .as_str() + .unwrap_or_default() + .contains("b.txt")) + }) + .await; + assert_eq!( + answered.iter().filter(|a| a.route == "actions").count(), + 2, + "each write asked on its own" + ); + assert_eq!(std::fs::read_to_string(&a).expect("a"), "A"); + assert!(!b.exists(), "a declined action never runs"); + let r = result_of(&s, "c3").await; + assert_eq!(r["status"], "denied", "{r}"); + assert!( + r["content"] + .as_str() + .unwrap_or_default() + .contains("declined"), + "{r}" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_decision_must_carry_the_arguments_that_were_shown() { + let fx = Fx::new("taint-subject"); + let proj = fx.proj(); + let f = proj.join("f.txt"); + let turns = vec![ + AssistantTurn::tools(vec![shell("c1", &["echo", "x"], &proj)]), + AssistantTurn::tools(vec![fs_write("c2", &f, "F")]), + AssistantTurn::text("ok"), + ]; + let mgr = Arc::new(manager_with(&fx, turns, Some(shell_open()))); + let id = mgr.create(create_req(fx.rw_doc(), true)).expect("session"); + let s = mgr.get(&id).expect("session"); + let app = app_for(mgr.clone()); + mgr.send(&id, "go".into(), None).expect("send"); + // Approve the command, then find the write's card. + let mut card = serde_json::Value::Null; + for _ in 0..1500 { + let (_, sh) = send(&app, "GET", &format!("/sessions/{id}/shell/pending"), None).await; + if let Some(c) = sh["pending"].as_array().and_then(|a| a.first()).cloned() { + let (st, _) = send( + &app, + "POST", + &format!("/sessions/{id}/shell/decide"), + Some(serde_json::json!({"id": c["id"], "allow": true, "argv": c["argv"], "cwd": c["cwd"]})), + ) + .await; + assert_eq!(st, StatusCode::OK); + } + let (_, ac) = send( + &app, + "GET", + &format!("/sessions/{id}/actions/pending"), + None, + ) + .await; + if let Some(c) = ac["pending"].as_array().and_then(|a| a.first()).cloned() { + card = c; + break; + } + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + assert!(card.is_object(), "no action card appeared"); + let url = format!("/sessions/{id}/actions/decide"); + let (st, _) = send( + &app, + "POST", + &url, + Some(serde_json::json!({"id": card["id"], "allow": true, "subject": "{\"path\":\"/etc/x\"}"})), + ) + .await; + assert_eq!(st, StatusCode::CONFLICT); + assert!(!f.exists(), "a mismatched decision releases nothing"); + let (st, _) = send( + &app, + "POST", + &url, + Some(serde_json::json!({"id": "act-999", "allow": true, "subject": card["subject"]})), + ) + .await; + assert_eq!(st, StatusCode::CONFLICT); + let (st, _) = send(&app, "POST", &url, Some(serde_json::json!({"id": 1}))).await; + assert_eq!(st, StatusCode::BAD_REQUEST); + let (st, _) = send( + &app, + "POST", + &url, + Some(serde_json::json!({"id": card["id"], "allow": true, "subject": card["subject"]})), + ) + .await; + assert_eq!(st, StatusCode::OK); + wait_done(&s).await; + assert_eq!(std::fs::read_to_string(&f).expect("f"), "F"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn without_an_hic_aware_app_tainted_actions_are_declined_and_say_why() { + let fx = Fx::new("taint-headless"); + let proj = fx.proj(); + let f = proj.join("f.txt"); + let turns = vec![ + AssistantTurn::tools(vec![shell("c1", &["echo", "x"], &proj)]), + AssistantTurn::tools(vec![fs_write("c2", &f, "F")]), + AssistantTurn::tools(vec![shell("c3", &["echo", "y"], &proj)]), + AssistantTurn::text("ok"), + ]; + let mgr = Arc::new(manager_with(&fx, turns, Some(shell_open()))); + let id = mgr.create(create_req(fx.rw_doc(), false)).expect("session"); + let s = mgr.get(&id).expect("session"); + let app = app_for(mgr.clone()); + mgr.send(&id, "go".into(), None).expect("send"); + // The untainted first command still waits for a decision on the shell route, as before. + let answered = member_answers(&app, &s, &id, |_, _| true).await; + assert_eq!( + answered.len(), + 1, + "only the untainted command was asked: {answered:?}" + ); + assert_eq!(answered[0].route, "shell"); + for c in ["c2", "c3"] { + let r = result_of(&s, c).await; + assert_eq!(r["status"], "denied", "{r}"); + let msg = r["content"].as_str().unwrap_or_default(); + assert!(msg.contains("no member can be asked"), "{msg}"); + assert!(msg.contains("from shell_run"), "{msg}"); + } + assert!(!f.exists()); + let (st, body) = send( + &app, + "GET", + &format!("/sessions/{id}/actions/pending"), + None, + ) + .await; + assert_eq!(st, StatusCode::OK); + assert_eq!(body["pending"], serde_json::json!([])); +} + +#[tokio::test(flavor = "multi_thread")] +async fn the_action_routes_need_the_bearer_and_a_known_session() { + use axum::body::Body; + use axum::http::Request; + use tower::ServiceExt; + let fx = Fx::new("taint-routes"); + let mgr = Arc::new(manager_with(&fx, vec![], Some(shell_open()))); + let id = mgr.create(create_req(fx.rw_doc(), true)).expect("session"); + let app = app_for(mgr); + let resp = app + .clone() + .oneshot( + Request::builder() + .uri(format!("/sessions/{id}/actions/pending")) + .body(Body::empty()) + .expect("req"), + ) + .await + .expect("resp"); + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); + let (st, _) = send(&app, "GET", "/sessions/nope/actions/pending", None).await; + assert_eq!(st, StatusCode::NOT_FOUND); + let (st, _) = send( + &app, + "POST", + "/sessions/nope/actions/decide", + Some(serde_json::json!({"id": "act-1", "allow": true, "subject": "{}"})), + ) + .await; + assert_eq!(st, StatusCode::NOT_FOUND); +} + +/// HUP-S2.6: every member decision on an action card lands in the decision log the nightly +/// anchor batches (write-ahead for an allow). +#[tokio::test(flavor = "multi_thread")] +async fn action_decisions_are_recorded_for_the_anchor() { + use citrate_agent_records::{Decision, DecisionLog, Entry, LogConfig, Outcome}; + let fx = Fx::new("taint-records"); + let records_dir = fx.base.join("records"); + let (log, _) = DecisionLog::open(&records_dir, LogConfig::default()).expect("log"); + let proj = fx.proj(); + let turns = vec![ + AssistantTurn::tools(vec![shell("c1", &["echo", "x"], &proj)]), + AssistantTurn::tools(vec![fs_write("c2", &proj.join("a.txt"), "A")]), + AssistantTurn::tools(vec![fs_write("c3", &proj.join("b.txt"), "B")]), + AssistantTurn::text("ok"), + ]; + let mgr = Arc::new(manager_with(&fx, turns, Some(shell_open())).with_records(Arc::new(log))); + let id = mgr.create(create_req(fx.rw_doc(), true)).expect("session"); + let s = mgr.get(&id).expect("session"); + let app = app_for(mgr.clone()); + mgr.send(&id, "go".into(), None).expect("send"); + member_answers(&app, &s, &id, |route, card| { + !(route == "actions" + && card["subject"] + .as_str() + .unwrap_or_default() + .contains("b.txt")) + }) + .await; + let mut recs = citrate_agent_records::read::page(&records_dir, None, 10_000).expect("records"); + recs.sort_by_key(|r| r.record.seq); + let decisions: Vec<(u64, Decision, String)> = recs + .iter() + .filter_map(|r| match &r.record.entry { + Entry::Decision(d) if d.kind == "sidecar.tool_call" => { + Some((r.record.seq, d.decision, d.subject.clone())) + } + _ => None, + }) + .collect(); + assert_eq!(decisions.len(), 2, "{decisions:?}"); + assert_eq!(decisions[0].1, Decision::Approved); + assert!(decisions[0].2.contains("fs_write"), "{decisions:?}"); + let outcome = recs.iter().find_map(|r| match &r.record.entry { + Entry::Outcome(o) if o.decision_seq == decisions[0].0 => Some(o.outcome), + _ => None, + }); + assert_eq!(outcome, Some(Outcome::Completed)); + assert_eq!(decisions[1].1, Decision::Denied); +} diff --git a/agent-sidecar/src/files.rs b/agent-sidecar/src/files.rs index 93aabda..384c407 100644 --- a/agent-sidecar/src/files.rs +++ b/agent-sidecar/src/files.rs @@ -46,8 +46,9 @@ //! **Honest scope.** A file changed between the grant check and the write is caught by the //! checkpoint's own path checks (no write through a symbolic link, no path through a symlinked //! directory), not by an OS sandbox. Taint (HUP-S2.7) is enforced by the loop: these tools are -//! effectful, so after the session reads untrusted content they are declined. This module never -//! holds a key and never signs (Rule 3). +//! effectful, so after the session reads untrusted content each call waits for the member on an +//! action card (HGW-1, [`crate::action_approvals`]), or is declined where nobody can be asked. +//! This module never holds a key and never signs (Rule 3). use std::fs; use std::io::{ErrorKind, Read, Write}; diff --git a/agent-sidecar/src/files_tests.rs b/agent-sidecar/src/files_tests.rs index 290040a..b3c0098 100644 --- a/agent-sidecar/src/files_tests.rs +++ b/agent-sidecar/src/files_tests.rs @@ -971,7 +971,7 @@ async fn the_file_tool_names_are_reserved_while_file_tools_are_on() { } #[tokio::test(flavor = "multi_thread")] -async fn after_untrusted_content_a_file_write_is_declined_and_nothing_is_written() { +async fn after_untrusted_content_a_file_write_waits_for_the_member_and_a_decline_writes_nothing() { let s = Scratch::new(); let (tools, store) = s.tools(); let target = s.proj().join("pwned.txt"); @@ -1012,12 +1012,30 @@ async fn after_untrusted_content_a_file_write_is_declined_and_nothing_is_written } tokio::time::sleep(Duration::from_millis(20)).await; } + // HGW-1: the write is not refused for the rest of the session; it waits on an action card. + // Nothing is written while it waits, and the member's decline writes nothing. + let mut card = None; + for _ in 0..500 { + if let Some(c) = sess.action_pending().unwrap_or_default().into_iter().next() { + card = Some(c); + break; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + let card = card.expect("the tainted write waits for the member"); + assert_eq!(card.tool, FS_WRITE_TOOL); + assert!(card.hic_reason.contains("web_fetch"), "{}", card.hic_reason); + assert!( + !target.exists(), + "nothing is written before the member decides" + ); + sess.action_decide(&card.id, false, &card.subject).unwrap(); events_until(&mgr, &id, "done", &mut after, &mut all).await; let w = all .iter() .find(|e| e["type"] == "tool_result" && e["call_id"] == "w1") .unwrap(); - assert_ne!(w["status"], "ok", "{w}"); + assert_eq!(w["status"], "denied", "{w}"); assert!(!target.exists()); assert!(store .steps(&SessionId::new(&id).unwrap()) diff --git a/agent-sidecar/src/hgw_support.rs b/agent-sidecar/src/hgw_support.rs new file mode 100644 index 0000000..cf02da6 --- /dev/null +++ b/agent-sidecar/src/hgw_support.rs @@ -0,0 +1,286 @@ +//! Test support for the HGW ("Hermes gets to work") session tests: a scratch home with a granted +//! project folder, a scripted model, a session manager with `shell_run` and the checkpointed file +//! tools on, the HTTP app, and a "member" that answers approval cards through the routes. + +#![cfg(all(test, unix))] +// Shared by several HGW test modules; each uses a different subset. +#![allow(dead_code)] + +use super::*; +use axum::body::Body; +use axum::http::{Request, StatusCode}; +use citrate_agent_checkpoints::{CheckpointStore, Config}; +use citrate_agent_grants::{Access, FolderGrants, GrantRequest}; +use citrate_agent_loop::{AssistantTurn, CompletionRequest, LlmClient, LlmError, ToolCall}; +use citrate_agent_shell::sandbox::{SandboxMode, SandboxPolicy}; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::Mutex; +use std::time::Duration; +use tower::ServiceExt; + +pub(crate) const BEARER: &str = "test-bearer-token-0123456789"; +pub(crate) const MEMBER: &str = "0x00000000000000000000000000000000000000aa"; +static N: AtomicUsize = AtomicUsize::new(0); + +pub(crate) fn now() -> u64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0) +} + +/// `base/home/proj` (granted read + write), `base/home/other` (not granted) and the undo store +/// at `base/ckpt` (outside the home). +pub(crate) struct Fx { + pub base: PathBuf, +} + +impl Fx { + pub fn new(tag: &str) -> Self { + let base = std::env::temp_dir().join(format!( + "citrate-hgw-{tag}-{}-{}", + std::process::id(), + N.fetch_add(1, Ordering::SeqCst) + )); + let _ = std::fs::remove_dir_all(&base); + for d in ["home/proj/src", "home/other", "ckpt"] { + std::fs::create_dir_all(base.join(d)).expect("fixture dirs"); + } + Fx { + base: base.canonicalize().expect("canonical base"), + } + } + pub fn home(&self) -> PathBuf { + self.base.join("home") + } + pub fn proj(&self) -> PathBuf { + self.base.join("home/proj") + } + pub fn other(&self) -> PathBuf { + self.base.join("home/other") + } + /// A grant document over `roots` (read and write each). + pub fn rw_doc_for(&self, roots: &[PathBuf]) -> serde_json::Value { + let mut g = FolderGrants::new(self.home(), self.home()); + for r in roots { + for a in [Access::Read, Access::Write] { + g.grant(GrantRequest::folder(r, a, MEMBER, "build it"), now()) + .expect("grant"); + } + } + serde_json::to_value(g.state()).expect("grant doc") + } + pub fn rw_doc(&self) -> serde_json::Value { + self.rw_doc_for(&[self.proj()]) + } +} + +impl Drop for Fx { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.base); + } +} + +/// `shell_run` with the sandbox off: for tests of the approval flow itself, which must pass on +/// machines without a backend. Production always requires the sandbox. +pub(crate) fn shell_open() -> shell_run::ShellRunConfig { + shell_run::ShellRunConfig { + search_path: vec![PathBuf::from("/bin"), PathBuf::from("/usr/bin")], + sandbox: SandboxPolicy::new(SandboxMode::Off), + approval_timeout: Duration::from_secs(20), + } +} + +/// The model: replays `turns`, then answers "(done)". +pub(crate) struct Script(pub Mutex>); +impl LlmClient for Script { + fn complete(&self, _req: &CompletionRequest) -> Result { + let mut t = self.0.lock().unwrap_or_else(|p| p.into_inner()); + Ok(if t.is_empty() { + AssistantTurn::text("(done)") + } else { + t.remove(0) + }) + } +} + +/// A session manager with grants, the undo store and `shell` (when given). +pub(crate) fn manager_with( + fx: &Fx, + turns: Vec, + shell: Option, +) -> sessions::SessionManager { + let script = Arc::new(Script(Mutex::new(turns))); + let store = + Arc::new(CheckpointStore::open(&fx.base.join("ckpt"), Config::default()).expect("store")); + let mut mgr = sessions::SessionManager::new( + Arc::new(move |_ep: &sessions::LlmEndpoint| script.clone() as Arc), + Duration::from_secs(5), + ) + .with_grants_home(fx.home()) + .with_checkpoints(store); + if let Some(c) = shell { + mgr = mgr.with_shell_run(Arc::new(c)); + } + mgr +} + +/// `POST /sessions` body with `grants` and `hicAware`. +pub(crate) fn create_req(grants: serde_json::Value, hic_aware: bool) -> sessions::CreateSessionReq { + serde_json::from_value(serde_json::json!({ + "model": "gemma-4", + "systemPrompt": "You are Hermes.", + "llm": {"baseUrl": "http://127.0.0.1:18080/v1", "bearer": "k"}, + "tools": [], + "maxToolsPerRequest": 8, + "hicAware": hic_aware, + "grants": grants, + })) + .expect("create req") +} + +pub(crate) fn app_for(mgr: Arc) -> axum::Router { + app(Arc::new(AppState { + estop: EmergencyStop::new(), + queue: Arc::new(ApprovalQueue::new()), + skills: vec![], + dispatch: None, + bearer: BEARER.to_string(), + run_slots: Arc::new(tokio::sync::Semaphore::new(MAX_CONCURRENT_SKILLS)), + sessions: mgr, + })) +} + +pub(crate) async fn send( + app: &axum::Router, + method: &str, + path: &str, + body: Option, +) -> (StatusCode, serde_json::Value) { + let req = Request::builder() + .method(method) + .uri(path) + .header("authorization", format!("Bearer {BEARER}")) + .header("content-type", "application/json") + .body(match body { + Some(b) => Body::from(b.to_string()), + None => Body::empty(), + }) + .expect("request"); + let resp = app.clone().oneshot(req).await.expect("response"); + let status = resp.status(); + let bytes = axum::body::to_bytes(resp.into_body(), 1 << 20) + .await + .unwrap_or_default(); + ( + status, + serde_json::from_slice(&bytes).unwrap_or(serde_json::Value::Null), + ) +} + +pub(crate) fn tool_call(id: &str, name: &str, args: serde_json::Value) -> ToolCall { + ToolCall { + id: id.into(), + name: name.into(), + arguments: args.to_string(), + } +} + +pub(crate) fn shell(id: &str, argv: &[&str], cwd: &Path) -> ToolCall { + tool_call( + id, + shell_run::SHELL_RUN_TOOL, + serde_json::json!({ "argv": argv, "cwd": cwd }), + ) +} + +pub(crate) fn fs_write(id: &str, path: &Path, content: &str) -> ToolCall { + tool_call( + id, + "fs_write", + serde_json::json!({ "path": path, "content": content }), + ) +} + +/// Every event of the session, as JSON. +pub(crate) fn events(s: &sessions::Session) -> Vec { + s.events_after(0) + .events + .iter() + .map(|e| serde_json::to_value(&e.event).unwrap_or(serde_json::Value::Null)) + .collect() +} + +/// The `tool_result` event for `call_id`, once it exists. +pub(crate) async fn result_of(s: &sessions::Session, call_id: &str) -> serde_json::Value { + for _ in 0..1500 { + if let Some(e) = events(s) + .into_iter() + .find(|e| e["type"] == "tool_result" && e["call_id"] == call_id) + { + return e; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + panic!("no tool_result for {call_id}"); +} + +/// Wait until the session's turn is over. +pub(crate) async fn wait_done(s: &sessions::Session) { + for _ in 0..3000 { + if !s.is_busy() && events(s).iter().any(|e| e["type"] == "done") { + return; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + panic!("the turn never finished"); +} + +/// One card the member answered, and on which route. +#[derive(Debug, Clone)] +pub(crate) struct Answered { + pub route: &'static str, + pub card: serde_json::Value, + pub allow: bool, +} + +/// Plays the member: answers every shell and action card of session `id` with `decide(card)` +/// (true = allow), until the turn is over. Returns the cards in the order they were answered. +pub(crate) async fn member_answers( + app: &axum::Router, + s: &sessions::Session, + id: &str, + decide: impl Fn(&'static str, &serde_json::Value) -> bool, +) -> Vec { + let mut out = Vec::new(); + for _ in 0..3000 { + for route in ["shell", "actions"] { + let (st, body) = + send(app, "GET", &format!("/sessions/{id}/{route}/pending"), None).await; + assert_eq!(st, StatusCode::OK, "{route}: {body}"); + for card in body["pending"].as_array().cloned().unwrap_or_default() { + let allow = decide(route, &card); + let decision = if route == "shell" { + serde_json::json!({"id": card["id"], "allow": allow, "argv": card["argv"], "cwd": card["cwd"]}) + } else { + serde_json::json!({"id": card["id"], "allow": allow, "subject": card["subject"]}) + }; + let (st, b) = send( + app, + "POST", + &format!("/sessions/{id}/{route}/decide"), + Some(decision), + ) + .await; + assert_eq!(st, StatusCode::OK, "{route} decide: {b}"); + out.push(Answered { route, card, allow }); + } + } + if !s.is_busy() && events(s).iter().any(|e| e["type"] == "done") { + return out; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + panic!("the turn never finished; answered so far: {out:?}"); +} diff --git a/agent-sidecar/src/lib.rs b/agent-sidecar/src/lib.rs index e1f3d97..6466106 100644 --- a/agent-sidecar/src/lib.rs +++ b/agent-sidecar/src/lib.rs @@ -18,6 +18,7 @@ //! //! NB — this is NOT `hermes/` (the Discord command-plane bot). Different program, distinct binary. +pub mod action_approvals; pub mod anchor; pub mod browser; pub mod capsule_sandbox; @@ -299,6 +300,8 @@ pub fn app(state: Arc) -> Router { .route("/sessions/:id/shell/decide", post(shell_decide)) .route("/sessions/:id/mcp/pending", get(mcp_pending)) .route("/sessions/:id/mcp/decide", post(mcp_decide)) + .route("/sessions/:id/actions/pending", get(action_pending)) + .route("/sessions/:id/actions/decide", post(action_decide)) // HUP-S1.4 — tracks + briefs (the interview every client shares). .route("/tracks", get(tracks)) .route("/briefs", post(create_brief)) @@ -1047,6 +1050,118 @@ async fn mcp_pending( Ok(Json(serde_json::json!({ "pending": pending }))) } +/// HGW-1: `GET /sessions/:id/actions/pending`: the action cards waiting for the member (tainted +/// effectful sidecar calls in an HIC-aware session). A session without them has none. +async fn action_pending( + headers: HeaderMap, + State(st): State>, + Path(id): Path, +) -> Result, JsonErr> { + if !authorized(&headers, &st.bearer) { + return Err(json_err(StatusCode::UNAUTHORIZED, "unauthorized")); + } + let session = st + .sessions + .get(&id) + .ok_or_else(|| json_err(StatusCode::NOT_FOUND, "no such session"))?; + let pending = session.action_pending().unwrap_or_default(); + Ok(Json(serde_json::json!({ "pending": pending }))) +} + +#[derive(Deserialize)] +struct ActionDecideReq { + id: String, + allow: bool, + subject: String, +} + +/// HGW-1: `POST /sessions/:id/actions/decide {id, allow, subject}`: the member's decision on one +/// action card. `subject` must be the arguments that were shown; otherwise 409 and nothing is +/// decided. Recorded like the other HIC decisions (write-ahead for an allow). +async fn action_decide( + headers: HeaderMap, + State(st): State>, + Path(id): Path, + body: axum::body::Bytes, +) -> Result, JsonErr> { + if !authorized(&headers, &st.bearer) { + return Err(json_err(StatusCode::UNAUTHORIZED, "unauthorized")); + } + let session = st + .sessions + .get(&id) + .ok_or_else(|| json_err(StatusCode::NOT_FOUND, "no such session"))?; + let req: ActionDecideReq = serde_json::from_slice(&body) + .map_err(|_| json_err(StatusCode::BAD_REQUEST, "expected {id, allow, subject}"))?; + let card = session + .action_waiting(&req.id) + .filter(|c| c.subject == req.subject) + .ok_or_else(|| { + json_err( + StatusCode::CONFLICT, + "that action is no longer waiting, or the decision does not match what is waiting; nothing was decided", + ) + })?; + let subject = format!("{} with {}", card.tool, card.subject); + let log = st.sessions.records(); + let record = |allow: bool, log: &citrate_agent_records::DecisionLog| { + hic_records::record_member_decision( + log, + "sidecar.tool_call", + &subject, + allow, + "the member decided on the exact action shown, in a session that read untrusted content", + vec![citrate_agent_records::EvidenceRef { + kind: "sidecar_tool_call".to_string(), + uri: format!("sidecar:sessions/{id}/actions/{}", req.id), + digest: None, + }], + ) + }; + let allow_seq = match &log { + Some(log) if req.allow => match record(true, log) { + Ok(seq) => Some(seq), + Err(e) => return Err(json_err(StatusCode::SERVICE_UNAVAILABLE, &e)), + }, + _ => None, + }; + match session.action_decide(&req.id, req.allow, &req.subject) { + Ok(()) => { + if let Some(log) = &log { + match allow_seq { + Some(seq) => hic_records::record_outcome( + log, + seq, + citrate_agent_records::Outcome::Completed, + "released; the call's own result is in the session", + ), + None => { + if let Err(e) = record(false, log) { + eprintln!("citrate-agent-sidecar: {e}"); + } + } + } + } + Ok(Json(serde_json::json!({ "ok": true }))) + } + Err(e) => { + if let (Some(log), Some(seq)) = (&log, allow_seq) { + hic_records::record_outcome( + log, + seq, + citrate_agent_records::Outcome::Failed, + "not released (no longer waiting, or the subject differed)", + ); + } + let msg = match e { + sessions::SessionError::Invalid(m) => m, + _ => "that action is no longer waiting for a decision".to_string(), + }; + Err(json_err(StatusCode::CONFLICT, &msg)) + } + } +} + #[derive(Deserialize)] struct McpDecideReq { id: String, @@ -2156,12 +2271,16 @@ mod tests; #[cfg(test)] mod verify_probes_tests; +#[cfg(all(test, unix))] +mod action_approvals_tests; #[cfg(test)] mod browser_session_tests; #[cfg(test)] mod decide_route_tests; #[cfg(all(test, unix))] mod deploy_guard_session_tests; +#[cfg(all(test, unix))] +mod hgw_support; #[cfg(test)] mod search_session_tests; #[cfg(test)] diff --git a/agent-sidecar/src/sessions.rs b/agent-sidecar/src/sessions.rs index 37dad32..d83ff33 100644 --- a/agent-sidecar/src/sessions.rs +++ b/agent-sidecar/src/sessions.rs @@ -65,6 +65,13 @@ //! session opened with folder grants also offers the sidecar-hosted `shell_run` tool //! ([`crate::shell_run`]): each command waits for the member's decision on the session's //! `/shell` routes and runs only in the OS sandbox. The name is reserved while it is on. +//! - HGW-1 (owner decision 2026-10-15): taint escalates to HIC approval instead of ending the +//! conversation. In a session opened with `hicAware: true`, a tainted effectful sidecar-hosted +//! call waits for the member on the session's action cards (`/actions` routes, +//! [`crate::action_approvals`]); `shell_run`, MCP and browser calls use their own cards, which +//! carry the same reason. Each approval releases one call and the session stays tainted. +//! Without `hicAware` nobody can be asked, so such calls are declined with +//! [`citrate_agent_loop::cannot_ask_reason`]. use std::collections::{HashMap, VecDeque}; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; @@ -96,6 +103,7 @@ use citrate_agent_metering::{MeteringSink, SystemClock}; use citrate_agent_trajectory::TrajectoryRecorder; use serde::{Deserialize, Serialize}; +use crate::action_approvals::{ActionApprovals, ActionPending}; use crate::anchor::AnchorService; use crate::decide::DecideService; use crate::files::{FileTools, FileToolsHost}; @@ -545,6 +553,8 @@ pub struct Session { /// refused), and its MCP approval cards (present only for an HIC-aware client). mcp_offered: Option>>, mcp_approvals: Option>, + /// HGW-1: the cards for tainted effectful sidecar calls (present only for an HIC-aware app). + actions: Option>, /// Test seam: runs on the emitting thread right after an event becomes visible to readers, so /// a test can act as a core client at the exact moment the event appears. #[cfg(test)] @@ -593,6 +603,23 @@ impl Session { a.decide(id, allow, subject).map_err(SessionError::Invalid) } + /// HGW-1: the action cards waiting for the member (`None` when the session has none, i.e. it + /// was not opened by an HIC-aware app). + pub fn action_pending(&self) -> Option> { + self.actions.as_ref().map(|a| a.pending()) + } + + /// HGW-1: the waiting action card `id` (for its decision record). + pub fn action_waiting(&self, id: &str) -> Option { + self.actions.as_ref().and_then(|a| a.waiting(id)) + } + + /// HGW-1: the member's decision on a waiting action card; it must carry the subject shown. + pub fn action_decide(&self, id: &str, allow: bool, subject: &str) -> Result<(), SessionError> { + let a = self.actions.as_ref().ok_or(SessionError::NotFound)?; + a.decide(id, allow, subject).map_err(SessionError::Invalid) + } + /// HUP-S1.2: how this session counts tokens and ranks tools and skills right now. pub fn retrieval_report(&self) -> RetrievalReport { RetrievalReport { @@ -929,6 +956,11 @@ pub(crate) struct SidecarHost { pub(crate) browser: Option, /// US-2.2 AC2: `shell_run`, when on and the session has grants. pub(crate) shell: Option, + /// HGW-1: the session's action cards (HIC-aware apps only), with its taint and stop, for + /// effectful calls after taint. + pub(crate) actions: Option>, + pub(crate) taint: TaintState, + pub(crate) stop: StopFlag, } impl ToolHost for SidecarHost { @@ -990,30 +1022,61 @@ impl ToolHost for SidecarHost { } /// HUP-S5.1: the browser can put an action in front of the member (its decision routes); - /// HUP-S4.1: so can MCP, through the session's MCP approval cards (HIC-aware clients only). - /// Without either this stays false, so the loop declines as before. + /// HUP-S4.1: so can MCP, through the session's MCP approval cards (HIC-aware clients only); + /// HGW-1: so can every other sidecar tool, through the session's action cards and the + /// `shell_run` card (HIC-aware clients only). Without any of them this stays false, so the + /// loop declines as before. fn honors_explicit_approval(&self) -> bool { self.browser.is_some() + || self.actions.is_some() || self .mcp .as_ref() .is_some_and(|m| m.honors_explicit_approval()) + || self + .shell + .as_ref() + .is_some_and(|s| s.honors_explicit_approval()) } fn execute_with_explicit_approval(&self, call: &ToolCall, reason: &str) -> ToolOutcome { + let cannot_ask = || { + ToolOutcome::Denied(citrate_agent_loop::cannot_ask_reason( + self.taint.record().as_ref().map(|r| r.source.as_str()), + )) + }; + // Tools with their own card show it (with the reason) themselves. + if call.name == SHELL_RUN_TOOL { + return match &self.shell { + Some(s) => s.execute_with_explicit_approval(call, reason), + None => cannot_ask(), + }; + } if let Some(m) = &self.mcp { - if m.handles(&call.name) && m.honors_explicit_approval() { - return m.execute_with_explicit_approval(call, reason); + if m.handles(&call.name) { + return if m.honors_explicit_approval() { + m.execute_with_explicit_approval(call, reason) + } else { + cannot_ask() + }; } } - match &self.browser { - Some(b) if browser_tools::handles(&call.name) => { - b.execute_with_explicit_approval(call, reason) + if let Some(b) = &self.browser { + if browser_tools::handles(&call.name) { + return b.execute_with_explicit_approval(call, reason); } - _ => ToolOutcome::Denied( - "this session read untrusted content and this action needs a member's explicit approval, which this tool's host cannot ask for" - .into(), - ), + } + // HGW-1: everything else waits on an action card; an allow releases this one call. + let Some(actions) = &self.actions else { + return cannot_ask(); + }; + let card = ActionPending::for_call(call, reason, self.taint.sources().unwrap_or_default()); + match actions.request(card, &self.stop) { + Ok(()) if self.stop.is_stopped() => { + ToolOutcome::Denied("the session was stopped; nothing was done".into()) + } + Ok(()) => self.execute(call), + Err(why) => ToolOutcome::Denied(why), } } } @@ -1525,6 +1588,8 @@ impl SessionManager { // HUP-S4.1: MCP approval cards only for a client that shows `hic: required` to a person. let mcp_approvals = (self.mcp.is_some() && req.hic_aware) .then(|| Arc::new(crate::mcp_approvals::McpApprovals::default())); + // HGW-1: action cards only for a client that shows `hic: required` to a person. + let actions = req.hic_aware.then(|| Arc::new(ActionApprovals::default())); if self.browser.is_some() { if let Some(t) = specs.iter().find(|t| browser_tools::handles(&t.name)) { return Err(SessionError::Invalid(format!( @@ -1770,6 +1835,7 @@ impl SessionManager { decide_llm, mcp_offered, mcp_approvals, + actions, #[cfg(test)] event_hook: Mutex::new(None), }); @@ -1907,10 +1973,13 @@ impl SessionManager { ), )) }); - let shell_host = session - .shell - .as_ref() - .map(|s| s.host(session.taint.clone(), session.stop.clone())); + let shell_host = session.shell.as_ref().map(|s| { + s.host( + session.taint.clone(), + session.stop.clone(), + session.hic_aware, + ) + }); if file_host.is_some() || shell_host.is_some() || skill_host.is_some() @@ -1936,6 +2005,9 @@ impl SessionManager { learn: learn_host, browser: browser_host, shell: shell_host, + actions: session.actions.clone(), + taint: session.taint.clone(), + stop: session.stop.clone(), }), ); } diff --git a/agent-sidecar/src/shell_run.rs b/agent-sidecar/src/shell_run.rs index 9f03d47..2fbca56 100644 --- a/agent-sidecar/src/shell_run.rs +++ b/agent-sidecar/src/shell_run.rs @@ -7,8 +7,12 @@ //! //! One call: //! -//! 1. **Refused in a tainted session.** Once a session has read untrusted content it never runs a -//! command (the loop declines the call; this host checks again). +//! 1. **After taint, still asked, never automatic (HGW-1).** Once a session has read untrusted +//! content, a command is still put in front of the member, with the card saying why it is +//! gated (`hicReason`, `taintSources`), but only in a session opened by an HIC-aware app +//! (`hicAware: true`). Without one nobody can be asked, so the call is declined with +//! [`citrate_agent_loop::cannot_ask_reason`]. An approval releases that one command; the +//! session stays tainted. //! 2. **Validated before anything is asked.** The model gives `argv` (program first, as a bare //! name resolved from a fixed search path, never a path; argv only, no shell expansion) and an //! absolute `cwd`. The cwd must be covered by live read and write folder grants that reach @@ -25,7 +29,7 @@ //! and stdout/stderr captured up to [`OUTPUT_CAP`] bytes each. //! 5. **Reports** a toolchain envelope (`tool: "shell_run"`) with the run facts, the captured //! output and the sandbox; citrate-core adds it to the Activity monitor. The output is -//! untrusted, so it taints the session (and the next `shell_run` is then refused). +//! untrusted, so it taints the session (and every later effectful call then needs a card). //! //! Keyless: nothing here holds a key or signs (Rule 3). @@ -169,6 +173,12 @@ pub struct ShellPending { pub sandbox: SandboxSummary, /// Seconds left before it is declined for want of a decision. pub expires_in_secs: u64, + /// HGW-1: why this command is gated beyond the usual card, when the session is tainted. + #[serde(skip_serializing_if = "Option::is_none")] + pub hic_reason: Option, + /// HGW-1: every tool that brought untrusted content into the session (empty when clean). + #[serde(skip_serializing_if = "Vec::is_empty")] + pub taint_sources: Vec, } struct Waiting { @@ -319,14 +329,17 @@ impl ShellRunSession { &self.approvals } - /// The host for one turn of the session. - pub fn host(&self, taint: TaintState, stop: StopFlag) -> ShellRunHost { + /// The host for one turn of the session. `hic_aware`: the session was opened by an app that + /// always puts a `hic: "required"` card in front of a person, so a tainted session may still + /// ask; without it a tainted session's commands are declined. + pub fn host(&self, taint: TaintState, stop: StopFlag, hic_aware: bool) -> ShellRunHost { ShellRunHost { runner: self.runner.clone(), approvals: self.approvals.clone(), approval_timeout: self.approval_timeout, taint, stop, + hic_aware, } } } @@ -338,6 +351,7 @@ pub struct ShellRunHost { approval_timeout: Duration, taint: TaintState, stop: StopFlag, + hic_aware: bool, } /// The model's proposal. @@ -499,16 +513,12 @@ fn report(argv: &[String], r: &RunReport) -> ToolOutcome { ) } -impl ToolHost for ShellRunHost { - fn execute(&self, call: &ToolCall) -> ToolOutcome { +impl ShellRunHost { + /// Validate, ask the member (with the taint reason when there is one), re-check, run. + fn ask_and_run(&self, call: &ToolCall, hic_reason: Option<&str>) -> ToolOutcome { if call.name != SHELL_RUN_TOOL { return ToolOutcome::Error(format!("'{}' is not shell_run", call.name)); } - if self.taint.is_tainted() { - return ToolOutcome::Denied( - "this session read untrusted content, so it does not run commands".into(), - ); - } let proposal = match parse(&call.arguments) { Ok(p) => p, Err(e) => return refuse(RunStatus::Refused, e), @@ -532,6 +542,12 @@ impl ToolHost for ShellRunHost { timeout_secs: plan.timeout.as_secs(), sandbox: plan.sandbox.clone(), expires_in_secs: 0, + hic_reason: hic_reason.map(str::to_string), + taint_sources: if hic_reason.is_some() { + self.taint.sources().unwrap_or_default() + } else { + Vec::new() + }, }; let stop = self.stop.clone(); if let Err(why) = self @@ -559,6 +575,42 @@ impl ToolHost for ShellRunHost { Err(e) => refusal_for(&e), } } + + /// The reason a tainted session's command is gated (the loop's wording, naming the source). + fn taint_reason(&self) -> Option { + self.taint.record().map(|r| { + format!( + "this session read untrusted content (from {}), so this action needs your explicit approval", + r.source + ) + }) + } +} + +impl ToolHost for ShellRunHost { + fn execute(&self, call: &ToolCall) -> ToolOutcome { + // The loop routes tainted calls through `execute_with_explicit_approval`; a session that + // became tainted in between is checked again here and never runs a command unasked. + match self.taint_reason() { + Some(reason) => self.execute_with_explicit_approval(call, &reason), + None => self.ask_and_run(call, None), + } + } + + fn honors_explicit_approval(&self) -> bool { + self.hic_aware + } + + /// HGW-1: the command's own card is the explicit approval; it carries the reason. Without an + /// HIC-aware app nobody can be asked, so nothing is asked or run. + fn execute_with_explicit_approval(&self, call: &ToolCall, reason: &str) -> ToolOutcome { + if !self.hic_aware { + return ToolOutcome::Denied(citrate_agent_loop::cannot_ask_reason( + self.taint.record().as_ref().map(|r| r.source.as_str()), + )); + } + self.ask_and_run(call, Some(reason)) + } } #[cfg(test)] diff --git a/agent-sidecar/src/shell_run_tests.rs b/agent-sidecar/src/shell_run_tests.rs index 09e6d16..22d38af 100644 --- a/agent-sidecar/src/shell_run_tests.rs +++ b/agent-sidecar/src/shell_run_tests.rs @@ -1,7 +1,8 @@ //! US-2.2 AC2 — the general `shell_run` tool: the agent proposes an exact argv and cwd, the member //! sees exactly that (plus the program it resolves to and the sandbox) and decides, and only an //! approved command runs, inside the OS sandbox, in a live folder grant, with a timeout and -//! capped output. Off by default; refused in a tainted session; fails closed without a sandbox. +//! capped output. Off by default; after taint asked with the reason (HIC-aware apps) or refused; +//! fails closed without a sandbox. //! //! BDD map: //! - "Anything else shows the exact command + cwd for approval": @@ -10,7 +11,9 @@ //! `a_decision_must_carry_the_argv_and_cwd_that_were_shown`. //! - Grants: `a_cwd_outside_the_grants_is_refused_before_asking`, //! `a_read_only_or_shallow_grant_is_not_enough`, `a_revoked_grant_stops_an_approved_run`. -//! - Taint: `a_tainted_session_is_refused_and_nothing_is_asked`, +//! - Taint (HGW-1): `a_tainted_session_without_an_hic_aware_app_is_refused_and_nothing_is_asked`, +//! `a_tainted_hic_aware_session_asks_with_the_reason_and_runs_only_on_allow`, +//! `an_untainted_card_has_no_taint_reason`, //! `an_unattended_session_never_offers_a_run`. //! - Sandbox: `no_sandbox_means_no_run_and_no_question`, `the_approved_run_is_sandboxed` (macOS). //! - Timeout + capture: `a_command_past_its_timeout_is_killed_and_reported`, @@ -207,7 +210,7 @@ fn a_call_waits_for_the_member_and_shows_exactly_what_will_run() { let s = session(&fx, &cfg_open()); let approvals = s.approvals().clone(); let h = spawn( - s.host(TaintState::default(), StopFlag::default()), + s.host(TaintState::default(), StopFlag::default(), false), call("c1", &["echo", "hello $(id)"], &fx.proj().join("src")), ); let p = wait_pending(&approvals); @@ -239,7 +242,7 @@ fn an_approved_command_runs_and_reports() { let approvals = s.approvals().clone(); let marker = fx.proj().join("built.txt"); let h = spawn( - s.host(TaintState::default(), StopFlag::default()), + s.host(TaintState::default(), StopFlag::default(), false), call( "c1", &[ @@ -275,7 +278,7 @@ fn a_declined_command_never_runs() { let approvals = s.approvals().clone(); let marker = fx.proj().join("never.txt"); let h = spawn( - s.host(TaintState::default(), StopFlag::default()), + s.host(TaintState::default(), StopFlag::default(), false), call( "c1", &["sh", "-c", &format!("echo x > '{}'", marker.display())], @@ -297,7 +300,7 @@ fn a_decision_must_carry_the_argv_and_cwd_that_were_shown() { let s = session(&fx, &cfg_open()); let approvals = s.approvals().clone(); let h = spawn( - s.host(TaintState::default(), StopFlag::default()), + s.host(TaintState::default(), StopFlag::default(), false), call("c1", &["echo", "a"], &fx.proj()), ); let p = wait_pending(&approvals); @@ -323,7 +326,7 @@ fn an_unanswered_approval_expires_as_declined() { cfg.approval_timeout = Duration::from_millis(200); let s = session(&fx, &cfg); let out = s - .host(TaintState::default(), StopFlag::default()) + .host(TaintState::default(), StopFlag::default(), false) .execute(&call("c1", &["echo", "a"], &fx.proj())); match out { ToolOutcome::Denied(why) => assert!(why.contains("no decision"), "{why}"), @@ -339,7 +342,7 @@ fn a_session_stop_declines_a_waiting_command() { let approvals = s.approvals().clone(); let stop = StopFlag::default(); let h = spawn( - s.host(TaintState::default(), stop.clone()), + s.host(TaintState::default(), stop.clone(), false), call("c1", &["echo", "a"], &fx.proj()), ); wait_pending(&approvals); @@ -366,7 +369,7 @@ fn refused(out: ToolOutcome, needle: &str) { fn bad_arguments_are_refused_before_asking() { let fx = Fx::new(); let s = session(&fx, &cfg_open()); - let host = s.host(TaintState::default(), StopFlag::default()); + let host = s.host(TaintState::default(), StopFlag::default(), false); let raw = |args: serde_json::Value| { host.execute(&ToolCall { id: "c".into(), @@ -416,7 +419,7 @@ fn bad_arguments_are_refused_before_asking() { fn a_cwd_outside_the_grants_is_refused_before_asking() { let fx = Fx::new(); let s = session(&fx, &cfg_open()); - let host = s.host(TaintState::default(), StopFlag::default()); + let host = s.host(TaintState::default(), StopFlag::default(), false); refused(host.execute(&call("c", &["echo"], &fx.other())), "refused"); refused(host.execute(&call("c", &["echo"], &fx.home())), "refused"); assert!(s.approvals().pending().is_empty()); @@ -433,7 +436,7 @@ fn a_read_only_or_shallow_grant_is_not_enough() { )]); let s = ShellRunSession::new(&cfg_open(), fx.grants(&read_only)).unwrap(); refused( - s.host(TaintState::default(), StopFlag::default()) + s.host(TaintState::default(), StopFlag::default(), false) .execute(&call("c", &["echo"], &fx.proj())), "refused", ); @@ -443,7 +446,7 @@ fn a_read_only_or_shallow_grant_is_not_enough() { ]); let s = ShellRunSession::new(&cfg_open(), fx.grants(&shallow)).unwrap(); refused( - s.host(TaintState::default(), StopFlag::default()) + s.host(TaintState::default(), StopFlag::default(), false) .execute(&call("c", &["echo"], &fx.proj())), "everything below", ); @@ -457,7 +460,7 @@ fn a_revoked_grant_stops_an_approved_run() { let approvals = s.approvals().clone(); let marker = fx.proj().join("late.txt"); let h = spawn( - s.host(TaintState::default(), StopFlag::default()), + s.host(TaintState::default(), StopFlag::default(), false), call( "c1", &["sh", "-c", &format!("echo x > '{}'", marker.display())], @@ -474,21 +477,103 @@ fn a_revoked_grant_stops_an_approved_run() { } #[test] -fn a_tainted_session_is_refused_and_nothing_is_asked() { +fn a_tainted_session_without_an_hic_aware_app_is_refused_and_nothing_is_asked() { let fx = Fx::new(); let s = session(&fx, &cfg_open()); let taint = TaintState::default(); taint.taint("read_url", "a web page"); - let out = s - .host(taint, StopFlag::default()) - .execute(&call("c1", &["echo", "a"], &fx.proj())); + let host = s.host(taint, StopFlag::default(), false); + assert!(!host.honors_explicit_approval()); + let out = host.execute(&call("c1", &["echo", "a"], &fx.proj())); match out { - ToolOutcome::Denied(why) => assert!(why.contains("untrusted"), "{why}"), + ToolOutcome::Denied(why) => { + assert!(why.contains("untrusted"), "{why}"); + assert!(why.contains("from read_url"), "{why}"); + assert!(why.contains("no member can be asked"), "{why}"); + } other => panic!("{other:?}"), } assert!(s.approvals().pending().is_empty()); } +/// HGW-1: after taint, an HIC-aware session still asks; the card says why the command is gated +/// and names every taint source, and only an explicit allow runs it. Taint is not cleared. +#[test] +fn a_tainted_hic_aware_session_asks_with_the_reason_and_runs_only_on_allow() { + let fx = Fx::new(); + let s = session(&fx, &cfg_open()); + let approvals = s.approvals().clone(); + let taint = TaintState::default(); + taint.taint("read_url", "a web page"); + taint.taint("shell_run", "output"); + let marker = fx.proj().join("tainted-run.txt"); + let host = s.host(taint.clone(), StopFlag::default(), true); + assert!(host.honors_explicit_approval()); + let c = call( + "c1", + &["sh", "-c", &format!("echo x > '{}'", marker.display())], + &fx.proj(), + ); + // Through the loop's explicit-approval path and through a plain execute (taint re-checked). + for plain in [false, true] { + let h = s.host(taint.clone(), StopFlag::default(), true); + let c2 = c.clone(); + let t = std::thread::spawn(move || { + if plain { + h.execute(&c2) + } else { + h.execute_with_explicit_approval(&c2, "gated: read_url") + } + }); + let p = wait_pending(&approvals); + let reason = p + .hic_reason + .clone() + .expect("a tainted card carries the reason"); + assert!(reason.contains("read_url"), "{reason}"); + assert_eq!(p.taint_sources, vec!["read_url", "shell_run"]); + let v = serde_json::to_value(&p).unwrap(); + assert!( + v.get("hicReason").is_some() && v.get("taintSources").is_some(), + "{v}" + ); + assert!(!marker.exists(), "nothing runs before the member decides"); + let allow = plain; + approvals.decide(&p.id, allow, &p.argv, &p.cwd).unwrap(); + let out = t.join().unwrap(); + if allow { + assert!(matches!(out, ToolOutcome::Ok(_)), "{out:?}"); + assert!(marker.exists()); + } else { + assert!(matches!(out, ToolOutcome::Denied(_)), "{out:?}"); + assert!(!marker.exists()); + } + } + assert!(taint.is_tainted(), "an approval never clears the taint"); + drop(host); +} + +#[test] +fn an_untainted_card_has_no_taint_reason() { + let fx = Fx::new(); + let s = session(&fx, &cfg_open()); + let approvals = s.approvals().clone(); + let h = spawn( + s.host(TaintState::default(), StopFlag::default(), true), + call("c1", &["echo", "a"], &fx.proj()), + ); + let p = wait_pending(&approvals); + assert!(p.hic_reason.is_none()); + assert!(p.taint_sources.is_empty()); + let v = serde_json::to_value(&p).unwrap(); + assert!( + v.get("hicReason").is_none() && v.get("taintSources").is_none(), + "{v}" + ); + approvals.decide(&p.id, false, &p.argv, &p.cwd).unwrap(); + assert!(matches!(h.join().unwrap(), ToolOutcome::Denied(_))); +} + #[test] fn no_sandbox_means_no_run_and_no_question() { let fx = Fx::new(); @@ -498,7 +583,7 @@ fn no_sandbox_means_no_run_and_no_question() { ); let s = session(&fx, &cfg); let out = s - .host(TaintState::default(), StopFlag::default()) + .host(TaintState::default(), StopFlag::default(), false) .execute(&call("c1", &["echo", "a"], &fx.proj())); refused(out, "sandbox"); assert!(s.approvals().pending().is_empty()); @@ -529,7 +614,7 @@ fn a_command_past_its_timeout_is_killed_and_reported() { }; let started = std::time::Instant::now(); let out = s - .host(TaintState::default(), StopFlag::default()) + .host(TaintState::default(), StopFlag::default(), false) .execute(&c); a.join().unwrap(); assert!(started.elapsed() < Duration::from_secs(10)); @@ -545,7 +630,7 @@ fn output_is_capped() { let s = session(&fx, &cfg_open()); let a = approve_next(s.approvals().clone()); let out = s - .host(TaintState::default(), StopFlag::default()) + .host(TaintState::default(), StopFlag::default(), false) .execute(&call( "c1", &["sh", "-c", "yes x | head -c 200000"], @@ -571,7 +656,7 @@ fn the_approved_run_is_sandboxed() { let inside = fx.proj().join("src/ok.txt"); let outside = fx.other().join("escape.txt"); let h = spawn( - s.host(TaintState::default(), StopFlag::default()), + s.host(TaintState::default(), StopFlag::default(), false), call( "c1", &[ @@ -621,7 +706,7 @@ fn backend_override_reaches_the_card() { let approvals = s.approvals().clone(); let stop = StopFlag::default(); let h = spawn( - s.host(TaintState::default(), stop.clone()), + s.host(TaintState::default(), stop.clone(), false), call("c1", &["echo", "a"], &fx.proj()), ); let p = wait_pending(&approvals); @@ -900,7 +985,7 @@ async fn an_unattended_session_never_offers_a_run() { /// HUP-S2.6: every member decision on a `shell_run` command lands in the decision log the nightly /// anchor batches: a refused (mismatched) allow closes as failed, the real allow as completed, and -/// a deny is recorded as denied. (One command per session: after a run the session is tainted.) +/// a deny is recorded as denied. (One session per command keeps each log entry easy to find.) #[tokio::test(flavor = "multi_thread")] async fn shell_run_decisions_are_recorded_for_the_anchor() { use citrate_agent_records::{Decision, DecisionLog, Entry, LogConfig, Outcome}; From 9d8f71722b9d3efa6f7b511e2104e831fb4aca14 Mon Sep 17 00:00:00 2001 From: Larry Klosowski Date: Thu, 15 Oct 2026 12:24:44 -0700 Subject: [PATCH 02/12] feat(sidecar): approvals routes per #74, explicit taint clear, ask-not-retry (HGW-1) Aligns the HGW-1 interface with the HGW-W1 issue (#74): - routes are GET /sessions/:id/approvals/pending and POST /sessions/:id/approvals/:aid/decide {allow, subject}; - POST /sessions/:id/taint/clear {note}: the member clears the taint from an explicit card (TaintState::clear_by_member). Recorded write-ahead (kind taint.clear); with no records log, or when the record cannot be written, nothing is cleared (503). Scheduled (unattended) runs are never cleared (409); empty note 400; not tainted 409. Approving one action still never clears the taint; - a declined card tells the model to ask the member instead of retrying (shell_run and approval cards). Test support opens one undo store per manager (a store is locked while open). Tests: action_approvals_tests (+2). Workspace: 2253 -> 2255 passed. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_018wWZZ8GRVU9USh3kKQHsFe --- agent-sidecar/src/action_approvals.rs | 8 +- agent-sidecar/src/action_approvals_tests.rs | 145 ++++++++++++++++++-- agent-sidecar/src/hgw_support.rs | 46 ++++--- agent-sidecar/src/lib.rs | 107 +++++++++++++-- agent-sidecar/src/sessions.rs | 12 +- agent-sidecar/src/shell_run.rs | 2 +- 6 files changed, 272 insertions(+), 48 deletions(-) diff --git a/agent-sidecar/src/action_approvals.rs b/agent-sidecar/src/action_approvals.rs index 82b20c8..e2665fc 100644 --- a/agent-sidecar/src/action_approvals.rs +++ b/agent-sidecar/src/action_approvals.rs @@ -4,7 +4,8 @@ //! output, a web page, an MCP result), an effectful sidecar-hosted call (`fs_*`, `file_write`, //! `sheet_write`, the toolchain tools, `project_create`, capsules) is not refused for the rest of //! the conversation. It waits here as an approval card that citrate-core shows to the member -//! (`GET /sessions/:id/actions/pending`, decided with `POST /sessions/:id/actions/decide`): +//! (`GET /sessions/:id/approvals/pending`, decided with +//! `POST /sessions/:id/approvals/:aid/decide {allow, subject}`): //! //! - the card shows the tool, the exact arguments that will be used (`subject`, canonical JSON), //! a one-line summary, why it is gated (`hicReason`) and every source that tainted the session @@ -12,7 +13,8 @@ //! - a decision must carry the `subject` that was shown; anything else is refused and changes //! nothing; //! - an allow releases **that one call**, with those arguments. The session stays tainted, so -//! the next effectful call asks again (no approval is ever global); +//! the next effectful call asks again (no approval is ever global). Clearing the taint is a +//! separate, explicit, recorded member action (`POST /sessions/:id/taint/clear`); //! - a decline, a session stop, or no decision within [`ACTION_APPROVAL_TIMEOUT`] declines it. //! //! The queue exists only for a session opened with `hicAware: true` (core's promise that a @@ -235,7 +237,7 @@ impl ActionApprovals { break if a { Ok(()) } else { - Err("the member declined this action".to_string()) + Err("the member declined this action; ask the member what to do instead of retrying it".to_string()) }; } if stop.is_stopped() { diff --git a/agent-sidecar/src/action_approvals_tests.rs b/agent-sidecar/src/action_approvals_tests.rs index f1cb77a..fde4c8b 100644 --- a/agent-sidecar/src/action_approvals_tests.rs +++ b/agent-sidecar/src/action_approvals_tests.rs @@ -15,6 +15,8 @@ //! - Headless: `without_an_hic_aware_app_tainted_actions_are_declined_and_say_why`. //! - Routes: `the_action_routes_need_the_bearer_and_a_known_session`. //! - Records: `action_decisions_are_recorded_for_the_anchor`. +//! - Explicit clear (HGW-W1, #74): `taint_clear_is_an_explicit_recorded_member_action`. +//! - Ask, don't retry: `a_declined_action_tells_the_model_to_ask_not_retry`. #![cfg(unix)] @@ -181,7 +183,7 @@ async fn a_decision_must_carry_the_arguments_that_were_shown() { let (_, ac) = send( &app, "GET", - &format!("/sessions/{id}/actions/pending"), + &format!("/sessions/{id}/approvals/pending"), None, ) .await; @@ -192,12 +194,13 @@ async fn a_decision_must_carry_the_arguments_that_were_shown() { tokio::time::sleep(std::time::Duration::from_millis(10)).await; } assert!(card.is_object(), "no action card appeared"); - let url = format!("/sessions/{id}/actions/decide"); + let aid = card["id"].as_str().unwrap_or_default().to_string(); + let url = format!("/sessions/{id}/approvals/{aid}/decide"); let (st, _) = send( &app, "POST", &url, - Some(serde_json::json!({"id": card["id"], "allow": true, "subject": "{\"path\":\"/etc/x\"}"})), + Some(serde_json::json!({"allow": true, "subject": "{\"path\":\"/etc/x\"}"})), ) .await; assert_eq!(st, StatusCode::CONFLICT); @@ -205,18 +208,24 @@ async fn a_decision_must_carry_the_arguments_that_were_shown() { let (st, _) = send( &app, "POST", - &url, - Some(serde_json::json!({"id": "act-999", "allow": true, "subject": card["subject"]})), + &format!("/sessions/{id}/approvals/act-999/decide"), + Some(serde_json::json!({"allow": true, "subject": card["subject"]})), ) .await; assert_eq!(st, StatusCode::CONFLICT); - let (st, _) = send(&app, "POST", &url, Some(serde_json::json!({"id": 1}))).await; + let (st, _) = send( + &app, + "POST", + &url, + Some(serde_json::json!({"allow": "yes"})), + ) + .await; assert_eq!(st, StatusCode::BAD_REQUEST); let (st, _) = send( &app, "POST", &url, - Some(serde_json::json!({"id": card["id"], "allow": true, "subject": card["subject"]})), + Some(serde_json::json!({"allow": true, "subject": card["subject"]})), ) .await; assert_eq!(st, StatusCode::OK); @@ -259,7 +268,7 @@ async fn without_an_hic_aware_app_tainted_actions_are_declined_and_say_why() { let (st, body) = send( &app, "GET", - &format!("/sessions/{id}/actions/pending"), + &format!("/sessions/{id}/approvals/pending"), None, ) .await; @@ -280,20 +289,20 @@ async fn the_action_routes_need_the_bearer_and_a_known_session() { .clone() .oneshot( Request::builder() - .uri(format!("/sessions/{id}/actions/pending")) + .uri(format!("/sessions/{id}/approvals/pending")) .body(Body::empty()) .expect("req"), ) .await .expect("resp"); assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); - let (st, _) = send(&app, "GET", "/sessions/nope/actions/pending", None).await; + let (st, _) = send(&app, "GET", "/sessions/nope/approvals/pending", None).await; assert_eq!(st, StatusCode::NOT_FOUND); let (st, _) = send( &app, "POST", - "/sessions/nope/actions/decide", - Some(serde_json::json!({"id": "act-1", "allow": true, "subject": "{}"})), + "/sessions/nope/approvals/act-1/decide", + Some(serde_json::json!({"allow": true, "subject": "{}"})), ) .await; assert_eq!(st, StatusCode::NOT_FOUND); @@ -348,3 +357,115 @@ async fn action_decisions_are_recorded_for_the_anchor() { assert_eq!(outcome, Some(Outcome::Completed)); assert_eq!(decisions[1].1, Decision::Denied); } + +/// HGW-W1 (#74): a member can clear a session's taint, but only as an explicit, recorded action: +/// not for a scheduled run, not with an empty note, and not when the decision cannot be recorded +/// (write-ahead, fail closed). After a clear, effectful calls run without a card again. +#[tokio::test(flavor = "multi_thread")] +async fn taint_clear_is_an_explicit_recorded_member_action() { + use citrate_agent_records::{Decision, DecisionLog, Entry, LogConfig}; + let fx = Fx::new("taint-clear"); + let f = fx.proj().join("after-clear.txt"); + // Without a records log, nothing is cleared. + let mgr = Arc::new(manager_with(&fx, vec![], Some(shell_open()))); + let id = mgr.create(create_req(fx.rw_doc(), true)).expect("session"); + let s = mgr.get(&id).expect("session"); + s.taint().taint("shell_run", "output"); + let app = app_for(mgr.clone()); + let url = format!("/sessions/{id}/taint/clear"); + let (st, _) = send( + &app, + "POST", + &url, + Some(serde_json::json!({"note": "I read that output"})), + ) + .await; + assert_eq!(st, StatusCode::SERVICE_UNAVAILABLE); + assert!(s.taint().is_tainted()); + // With records: an empty note is refused, a real one clears and is recorded. + let dir = fx.base.join("records"); + let (log, _) = DecisionLog::open(&dir, LogConfig::default()).expect("log"); + let turns = vec![ + AssistantTurn::tools(vec![fs_write("c1", &f, "F")]), + AssistantTurn::text("ok"), + ]; + let mgr = Arc::new(manager_with(&fx, turns, Some(shell_open())).with_records(Arc::new(log))); + let id = mgr.create(create_req(fx.rw_doc(), true)).expect("session"); + let s = mgr.get(&id).expect("session"); + s.taint().taint("shell_run", "output"); + let app = app_for(mgr.clone()); + let url = format!("/sessions/{id}/taint/clear"); + let (st, _) = send(&app, "POST", &url, Some(serde_json::json!({"note": " "}))).await; + assert_eq!(st, StatusCode::BAD_REQUEST); + let (st, body) = send( + &app, + "POST", + &url, + Some(serde_json::json!({"note": "I read that output"})), + ) + .await; + assert_eq!(st, StatusCode::OK, "{body}"); + assert_eq!(body["cleared"]["source"], "shell_run"); + assert!(!s.taint().is_tainted()); + let (st, _) = send( + &app, + "POST", + &url, + Some(serde_json::json!({"note": "again"})), + ) + .await; + assert_eq!(st, StatusCode::CONFLICT, "nothing to clear"); + // The next write runs without a card. + mgr.send(&id, "go".into(), None).expect("send"); + wait_done(&s).await; + assert_eq!(result_of(&s, "c1").await["status"], "ok"); + assert!(f.exists()); + let recs = citrate_agent_records::read::page(&dir, None, 10_000).expect("records"); + assert!(recs.iter().any(|r| matches!(&r.record.entry, + Entry::Decision(d) if d.kind == "taint.clear" && d.decision == Decision::Approved && d.subject.contains("shell_run")))); + // A scheduled run's taint is never cleared. + let mut req = create_req(fx.rw_doc(), true); + req.unattended = true; + let id = mgr.create(req).expect("session"); + let (st, _) = send( + &app, + "POST", + &format!("/sessions/{id}/taint/clear"), + Some(serde_json::json!({"note": "please"})), + ) + .await; + assert_eq!(st, StatusCode::CONFLICT); + assert!(mgr.get(&id).expect("session").taint().is_tainted()); +} + +/// HGW-W1 (#74 AC3): a declined action tells the model to ask the member, not to retry. +#[tokio::test(flavor = "multi_thread")] +async fn a_declined_action_tells_the_model_to_ask_not_retry() { + let fx = Fx::new("taint-decline-hint"); + let proj = fx.proj(); + let turns = vec![ + AssistantTurn::tools(vec![shell("c1", &["echo", "x"], &proj)]), + AssistantTurn::tools(vec![fs_write("c2", &proj.join("a.txt"), "A")]), + AssistantTurn::tools(vec![shell("c3", &["echo", "y"], &proj)]), + AssistantTurn::text("ok"), + ]; + let mgr = Arc::new(manager_with(&fx, turns, Some(shell_open()))); + let id = mgr.create(create_req(fx.rw_doc(), true)).expect("session"); + let s = mgr.get(&id).expect("session"); + let app = app_for(mgr.clone()); + mgr.send(&id, "go".into(), None).expect("send"); + // Allow the first command; decline the write and the second command. + let mut n = 0; + member_answers(&app, &s, &id, move |_, _| { + n += 1; + n == 1 + }) + .await; + for c in ["c2", "c3"] { + let r = result_of(&s, c).await; + assert_eq!(r["status"], "denied", "{r}"); + let msg = r["content"].as_str().unwrap_or_default(); + assert!(msg.contains("ask the member"), "{msg}"); + assert!(msg.contains("instead of retrying"), "{msg}"); + } +} diff --git a/agent-sidecar/src/hgw_support.rs b/agent-sidecar/src/hgw_support.rs index cf02da6..ad6ec9f 100644 --- a/agent-sidecar/src/hgw_support.rs +++ b/agent-sidecar/src/hgw_support.rs @@ -30,8 +30,8 @@ pub(crate) fn now() -> u64 { .unwrap_or(0) } -/// `base/home/proj` (granted read + write), `base/home/other` (not granted) and the undo store -/// at `base/ckpt` (outside the home). +/// `base/home/proj` (granted read + write), `base/home/other` (not granted); undo stores go in +/// `base/ckpt-` (outside the home). pub(crate) struct Fx { pub base: PathBuf, } @@ -44,7 +44,7 @@ impl Fx { N.fetch_add(1, Ordering::SeqCst) )); let _ = std::fs::remove_dir_all(&base); - for d in ["home/proj/src", "home/other", "ckpt"] { + for d in ["home/proj/src", "home/other"] { std::fs::create_dir_all(base.join(d)).expect("fixture dirs"); } Fx { @@ -112,8 +112,11 @@ pub(crate) fn manager_with( shell: Option, ) -> sessions::SessionManager { let script = Arc::new(Script(Mutex::new(turns))); - let store = - Arc::new(CheckpointStore::open(&fx.base.join("ckpt"), Config::default()).expect("store")); + // One store per manager: a store is locked while it is open. + let dir = fx + .base + .join(format!("ckpt-{}", N.fetch_add(1, Ordering::SeqCst))); + let store = Arc::new(CheckpointStore::open(&dir, Config::default()).expect("store")); let mut mgr = sessions::SessionManager::new( Arc::new(move |_ep: &sessions::LlmEndpoint| script.clone() as Arc), Duration::from_secs(5), @@ -251,28 +254,37 @@ pub(crate) async fn member_answers( app: &axum::Router, s: &sessions::Session, id: &str, - decide: impl Fn(&'static str, &serde_json::Value) -> bool, + mut decide: impl FnMut(&'static str, &serde_json::Value) -> bool, ) -> Vec { let mut out = Vec::new(); for _ in 0..3000 { for route in ["shell", "actions"] { + // Labels: `shell` = the shell_run cards, `actions` = the session's approval cards. + let base = if route == "shell" { + "shell" + } else { + "approvals" + }; let (st, body) = - send(app, "GET", &format!("/sessions/{id}/{route}/pending"), None).await; + send(app, "GET", &format!("/sessions/{id}/{base}/pending"), None).await; assert_eq!(st, StatusCode::OK, "{route}: {body}"); for card in body["pending"].as_array().cloned().unwrap_or_default() { let allow = decide(route, &card); - let decision = if route == "shell" { - serde_json::json!({"id": card["id"], "allow": allow, "argv": card["argv"], "cwd": card["cwd"]}) + let (url, decision) = if route == "shell" { + ( + format!("/sessions/{id}/shell/decide"), + serde_json::json!({"id": card["id"], "allow": allow, "argv": card["argv"], "cwd": card["cwd"]}), + ) } else { - serde_json::json!({"id": card["id"], "allow": allow, "subject": card["subject"]}) + ( + format!( + "/sessions/{id}/approvals/{}/decide", + card["id"].as_str().unwrap_or_default() + ), + serde_json::json!({"allow": allow, "subject": card["subject"]}), + ) }; - let (st, b) = send( - app, - "POST", - &format!("/sessions/{id}/{route}/decide"), - Some(decision), - ) - .await; + let (st, b) = send(app, "POST", &url, Some(decision)).await; assert_eq!(st, StatusCode::OK, "{route} decide: {b}"); out.push(Answered { route, card, allow }); } diff --git a/agent-sidecar/src/lib.rs b/agent-sidecar/src/lib.rs index 6466106..6251a21 100644 --- a/agent-sidecar/src/lib.rs +++ b/agent-sidecar/src/lib.rs @@ -300,8 +300,9 @@ pub fn app(state: Arc) -> Router { .route("/sessions/:id/shell/decide", post(shell_decide)) .route("/sessions/:id/mcp/pending", get(mcp_pending)) .route("/sessions/:id/mcp/decide", post(mcp_decide)) - .route("/sessions/:id/actions/pending", get(action_pending)) - .route("/sessions/:id/actions/decide", post(action_decide)) + .route("/sessions/:id/approvals/pending", get(action_pending)) + .route("/sessions/:id/approvals/:aid/decide", post(action_decide)) + .route("/sessions/:id/taint/clear", post(taint_clear)) // HUP-S1.4 — tracks + briefs (the interview every client shares). .route("/tracks", get(tracks)) .route("/briefs", post(create_brief)) @@ -1050,8 +1051,9 @@ async fn mcp_pending( Ok(Json(serde_json::json!({ "pending": pending }))) } -/// HGW-1: `GET /sessions/:id/actions/pending`: the action cards waiting for the member (tainted -/// effectful sidecar calls in an HIC-aware session). A session without them has none. +/// HGW-1: `GET /sessions/:id/approvals/pending`: the approval cards waiting for the member +/// (tainted effectful sidecar calls in an HIC-aware session). A session without them has none. +/// `shell_run` keeps its own cards on `/sessions/:id/shell/pending`. async fn action_pending( headers: HeaderMap, State(st): State>, @@ -1070,18 +1072,17 @@ async fn action_pending( #[derive(Deserialize)] struct ActionDecideReq { - id: String, allow: bool, subject: String, } -/// HGW-1: `POST /sessions/:id/actions/decide {id, allow, subject}`: the member's decision on one -/// action card. `subject` must be the arguments that were shown; otherwise 409 and nothing is -/// decided. Recorded like the other HIC decisions (write-ahead for an allow). +/// HGW-1: `POST /sessions/:id/approvals/:aid/decide {allow, subject}`: the member's decision on +/// one approval card. `subject` must be the arguments that were shown; otherwise 409 and nothing +/// is decided. Recorded like the other HIC decisions (write-ahead for an allow). async fn action_decide( headers: HeaderMap, State(st): State>, - Path(id): Path, + Path((id, aid)): Path<(String, String)>, body: axum::body::Bytes, ) -> Result, JsonErr> { if !authorized(&headers, &st.bearer) { @@ -1092,9 +1093,9 @@ async fn action_decide( .get(&id) .ok_or_else(|| json_err(StatusCode::NOT_FOUND, "no such session"))?; let req: ActionDecideReq = serde_json::from_slice(&body) - .map_err(|_| json_err(StatusCode::BAD_REQUEST, "expected {id, allow, subject}"))?; + .map_err(|_| json_err(StatusCode::BAD_REQUEST, "expected {allow, subject}"))?; let card = session - .action_waiting(&req.id) + .action_waiting(&aid) .filter(|c| c.subject == req.subject) .ok_or_else(|| { json_err( @@ -1113,7 +1114,7 @@ async fn action_decide( "the member decided on the exact action shown, in a session that read untrusted content", vec![citrate_agent_records::EvidenceRef { kind: "sidecar_tool_call".to_string(), - uri: format!("sidecar:sessions/{id}/actions/{}", req.id), + uri: format!("sidecar:sessions/{id}/approvals/{aid}"), digest: None, }], ) @@ -1125,7 +1126,7 @@ async fn action_decide( }, _ => None, }; - match session.action_decide(&req.id, req.allow, &req.subject) { + match session.action_decide(&aid, req.allow, &req.subject) { Ok(()) => { if let Some(log) = &log { match allow_seq { @@ -1162,6 +1163,86 @@ async fn action_decide( } } +#[derive(Deserialize)] +struct TaintClearReq { + note: String, +} + +/// HGW-W1 (#74): `POST /sessions/:id/taint/clear {note}`: the member clears the session's taint +/// (`TaintState::clear_by_member`), from an explicit card in the app. Refused (409) for a +/// scheduled run, whose taint is never cleared, and when nothing is tainted; 400 for an empty +/// note. The decision is recorded write-ahead, and without a records log (or when the record +/// cannot be written) nothing is cleared (503). Approving one action never clears the taint. +async fn taint_clear( + headers: HeaderMap, + State(st): State>, + Path(id): Path, + body: axum::body::Bytes, +) -> Result, JsonErr> { + if !authorized(&headers, &st.bearer) { + return Err(json_err(StatusCode::UNAUTHORIZED, "unauthorized")); + } + let session = st + .sessions + .get(&id) + .ok_or_else(|| json_err(StatusCode::NOT_FOUND, "no such session"))?; + let req: TaintClearReq = serde_json::from_slice(&body) + .map_err(|_| json_err(StatusCode::BAD_REQUEST, "expected {note}"))?; + let ack = citrate_agent_loop::MemberClear::new(req.note.clone()).ok_or_else(|| { + json_err( + StatusCode::BAD_REQUEST, + "the note is empty; nothing was cleared", + ) + })?; + if session.is_unattended() { + return Err(json_err( + StatusCode::CONFLICT, + "a scheduled run's taint is never cleared", + )); + } + let sources = session.taint().sources().unwrap_or_default(); + if !session.taint().is_tainted() { + return Err(json_err( + StatusCode::CONFLICT, + "this session is not tainted", + )); + } + let log = st.sessions.records().ok_or_else(|| { + json_err( + StatusCode::SERVICE_UNAVAILABLE, + "clearing a taint must be recorded, and no decision records are configured; nothing was cleared", + ) + })?; + let seq = hic_records::record_member_decision( + &log, + "taint.clear", + &format!( + "clear the untrusted-content taint of session {id} (sources: {})", + sources.join(", ") + ), + true, + &req.note, + vec![citrate_agent_records::EvidenceRef { + kind: "taint_clear".to_string(), + uri: format!("sidecar:sessions/{id}/taint"), + digest: None, + }], + ) + .map_err(|e| json_err(StatusCode::SERVICE_UNAVAILABLE, &e))?; + let cleared = session.taint().clear_by_member(ack); + hic_records::record_outcome( + &log, + seq, + citrate_agent_records::Outcome::Completed, + "the session's taint was cleared", + ); + Ok(Json(serde_json::json!({ + "ok": true, + "cleared": cleared.map(|r| serde_json::json!({"source": r.source, "reason": r.reason})), + "sources": sources, + }))) +} + #[derive(Deserialize)] struct McpDecideReq { id: String, diff --git a/agent-sidecar/src/sessions.rs b/agent-sidecar/src/sessions.rs index d83ff33..37cc9f9 100644 --- a/agent-sidecar/src/sessions.rs +++ b/agent-sidecar/src/sessions.rs @@ -67,8 +67,8 @@ //! `/shell` routes and runs only in the OS sandbox. The name is reserved while it is on. //! - HGW-1 (owner decision 2026-10-15): taint escalates to HIC approval instead of ending the //! conversation. In a session opened with `hicAware: true`, a tainted effectful sidecar-hosted -//! call waits for the member on the session's action cards (`/actions` routes, -//! [`crate::action_approvals`]); `shell_run`, MCP and browser calls use their own cards, which +//! call waits for the member on the session's approval cards (`/sessions/:id/approvals` +//! routes, [`crate::action_approvals`]); `shell_run`, MCP and browser calls use their own cards, which //! carry the same reason. Each approval releases one call and the session stays tainted. //! Without `hicAware` nobody can be asked, so such calls are declined with //! [`citrate_agent_loop::cannot_ask_reason`]. @@ -555,6 +555,8 @@ pub struct Session { mcp_approvals: Option>, /// HGW-1: the cards for tainted effectful sidecar calls (present only for an HIC-aware app). actions: Option>, + /// HUP-S10.3: a scheduled run nobody watches; its taint is never cleared. + unattended: bool, /// Test seam: runs on the emitting thread right after an event becomes visible to readers, so /// a test can act as a core client at the exact moment the event appears. #[cfg(test)] @@ -603,6 +605,11 @@ impl Session { a.decide(id, allow, subject).map_err(SessionError::Invalid) } + /// HUP-S10.3: whether this is a scheduled run nobody watches (its taint is never cleared). + pub fn is_unattended(&self) -> bool { + self.unattended + } + /// HGW-1: the action cards waiting for the member (`None` when the session has none, i.e. it /// was not opened by an HIC-aware app). pub fn action_pending(&self) -> Option> { @@ -1836,6 +1843,7 @@ impl SessionManager { mcp_offered, mcp_approvals, actions, + unattended: req.unattended, #[cfg(test)] event_hook: Mutex::new(None), }); diff --git a/agent-sidecar/src/shell_run.rs b/agent-sidecar/src/shell_run.rs index 2fbca56..ebccc65 100644 --- a/agent-sidecar/src/shell_run.rs +++ b/agent-sidecar/src/shell_run.rs @@ -270,7 +270,7 @@ impl ShellApprovals { break if a { Ok(()) } else { - Err("the member declined this command".to_string()) + Err("the member declined this command; ask the member what to do instead of retrying it".to_string()) }; } if stopped() { From bb194f0ce9158189b43dc7be0aafaa528f7dea73 Mon Sep 17 00:00:00 2001 From: Larry Klosowski Date: Thu, 15 Oct 2026 11:45:42 -0700 Subject: [PATCH 03/12] feat(sidecar,shell): project write policy and git inside the sandbox (HGW-2) Owner decision 2026-10-15 (HGW). Hermes could not set up a project: the file tools refused package.json, foundry.toml, remappings.txt and the rest of the build configuration everywhere, and the macOS sandbox denied every write to .git/config and .git/hooks, so `git init` failed even offline. Project roots (agent-sidecar): - POST /sessions takes `projectRoots` (absolute, existing folders under live read+write subtree folder grants; needs `grants`). SessionGrants keeps them; project_create (next PR) adds the projects it makes. - Inside a project root the file tools (fs_*, file_write, sheet_write) may create and change build configuration. Outside, refused as before (the message now points at projects). A marker file alone never makes a folder a project. - Env files stay the member's everywhere. The file tools never write inside a .git folder (git does, through the sandboxed shell_run). - The toolchain's own config check (ffi, fs_permissions, ...) is unchanged: forge still refuses risky config whoever wrote it. Sandbox (agent-shell): - Seatbelt: .git/ is writable inside write roots (git init/config/commit work); a hook file other than git's *.sample files is still denied, as are removing or moving .git/hooks and symlinking a new one in. - bwrap: .git/config is no longer masked (parity); existing .git/hooks stays read-only. - RunReport.git_warnings: risky git state a run leaves behind that would run a program outside the sandbox later (hooks, core.hooksPath, core.fsmonitor, `!` aliases, filters, includes, ...), new since before the run. shell_run puts it in the run facts and the summary. - Linux bwrap run tests (skip unless bwrap works; CITRATE_REQUIRE_BWRAP=1 makes them fail instead, for the Linux CI lane). Tests: project_policy_tests (+6), agent-shell sandbox (+3 macOS real Seatbelt incl. git init/commit and hooks closed; +3 Linux), git_state unit (+2), shell_run git warning (+1). Workspace: 2253 -> 2265 passed. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_018wWZZ8GRVU9USh3kKQHsFe --- agent-shell/src/git_state.rs | 209 ++++++++++++++++++++ agent-shell/src/lib.rs | 16 ++ agent-shell/src/sandbox.rs | 32 +-- agent-shell/tests/sandbox.rs | 173 ++++++++++++++++ agent-shell/tests/windows.rs | 1 + agent-sidecar/src/files.rs | 71 +++++-- agent-sidecar/src/grants.rs | 54 +++++ agent-sidecar/src/hgw_support.rs | 23 +++ agent-sidecar/src/lib.rs | 2 + agent-sidecar/src/project_policy_tests.rs | 230 ++++++++++++++++++++++ agent-sidecar/src/sessions.rs | 19 ++ agent-sidecar/src/shell_run.rs | 13 +- agent-sidecar/src/shell_run_tests.rs | 36 ++++ agent-sidecar/src/toolchain.rs | 1 + agent-sidecar/src/toolchain_config.rs | 47 ++++- 15 files changed, 893 insertions(+), 34 deletions(-) create mode 100644 agent-shell/src/git_state.rs create mode 100644 agent-sidecar/src/project_policy_tests.rs diff --git a/agent-shell/src/git_state.rs b/agent-shell/src/git_state.rs new file mode 100644 index 0000000..815de35 --- /dev/null +++ b/agent-shell/src/git_state.rs @@ -0,0 +1,209 @@ +//! HGW-2: git state that would run a program later, outside the sandbox. +//! +//! The sandbox lets a run write `.git/` inside its write roots, so `git init`, `git config` and +//! `git commit` work. A run can then leave git set up to start a program the next time the +//! member uses git in a terminal, where no sandbox applies: a hook, `core.hooksPath`, +//! `core.fsmonitor`, an alias that starts a shell, a diff or filter driver, an include. The +//! Seatbelt profile keeps hook files out of `.git/hooks`, but configuration cannot be filtered by +//! path. So the runner looks at the repositories a run could have touched before and after it, +//! and reports what is new ([`crate::RunReport::git_warnings`]) for the member to see. It reports, +//! it does not change anything. +//! +//! Honest scope: the repositories checked are the run's folder, its parents up to the write root +//! that holds it, and the folders directly inside the run's folder. A repository created deeper +//! than that is not checked. + +use std::collections::BTreeSet; +use std::path::{Path, PathBuf}; + +/// Most entries of the run's folder looked at for nested repositories. +const MAX_CHILDREN: usize = 256; +/// Largest `.git/config` read. +const MAX_CONFIG_BYTES: u64 = 256 * 1024; + +/// The repositories (their `.git` folders) a run in `cwd` with `write_roots` could touch. +pub(crate) fn repo_dirs(cwd: &Path, write_roots: &[PathBuf]) -> Vec { + let mut out: Vec = Vec::new(); + let mut push = |p: PathBuf| { + if !out.contains(&p) { + out.push(p); + } + }; + if let Some(root) = write_roots + .iter() + .filter(|r| cwd.starts_with(r)) + .max_by_key(|r| r.components().count()) + { + for dir in cwd.ancestors() { + push(dir.join(".git")); + if dir == root.as_path() { + break; + } + } + } + if let Ok(rd) = std::fs::read_dir(cwd) { + for e in rd.flatten().take(MAX_CHILDREN) { + if e.file_type().map(|t| t.is_dir()).unwrap_or(false) { + push(e.path().join(".git")); + } + } + } + out +} + +/// One line per risky item across `git_dirs` (sorted, without repeats). +pub(crate) fn risks(git_dirs: &[PathBuf]) -> BTreeSet { + let mut out = BTreeSet::new(); + for g in git_dirs { + let Ok(meta) = std::fs::symlink_metadata(g) else { + continue; + }; + if !meta.is_dir() { + continue; + } + let hooks = g.join("hooks"); + match std::fs::symlink_metadata(&hooks) { + Ok(m) if m.file_type().is_symlink() => { + out.insert(format!("{} is a symbolic link", hooks.display())); + } + Ok(m) if m.is_dir() => { + if let Ok(rd) = std::fs::read_dir(&hooks) { + for e in rd.flatten() { + let name = e.file_name().to_string_lossy().into_owned(); + if !name.ends_with(".sample") { + out.insert(format!( + "git hook {name} in {} (git runs it outside the sandbox)", + hooks.display() + )); + } + } + } + } + _ => {} + } + let config = g.join("config"); + if let Ok(m) = std::fs::symlink_metadata(&config) { + if m.file_type().is_symlink() { + out.insert(format!("{} is a symbolic link", config.display())); + } else if m.len() <= MAX_CONFIG_BYTES { + if let Ok(text) = std::fs::read_to_string(&config) { + for (key, value) in risky_config(&text) { + out.insert(format!( + "git config {key} = {value} in {} (git may run a program from it)", + config.display() + )); + } + } + } else { + out.insert(format!("{} is too large to check", config.display())); + } + } + } + out +} + +/// `(section.key, value)` pairs in a git config text that make git start a program. Keys are +/// lowercased (git's own rule for section and key names; a subsection keeps its case). +pub(crate) fn risky_config(text: &str) -> Vec<(String, String)> { + let mut out = Vec::new(); + let mut section = String::new(); + for raw in text.lines() { + let line = raw.trim(); + if line.is_empty() || line.starts_with('#') || line.starts_with(';') { + continue; + } + if let Some(rest) = line.strip_prefix('[') { + let inner = rest.split(']').next().unwrap_or("").trim(); + section = match inner.split_once(char::is_whitespace) { + Some((name, sub)) => { + format!( + "{}.{}", + name.to_ascii_lowercase(), + sub.trim().trim_matches('"') + ) + } + None => inner.to_ascii_lowercase(), + }; + continue; + } + let (k, v) = match line.split_once('=') { + Some((k, v)) => (k.trim(), v.trim().trim_matches('"')), + None => (line, "true"), + }; + let key = format!("{section}.{}", k.to_ascii_lowercase()); + if is_risky(&key, v) { + out.push((key, v.to_string())); + } + } + out +} + +fn is_risky(key: &str, value: &str) -> bool { + let lower = key.to_ascii_lowercase(); + let (first, rest) = lower.split_once('.').unwrap_or((lower.as_str(), "")); + let last = rest.rsplit('.').next().unwrap_or(rest); + let off = matches!( + value.to_ascii_lowercase().as_str(), + "" | "false" | "0" | "no" | "off" + ); + match first { + "core" => match last { + "fsmonitor" => !off, + "hookspath" | "sshcommand" | "gitproxy" | "editor" | "pager" | "askpass" => true, + _ => false, + }, + "alias" => value.trim_start().starts_with('!'), + "sequence" => last == "editor", + "credential" => last == "helper", + "gpg" => last == "program", + "diff" => matches!(last, "external" | "command" | "textconv"), + "filter" => matches!(last, "clean" | "smudge" | "process"), + "merge" => last == "driver", + "include" | "includeif" => last == "path", + "uploadpack" => last == "packobjectshook", + "remote" => matches!(last, "uploadpack" | "receivepack"), + "pager" | "browser" | "man" | "difftool" | "mergetool" => { + matches!(last, "cmd" | "path") || first == "pager" + } + _ => false, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn risky_keys_are_found_and_harmless_ones_are_not() { + let text = "[core]\n\trepositoryformatversion = 0\n\tfsmonitor = false\n\thooksPath = .h\n[alias]\n\tst = status\n\tx = !sh -c id\n[filter \"lfs\"]\n\tsmudge = git-lfs smudge %f\n[remote \"origin\"]\n\turl = https://example.invalid/r.git\n[includeIf \"gitdir:~/\"]\n\tpath = ~/.evil\n"; + let keys: Vec = risky_config(text).into_iter().map(|(k, _)| k).collect(); + assert_eq!( + keys, + vec![ + "core.hookspath", + "alias.x", + "filter.lfs.smudge", + "includeif.gitdir:~/.path" + ] + ); + assert!(risky_config("[core]\n\tfsmonitor = true\n") + .iter() + .any(|(k, _)| k == "core.fsmonitor")); + } + + #[test] + fn repo_dirs_cover_the_cwd_its_parents_to_the_root_and_its_children() { + let d = tempfile::tempdir().expect("tmp"); + let root = d.path().join("root"); + std::fs::create_dir_all(root.join("a/b/c")).expect("dirs"); + let got = repo_dirs(&root.join("a/b"), std::slice::from_ref(&root)); + assert!(got.contains(&root.join("a/b/.git"))); + assert!(got.contains(&root.join("a/.git"))); + assert!(got.contains(&root.join(".git"))); + assert!(got.contains(&root.join("a/b/c/.git"))); + assert!( + !got.contains(&d.path().join(".git")), + "never above the write root" + ); + } +} diff --git a/agent-shell/src/lib.rs b/agent-shell/src/lib.rs index db61bf7..0ef78d6 100644 --- a/agent-shell/src/lib.rs +++ b/agent-shell/src/lib.rs @@ -75,6 +75,7 @@ //! That crate is async and tied to the legacy tool trait, so the logic is re-homed here as a //! synchronous, dependency-light library. +mod git_state; pub mod sandbox; #[cfg(windows)] mod win; @@ -847,6 +848,12 @@ pub struct RunReport { pub cleanup_error: Option, /// Whether (and how) the run was wrapped in the OS sandbox. pub sandbox: SandboxSummary, + /// HGW-2: git state the run left behind that would start a program the next time the member + /// uses git outside the sandbox (a hook, `core.hooksPath`, an alias that runs a shell, …), and + /// that was not there before the run. Reported, never changed. Empty (and not serialized) + /// when there is nothing new. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub git_warnings: Vec, } impl RunReport { @@ -1141,6 +1148,9 @@ impl ShellRunner { cmd.env(k, v); } + // HGW-2: what git would run later, before the run (only what is new is reported). + let repos = git_state::repo_dirs(&plan.cwd, &plan.write_roots); + let git_before = git_state::risks(&repos); let tree = ProcessTree::prepare().map_err(|e| ShellError::Spawn { program: req.program.clone(), reason: format!("cannot create the process tree container: {e}"), @@ -1199,6 +1209,10 @@ impl ShellRunner { ); } + let git_warnings: Vec = git_state::risks(&repos) + .difference(&git_before) + .cloned() + .collect(); Ok(RunReport { program: req.program.clone(), resolved_path: plan.resolved_path, @@ -1218,6 +1232,7 @@ impl ShellRunner { output_incomplete, cleanup_error, sandbox: plan.sandbox, + git_warnings, }) } } @@ -1794,6 +1809,7 @@ mod tests { output_incomplete: false, cleanup_error: Some("scratch removal failed".into()), sandbox: SandboxSummary::not_enforced("test", &[]), + git_warnings: Vec::new(), }; assert!(!report.passed()); diff --git a/agent-shell/src/sandbox.rs b/agent-shell/src/sandbox.rs index 72337fb..837d5ed 100644 --- a/agent-shell/src/sandbox.rs +++ b/agent-shell/src/sandbox.rs @@ -7,14 +7,18 @@ //! - **macOS: Seatbelt** through `/usr/bin/sandbox-exec`. The profile is generated per run //! ([`seatbelt_profile`]) and starts from `(deny default)`. It never allows a network //! operation. Every path is passed as a `-D` parameter, never spliced into the profile text, -//! so a folder name cannot change the policy. Writes to `.git/hooks`, `.git/config` and -//! `.env*` files are denied even inside a granted folder (a hook planted there would run later -//! outside the sandbox). +//! so a folder name cannot change the policy. Inside a granted folder `.git/` is writable +//! (HGW-2: `git init`, `git config` and `git commit` work), but a hook file other than git's own +//! `*.sample` files cannot be written, the hooks folder cannot be removed or moved, and a +//! symbolic link cannot be put in its place (a hook planted there would run later outside the +//! sandbox). `.env*` files are neither readable nor writable. Configuration that makes git run +//! a program cannot be filtered by path, so the runner reports any it finds new after a run +//! (`RunReport::git_warnings`). //! - **Linux: bubblewrap** (`bwrap`, found only in fixed system directories). The run gets fresh //! namespaces with networking unshared (`--unshare-all`), read-only binds of the system and //! toolchain directories, writable binds of the granted folders and the scratch HOME, and -//! read-only masks over `.git/hooks`, `.git/config` and the `.env*` files that exist at the top -//! of a granted folder ([`bwrap_command`]). A Landlock fallback is not implemented: with no +//! read-only masks over `.git/hooks` and the `.env*` files that exist at the top of a granted +//! folder ([`bwrap_command`]). `.git/config` is writable (HGW-2), as on macOS. A Landlock fallback is not implemented: with no //! working `bwrap` the backend is unavailable. //! - **Windows**: the runner builds and uses a Job Object for bounded process cleanup, but has //! no OS sandbox backend. A Job Object is cleanup containment, not a filesystem, network, or @@ -364,8 +368,16 @@ pub fn seatbelt_profile(n_read: usize, n_write: usize) -> String { "(allow file-write-data (literal \"/dev/null\") (literal \"/dev/zero\") (literal \"/dev/dtracehelper\"))\n", ); p.push_str("(allow file-ioctl (literal \"/dev/dtracehelper\"))\n"); - // Later rules win: these hold even inside a granted folder. - p.push_str("(deny file-write* (regex #\"/\\.git/hooks(/|$)\") (regex #\"/\\.git/config$\"))\n"); + // Later rules win: these hold even inside a granted folder. HGW-2: `.git/` is writable (git + // init/config/commit), except a hook that is not one of git's `*.sample` files, removing or + // moving the hooks folder, and a symbolic link in its place. + p.push_str( + "(deny file-write* (require-all (regex #\"/\\.git/hooks/[^/]+$\") (require-not (regex #\"\\.sample$\"))))\n", + ); + p.push_str("(deny file-write-unlink (regex #\"/\\.git/hooks$\"))\n"); + p.push_str( + "(deny file-write-create (require-all (regex #\"/\\.git/hooks$\") (vnode-type SYMLINK)))\n", + ); p.push_str("(deny file-read* file-write* (regex #\"/\\.env[^/]*$\"))\n"); p } @@ -436,10 +448,8 @@ pub fn bwrap_command(spec: &SandboxSpec, program: &Path, args: &[String]) -> Vec ]); // Masks go after the writable binds, so they sit on top of them. for w in &spec.write_roots { - for sub in [".git/hooks", ".git/config"] { - let p = w.join(sub); - push(&["--ro-bind-try".as_ref(), p.as_os_str(), p.as_os_str()]); - } + let p = w.join(".git/hooks"); + push(&["--ro-bind-try".as_ref(), p.as_os_str(), p.as_os_str()]); } for f in &spec.masked_files { push(&["--ro-bind".as_ref(), "/dev/null".as_ref(), f.as_os_str()]); diff --git a/agent-shell/tests/sandbox.rs b/agent-shell/tests/sandbox.rs index 245201e..a6ed660 100644 --- a/agent-shell/tests/sandbox.rs +++ b/agent-shell/tests/sandbox.rs @@ -23,6 +23,7 @@ fn allow() -> Allowlist { .allow("sh", ArgPolicy::Any) .allow("curl", ArgPolicy::Any) .allow("echo", ArgPolicy::Any) + .allow("git", ArgPolicy::Any) } /// `base/grant` (the granted folder, with `sub/`), `base/other` (not granted). @@ -199,6 +200,12 @@ fn seatbelt_profile_denies_by_default_and_never_allows_network() { let deny_hooks = p.find(".git/hooks").expect("hooks deny"); assert!(deny_hooks > allow_w); assert!(p.contains("\\.env")); + // HGW-2: `.git/config` is writable inside a write root (git init and git config work); the + // hooks stay closed: no non-sample hook file, no removing or symlinking the hooks folder. + assert!(!p.contains(".git/config"), "{p}"); + assert!(p.contains("require-not"), "{p}"); + assert!(p.contains("\\.sample$"), "{p}"); + assert!(p.contains("vnode-type SYMLINK"), "{p}"); } #[test] @@ -237,6 +244,43 @@ fn bwrap_command_isolates_network_and_binds_only_the_grant_writable() { let bind_at = a.iter().position(|x| x == &g).expect("bind"); let mask_at = a.iter().position(|x| x == &env).expect("mask"); assert!(mask_at > bind_at); + // HGW-2: an existing hooks folder stays read-only; `.git/config` is writable (git config). + let hooks = fx.grant().join(".git/hooks").to_string_lossy().into_owned(); + assert!(has(&["--ro-bind-try", &hooks, &hooks]), "{a:?}"); + assert!(!a.iter().any(|x| x.ends_with(".git/config")), "{a:?}"); +} + +/// HGW-2: a run that leaves git in a state that would run a program the next time the member uses +/// git in a terminal (a hook, `core.hooksPath`, `core.fsmonitor`, an alias that runs a shell…) is +/// reported in `git_warnings`. What was already there before the run is not reported again. +#[test] +fn new_risky_git_state_is_reported_after_a_run() { + let fx = Fx::new(); + let repo = fx.grant().join("repo"); + std::fs::create_dir_all(repo.join(".git/hooks")).expect("repo"); + // Already there before the run (a member's own hook manager): not reported. + std::fs::write(repo.join(".git/hooks/pre-push"), "#!/bin/sh\n").expect("hook"); + std::fs::write(repo.join(".git/config"), "[core]\n\tbare = false\n").expect("config"); + let r = scoped(fx.grant(), SandboxPolicy::new(SandboxMode::Off)); + let cmd = "printf '[core]\\n\\thooksPath = .h\\n[alias]\\n\\tx = !sh -c id\\n' >> .git/config; echo '#!/bin/sh' > .git/hooks/post-checkout; echo ok"; + let rep = r.run(&sh(cmd, &repo)).expect("run"); + assert_eq!(rep.exit_code, Some(0), "{}", rep.stderr); + let w = rep.git_warnings.join("\n"); + assert!(w.contains("core.hookspath"), "{w}"); + assert!(w.contains("alias.x"), "{w}"); + assert!(w.contains("post-checkout"), "{w}"); + assert!( + !w.contains("pre-push"), + "pre-existing state is not reported: {w}" + ); + // A run that changes nothing about git reports nothing. + let rep = r.run(&sh("echo quiet", &repo)).expect("run"); + assert!(rep.git_warnings.is_empty(), "{:?}", rep.git_warnings); + let v = rep.to_json(); + assert!( + v.get("git_warnings").is_none(), + "empty warnings are not serialized: {v}" + ); } #[test] @@ -396,6 +440,59 @@ mod macos { } } + /// HGW-2: `git init`, `git add` and `git commit` work inside the grant under Seatbelt (they + /// write `.git/config` and the sample hooks), and nothing lands outside it. + #[test] + fn git_init_add_and_commit_work_inside_the_grant() { + let fx = Fx::new(); + let r = scoped(fx.grant(), required()); + let cmd = "git init -q . && echo hi > a.txt && git add a.txt && git -c user.name=Hermes -c user.email=hermes@example.invalid commit -q -m init && git config user.name Hermes && git log --oneline | wc -l"; + let rep = r.run(&sh(cmd, &fx.grant())).expect("run"); + assert!(rep.sandbox.enforced); + assert_eq!(rep.exit_code, Some(0), "stderr: {}", rep.stderr); + assert_eq!(rep.stdout.trim(), "1"); + assert!(fx.grant().join(".git/config").is_file()); + assert!(rep.git_warnings.is_empty(), "{:?}", rep.git_warnings); + } + + /// HGW-2: with `.git` writable, hooks stay closed: no hook file but samples, no renaming a + /// sample into a hook, no removing the hooks folder, no symlinking a new one in. + #[test] + fn git_hooks_stay_closed_when_git_is_writable() { + let fx = Fx::new(); + let r = scoped(fx.grant(), required()); + let rep = r.run(&sh("git init -q .", &fx.grant())).expect("init"); + assert_eq!(rep.exit_code, Some(0), "stderr: {}", rep.stderr); + let hooks = fx.grant().join(".git/hooks"); + for cmd in [ + "echo x > .git/hooks/pre-commit", + "mv .git/hooks/pre-commit.sample .git/hooks/pre-commit", + "cp .git/hooks/pre-commit.sample .git/hooks/post-merge", + ] { + let rep = r.run(&sh(cmd, &fx.grant())).expect("run"); + assert_ne!(rep.exit_code, Some(0), "{cmd}"); + } + assert!(!hooks.join("pre-commit").exists()); + assert!(!hooks.join("post-merge").exists()); + let _ = r.run(&sh("rm -rf .git/hooks", &fx.grant())).expect("rm"); + assert!(hooks.is_dir(), "the hooks folder cannot be removed"); + let _ = r + .run(&sh("mv .git/hooks .git/h2", &fx.grant())) + .expect("mv"); + assert!(hooks.is_dir(), "the hooks folder cannot be moved away"); + std::fs::create_dir_all(fx.grant().join("sub/fresh/.git")).expect("fresh"); + let rep = r + .run(&sh( + "mkdir -p ../../h && ln -s ../../h .git/hooks", + &fx.grant().join("sub/fresh"), + )) + .expect("ln"); + assert_ne!(rep.exit_code, Some(0)); + assert!(!fx.grant().join("sub/fresh/.git/hooks").exists()); + // The sample a fresh `git init` writes is fine. + assert!(hooks.join("pre-commit.sample").exists() || hooks.is_dir()); + } + #[test] fn plan_shows_the_sandbox_before_anything_runs() { let fx = Fx::new(); @@ -414,3 +511,79 @@ mod macos { ); } } + +// ------------------------------------------------------------------------------------------ +// Linux: the real thing (bubblewrap). Runs where bwrap can create a sandbox; the Linux CI lane +// sets CITRATE_REQUIRE_BWRAP=1 so a missing or broken bwrap fails instead of skipping. +// ------------------------------------------------------------------------------------------ + +#[cfg(target_os = "linux")] +mod linux { + use super::*; + + fn required() -> Option { + match citrate_agent_shell::sandbox::detect() { + Ok(_) => Some(SandboxPolicy::new(SandboxMode::Required)), + Err(why) => { + assert!( + std::env::var("CITRATE_REQUIRE_BWRAP").as_deref() != Ok("1"), + "CITRATE_REQUIRE_BWRAP=1 but bwrap is unusable here: {why}" + ); + eprintln!("skipping the bwrap run test: {why}"); + None + } + } + } + + #[test] + fn writes_outside_the_grant_are_denied_under_bwrap() { + let Some(p) = required() else { return }; + let fx = Fx::new(); + let r = scoped(fx.grant(), p); + let f = fx.other().join("escape.txt"); + let rep = r + .run(&sh(&format!("echo x > '{}'", f.display()), &fx.grant())) + .expect("run"); + assert!(rep.sandbox.enforced); + assert_ne!(rep.exit_code, Some(0)); + assert!(!f.exists()); + let ok = fx.grant().join("in.txt"); + let rep = r + .run(&sh(&format!("echo x > '{}'", ok.display()), &fx.grant())) + .expect("run"); + assert_eq!(rep.exit_code, Some(0), "{}", rep.stderr); + assert!(ok.exists()); + } + + #[test] + fn git_init_add_and_commit_work_inside_the_grant_under_bwrap() { + let Some(p) = required() else { return }; + if !Path::new("/usr/bin/git").exists() { + eprintln!("skipping: no /usr/bin/git"); + return; + } + let fx = Fx::new(); + let r = scoped(fx.grant(), p); + let cmd = "git init -q . && echo hi > a.txt && git add a.txt && git -c user.name=Hermes -c user.email=hermes@example.invalid commit -q -m init && git config user.name Hermes && git log --oneline | wc -l"; + let rep = r.run(&sh(cmd, &fx.grant())).expect("run"); + assert_eq!(rep.exit_code, Some(0), "stderr: {}", rep.stderr); + assert_eq!(rep.stdout.trim(), "1"); + } + + #[test] + fn network_is_unshared_under_bwrap() { + let Some(p) = required() else { return }; + let fx = Fx::new(); + let r = scoped(fx.grant(), p); + // Only a loopback interface exists in a fresh network namespace. + let rep = r + .run(&sh( + "cat /proc/net/dev | tail -n +3 | cut -d: -f1 | tr -d ' '", + &fx.grant(), + )) + .expect("run"); + assert_eq!(rep.exit_code, Some(0), "{}", rep.stderr); + let ifaces: Vec<&str> = rep.stdout.lines().filter(|l| !l.is_empty()).collect(); + assert_eq!(ifaces, vec!["lo"], "{ifaces:?}"); + } +} diff --git a/agent-shell/tests/windows.rs b/agent-shell/tests/windows.rs index 2969f0b..723ae13 100644 --- a/agent-shell/tests/windows.rs +++ b/agent-shell/tests/windows.rs @@ -287,6 +287,7 @@ fn cleanup_error_makes_serialized_report_fail() { readable_extra: Vec::new(), summary: "test".into(), }, + git_warnings: Vec::new(), }; assert!(!report.passed()); diff --git a/agent-sidecar/src/files.rs b/agent-sidecar/src/files.rs index 384c407..33b57e4 100644 --- a/agent-sidecar/src/files.rs +++ b/agent-sidecar/src/files.rs @@ -13,7 +13,10 @@ //! Every call runs in this order: //! //! 1. **Grant and deny list.** Build configuration (`foundry.toml`, env files and the rest of -//! `toolchain_config::build_config_file`) is refused first: the member edits it. Each path +//! `toolchain_config::build_config_file`) is refused first: the member edits it. HGW-2: +//! inside a session's project root (`SessionGrants::project_root_of`) build configuration +//! other than env files may be written; env files and anything inside a `.git` folder are +//! refused everywhere (git writes there, through the sandboxed `shell_run`). Each path //! then goes through [`FolderGrants::check`] for a write, which //! asks the agent-guard default-deny list first (credentials, keychains, browser profiles, //! wallet storage, app data, shell history) and then needs a live write grant covering the @@ -195,16 +198,14 @@ pub(crate) fn checked_whole_file_write( if !path.is_absolute() { return Err(Refusal::Failed("path must be absolute".into())); } - if let Some(name) = crate::toolchain_config::build_config_file(path) { - return Err(Refusal::Policy( - crate::toolchain_config::build_config_refusal(&name), - )); + // HGW-2: env files and `.git` internals are refused on the path as given, before any I/O; + // other build configuration waits for the resolved path (it is allowed in a project root). + if let Some(r) = config_refusal_as_given(path) { + return Err(Refusal::Policy(r)); } let (file, root) = grants.check_write(path).map_err(Refusal::Policy)?; - if let Some(name) = crate::toolchain_config::build_config_file(&file) { - return Err(Refusal::Policy( - crate::toolchain_config::build_config_refusal(&name), - )); + if let Some(r) = config_refusal_resolved(path, &file, grants.project_root_of(&file)) { + return Err(Refusal::Policy(r)); } if let Some(r) = leaf_refusal(&[path, &file]) { return Err(r); @@ -229,6 +230,27 @@ pub(crate) fn checked_whole_file_write( }) } +/// HGW-2: the refusal for a path as given (no I/O yet): `.git` internals and env files always; +/// other build configuration is decided on the resolved path. +fn config_refusal_as_given(path: &Path) -> Option { + crate::toolchain_config::git_internal_refusal(path) + .or_else(|| crate::toolchain_config::env_file_refusal(path)) +} + +/// HGW-2: the refusal once the path is resolved: `.git` internals, env files, and other build +/// configuration outside a project root, checked on the path as given and as resolved (the +/// project is the one holding the resolved path). +fn config_refusal_resolved( + given: &Path, + resolved: &Path, + project: Option, +) -> Option { + let project = project.as_deref(); + crate::toolchain_config::git_internal_refusal(resolved) + .or_else(|| crate::toolchain_config::build_config_write_refusal(given, project)) + .or_else(|| crate::toolchain_config::build_config_write_refusal(resolved, project)) +} + /// A refusal when a leaf in `paths` is a symbolic link, is not a regular file, or has other hard /// links (writing it could change a file outside the grant). A missing leaf is fine (a new file). /// The grant-session write tools pass the path as given and as resolved (they never follow a @@ -415,6 +437,15 @@ impl FileTools { ] } + /// HGW-2: the project root holding `canonical`, when the grants are a session's (only a + /// session has project roots). + fn project_of(&self, canonical: &Path) -> Option { + match &self.grants { + GrantSource::Session(g) => g.project_root_of(canonical), + GrantSource::File(_) | GrantSource::Fixed(_) => None, + } + } + /// The grant set to check against now, and the time to check it at. fn load_grants(&self) -> Result<(FolderGrants, u64), Refusal> { match &self.grants { @@ -445,23 +476,23 @@ impl FileTools { "{raw:?} is not an absolute path; use an absolute path inside a folder the member granted" ))); } - // Build configuration is the member's to edit: no fs tool creates, changes, moves or - // removes it (checked on the path as given and as resolved). - if let Some(name) = crate::toolchain_config::build_config_file(p) { - return Err(Refusal::Policy( - crate::toolchain_config::build_config_refusal(&name), - )); + // Build configuration is the member's to edit outside a project root (HGW-2): no fs tool + // creates, changes, moves or removes it there (checked on the path as given and as + // resolved). Env files and `.git` internals are refused everywhere. + if let Some(r) = config_refusal_as_given(p) { + return Err(Refusal::Policy(r)); } match grants.check(p, Op::Write, now) { Decision::Allowed { canonical, grant_id, } => { - if let Some(name) = crate::toolchain_config::build_config_file(canonical.as_path()) - { - return Err(Refusal::Policy( - crate::toolchain_config::build_config_refusal(&name), - )); + if let Some(r) = config_refusal_resolved( + p, + canonical.as_path(), + self.project_of(canonical.as_path()), + ) { + return Err(Refusal::Policy(r)); } // A hard link can name a file kept outside the grant (the deny list is // path-based): no fs tool reads, snapshots, edits, moves or removes one. diff --git a/agent-sidecar/src/grants.rs b/agent-sidecar/src/grants.rs index b92401e..5e123d0 100644 --- a/agent-sidecar/src/grants.rs +++ b/agent-sidecar/src/grants.rs @@ -79,6 +79,10 @@ pub struct SessionGrants { clock: fn() -> u64, inner: RwLock, ignored: std::sync::atomic::AtomicUsize, + /// HGW-2: the session's project roots (canonical): folders where the agent's file tools may + /// create and change build configuration. Named by citrate-core (`projectRoots`) or created by + /// this session's `project_create`. A marker file alone never adds one. + projects: RwLock>, } impl std::fmt::Debug for SessionGrants { @@ -100,9 +104,59 @@ impl SessionGrants { home, clock: now_secs, ignored: std::sync::atomic::AtomicUsize::new(0), + projects: RwLock::new(Vec::new()), } } + /// HGW-2: add a project root. It must be an absolute, existing folder covered by live read + /// and write folder grants over its whole subtree (the same rule as `shell_run`'s folder). + /// Returns its canonical path. The grants are checked again at every write. + pub fn add_project_root(&self, root: &Path) -> Result { + if !root.is_absolute() { + return Err(format!( + "the project root {} is not an absolute path", + root.display() + )); + } + if !root.is_dir() { + return Err(format!( + "the project root {} is not an existing folder", + root.display() + )); + } + let (canonical, _) = self.shell_scope(root).map_err(|e| { + format!( + "the project root {} is not inside folders granted for reading and writing: {e}", + root.display() + ) + })?; + let mut p = self + .projects + .write() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if !p.contains(&canonical) { + p.push(canonical.clone()); + } + Ok(canonical) + } + + /// HGW-2: the session's project roots, in the order they were added. + pub fn project_roots(&self) -> Vec { + self.projects.read().map(|p| p.clone()).unwrap_or_default() + } + + /// HGW-2: the project root that contains `canonical` (an already-resolved path), if any. A + /// poisoned lock answers none (build configuration then stays refused). + pub fn project_root_of(&self, canonical: &Path) -> Option { + self.projects + .read() + .ok()? + .iter() + .filter(|r| canonical.starts_with(r)) + .max_by_key(|r| r.components().count()) + .cloned() + } + /// Use a different clock (tests). pub fn with_clock(mut self, clock: fn() -> u64) -> Self { self.clock = clock; diff --git a/agent-sidecar/src/hgw_support.rs b/agent-sidecar/src/hgw_support.rs index ad6ec9f..29dbab5 100644 --- a/agent-sidecar/src/hgw_support.rs +++ b/agent-sidecar/src/hgw_support.rs @@ -129,6 +129,29 @@ pub(crate) fn manager_with( mgr } +/// `POST /sessions` body with `grants`, `hicAware` and every key of `extra` (an object). +pub(crate) fn create_req_with( + grants: serde_json::Value, + hic_aware: bool, + extra: serde_json::Value, +) -> serde_json::Result { + let mut b = serde_json::json!({ + "model": "gemma-4", + "systemPrompt": "You are Hermes.", + "llm": {"baseUrl": "http://127.0.0.1:18080/v1", "bearer": "k"}, + "tools": [], + "maxToolsPerRequest": 8, + "hicAware": hic_aware, + "grants": grants, + }); + if let (Some(o), Some(e)) = (b.as_object_mut(), extra.as_object()) { + for (k, v) in e { + o.insert(k.clone(), v.clone()); + } + } + serde_json::from_value(b) +} + /// `POST /sessions` body with `grants` and `hicAware`. pub(crate) fn create_req(grants: serde_json::Value, hic_aware: bool) -> sessions::CreateSessionReq { serde_json::from_value(serde_json::json!({ diff --git a/agent-sidecar/src/lib.rs b/agent-sidecar/src/lib.rs index 6251a21..ddecf0a 100644 --- a/agent-sidecar/src/lib.rs +++ b/agent-sidecar/src/lib.rs @@ -2362,6 +2362,8 @@ mod decide_route_tests; mod deploy_guard_session_tests; #[cfg(all(test, unix))] mod hgw_support; +#[cfg(all(test, unix))] +mod project_policy_tests; #[cfg(test)] mod search_session_tests; #[cfg(test)] diff --git a/agent-sidecar/src/project_policy_tests.rs b/agent-sidecar/src/project_policy_tests.rs new file mode 100644 index 0000000..c8fbc31 --- /dev/null +++ b/agent-sidecar/src/project_policy_tests.rs @@ -0,0 +1,230 @@ +//! HGW-2 — project write policy: inside a project root, Hermes's file tools may create and change +//! build configuration (`package.json`, `foundry.toml`, `remappings.txt`, `tsconfig.json`, …); +//! everywhere else the member still edits it. `.env*` files stay the member's everywhere, and the +//! file tools never write inside `.git/` (git itself does, through the sandboxed `shell_run`). +//! +//! A project root is a folder citrate-core names in `projectRoots` when it opens the session (a +//! project Hermes created earlier, or a folder the member marked as a project) or one this +//! session's `project_create` made. A marker file alone never makes a folder a project. +//! +//! BDD map: +//! - `build_config_is_writable_inside_a_project_root_and_nowhere_else` +//! - `env_files_stay_the_members_even_inside_a_project` +//! - `a_marker_file_alone_does_not_make_a_project` +//! - `the_file_tools_never_write_inside_git` +//! - `project_roots_must_be_absolute_existing_folders_inside_the_grants` + +#![cfg(unix)] + +use super::hgw_support::*; +use super::*; +use citrate_agent_loop::AssistantTurn; + +fn file_write(id: &str, path: &std::path::Path, content: &str) -> citrate_agent_loop::ToolCall { + tool_call( + id, + grants::FILE_WRITE_TOOL, + serde_json::json!({ "path": path, "content": content }), + ) +} + +async fn run_calls( + fx: &Fx, + calls: Vec, + extra: serde_json::Value, + roots: &[std::path::PathBuf], +) -> Vec { + let ids: Vec = calls.iter().map(|c| c.id.clone()).collect(); + let mut turns: Vec = calls + .into_iter() + .map(|c| AssistantTurn::tools(vec![c])) + .collect(); + turns.push(AssistantTurn::text("ok")); + let mgr = Arc::new(manager_with(fx, turns, None)); + let id = mgr + .create(create_req_with(fx.rw_doc_for(roots), true, extra).expect("req")) + .expect("session"); + let s = mgr.get(&id).expect("session"); + mgr.send(&id, "go".into(), None).expect("send"); + let mut out = Vec::new(); + for c in ids { + out.push(result_of(&s, &c).await); + } + wait_done(&s).await; + out +} + +fn content(r: &serde_json::Value) -> String { + r["content"].as_str().unwrap_or_default().to_string() +} + +#[tokio::test(flavor = "multi_thread")] +async fn build_config_is_writable_inside_a_project_root_and_nowhere_else() { + let fx = Fx::new("proj-config"); + let proj = fx.proj(); + let other = fx.other(); + let res = run_calls( + &fx, + vec![ + fs_write("c1", &proj.join("package.json"), "{\"name\":\"demo\"}\n"), + file_write("c2", &proj.join("foundry.toml"), "[profile.default]\n"), + fs_write( + "c3", + &proj.join("remappings.txt"), + "forge-std/=lib/forge-std/src/\n", + ), + fs_write("c4", &proj.join("tsconfig.json"), "{}\n"), + fs_write("c5", &proj.join("src/Makefile"), "all:\n"), + fs_write("c6", &other.join("package.json"), "{}\n"), + file_write("c7", &other.join("foundry.toml"), "x\n"), + ], + serde_json::json!({ "projectRoots": [proj] }), + &[proj.clone(), other.clone()], + ) + .await; + for (i, r) in res.iter().take(5).enumerate() { + assert_eq!(r["status"], "ok", "call {}: {r}", i + 1); + } + assert_eq!( + std::fs::read_to_string(proj.join("package.json")).expect("package.json"), + "{\"name\":\"demo\"}\n" + ); + assert!(proj.join("foundry.toml").exists()); + assert!(proj.join("remappings.txt").exists()); + for r in &res[5..] { + assert_ne!(r["status"], "ok", "{r}"); + let c = content(r); + assert!(c.contains("build configuration"), "{c}"); + assert!(c.contains("project"), "the refusal points at projects: {c}"); + } + assert!(!other.join("package.json").exists()); + assert!(!other.join("foundry.toml").exists()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn without_project_roots_build_config_is_refused_as_before() { + let fx = Fx::new("proj-none"); + let proj = fx.proj(); + let res = run_calls( + &fx, + vec![fs_write("c1", &proj.join("package.json"), "{}\n")], + serde_json::json!({}), + std::slice::from_ref(&proj), + ) + .await; + assert_ne!(res[0]["status"], "ok"); + assert!(content(&res[0]).contains("build configuration")); + assert!(!proj.join("package.json").exists()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn env_files_stay_the_members_even_inside_a_project() { + let fx = Fx::new("proj-env"); + let proj = fx.proj(); + let res = run_calls( + &fx, + vec![ + fs_write("c1", &proj.join(".env"), "KEY=1\n"), + file_write("c2", &proj.join(".env.local"), "KEY=1\n"), + ], + serde_json::json!({ "projectRoots": [proj] }), + std::slice::from_ref(&proj), + ) + .await; + for r in &res { + assert_ne!(r["status"], "ok", "{r}"); + } + assert!(!proj.join(".env").exists()); + assert!(!proj.join(".env.local").exists()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_marker_file_alone_does_not_make_a_project() { + let fx = Fx::new("proj-marker"); + let other = fx.other(); + std::fs::create_dir_all(other.join(".citrate")).expect("marker dir"); + std::fs::write( + other.join(".citrate/project.json"), + serde_json::json!({"version": 1, "name": "other", "root": other}).to_string(), + ) + .expect("marker"); + let res = run_calls( + &fx, + vec![fs_write("c1", &other.join("package.json"), "{}\n")], + serde_json::json!({ "projectRoots": [fx.proj()] }), + &[fx.proj(), other.clone()], + ) + .await; + assert_ne!(res[0]["status"], "ok", "{}", res[0]); + assert!(!other.join("package.json").exists()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn the_file_tools_never_write_inside_git() { + let fx = Fx::new("proj-git"); + let proj = fx.proj(); + std::fs::create_dir_all(proj.join(".git/hooks")).expect("git dir"); + let res = run_calls( + &fx, + vec![ + fs_write("c1", &proj.join(".git/hooks/pre-commit"), "#!/bin/sh\n"), + file_write("c2", &proj.join(".git/config"), "[core]\n"), + fs_write("c3", &fx.other().join(".git/HEAD"), "ref: x\n"), + ], + serde_json::json!({ "projectRoots": [proj] }), + &[proj.clone(), fx.other()], + ) + .await; + for r in &res { + assert_ne!(r["status"], "ok", "{r}"); + assert!(content(r).contains(".git"), "{r}"); + } + assert!(!proj.join(".git/hooks/pre-commit").exists()); + assert!(!proj.join(".git/config").exists()); +} + +#[test] +fn project_roots_must_be_absolute_existing_folders_inside_the_grants() { + let fx = Fx::new("proj-roots"); + let mgr = manager_with(&fx, vec![], None); + let doc = fx.rw_doc(); + for bad in [ + serde_json::json!(["relative/dir"]), + serde_json::json!([fx.proj().join("missing")]), + serde_json::json!([fx.other()]), + ] { + let req = create_req_with( + doc.clone(), + true, + serde_json::json!({ "projectRoots": bad }), + ) + .expect("req"); + assert!( + matches!(mgr.create(req), Err(sessions::SessionError::Invalid(_))), + "{bad}" + ); + } + // Without a grant document there are no project roots. + let mut req = create_req_with( + serde_json::Value::Null, + true, + serde_json::json!({ "projectRoots": [fx.proj()] }), + ) + .expect("req"); + req.grants = None; + assert!(matches!( + mgr.create(req), + Err(sessions::SessionError::Invalid(_)) + )); + let ok = create_req_with( + doc, + true, + serde_json::json!({ "projectRoots": [fx.proj()] }), + ) + .expect("req"); + let id = mgr + .create(ok) + .expect("a granted, existing project root is accepted"); + let s = mgr.get(&id).expect("session"); + assert_eq!(s.grants().expect("grants").project_roots(), vec![fx.proj()]); +} diff --git a/agent-sidecar/src/sessions.rs b/agent-sidecar/src/sessions.rs index 37cc9f9..bdbb552 100644 --- a/agent-sidecar/src/sessions.rs +++ b/agent-sidecar/src/sessions.rs @@ -358,6 +358,13 @@ pub struct CreateSessionReq { /// the same rules as `POST /personas/check`). Never both. #[serde(default)] pub custom_persona: Option, + /// HGW-2: the session's project roots: absolute, existing folders covered by live read and + /// write folder grants (subtree), where the file tools may create and change build + /// configuration. citrate-core sends the projects Hermes created before (`project_create` + /// results it kept) and folders the member marked as projects. Needs `grants`. Absent = + /// none (build configuration stays the member's, as before). + #[serde(default)] + pub project_roots: Vec, } /// HUP-S3.3: what a session did with its persona (`POST /sessions` answers it as `persona`). @@ -1623,6 +1630,11 @@ impl SessionManager { "the tool name '{SHELL_RUN_TOOL}' is reserved by the sidecar while shell_run is on" ))); } + if req.grants.is_none() && !req.project_roots.is_empty() { + return Err(SessionError::Invalid( + "projectRoots needs folder grants (a grant document) to check them against".into(), + )); + } let grants = match req.grants { None => None, Some(doc) => { @@ -1655,6 +1667,13 @@ impl SessionManager { g.replace(&doc).map_err(|e| { SessionError::Invalid(format!("the grant document was refused: {e}")) })?; + // HGW-2: the project roots core names, each checked against these grants now. + for root in &req.project_roots { + g.add_project_root(std::path::Path::new(root)) + .map_err(|e| { + SessionError::Invalid(format!("projectRoots was refused: {e}")) + })?; + } specs.extend(crate::grants::file_tool_specs()); specs.extend(crate::sheets::sheet_tool_specs()); if fs_on_grants { diff --git a/agent-sidecar/src/shell_run.rs b/agent-sidecar/src/shell_run.rs index ebccc65..e1c61cb 100644 --- a/agent-sidecar/src/shell_run.rs +++ b/agent-sidecar/src/shell_run.rs @@ -462,7 +462,17 @@ fn report(argv: &[String], r: &RunReport) -> ToolOutcome { "output_incomplete": r.output_incomplete, "cleanup_error": r.cleanup_error, "sandbox": r.sandbox, + "git_warnings": r.git_warnings, }); + // HGW-2: git state the command left that would run a program outside the sandbox later. + let git_note = if r.git_warnings.is_empty() { + String::new() + } else { + format!( + ". Warning: the command left git set up to run a program outside the sandbox the next time git is used ({}); the member should review it", + r.git_warnings.join("; ") + ) + }; let isolation = if r.sandbox.enforced { format!("in the {} sandbox, no network", r.sandbox.backend) } else { @@ -504,7 +514,7 @@ fn report(argv: &[String], r: &RunReport) -> ToolOutcome { SHELL_RUN_TOOL, RunStatus::Completed, format!( - "{} {ended} after {} ms ({isolation})", + "{} {ended} after {} ms ({isolation}){git_note}", r.program, r.duration_ms ), ) @@ -644,6 +654,7 @@ mod tests { readable_extra: Vec::new(), summary: "test".into(), }, + git_warnings: Vec::new(), } } diff --git a/agent-sidecar/src/shell_run_tests.rs b/agent-sidecar/src/shell_run_tests.rs index 22d38af..b0e7a31 100644 --- a/agent-sidecar/src/shell_run_tests.rs +++ b/agent-sidecar/src/shell_run_tests.rs @@ -553,6 +553,42 @@ fn a_tainted_hic_aware_session_asks_with_the_reason_and_runs_only_on_allow() { drop(host); } +/// HGW-2: a command that leaves git set up to run a program later (outside the sandbox) is +/// reported to the model and the member: the run facts carry `git_warnings` and the summary says +/// so. +#[test] +fn risky_git_state_left_by_a_command_is_reported() { + let fx = Fx::new(); + std::fs::create_dir_all(fx.proj().join(".git/hooks")).unwrap(); + let s = session(&fx, &cfg_open()); + let approvals = s.approvals().clone(); + let h = spawn( + s.host(TaintState::default(), StopFlag::default(), false), + call( + "c1", + &[ + "sh", + "-c", + "printf '[core]\\n\\thooksPath = .h\\n' > .git/config", + ], + &fx.proj(), + ), + ); + let p = wait_pending(&approvals); + approvals.decide(&p.id, true, &p.argv, &p.cwd).unwrap(); + let out = h.join().unwrap(); + let env = envelope(&out); + let facts = env.run.expect("run facts"); + let w = facts["git_warnings"].to_string(); + assert!(w.contains("core.hookspath"), "{w}"); + assert!(env.summary.contains("git"), "{}", env.summary); + assert!( + env.summary.contains("outside the sandbox"), + "{}", + env.summary + ); +} + #[test] fn an_untainted_card_has_no_taint_reason() { let fx = Fx::new(); diff --git a/agent-sidecar/src/toolchain.rs b/agent-sidecar/src/toolchain.rs index 78a9020..6b139ad 100644 --- a/agent-sidecar/src/toolchain.rs +++ b/agent-sidecar/src/toolchain.rs @@ -777,6 +777,7 @@ mod tests { readable_extra: Vec::new(), summary: "test".into(), }, + git_warnings: Vec::new(), } } diff --git a/agent-sidecar/src/toolchain_config.rs b/agent-sidecar/src/toolchain_config.rs index c950326..68f5d4a 100644 --- a/agent-sidecar/src/toolchain_config.rs +++ b/agent-sidecar/src/toolchain_config.rs @@ -29,7 +29,11 @@ //! [`build_config_file`] is the matching rule for the agent's file tools: build configuration //! (the files above, plus `foundry.toml`, `remappings.txt`, `package.json`, `aderyn.toml`, //! make/just files and `.cargo/config*`) is edited by the member, never created, changed, moved -//! or removed by the agent. +//! or removed by the agent, except (HGW-2, owner decision 2026-10-15) inside a project root +//! ([`build_config_write_refusal`]): a project Hermes created or a folder the member marked as a +//! project, where the agent sets up its own build. Env files stay the member's everywhere. This +//! does not loosen the toolchain check above: forge still refuses to run with `ffi` or wide +//! `fs_permissions`, whoever wrote the file. use std::path::{Component, Path, PathBuf}; @@ -86,10 +90,49 @@ pub(crate) fn build_config_file(path: &Path) -> Option { /// The refusal the file tools give for [`build_config_file`]. pub(crate) fn build_config_refusal(name: &str) -> String { format!( - "{name} is build configuration, which the member edits; the agent's file tools do not create, change, move or remove it" + "{name} is build configuration, which the member edits; the agent's file tools do not create, change, move or remove it outside a project (a folder made with project_create, or one the member marked as a project)" ) } +/// HGW-2: the file tools' rule for one write, given the project root that holds the resolved +/// path (if any). Env files are always the member's; other build configuration is the agent's +/// to write only inside a project root. `None` = allowed. +pub(crate) fn build_config_write_refusal(path: &Path, project: Option<&Path>) -> Option { + if let Some(r) = env_file_refusal(path) { + return Some(r); + } + let name = build_config_file(path)?; + match project { + Some(_) => None, + None => Some(build_config_refusal(&name)), + } +} + +/// HGW-2: env files are only ever the member's: `Some(refusal)` for one (matching on the last +/// component, any case). +pub(crate) fn env_file_refusal(path: &Path) -> Option { + let name = path.file_name()?.to_string_lossy().into_owned(); + env_file(&name).then(|| { + format!( + "{name} is an env file, build configuration that may hold secrets, which only the member creates or changes; the agent's file tools never write it, in a project or not" + ) + }) +} + +/// HGW-2: the file tools never write inside a `.git` folder: git does that through the sandboxed +/// `shell_run`, whose sandbox keeps hooks out. `Some(refusal)` for such a path. +pub(crate) fn git_internal_refusal(path: &Path) -> Option { + let inside = path + .components() + .any(|c| matches!(c, Component::Normal(n) if n.eq_ignore_ascii_case(".git"))); + inside.then(|| { + format!( + "{} is inside a .git folder; the agent's file tools do not write there (git commands run through shell_run do)", + path.display() + ) + }) +} + /// Refuse (with the reason) when the project's build configuration asks for more than the agent /// toolchain allows. `project` is the canonical project folder. pub(crate) fn check_project_config(project: &Path) -> Result<(), String> { From 2a67b1d67d912acae238bb446ac0d14dda2f8814 Mon Sep 17 00:00:00 2001 From: Larry Klosowski Date: Thu, 15 Oct 2026 11:53:39 -0700 Subject: [PATCH 04/12] feat(sidecar): project_create tool, HIC-approved (HGW-3) Hermes had no way to start a project: there was no default folder and no tool to make one, and template rendering was a panel action only. project_create {name, template?, params?} (sidecar-hosted, offered to sessions with folder grants when CITRATE_HERMES_PROJECTS=1): - name is one plain folder name (no separators, no `..`, no leading `.`/`-`/`~`, at most 64 chars); refused before anything is asked; - the projects root (CITRATE_HERMES_PROJECTS_ROOT, passed by core; default ~/Citrate/Projects) must exist and be under live read+write subtree folder grants; / must not exist (never reused); - always waits on an action card (HGW-1 queue) showing the exact folder and template; no HIC-aware app = nothing created; a tainted session shows one card carrying the taint reason; - after approval (grants re-checked): creates the folder, renders the template if asked, writes .citrate/project.json, and makes the folder a session project root (HGW-2 build-config writes apply); a failed render removes the new folder and says why; returns the path. Template rendering stays core's: the TemplateRenderer trait is the seam, and CliTemplateRenderer runs core's `citrate-templates render --root --template --tier --out [--param k=v]...` (CITRATE_HERMES_TEMPLATE_RENDERER + CITRATE_HERMES_TEMPLATES_ROOT, CITRATE_HERMES_TEMPLATE_TIER) through the shell runner, in the OS sandbox when there is one, writing only the new folder. Tests: projects unit (+4), projects_tests session/route (+10, incl. the CLI renderer under real Seatbelt). Workspace: 2265 -> 2279 passed. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_018wWZZ8GRVU9USh3kKQHsFe --- agent-sidecar/src/lib.rs | 22 + agent-sidecar/src/main.rs | 7 + agent-sidecar/src/projects.rs | 737 ++++++++++++++++++++++++++++ agent-sidecar/src/projects_tests.rs | 464 +++++++++++++++++ agent-sidecar/src/sessions.rs | 53 ++ 5 files changed, 1283 insertions(+) create mode 100644 agent-sidecar/src/projects.rs create mode 100644 agent-sidecar/src/projects_tests.rs diff --git a/agent-sidecar/src/lib.rs b/agent-sidecar/src/lib.rs index ddecf0a..8bda1fb 100644 --- a/agent-sidecar/src/lib.rs +++ b/agent-sidecar/src/lib.rs @@ -35,6 +35,7 @@ pub mod llm_http; pub mod mcp_approvals; pub mod mcp_probe; pub mod metering; +pub mod projects; pub mod resources; pub mod retrieval_http; pub mod search; @@ -1859,6 +1860,25 @@ pub fn production_sessions_with( let mgr = mgr.with_self_review(self_review_from_env_var( std::env::var("CITRATE_HERMES_SELF_REVIEW").ok().as_deref(), )); + // HGW-3: project_create (off by default; every project needs the member's approval). + let mgr = match projects::ProjectsConfig::from_env_vars( + |k| std::env::var(k).ok(), + projects::production_render_sandbox(), + ) { + Ok(Some(cfg)) => { + eprintln!( + "citrate-agent-sidecar: project_create on for sessions with folder grants; projects go in {}{}", + cfg.root.display(), + if cfg.renderer.is_some() { ", with templates" } else { "" } + ); + mgr.with_projects(Arc::new(cfg)) + } + Ok(None) => mgr, + Err(e) => { + eprintln!("citrate-agent-sidecar: project_create stays off: {e}"); + mgr + } + }; // US-2.2 AC2: shell_run (off by default; always inside the OS sandbox). let mgr = match shell_run::ShellRunConfig::from_env() { Some(cfg) => { @@ -2364,6 +2384,8 @@ mod deploy_guard_session_tests; mod hgw_support; #[cfg(all(test, unix))] mod project_policy_tests; +#[cfg(all(test, unix))] +mod projects_tests; #[cfg(test)] mod search_session_tests; #[cfg(test)] diff --git a/agent-sidecar/src/main.rs b/agent-sidecar/src/main.rs index 1c2872b..8241660 100644 --- a/agent-sidecar/src/main.rs +++ b/agent-sidecar/src/main.rs @@ -30,6 +30,13 @@ //! approves, run in the OS sandbox, never without it) in every //! session opened with folder grants; anything else = off (optional) //! CITRATE_HERMES_SHELL_PATH shell_run search path override, a path list (optional) +//! CITRATE_HERMES_PROJECTS HGW-3: `1` offers project_create (a new project folder, after the +//! member approves it) in sessions opened with folder grants (optional) +//! CITRATE_HERMES_PROJECTS_ROOT absolute folder new projects go in; default ~/Citrate/Projects +//! CITRATE_HERMES_TEMPLATE_RENDERER / CITRATE_HERMES_TEMPLATES_ROOT absolute paths of core's +//! citrate-templates program and the bundled templates folder; both +//! set = project_create can start from a template (optional) +//! CITRATE_HERMES_TEMPLATE_TIER T0 (default), T1 or T2: the tier passed to the renderer //! CITRATE_HERMES_MCP HUP-S4.1: path to the MCP server allowlist (TOML, or JSON by //! `.json` extension); unset = no MCP (optional) //! CITRATE_HERMES_MCP_REGISTRY HUP-S4.4: core's owner-only saved MCP server list diff --git a/agent-sidecar/src/projects.rs b/agent-sidecar/src/projects.rs new file mode 100644 index 0000000..705fc9e --- /dev/null +++ b/agent-sidecar/src/projects.rs @@ -0,0 +1,737 @@ +//! HGW-3 — `project_create`: Hermes makes a new project folder, with the member's approval. +//! +//! One call `{name, template?, params?}`: +//! +//! 1. **Checked before anything is asked.** `name` is one plain folder name (letters, digits, +//! `.`, `_`, `-`; starts with a letter or digit; at most [`MAX_NAME_LEN`] characters; never +//! `..`, a path separator or `~`). The projects root (`CITRATE_HERMES_PROJECTS_ROOT`, which +//! citrate-core passes; default `~/Citrate/Projects`) must exist and be covered by the +//! session's live read and write folder grants over its whole subtree (citrate-core's +//! workspace card grants it). `/` must not exist yet. A template id, when given, +//! is a plain id and needs a configured renderer. +//! 2. **HIC, always.** The call waits on the session's action card (`/actions` routes, +//! [`crate::action_approvals`]) showing the exact folder, template and parameters. Only an +//! explicit allow creates anything. A session not opened by an HIC-aware app cannot ask, so it +//! creates nothing. In a tainted session the one card carries the taint reason too. +//! 3. **Creates** `/` (the grants are checked again first), renders the template into +//! it when one was asked for ([`TemplateRenderer`]; production runs citrate-core's +//! `citrate-templates render` program in the OS sandbox, writing only the new folder), writes +//! the project marker `.citrate/project.json` ([`ProjectMarker`]), and makes the folder one of +//! the session's project roots, so the file tools may write its build configuration (HGW-2). +//! A failed render removes the new folder again and reports why. +//! 4. **Returns** the absolute path (`{"ok": true, "path": …, "name": …, "template": …, +//! "marker": …, "rendered": …}`). citrate-core keeps the path and sends it back in +//! `projectRoots` when it opens later sessions. +//! +//! The marker is a record for people and for citrate-core; it does not make a folder a project +//! by itself (a cloned repository could carry one). Nothing here signs or holds a key (Rule 3). + +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; +use std::sync::Arc; +use std::time::Duration; + +use serde::Serialize; + +use crate::action_approvals::{ActionApprovals, ActionPending}; +use crate::grants::SessionGrants; +use citrate_agent_loop::{ + Effect, HostKind, StopFlag, TaintState, ToolAnnotations, ToolCall, ToolHost, ToolOutcome, + ToolSpec, Trust, +}; +use citrate_agent_shell::sandbox::{SandboxMode, SandboxPolicy}; +use citrate_agent_shell::{Allowlist, ArgPolicy, RunRequest, ShellPolicy, ShellRunner}; + +pub const PROJECT_CREATE_TOOL: &str = "project_create"; +/// `1` offers `project_create` to sessions opened with folder grants; anything else leaves it off. +pub const PROJECTS_ENV: &str = "CITRATE_HERMES_PROJECTS"; +/// The folder new projects go in (absolute). Unset = `$HOME/Citrate/Projects`. +pub const PROJECTS_ROOT_ENV: &str = "CITRATE_HERMES_PROJECTS_ROOT"; +/// Absolute path of citrate-core's `citrate-templates` program. Unset = no templates. +pub const TEMPLATE_RENDERER_ENV: &str = "CITRATE_HERMES_TEMPLATE_RENDERER"; +/// Absolute path of the bundled templates folder (`templates/` in the app's resources). +pub const TEMPLATES_ROOT_ENV: &str = "CITRATE_HERMES_TEMPLATES_ROOT"; +/// The hardware tier passed to the renderer (`T0`, `T1` or `T2`). Unset = `T0`. +pub const TEMPLATE_TIER_ENV: &str = "CITRATE_HERMES_TEMPLATE_TIER"; +/// The default projects root, under the member's home. +pub const DEFAULT_PROJECTS_DIR: &str = "Citrate/Projects"; +/// The marker, relative to the project folder. +pub const MARKER_DIR: &str = ".citrate"; +pub const MARKER_FILE: &str = "project.json"; +/// Longest project name. +pub const MAX_NAME_LEN: usize = 64; +/// Most template parameters, and the longest key and value. +pub const MAX_PARAMS: usize = 16; +pub const MAX_PARAM_LEN: usize = 256; +/// How long a render may take. +pub const RENDER_TIMEOUT: Duration = Duration::from_secs(60); + +/// What `project_create` needs. +#[derive(Clone)] +pub struct ProjectsConfig { + /// Where new projects go (absolute). + pub root: PathBuf, + /// Renders a bundled template into a new folder; `None` = templates are refused. + pub renderer: Option>, +} + +impl std::fmt::Debug for ProjectsConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ProjectsConfig") + .field("root", &self.root) + .field("renderer", &self.renderer.is_some()) + .finish() + } +} + +impl ProjectsConfig { + /// `None` unless `CITRATE_HERMES_PROJECTS` is exactly `1` and a projects root is known + /// (`CITRATE_HERMES_PROJECTS_ROOT`, absolute, or `$HOME/Citrate/Projects`). Templates need + /// both `CITRATE_HERMES_TEMPLATE_RENDERER` and `CITRATE_HERMES_TEMPLATES_ROOT` (absolute); + /// `sandbox` is how the renderer runs (production: the OS sandbox when there is one). + pub fn from_env_vars( + get: impl Fn(&str) -> Option, + sandbox: SandboxPolicy, + ) -> Result, String> { + if get(PROJECTS_ENV).as_deref() != Some("1") { + return Ok(None); + } + let root = match get(PROJECTS_ROOT_ENV).filter(|v| !v.trim().is_empty()) { + Some(r) => { + let p = PathBuf::from(r); + if !p.is_absolute() { + return Err(format!("{PROJECTS_ROOT_ENV} must be an absolute path")); + } + p + } + None => match get("HOME").filter(|h| !h.is_empty()) { + Some(h) => PathBuf::from(h).join(DEFAULT_PROJECTS_DIR), + None => return Err("no home directory is known for the projects root".into()), + }, + }; + let renderer = match ( + get(TEMPLATE_RENDERER_ENV).filter(|v| !v.is_empty()), + get(TEMPLATES_ROOT_ENV).filter(|v| !v.is_empty()), + ) { + (None, None) => None, + (Some(bin), Some(templates)) => { + let tier = get(TEMPLATE_TIER_ENV).unwrap_or_else(|| "T0".into()); + Some(Arc::new(CliTemplateRenderer::new( + PathBuf::from(bin), + PathBuf::from(templates), + &tier, + sandbox, + )?) as Arc) + } + _ => { + return Err(format!( + "templates need both {TEMPLATE_RENDERER_ENV} and {TEMPLATES_ROOT_ENV}" + )) + } + }; + Ok(Some(ProjectsConfig { root, renderer })) + } +} + +/// HGW-3: renders one bundled template into an empty folder. The interface citrate-core's +/// renderer is called through: `template` is the template id, `params` its parameters, `dest` +/// the new, empty project folder. Returns the renderer's own report (JSON), shown to the model. +pub trait TemplateRenderer: Send + Sync { + fn render( + &self, + template: &str, + params: &BTreeMap, + dest: &Path, + ) -> Result; +} + +/// The production renderer: runs citrate-core's `citrate-templates` program, +/// +/// ```text +/// citrate-templates render --root --template --tier +/// --out [--param key=value]... +/// ``` +/// +/// which prints a JSON report on stdout (exit 0) or an error on stderr. It runs through the +/// shell runner: argv only, a scrubbed environment, a timeout, and (with the OS sandbox) no +/// network, writes only in `dest`, reads of the templates folder. +pub struct CliTemplateRenderer { + runner: ShellRunner, + program: String, + templates: PathBuf, + tier: String, +} + +impl CliTemplateRenderer { + pub fn new( + bin: PathBuf, + templates: PathBuf, + tier: &str, + sandbox: SandboxPolicy, + ) -> Result { + if !bin.is_absolute() || !templates.is_absolute() { + return Err( + "the template renderer and the templates folder must be absolute paths".into(), + ); + } + if !matches!(tier, "T0" | "T1" | "T2") { + return Err(format!( + "{TEMPLATE_TIER_ENV} must be T0, T1 or T2, not {tier:?}" + )); + } + let program = bin + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .ok_or("the template renderer path names no program")?; + let dir = bin + .parent() + .map(Path::to_path_buf) + .ok_or("the template renderer path has no folder")?; + let sandbox = sandbox.with_extra_read_roots(vec![templates.clone()]); + let policy = ShellPolicy::new( + Allowlist::empty().allow(&program, ArgPolicy::Any), + vec![dir], + ) + .map_err(|e| e.to_string())? + .with_default_timeout(RENDER_TIMEOUT) + .with_max_timeout(RENDER_TIMEOUT) + .with_sandbox(sandbox); + // The run's only writable folder is the new project (its cwd). + let runner = ShellRunner::new(policy, |_cwd: &Path| Ok(())); + Ok(CliTemplateRenderer { + runner, + program, + templates, + tier: tier.to_string(), + }) + } +} + +impl TemplateRenderer for CliTemplateRenderer { + fn render( + &self, + template: &str, + params: &BTreeMap, + dest: &Path, + ) -> Result { + let mut args: Vec = vec![ + "render".into(), + "--root".into(), + self.templates.display().to_string(), + "--template".into(), + template.into(), + "--tier".into(), + self.tier.clone(), + "--out".into(), + dest.display().to_string(), + ]; + for (k, v) in params { + args.push("--param".into()); + args.push(format!("{k}={v}")); + } + let req = RunRequest::new(&self.program, args, dest).timeout(RENDER_TIMEOUT); + let r = self.runner.run(&req).map_err(|e| e.to_string())?; + if !r.passed() { + let why = r.stderr.trim(); + return Err(format!( + "the template renderer failed (exit {:?}){}", + r.exit_code, + if why.is_empty() { + String::new() + } else { + format!(": {}", clip(why, 2000)) + } + )); + } + serde_json::from_str(r.stdout.trim()) + .map_err(|_| "the template renderer did not print a JSON report".to_string()) + } +} + +fn clip(s: &str, max: usize) -> String { + if s.chars().count() <= max { + s.to_string() + } else { + let mut out: String = s.chars().take(max).collect(); + out.push_str("..."); + out + } +} + +/// `.citrate/project.json`: what Hermes created, where, when, and from which template. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, serde::Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ProjectMarker { + pub version: u32, + pub name: String, + /// The canonical project folder. + pub root: String, + pub created_by: String, + /// Unix seconds. + pub created_at: u64, + #[serde(skip_serializing_if = "Option::is_none")] + pub template: Option, + #[serde(skip_serializing_if = "BTreeMap::is_empty", default)] + pub params: BTreeMap, +} + +/// The tool offered to the model. +pub fn project_create_spec() -> ToolSpec { + ToolSpec { + name: PROJECT_CREATE_TOOL.into(), + description: "Create a new project folder in the member's projects folder, optionally from a bundled template, and get its absolute path. The member approves it first. Inside the new project you may create build files such as package.json, foundry.toml and remappings.txt with the file tools.".into(), + parameters: serde_json::json!({ + "type": "object", + "properties": { + "name": { + "type": "string", "maxLength": MAX_NAME_LEN, + "description": "The folder name: letters, digits, '.', '_' or '-', starting with a letter or digit (for example demo or my-token)." + }, + "template": { + "type": "string", + "description": "Optional bundled template id (for example erc20 or hello-mint)." + }, + "params": { + "type": "object", "additionalProperties": {"type": "string"}, + "description": "Optional template parameters, such as name and symbol." + } + }, + "required": ["name"], + }), + host: HostKind::Sidecar, + annotations: ToolAnnotations { + read_only: false, + destructive: false, + idempotent: false, + open_world: false, + effect: Some(Effect::Write), + // The result is the sidecar's own (paths and the renderer's report). + trust: Some(Trust::Trusted), + }, + } +} + +/// Whether `name` is one plain folder name (see the module docs). +pub fn valid_name(name: &str) -> Result<(), String> { + let refuse = |why: &str| Err(format!("the project name {name:?} {why}")); + if name.is_empty() { + return refuse("is empty"); + } + if name.chars().count() > MAX_NAME_LEN { + return refuse(&format!("is longer than {MAX_NAME_LEN} characters")); + } + let first = name.chars().next().unwrap_or('.'); + if !first.is_ascii_alphanumeric() { + return refuse("must start with a letter or a digit"); + } + if name.contains("..") { + return refuse("may not contain '..'"); + } + if !name + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-')) + { + return refuse("may only use letters, digits, '.', '_' and '-' (no path separators)"); + } + Ok(()) +} + +fn valid_template(id: &str) -> Result<(), String> { + if !id.is_empty() + && id.len() <= MAX_NAME_LEN + && id + .chars() + .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-') + { + Ok(()) + } else { + Err(format!( + "the template id {id:?} is not a plain id (a-z, 0-9, '-')" + )) + } +} + +/// The model's request. +struct Request { + name: String, + template: Option, + params: BTreeMap, +} + +fn parse(raw: &str) -> Result { + let raw = if raw.trim().is_empty() { "{}" } else { raw }; + let args: serde_json::Map = match serde_json::from_str(raw) { + Ok(serde_json::Value::Object(m)) => m, + _ => return Err("the arguments must be a JSON object".into()), + }; + let name = match args.get("name") { + Some(serde_json::Value::String(s)) => s.clone(), + _ => return Err("name (the new folder's name) is required".into()), + }; + valid_name(&name)?; + let template = match args.get("template") { + None | Some(serde_json::Value::Null) => None, + Some(serde_json::Value::String(t)) if t.is_empty() => None, + Some(serde_json::Value::String(t)) => { + valid_template(t)?; + Some(t.clone()) + } + Some(_) => return Err("template must be a string".into()), + }; + let mut params = BTreeMap::new(); + match args.get("params") { + None | Some(serde_json::Value::Null) => {} + Some(serde_json::Value::Object(m)) => { + if m.len() > MAX_PARAMS { + return Err(format!("at most {MAX_PARAMS} template parameters")); + } + for (k, v) in m { + let v = v + .as_str() + .ok_or_else(|| format!("template parameter {k} must be a string"))?; + let key_ok = !k.is_empty() + && k.len() <= MAX_PARAM_LEN + && k.chars().all(|c| c.is_ascii_alphanumeric() || c == '_'); + if !key_ok || v.len() > MAX_PARAM_LEN || v.contains('\0') { + return Err(format!( + "template parameter {k:?} must be a plain key with a value of at most {MAX_PARAM_LEN} bytes" + )); + } + params.insert(k.clone(), v.to_string()); + } + } + Some(_) => return Err("params must be an object of strings".into()), + } + if !params.is_empty() && template.is_none() { + return Err("params are only for a template".into()); + } + Ok(Request { + name, + template, + params, + }) +} + +/// `project_create` for one session turn. +pub struct ProjectCreateHost { + cfg: Arc, + grants: Arc, + actions: Option>, + taint: TaintState, + stop: StopFlag, +} + +impl ProjectCreateHost { + pub fn new( + cfg: Arc, + grants: Arc, + actions: Option>, + taint: TaintState, + stop: StopFlag, + ) -> Self { + ProjectCreateHost { + cfg, + grants, + actions, + taint, + stop, + } + } + + pub fn handles(name: &str) -> bool { + name == PROJECT_CREATE_TOOL + } + + /// The projects root, checked against the grants now: its canonical path. + fn checked_root(&self) -> Result { + let root = &self.cfg.root; + if !root.is_dir() { + return Err(format!( + "the projects folder {} does not exist yet; the app creates it when the member lets Hermes work there", + root.display() + )); + } + self.grants.shell_scope(root).map(|(c, _)| c).map_err(|e| { + format!( + "the projects folder {} is not granted for reading and writing ({e}); the member can allow it in the app", + root.display() + ) + }) + } + + /// Validate, ask, then create. `reason` names why the card is shown. + fn ask_and_create(&self, call: &ToolCall, reason: &str) -> ToolOutcome { + if call.name != PROJECT_CREATE_TOOL { + return ToolOutcome::Error(format!("'{}' is not project_create", call.name)); + } + let req = match parse(&call.arguments) { + Ok(r) => r, + Err(e) => return ToolOutcome::Error(e), + }; + if req.template.is_some() && self.cfg.renderer.is_none() { + return ToolOutcome::Error( + "this app cannot render templates from Hermes (no template renderer is configured); create the project without a template".into(), + ); + } + let root = match self.checked_root() { + Ok(r) => r, + Err(e) => return ToolOutcome::Error(e), + }; + let dest = root.join(&req.name); + if std::fs::symlink_metadata(&dest).is_ok() { + return ToolOutcome::Error(format!( + "{} already exists; pick another name (an existing folder is never reused)", + dest.display() + )); + } + let Some(actions) = &self.actions else { + return ToolOutcome::Denied( + "creating a project needs the member's approval, and no member can be asked in this session (it was not opened by an app that shows approval cards)".into(), + ); + }; + let mut card = ActionPending::for_call( + call, + reason, + if self.taint.is_tainted() { + self.taint.sources().unwrap_or_default() + } else { + Vec::new() + }, + ); + card.summary = match &req.template { + Some(t) => format!( + "create the project folder {} from the {t} template", + dest.display() + ), + None => format!("create the project folder {}", dest.display()), + }; + if let Err(why) = actions.request(card, &self.stop) { + return ToolOutcome::Denied(why); + } + if self.stop.is_stopped() { + return ToolOutcome::Denied("the session was stopped; nothing was created".into()); + } + // Checked again: a grant revoked while the card was open stops it. + let root = match self.checked_root() { + Ok(r) => r, + Err(e) => return ToolOutcome::Error(e), + }; + let dest = root.join(&req.name); + match self.create(&dest, &req) { + Ok(v) => ToolOutcome::Ok(v.to_string()), + Err(e) => ToolOutcome::Error(e), + } + } + + fn create(&self, dest: &Path, req: &Request) -> Result { + // `create_dir` (not `_all`): the root exists, and an existing folder is never reused. + std::fs::create_dir(dest) + .map_err(|e| format!("{} could not be created: {e}", dest.display()))?; + let rendered = match (&req.template, &self.cfg.renderer) { + (Some(t), Some(r)) => match r.render(t, &req.params, dest) { + Ok(report) => Some(report), + Err(e) => { + let _ = std::fs::remove_dir_all(dest); + return Err(format!( + "the {t} template could not be rendered, so no project was created: {e}" + )); + } + }, + _ => None, + }; + let canonical = std::fs::canonicalize(dest).map_err(|e| e.to_string())?; + let marker = ProjectMarker { + version: 1, + name: req.name.clone(), + root: canonical.display().to_string(), + created_by: "hermes".into(), + created_at: std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0), + template: req.template.clone(), + params: req.params.clone(), + }; + let marker_dir = canonical.join(MARKER_DIR); + let written = std::fs::create_dir_all(&marker_dir) + .and_then(|()| { + std::fs::write( + marker_dir.join(MARKER_FILE), + serde_json::to_vec_pretty(&marker).unwrap_or_default(), + ) + }) + .map_err(|e| format!("the project marker could not be written: {e}")); + if let Err(e) = written { + let _ = std::fs::remove_dir_all(&canonical); + return Err(e); + } + self.grants.add_project_root(&canonical)?; + Ok(serde_json::json!({ + "ok": true, + "tool": PROJECT_CREATE_TOOL, + "name": req.name, + "path": canonical, + "template": req.template, + "marker": marker_dir.join(MARKER_FILE), + "rendered": rendered, + "note": "This folder is a project: the file tools may create its build files (package.json, foundry.toml, remappings.txt, ...). Env files stay the member's.", + })) + } +} + +impl ToolHost for ProjectCreateHost { + fn execute(&self, call: &ToolCall) -> ToolOutcome { + let reason = match self.taint.record() { + Some(r) => format!( + "creating a project folder needs your approval, and this session read untrusted content (from {})", + r.source + ), + None => "creating a project folder needs your approval".to_string(), + }; + self.ask_and_create(call, &reason) + } + + fn honors_explicit_approval(&self) -> bool { + self.actions.is_some() + } + + /// The one card is the explicit approval; it carries the loop's reason. + fn execute_with_explicit_approval(&self, call: &ToolCall, reason: &str) -> ToolOutcome { + self.ask_and_create(call, reason) + } +} + +/// How the renderer runs in production: the OS sandbox when this machine has one. +pub fn production_render_sandbox() -> SandboxPolicy { + SandboxPolicy::new(SandboxMode::Preferred) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn names_are_one_plain_folder_name() { + for ok in ["demo", "my-token", "App_2", "v1.0", "a"] { + assert!(valid_name(ok).is_ok(), "{ok}"); + } + for bad in [ + "", "..", "../x", "a/b", "a\\b", ".hidden", "-x", "~x", "a..b", "a b", "a;b", "é", + "/abs", + ] { + assert!(valid_name(bad).is_err(), "{bad:?}"); + } + assert!(valid_name(&"x".repeat(MAX_NAME_LEN + 1)).is_err()); + } + + #[test] + fn parse_checks_template_and_params() { + assert!(parse( + r#"{"name":"d","template":"erc20","params":{"name":"Demo","symbol":"DMO"}}"# + ) + .is_ok()); + assert!(parse(r#"{"name":"d","template":"../x"}"#).is_err()); + assert!( + parse(r#"{"name":"d","params":{"a":"b"}}"#).is_err(), + "params need a template" + ); + assert!(parse(r#"{"name":"d","template":"erc20","params":{"a b":"c"}}"#).is_err()); + assert!(parse(r#"{"name":"d","template":"erc20","params":{"a":1}}"#).is_err()); + assert!(parse(r#"{"template":"erc20"}"#).is_err()); + assert!(parse("[]").is_err()); + } + + fn vars(pairs: &[(&str, &str)]) -> impl Fn(&str) -> Option { + let m: std::collections::HashMap = pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(); + move |k| m.get(k).cloned() + } + + #[test] + fn config_is_off_unless_the_flag_is_exactly_1_and_defaults_under_home() { + let off = SandboxPolicy::off; + assert!( + ProjectsConfig::from_env_vars(vars(&[("HOME", "/h")]), off()) + .unwrap() + .is_none() + ); + assert!(ProjectsConfig::from_env_vars( + vars(&[(PROJECTS_ENV, "true"), ("HOME", "/h")]), + off() + ) + .unwrap() + .is_none()); + let c = ProjectsConfig::from_env_vars(vars(&[(PROJECTS_ENV, "1"), ("HOME", "/h")]), off()) + .unwrap() + .unwrap(); + assert_eq!(c.root, PathBuf::from("/h/Citrate/Projects")); + assert!(c.renderer.is_none()); + let c = ProjectsConfig::from_env_vars( + vars(&[ + (PROJECTS_ENV, "1"), + (PROJECTS_ROOT_ENV, "/w/p"), + ("HOME", "/h"), + ]), + off(), + ) + .unwrap() + .unwrap(); + assert_eq!(c.root, PathBuf::from("/w/p")); + assert!(ProjectsConfig::from_env_vars( + vars(&[ + (PROJECTS_ENV, "1"), + (PROJECTS_ROOT_ENV, "rel"), + ("HOME", "/h") + ]), + off() + ) + .is_err()); + assert!( + ProjectsConfig::from_env_vars( + vars(&[ + (PROJECTS_ENV, "1"), + ("HOME", "/h"), + (TEMPLATE_RENDERER_ENV, "/bin/r") + ]), + off() + ) + .is_err(), + "a renderer needs the templates folder too" + ); + let c = ProjectsConfig::from_env_vars( + vars(&[ + (PROJECTS_ENV, "1"), + ("HOME", "/h"), + (TEMPLATE_RENDERER_ENV, "/opt/c/citrate-templates"), + (TEMPLATES_ROOT_ENV, "/opt/c/templates"), + ]), + off(), + ) + .unwrap() + .unwrap(); + assert!(c.renderer.is_some()); + assert!(ProjectsConfig::from_env_vars( + vars(&[ + (PROJECTS_ENV, "1"), + ("HOME", "/h"), + (TEMPLATE_RENDERER_ENV, "/opt/c/citrate-templates"), + (TEMPLATES_ROOT_ENV, "/opt/c/templates"), + (TEMPLATE_TIER_ENV, "T9"), + ]), + off() + ) + .is_err()); + } + + #[test] + fn the_spec_is_a_trusted_sidecar_write_tool() { + let s = project_create_spec(); + assert_eq!(s.name, PROJECT_CREATE_TOOL); + assert_eq!(s.host, HostKind::Sidecar); + assert_eq!(s.annotations.effect, Some(Effect::Write)); + assert_eq!(s.annotations.trust, Some(Trust::Trusted)); + assert_eq!(s.parameters["required"], serde_json::json!(["name"])); + } +} diff --git a/agent-sidecar/src/projects_tests.rs b/agent-sidecar/src/projects_tests.rs new file mode 100644 index 0000000..88f288f --- /dev/null +++ b/agent-sidecar/src/projects_tests.rs @@ -0,0 +1,464 @@ +//! HGW-3 — `project_create`: a new project folder in the member's projects folder, after the +//! member approves it, optionally rendered from a bundled template, that the file tools may then +//! set up (build configuration included). +//! +//! BDD map: +//! - `project_create_asks_then_creates_the_folder_and_marker_and_the_project_is_writable` +//! - `bad_names_are_refused_before_anything_is_asked` +//! - `an_existing_folder_is_never_reused` +//! - `the_projects_root_must_exist_and_be_granted` +//! - `a_declined_card_creates_nothing` +//! - `without_an_hic_aware_app_nothing_is_created` +//! - `in_a_tainted_session_one_card_carries_the_taint_reason` +//! - `a_template_is_rendered_into_the_new_folder_and_a_failed_render_leaves_nothing` +//! - `the_cli_renderer_calls_the_program_with_the_documented_argv` (+ Seatbelt on macOS) + +#![cfg(unix)] + +use super::hgw_support::*; +use super::projects::*; +use super::*; +use citrate_agent_loop::AssistantTurn; +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +fn projects_root(fx: &Fx) -> PathBuf { + let r = fx.home().join("Projects"); + std::fs::create_dir_all(&r).expect("projects root"); + r +} + +fn create_call(id: &str, args: serde_json::Value) -> citrate_agent_loop::ToolCall { + tool_call(id, PROJECT_CREATE_TOOL, args) +} + +/// One render request: the template, its parameters and the destination. +type Rendered = (String, BTreeMap, PathBuf); + +/// A renderer that writes `README.md` (and records what it was asked), or fails. +struct FakeRenderer { + fail: bool, + seen: std::sync::Mutex>, +} +impl TemplateRenderer for FakeRenderer { + fn render( + &self, + template: &str, + params: &BTreeMap, + dest: &Path, + ) -> Result { + self.seen.lock().unwrap_or_else(|p| p.into_inner()).push(( + template.into(), + params.clone(), + dest.to_path_buf(), + )); + // The destination is the new, empty folder. + assert_eq!(std::fs::read_dir(dest).map(|d| d.count()).unwrap_or(99), 0); + std::fs::write(dest.join("README.md"), format!("# {template}\n")) + .map_err(|e| e.to_string())?; + if self.fail { + return Err("unknown parameter `colour`".into()); + } + Ok(serde_json::json!({"files": ["README.md"]})) + } +} + +struct Run { + s: Arc, + answered: Vec, +} + +async fn run( + fx: &Fx, + calls: Vec, + cfg: ProjectsConfig, + roots: &[PathBuf], + hic_aware: bool, + decide: impl Fn(&'static str, &serde_json::Value) -> bool, +) -> Run { + let mut turns: Vec = calls + .into_iter() + .map(|c| AssistantTurn::tools(vec![c])) + .collect(); + turns.push(AssistantTurn::text("ok")); + let mgr = Arc::new(manager_with(fx, turns, Some(shell_open())).with_projects(Arc::new(cfg))); + let id = mgr + .create(create_req(fx.rw_doc_for(roots), hic_aware)) + .expect("session"); + let s = mgr.get(&id).expect("session"); + let app = app_for(mgr.clone()); + mgr.send(&id, "make a project".into(), None).expect("send"); + let answered = member_answers(&app, &s, &id, decide).await; + Run { s, answered } +} + +fn plain(root: PathBuf) -> ProjectsConfig { + ProjectsConfig { + root, + renderer: None, + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn project_create_asks_then_creates_the_folder_and_marker_and_the_project_is_writable() { + let fx = Fx::new("pc-create"); + let root = projects_root(&fx); + let demo = root.join("demo"); + let demo_shown = demo.clone(); + let r = run( + &fx, + vec![ + create_call("c1", serde_json::json!({"name": "demo"})), + fs_write("c2", &demo.join("package.json"), "{\"name\":\"demo\"}\n"), + ], + plain(root.clone()), + std::slice::from_ref(&root), + true, + move |route, card| { + if route == "actions" { + assert!( + !demo_shown.exists(), + "nothing is created before the member decides" + ); + } + let _ = card; + true + }, + ) + .await; + assert_eq!(r.answered.len(), 1, "{:?}", r.answered); + let card = &r.answered[0].card; + assert_eq!(card["tool"], PROJECT_CREATE_TOOL); + assert!( + card["summary"] + .as_str() + .unwrap_or_default() + .contains(&demo.display().to_string()), + "the card shows the exact folder: {card}" + ); + let res = result_of(&r.s, "c1").await; + assert_eq!(res["status"], "ok", "{res}"); + let body: serde_json::Value = + serde_json::from_str(res["content"].as_str().unwrap_or_default()).expect("json"); + assert_eq!(body["path"], serde_json::json!(demo)); + assert!(demo.is_dir()); + let marker: ProjectMarker = + serde_json::from_slice(&std::fs::read(demo.join(".citrate/project.json")).expect("marker")) + .expect("marker json"); + assert_eq!(marker.name, "demo"); + assert_eq!(marker.root, demo.display().to_string()); + assert_eq!(marker.created_by, "hermes"); + // The new folder is a project root: its build configuration is the agent's to write. + assert_eq!(result_of(&r.s, "c2").await["status"], "ok"); + assert!(demo.join("package.json").is_file()); + assert_eq!(r.s.grants().expect("grants").project_roots(), vec![demo]); +} + +#[tokio::test(flavor = "multi_thread")] +async fn bad_names_are_refused_before_anything_is_asked() { + let fx = Fx::new("pc-names"); + let root = projects_root(&fx); + let long = "x".repeat(65); + let names = ["../escape", "a/b", ".hidden", "", "..", long.as_str()]; + let calls = names + .iter() + .enumerate() + .map(|(i, n)| create_call(&format!("c{i}"), serde_json::json!({ "name": n }))) + .collect(); + let r = run( + &fx, + calls, + plain(root.clone()), + std::slice::from_ref(&root), + true, + |_, _| true, + ) + .await; + assert!(r.answered.is_empty(), "{:?}", r.answered); + for i in 0..names.len() { + let res = result_of(&r.s, &format!("c{i}")).await; + assert_eq!(res["status"], "error", "{res}"); + } + assert!(!fx.home().join("escape").exists()); + assert_eq!(std::fs::read_dir(&root).expect("root").count(), 0); +} + +#[tokio::test(flavor = "multi_thread")] +async fn an_existing_folder_is_never_reused() { + let fx = Fx::new("pc-exists"); + let root = projects_root(&fx); + std::fs::create_dir_all(root.join("demo")).expect("existing"); + std::fs::write(root.join("demo/keep.txt"), "mine").expect("keep"); + let r = run( + &fx, + vec![create_call("c1", serde_json::json!({"name": "demo"}))], + plain(root.clone()), + std::slice::from_ref(&root), + true, + |_, _| true, + ) + .await; + assert!(r.answered.is_empty()); + let res = result_of(&r.s, "c1").await; + assert!( + res["content"] + .as_str() + .unwrap_or_default() + .contains("already exists"), + "{res}" + ); + assert!(!root.join("demo/.citrate").exists()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn the_projects_root_must_exist_and_be_granted() { + let fx = Fx::new("pc-root"); + let missing = fx.home().join("NotThere"); + let r = run( + &fx, + vec![create_call("c1", serde_json::json!({"name": "demo"}))], + plain(missing.clone()), + &[fx.proj()], + true, + |_, _| true, + ) + .await; + let res = result_of(&r.s, "c1").await; + assert!( + res["content"] + .as_str() + .unwrap_or_default() + .contains("does not exist"), + "{res}" + ); + assert!(!missing.exists()); + // Exists, but is not granted. + let root = projects_root(&fx); + let r = run( + &fx, + vec![create_call("c1", serde_json::json!({"name": "demo"}))], + plain(root.clone()), + &[fx.proj()], + true, + |_, _| true, + ) + .await; + let res = result_of(&r.s, "c1").await; + assert!( + res["content"] + .as_str() + .unwrap_or_default() + .contains("not granted"), + "{res}" + ); + assert!(r.answered.is_empty()); + assert!(!root.join("demo").exists()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_declined_card_creates_nothing() { + let fx = Fx::new("pc-decline"); + let root = projects_root(&fx); + let r = run( + &fx, + vec![create_call("c1", serde_json::json!({"name": "demo"}))], + plain(root.clone()), + std::slice::from_ref(&root), + true, + |_, _| false, + ) + .await; + assert_eq!(r.answered.len(), 1); + assert_eq!(result_of(&r.s, "c1").await["status"], "denied"); + assert!(!root.join("demo").exists()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn without_an_hic_aware_app_nothing_is_created() { + let fx = Fx::new("pc-headless"); + let root = projects_root(&fx); + let r = run( + &fx, + vec![create_call("c1", serde_json::json!({"name": "demo"}))], + plain(root.clone()), + std::slice::from_ref(&root), + false, + |_, _| true, + ) + .await; + let res = result_of(&r.s, "c1").await; + assert_eq!(res["status"], "denied", "{res}"); + assert!(res["content"] + .as_str() + .unwrap_or_default() + .contains("no member can be asked")); + assert!(!root.join("demo").exists()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn in_a_tainted_session_one_card_carries_the_taint_reason() { + let fx = Fx::new("pc-taint"); + let root = projects_root(&fx); + let r = run( + &fx, + vec![ + shell("c0", &["echo", "x"], &root), + create_call("c1", serde_json::json!({"name": "demo"})), + ], + plain(root.clone()), + std::slice::from_ref(&root), + true, + |_, _| true, + ) + .await; + let actions: Vec<&Answered> = r.answered.iter().filter(|a| a.route == "actions").collect(); + assert_eq!(actions.len(), 1, "one card, not two: {:?}", r.answered); + let reason = actions[0].card["hicReason"].as_str().unwrap_or_default(); + assert!(reason.contains("shell_run"), "{reason}"); + assert_eq!(result_of(&r.s, "c1").await["status"], "ok"); + assert!(root.join("demo/.citrate/project.json").is_file()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_template_is_rendered_into_the_new_folder_and_a_failed_render_leaves_nothing() { + let fx = Fx::new("pc-template"); + let root = projects_root(&fx); + let ok = Arc::new(FakeRenderer { + fail: false, + seen: Default::default(), + }); + let r = run( + &fx, + vec![create_call( + "c1", + serde_json::json!({"name": "tok", "template": "erc20", "params": {"name": "Demo", "symbol": "DMO"}}), + )], + ProjectsConfig { + root: root.clone(), + renderer: Some(ok.clone()), + }, + std::slice::from_ref(&root), + true, + |_, _| true, + ) + .await; + let res = result_of(&r.s, "c1").await; + assert_eq!(res["status"], "ok", "{res}"); + assert!(r.answered[0].card["summary"] + .as_str() + .unwrap_or_default() + .contains("erc20")); + let seen = ok.seen.lock().expect("seen").clone(); + assert_eq!(seen.len(), 1); + assert_eq!(seen[0].0, "erc20"); + assert_eq!(seen[0].1.get("symbol").map(String::as_str), Some("DMO")); + assert_eq!(seen[0].2, root.join("tok")); + assert!(root.join("tok/README.md").is_file()); + let marker: ProjectMarker = + serde_json::from_slice(&std::fs::read(root.join("tok/.citrate/project.json")).expect("m")) + .expect("marker"); + assert_eq!(marker.template.as_deref(), Some("erc20")); + // A render that fails removes the new folder and says why. + let bad = Arc::new(FakeRenderer { + fail: true, + seen: Default::default(), + }); + let r = run( + &fx, + vec![create_call( + "c1", + serde_json::json!({"name": "broken", "template": "erc20"}), + )], + ProjectsConfig { + root: root.clone(), + renderer: Some(bad), + }, + std::slice::from_ref(&root), + true, + |_, _| true, + ) + .await; + let res = result_of(&r.s, "c1").await; + assert_eq!(res["status"], "error", "{res}"); + assert!( + res["content"] + .as_str() + .unwrap_or_default() + .contains("colour"), + "{res}" + ); + assert!(!root.join("broken").exists()); + // Without a renderer a template is refused before asking. + let r = run( + &fx, + vec![create_call( + "c1", + serde_json::json!({"name": "t2", "template": "erc20"}), + )], + plain(root.clone()), + std::slice::from_ref(&root), + true, + |_, _| true, + ) + .await; + assert!(r.answered.is_empty()); + assert!(!root.join("t2").exists()); +} + +/// A stand-in for citrate-core's `citrate-templates` program: records its argv and writes a file. +fn fake_cli(dir: &Path) -> PathBuf { + use std::os::unix::fs::PermissionsExt; + let bin = dir.join("citrate-templates"); + std::fs::write( + &bin, + "#!/bin/sh\nout=''\nprev=''\nfor a in \"$@\"; do if [ \"$prev\" = --out ]; then out=\"$a\"; fi; prev=\"$a\"; done\nprintf '%s\\n' \"$@\" > \"$out/argv.txt\"\necho '{\"files\":[\"argv.txt\"]}'\n", + ) + .expect("script"); + std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)).expect("chmod"); + bin +} + +fn check_cli(sandbox: citrate_agent_shell::sandbox::SandboxPolicy) { + let fx = Fx::new("pc-cli"); + let tools = fx.base.join("app"); + let templates = tools.join("templates"); + std::fs::create_dir_all(&templates).expect("templates"); + let bin = fake_cli(&tools); + let dest = fx.proj().join("new"); + std::fs::create_dir(&dest).expect("dest"); + let r = CliTemplateRenderer::new(bin, templates.clone(), "T1", sandbox).expect("renderer"); + let mut params = BTreeMap::new(); + params.insert("name".to_string(), "Demo".to_string()); + let report = r.render("erc20", ¶ms, &dest).expect("render"); + assert_eq!(report["files"], serde_json::json!(["argv.txt"])); + let argv = std::fs::read_to_string(dest.join("argv.txt")).expect("argv"); + let lines: Vec<&str> = argv.lines().collect(); + assert_eq!( + lines, + vec![ + "render", + "--root", + &templates.display().to_string(), + "--template", + "erc20", + "--tier", + "T1", + "--out", + &dest.display().to_string(), + "--param", + "name=Demo", + ] + ); +} + +#[test] +fn the_cli_renderer_calls_the_program_with_the_documented_argv() { + check_cli(citrate_agent_shell::sandbox::SandboxPolicy::off()); +} + +#[cfg(target_os = "macos")] +#[test] +fn the_cli_renderer_runs_under_seatbelt_and_writes_only_the_new_folder() { + use citrate_agent_shell::sandbox::{SandboxMode, SandboxPolicy}; + check_cli(SandboxPolicy::new(SandboxMode::Required)); +} diff --git a/agent-sidecar/src/sessions.rs b/agent-sidecar/src/sessions.rs index bdbb552..e81aef7 100644 --- a/agent-sidecar/src/sessions.rs +++ b/agent-sidecar/src/sessions.rs @@ -72,6 +72,10 @@ //! carry the same reason. Each approval releases one call and the session stays tainted. //! Without `hicAware` nobody can be asked, so such calls are declined with //! [`citrate_agent_loop::cannot_ask_reason`]. +//! - HGW-3: when projects are on (`CITRATE_HERMES_PROJECTS=1`, default off), every session opened +//! with folder grants also offers the sidecar-hosted `project_create` ([`crate::projects`]): +//! it creates `/` after the member approves the action card, and the new +//! folder becomes one of the session's project roots. The name is reserved while it is on. use std::collections::{HashMap, VecDeque}; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; @@ -975,6 +979,8 @@ pub(crate) struct SidecarHost { pub(crate) actions: Option>, pub(crate) taint: TaintState, pub(crate) stop: StopFlag, + /// HGW-3: `project_create`, when projects are on and the session has grants. + pub(crate) projects: Option, } impl ToolHost for SidecarHost { @@ -984,6 +990,11 @@ impl ToolHost for SidecarHost { return s.execute(call); } } + if crate::projects::ProjectCreateHost::handles(&call.name) { + if let Some(p) = &self.projects { + return p.execute(call); + } + } if browser_tools::handles(&call.name) { if let Some(b) = &self.browser { return b.execute(call); @@ -1066,6 +1077,14 @@ impl ToolHost for SidecarHost { None => cannot_ask(), }; } + if crate::projects::ProjectCreateHost::handles(&call.name) { + return match &self.projects { + Some(p) if p.honors_explicit_approval() => { + p.execute_with_explicit_approval(call, reason) + } + _ => cannot_ask(), + }; + } if let Some(m) = &self.mcp { if m.handles(&call.name) { return if m.honors_explicit_approval() { @@ -1144,6 +1163,8 @@ pub struct SessionManager { uses: AtomicU64, /// US-2.2 AC2: `shell_run` for sessions opened with folder grants (default off). shell_run: Option>, + /// HGW-3: `project_create` for sessions opened with folder grants (default off). + projects: Option>, /// HUP-S2.6: the one writer of the decision records the nightly anchor batches. records: Option>, /// US-1.3 AC2: ask the model for a self-review of every workflow step attempt and record it @@ -1179,6 +1200,7 @@ impl SessionManager { mcp_registry: None, uses: AtomicU64::new(0), shell_run: None, + projects: None, records: None, self_review: false, } @@ -1226,6 +1248,12 @@ impl SessionManager { self } + /// HGW-3: offer `project_create` to every new session opened with folder grants. + pub fn with_projects(mut self, cfg: Arc) -> Self { + self.projects = Some(cfg); + self + } + /// Whether `shell_run` is on. pub fn shell_run_enabled(&self) -> bool { self.shell_run.is_some() @@ -1625,6 +1653,16 @@ impl SessionManager { } specs.push(crate::learn::learn_propose_spec()); } + if self.projects.is_some() + && specs + .iter() + .any(|t| t.name == crate::projects::PROJECT_CREATE_TOOL) + { + return Err(SessionError::Invalid(format!( + "the tool name '{}' is reserved by the sidecar while projects are on", + crate::projects::PROJECT_CREATE_TOOL + ))); + } if self.shell_run.is_some() && specs.iter().any(|t| t.name == SHELL_RUN_TOOL) { return Err(SessionError::Invalid(format!( "the tool name '{SHELL_RUN_TOOL}' is reserved by the sidecar while shell_run is on" @@ -1682,6 +1720,9 @@ impl SessionManager { Some(Arc::new(g)) } }; + if self.projects.is_some() && grants.is_some() { + specs.push(crate::projects::project_create_spec()); + } let shell = match (&self.shell_run, &grants) { (Some(cfg), Some(g)) => { specs.push(shell_run_spec()); @@ -2007,8 +2048,19 @@ impl SessionManager { session.hic_aware, ) }); + let projects_host = match (&self.projects, &session.grants) { + (Some(cfg), Some(g)) => Some(crate::projects::ProjectCreateHost::new( + cfg.clone(), + g.clone(), + session.actions.clone(), + session.taint.clone(), + session.stop.clone(), + )), + _ => None, + }; if file_host.is_some() || shell_host.is_some() + || projects_host.is_some() || skill_host.is_some() || capsule_host.is_some() || toolchain.is_some() @@ -2035,6 +2087,7 @@ impl SessionManager { actions: session.actions.clone(), taint: session.taint.clone(), stop: session.stop.clone(), + projects: projects_host, }), ); } From 7c96c680438527a08458f7b0ceb29a288067c1a8 Mon Sep 17 00:00:00 2001 From: Larry Klosowski Date: Thu, 15 Oct 2026 12:29:26 -0700 Subject: [PATCH 05/12] feat(shell): .env.example and .env.sample stay readable in the sandbox (HGW-2, #75) Per the HGW-W2 issue (#75): the Seatbelt `.env*` deny and the bwrap `.env*` masks exempt the templates' `.env.example` and `.env.sample` (not secrets; agent-guard exempts them the same way). `.env` and `.env.local` stay neither readable nor writable. Tests: sandbox env_files unit extended; real-Seatbelt test (+1). Workspace: 2267 -> 2268 passed. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_018wWZZ8GRVU9USh3kKQHsFe --- agent-shell/src/sandbox.rs | 18 ++++++++++++++---- agent-shell/tests/sandbox.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/agent-shell/src/sandbox.rs b/agent-shell/src/sandbox.rs index 837d5ed..0232613 100644 --- a/agent-shell/src/sandbox.rs +++ b/agent-shell/src/sandbox.rs @@ -11,7 +11,8 @@ //! (HGW-2: `git init`, `git config` and `git commit` work), but a hook file other than git's own //! `*.sample` files cannot be written, the hooks folder cannot be removed or moved, and a //! symbolic link cannot be put in its place (a hook planted there would run later outside the -//! sandbox). `.env*` files are neither readable nor writable. Configuration that makes git run +//! sandbox). `.env*` files are neither readable nor writable, except `.env.example` and +//! `.env.sample`. Configuration that makes git run //! a program cannot be filtered by path, so the runner reports any it finds new after a run //! (`RunReport::git_warnings`). //! - **Linux: bubblewrap** (`bwrap`, found only in fixed system directories). The run gets fresh @@ -378,7 +379,11 @@ pub fn seatbelt_profile(n_read: usize, n_write: usize) -> String { p.push_str( "(deny file-write-create (require-all (regex #\"/\\.git/hooks$\") (vnode-type SYMLINK)))\n", ); - p.push_str("(deny file-read* file-write* (regex #\"/\\.env[^/]*$\"))\n"); + // `.env*` files are neither readable nor writable, except the templates' `.env.example` and + // `.env.sample` (HGW-W2: not secrets; agent-guard exempts them the same way). + p.push_str( + "(deny file-read* file-write* (require-all (regex #\"/\\.env[^/]*$\") (require-not (regex #\"/\\.env\\.(example|sample)$\"))))\n", + ); p } @@ -460,7 +465,8 @@ pub fn bwrap_command(spec: &SandboxSpec, program: &Path, args: &[String]) -> Vec a } -/// `.env*` files at the top of each write root (bwrap masks them; Seatbelt denies by pattern). +/// `.env*` files at the top of each write root (bwrap masks them; Seatbelt denies by pattern), +/// except the templates' `.env.example` and `.env.sample`. pub(crate) fn env_files(write_roots: &[PathBuf]) -> Vec { let mut out = Vec::new(); for w in write_roots { @@ -470,7 +476,8 @@ pub(crate) fn env_files(write_roots: &[PathBuf]) -> Vec { for e in rd.flatten() { let name = e.file_name().to_string_lossy().to_ascii_lowercase(); let is_file = e.file_type().map(|t| t.is_file()).unwrap_or(false); - if is_file && (name == ".env" || name.starts_with(".env.")) { + let template = name == ".env.example" || name == ".env.sample"; + if is_file && !template && (name == ".env" || name.starts_with(".env.")) { out.push(e.path()); } } @@ -567,6 +574,9 @@ mod tests { std::fs::write(root.join(".env"), "x").expect("w"); std::fs::write(root.join(".env.local"), "x").expect("w"); std::fs::write(root.join("env.txt"), "x").expect("w"); + // HGW-W2 (#75): templates' example files are not secrets (agent-guard exempts them too). + std::fs::write(root.join(".env.example"), "x").expect("w"); + std::fs::write(root.join(".env.sample"), "x").expect("w"); std::fs::create_dir(root.join(".env.d")).expect("d"); let got = env_files(std::slice::from_ref(&root)); assert_eq!(got, vec![root.join(".env"), root.join(".env.local")]); diff --git a/agent-shell/tests/sandbox.rs b/agent-shell/tests/sandbox.rs index a6ed660..84f02cb 100644 --- a/agent-shell/tests/sandbox.rs +++ b/agent-shell/tests/sandbox.rs @@ -493,6 +493,34 @@ mod macos { assert!(hooks.join("pre-commit.sample").exists() || hooks.is_dir()); } + /// HGW-W2 (#75): `.env.example` and `.env.sample` (template files, not secrets) are readable; + /// `.env` and `.env.local` stay unreadable and unwritable. + #[test] + fn env_example_files_are_readable_and_real_env_files_are_not() { + let fx = Fx::new(); + for (name, body) in [ + (".env.example", "RPC_URL=\n"), + (".env.sample", "KEY=\n"), + (".env", "SECRET=1\n"), + (".env.local", "SECRET=2\n"), + ] { + std::fs::write(fx.grant().join(name), body).expect("seed"); + } + let r = scoped(fx.grant(), required()); + let rep = r + .run(&sh("cat .env.example .env.sample", &fx.grant())) + .expect("run"); + assert_eq!(rep.exit_code, Some(0), "{}", rep.stderr); + assert_eq!(rep.stdout, "RPC_URL=\nKEY=\n"); + for secret in [".env", ".env.local"] { + let rep = r + .run(&sh(&format!("cat {secret}"), &fx.grant())) + .expect("run"); + assert_ne!(rep.exit_code, Some(0), "{secret}"); + assert!(!rep.stdout.contains("SECRET"), "{secret}"); + } + } + #[test] fn plan_shows_the_sandbox_before_anything_runs() { let fx = Fx::new(); From 368b87bbb6db1788e925e174a5f384f60932ffaa Mon Sep 17 00:00:00 2001 From: Larry Klosowski Date: Thu, 15 Oct 2026 12:18:18 -0700 Subject: [PATCH 06/12] feat(sidecar): auto-approve list for sandboxed project commands + e2e (HGW-5) Owner decision 2026-10-15 (HGW (3), D-16 as amended): sandboxed, offline, project-scoped read and build commands run without a card. - The list is data: agent-sidecar/policy/auto_approve.toml (compiled in; CITRATE_HERMES_AUTO_APPROVE=0 turns it off, an absolute path replaces it). Rules are argv templates (program, subcommands, flags, value flags with kinds, required flags, operand kind and count). The shipped list: ls, pwd, cat, head, tail, wc, node , npm run