From fa4ff042b19e48ffe0daa361debb2deba370fb5c Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 1 Aug 2026 11:20:02 +0200 Subject: [PATCH] fix(eval): scope external requester inbox authority agent-tool: Codex CLI agent-tool-version: 0.145.0 agent-runtime: Codex CLI 0.145.0 agent-session-lookup: sha256:ad381a588faa911d70bf08ee2ae4305e96d980701e6405897e32fe38c3163d98 tooling-profile: dotfiles@de765ec --- src/eval_run.rs | 30 +++++++++------ src/main.rs | 22 ++++++++--- src/message.rs | 89 +++++++++++++++++++++++++++++++++++++------ tests/eval_run_e2e.rs | 9 +---- tests/message_cli.rs | 37 ++++++++++++++---- 5 files changed, 142 insertions(+), 45 deletions(-) diff --git a/src/eval_run.rs b/src/eval_run.rs index c29e7190..8c95794a 100644 --- a/src/eval_run.rs +++ b/src/eval_run.rs @@ -221,7 +221,7 @@ fn load_canonical_eval_team(catalog: &Path, host: &str) -> Result//inbox. Status lives beside it either way. let agent_dir = message::resolve_agent_dir(&catalog_root, &id, &this_host) .unwrap_or_else(|| catalog_root.join(&id)); - let inbox = message::resolve_inbox(&catalog_root, &id, &this_host)?; + let inbox = resolve_message_inbox(&catalog_root, &id, &this_host)?; let status_path = st2::status::status_path(&agent_dir); eprintln!( "st2 ding: watching {}'s inbox ({}) → poking pty '{session}'", @@ -1402,6 +1402,16 @@ fn agent_dir_of(root: &Path, id: &str, host: &str) -> Result { .with_context(|| format!("no agent '{id}' found in catalog {}", root.display())) } +/// Resolve ordinary declared messaging authority plus the exact external requester capability +/// injected only into canonical eval seats. +fn resolve_message_inbox(root: &Path, id: &str, host: &str) -> Result { + let external = std::env::var("ST2_EVAL_REQUESTER") + .ok() + .map(|identity| message::ExternalInbox::new(root, &identity)) + .transpose()?; + message::resolve_inbox_with_external(root, id, host, external.as_ref()) +} + /// Body from `-m`, else stdin (so `st2 message send x < file` works). fn body_or_stdin(body: Option) -> Result { match body { @@ -1439,7 +1449,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { let (root, host) = resolve_ctx(&ctx)?; let from = acting_id(&ctx)?; let body = body_or_stdin(body)?; - let dir = message::resolve_inbox(&root, &to, &host)?; + let dir = resolve_message_inbox(&root, &to, &host)?; let filename = message::send_to_inbox( &dir, &from, @@ -1459,7 +1469,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { } => { let (root, host) = resolve_ctx(&ctx)?; let from = acting_id(&ctx)?; - let my_inbox = message::resolve_inbox(&root, &from, &host)?; + let my_inbox = resolve_message_inbox(&root, &from, &host)?; let original = message::read_msg(&my_inbox, &filename) .with_context(|| format!("no message '{filename}' in {}'s inbox", from))?; let to = original @@ -1468,7 +1478,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { .with_context(|| format!("message '{filename}' has no `from` to reply to"))?; let subject = subject.or_else(|| message::reply_subject(original.subject.as_deref())); let body = body_or_stdin(body)?; - let dir = message::resolve_inbox(&root, &to, &host)?; + let dir = resolve_message_inbox(&root, &to, &host)?; let sent = message::send_to_inbox( &dir, &from, @@ -1546,7 +1556,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { let dir = if archive { message::resolve_archive(&root, &id, &host) } else { - message::resolve_inbox(&root, &id, &host) + resolve_message_inbox(&root, &id, &host) }?; if raw { print!("{}", std::fs::read_to_string(dir.join(&filename))?); @@ -1574,7 +1584,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> { MessageCmd::Archive { first, second, ctx } => { let (root, host) = resolve_ctx(&ctx)?; let (id, filename) = box_target(first, second, &ctx)?; - let inbox = message::resolve_inbox(&root, &id, &host)?; + let inbox = resolve_message_inbox(&root, &id, &host)?; let archive = message::resolve_archive(&root, &id, &host)?; message::archive_msg(&inbox, &archive, &filename)?; println!("archived"); diff --git a/src/message.rs b/src/message.rs index 25eb2c94..f6520a4a 100644 --- a/src/message.rs +++ b/src/message.rs @@ -14,7 +14,7 @@ use std::collections::{HashMap, HashSet}; use std::fs; use std::io::Read; -use std::path::{Path, PathBuf}; +use std::path::{Component, Path, PathBuf}; use std::sync::atomic::{AtomicU64, Ordering}; use std::time::{SystemTime, UNIX_EPOCH}; @@ -314,19 +314,65 @@ pub fn archive_dir(agent_dir: &Path) -> PathBuf { agent_dir.join("resources").join("archive") } -/// Resolve an inbox by stable identity. A proven catalog-less root retains the legacy flat bus. -/// Inside a catalog, an absent identity fails closed unless a real flat inbox was explicitly -/// provisioned, as eval does for its external requester. +/// Eval-owned authority for one external flat requester mailbox. General catalog routing remains +/// declaration-only; possessing this value is the explicit exception at message call sites. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ExternalInbox { + root: PathBuf, + identity: String, + inbox: PathBuf, +} + +impl ExternalInbox { + pub fn new(root: &Path, identity: &str) -> anyhow::Result { + let mut components = Path::new(identity).components(); + let safe = matches!(components.next(), Some(Component::Normal(component)) if component == identity) + && components.next().is_none(); + anyhow::ensure!(safe, "external requester identity must be one non-empty relative path component"); + Ok(Self { + root: root.to_path_buf(), + identity: identity.to_owned(), + inbox: root.join(identity).join("inbox"), + }) + } + + pub fn provision(root: &Path, identity: &str) -> anyhow::Result { + let external = Self::new(root, identity)?; + fs::create_dir_all(&external.inbox).map_err(|error| { + anyhow::anyhow!( + "provisioning external requester {identity:?} inbox {}: {error}", + external.inbox.display() + ) + })?; + Ok(external) + } +} + +/// Resolve an inbox by stable identity. A proven catalog-less root retains the legacy flat bus; +/// inside a catalog an absent identity always fails closed. pub fn resolve_inbox(root: &Path, id: &str, host: &str) -> anyhow::Result { - match resolve_list_box(root, id, host, false, false) { + resolve_list_box(root, id, host, false, false) +} + +/// Resolve a normal declared inbox or one exact eval-owned external requester capability. +pub fn resolve_inbox_with_external( + root: &Path, + id: &str, + host: &str, + external: Option<&ExternalInbox>, +) -> anyhow::Result { + match resolve_inbox(root, id, host) { Ok(inbox) => Ok(inbox), - Err(error) => { - let flat = root.join(id).join("inbox"); - match fs::symlink_metadata(&flat) { - Ok(metadata) if metadata.file_type().is_dir() => Ok(flat), - _ => Err(error), + Err(error) => match external { + Some(external) + if external.root == root + && external.identity == id + && external.inbox.is_dir() => + { + Ok(external.inbox.clone()) } - } + _ => Err(error), + }, } } @@ -675,8 +721,27 @@ mod tests { ); assert!(resolve_inbox(root, "Shared Worker", "h").is_err()); + let external = ExternalInbox::new(root, "requester").unwrap(); + assert!(resolve_inbox_with_external(root, "requester", "h", Some(&external)).is_err()); + let requester = root.join("requester").join("inbox"); std::fs::create_dir_all(&requester).unwrap(); - assert_eq!(resolve_inbox(root, "requester", "h").unwrap(), requester); + assert!(resolve_inbox(root, "requester", "h").is_err()); + assert_eq!( + resolve_inbox_with_external(root, "requester", "h", Some(&external)).unwrap(), + requester + ); + assert!(resolve_inbox_with_external(root, "missing", "h", Some(&external)).is_err()); + } + + #[test] + fn external_inbox_rejects_unsafe_or_nested_identities() { + let tmp = tempfile::tempdir().unwrap(); + for identity in ["", ".", "..", "nested/requester", "../requester", "/requester"] { + assert!( + ExternalInbox::new(tmp.path(), identity).is_err(), + "accepted unsafe external identity {identity:?}" + ); + } } } diff --git a/tests/eval_run_e2e.rs b/tests/eval_run_e2e.rs index 5dc150ed..d6a4928c 100644 --- a/tests/eval_run_e2e.rs +++ b/tests/eval_run_e2e.rs @@ -271,6 +271,7 @@ eval { fixture.join("agents/evalhost/worker/agent.kdl"), r#"agent "worker" { identity "worker" + name "requester" host "evalhost" workspace "$CATALOG/worker" supervisor "sup" @@ -802,14 +803,6 @@ fn canonical_agents_fail_closed_matrix_is_pre_spawn_and_non_vacuous() { r#"agent "worker" { identity "worker"; host "evalhost"; pty "agent" { id ""; command "touch \"$CATALOG/SPAWNED\"; sleep 60" } }"#, )], ), - ( - "presentation name", - "evalhost.worker", - vec![( - "worker", - r#"agent "worker" { identity "worker"; name "requester"; host "evalhost"; argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60" }"#, - )], - ), ( "duplicate canonical route", "worker", diff --git a/tests/message_cli.rs b/tests/message_cli.rs index dde32fb5..bd4352ba 100644 --- a/tests/message_cli.rs +++ b/tests/message_cli.rs @@ -196,16 +196,19 @@ fn known_empty_native_and_catalog_less_flat_boxes_remain_valid() { #[test] fn send_routes_only_by_stable_identity_in_a_catalog_and_preserves_catalogless_bus() { - let send = |root: &Path, recipient: &str, root_flag: &str| { - let mut child = Command::new(env!("CARGO_BIN_EXE_st2")) + let send = |root: &Path, recipient: &str, root_flag: &str, external: Option<&str>| { + let mut command = Command::new(env!("CARGO_BIN_EXE_st2")); + command .args(["message", "send", recipient, root_flag]) .arg(root) .args(["--host", "h", "--as", "h.sender"]) .stdin(Stdio::piped()) .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .unwrap(); + .stderr(Stdio::piped()); + if let Some(identity) = external { + command.env("ST2_EVAL_REQUESTER", identity); + } + let mut child = command.spawn().unwrap(); child.stdin.take().unwrap().write_all(b"work\n").unwrap(); child.wait_with_output().unwrap() }; @@ -222,7 +225,7 @@ fn send_routes_only_by_stable_identity_in_a_catalog_and_preserves_catalogless_bu ) .unwrap(); - let display = send(catalog.path(), "Shared Worker", "--catalog"); + let display = send(catalog.path(), "Shared Worker", "--catalog", None); assert!(!display.status.success()); assert!( String::from_utf8_lossy(&display.stderr) @@ -230,7 +233,7 @@ fn send_routes_only_by_stable_identity_in_a_catalog_and_preserves_catalogless_bu ); assert!(!catalog.path().join("Shared Worker").exists()); - let stable = send(catalog.path(), "h.worker", "--catalog"); + let stable = send(catalog.path(), "h.worker", "--catalog", None); assert!( stable.status.success(), "{}", @@ -243,8 +246,26 @@ fn send_routes_only_by_stable_identity_in_a_catalog_and_preserves_catalogless_bu 1 ); + fs::create_dir_all(catalog.path().join("requester/inbox")).unwrap(); + assert!(!send(catalog.path(), "requester", "--catalog", None).status.success()); + assert!(!send(catalog.path(), "requester", "--catalog", Some("other")) + .status + .success()); + let external = send(catalog.path(), "requester", "--catalog", Some("requester")); + assert!( + external.status.success(), + "{}", + String::from_utf8_lossy(&external.stderr) + ); + assert_eq!( + fs::read_dir(catalog.path().join("requester/inbox")) + .unwrap() + .count(), + 1 + ); + let flat = tempfile::tempdir().unwrap(); - let raw = send(flat.path(), "requester", "--root"); + let raw = send(flat.path(), "requester", "--root", None); assert!( raw.status.success(), "{}",