Skip to content

Commit fa4ff04

Browse files
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
1 parent 96a7351 commit fa4ff04

5 files changed

Lines changed: 142 additions & 45 deletions

File tree

src/eval_run.rs

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ fn load_canonical_eval_team(catalog: &Path, host: &str) -> Result<CanonicalEvalT
221221
anyhow::bail!("canonical-agents Agent Spec `{bus_id}` is not runnable");
222222
}
223223
for task in &spec.tasks {
224-
for root in ["CATALOG", "ST_ROOT", "PTY_ROOT"] {
224+
for root in ["CATALOG", "ST_ROOT", "PTY_ROOT", "ST2_EVAL_REQUESTER"] {
225225
if task.env.contains_key(root) {
226226
anyhow::bail!(
227227
"canonical-agents Agent Spec `{bus_id}` must not override eval-owned `{root}`"
@@ -1092,17 +1092,13 @@ fn run_eval_inner(spec: &Spec, eval: &Eval, spec_dir: &Path, catalog: &Path, hos
10921092
"canonical-agents requester `{requester}` must be external to the admitted Agent Specs"
10931093
);
10941094
}
1095-
if let Some(owner) = specs
1096-
.iter()
1097-
.find(|spec| spec.name.as_deref() == Some(requester.as_str()))
1098-
{
1099-
anyhow::bail!(
1100-
"canonical-agents requester `{requester}` matches the presentation name of admitted Agent Spec `{}`",
1101-
owner.bus_id(host)
1102-
);
1095+
crate::message::ExternalInbox::provision(&bus, &requester)?;
1096+
for spec in &mut specs {
1097+
for task in &mut spec.tasks {
1098+
task.env
1099+
.insert("ST2_EVAL_REQUESTER".to_owned(), requester.clone());
1100+
}
11031101
}
1104-
std::fs::create_dir_all(bus.join(&requester).join("inbox"))
1105-
.with_context(|| format!("provisioning external requester `{requester}` inbox"))?;
11061102
}
11071103

11081104
eval_log!("== boot team ({} agents) ==", specs.len());
@@ -1677,6 +1673,18 @@ agent "worker" { identity "worker"; host "evalhost"; argv "true" }
16771673
host "evalhost"
16781674
workspace "$CATALOG/missing"
16791675
argv "true"
1676+
}"#,
1677+
)],
1678+
),
1679+
(
1680+
"ST2_EVAL_REQUESTER",
1681+
vec![(
1682+
"agents/evalhost/worker/agent.kdl",
1683+
r#"agent "worker" {
1684+
identity "worker"
1685+
host "evalhost"
1686+
env { ST2_EVAL_REQUESTER "shadow-requester" }
1687+
argv "true"
16801688
}"#,
16811689
)],
16821690
),

src/main.rs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1363,7 +1363,7 @@ fn ding_cmd(
13631363
// ST_ROOT) → the flat <root>/<id>/inbox. Status lives beside it either way.
13641364
let agent_dir = message::resolve_agent_dir(&catalog_root, &id, &this_host)
13651365
.unwrap_or_else(|| catalog_root.join(&id));
1366-
let inbox = message::resolve_inbox(&catalog_root, &id, &this_host)?;
1366+
let inbox = resolve_message_inbox(&catalog_root, &id, &this_host)?;
13671367
let status_path = st2::status::status_path(&agent_dir);
13681368
eprintln!(
13691369
"st2 ding: watching {}'s inbox ({}) → poking pty '{session}'",
@@ -1402,6 +1402,16 @@ fn agent_dir_of(root: &Path, id: &str, host: &str) -> Result<PathBuf> {
14021402
.with_context(|| format!("no agent '{id}' found in catalog {}", root.display()))
14031403
}
14041404

1405+
/// Resolve ordinary declared messaging authority plus the exact external requester capability
1406+
/// injected only into canonical eval seats.
1407+
fn resolve_message_inbox(root: &Path, id: &str, host: &str) -> Result<PathBuf> {
1408+
let external = std::env::var("ST2_EVAL_REQUESTER")
1409+
.ok()
1410+
.map(|identity| message::ExternalInbox::new(root, &identity))
1411+
.transpose()?;
1412+
message::resolve_inbox_with_external(root, id, host, external.as_ref())
1413+
}
1414+
14051415
/// Body from `-m`, else stdin (so `st2 message send x < file` works).
14061416
fn body_or_stdin(body: Option<String>) -> Result<String> {
14071417
match body {
@@ -1439,7 +1449,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> {
14391449
let (root, host) = resolve_ctx(&ctx)?;
14401450
let from = acting_id(&ctx)?;
14411451
let body = body_or_stdin(body)?;
1442-
let dir = message::resolve_inbox(&root, &to, &host)?;
1452+
let dir = resolve_message_inbox(&root, &to, &host)?;
14431453
let filename = message::send_to_inbox(
14441454
&dir,
14451455
&from,
@@ -1459,7 +1469,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> {
14591469
} => {
14601470
let (root, host) = resolve_ctx(&ctx)?;
14611471
let from = acting_id(&ctx)?;
1462-
let my_inbox = message::resolve_inbox(&root, &from, &host)?;
1472+
let my_inbox = resolve_message_inbox(&root, &from, &host)?;
14631473
let original = message::read_msg(&my_inbox, &filename)
14641474
.with_context(|| format!("no message '{filename}' in {}'s inbox", from))?;
14651475
let to = original
@@ -1468,7 +1478,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> {
14681478
.with_context(|| format!("message '{filename}' has no `from` to reply to"))?;
14691479
let subject = subject.or_else(|| message::reply_subject(original.subject.as_deref()));
14701480
let body = body_or_stdin(body)?;
1471-
let dir = message::resolve_inbox(&root, &to, &host)?;
1481+
let dir = resolve_message_inbox(&root, &to, &host)?;
14721482
let sent = message::send_to_inbox(
14731483
&dir,
14741484
&from,
@@ -1546,7 +1556,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> {
15461556
let dir = if archive {
15471557
message::resolve_archive(&root, &id, &host)
15481558
} else {
1549-
message::resolve_inbox(&root, &id, &host)
1559+
resolve_message_inbox(&root, &id, &host)
15501560
}?;
15511561
if raw {
15521562
print!("{}", std::fs::read_to_string(dir.join(&filename))?);
@@ -1574,7 +1584,7 @@ fn message_cmd(cmd: MessageCmd) -> Result<()> {
15741584
MessageCmd::Archive { first, second, ctx } => {
15751585
let (root, host) = resolve_ctx(&ctx)?;
15761586
let (id, filename) = box_target(first, second, &ctx)?;
1577-
let inbox = message::resolve_inbox(&root, &id, &host)?;
1587+
let inbox = resolve_message_inbox(&root, &id, &host)?;
15781588
let archive = message::resolve_archive(&root, &id, &host)?;
15791589
message::archive_msg(&inbox, &archive, &filename)?;
15801590
println!("archived");

src/message.rs

Lines changed: 77 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
use std::collections::{HashMap, HashSet};
1515
use std::fs;
1616
use std::io::Read;
17-
use std::path::{Path, PathBuf};
17+
use std::path::{Component, Path, PathBuf};
1818
use std::sync::atomic::{AtomicU64, Ordering};
1919
use std::time::{SystemTime, UNIX_EPOCH};
2020

@@ -314,19 +314,65 @@ pub fn archive_dir(agent_dir: &Path) -> PathBuf {
314314
agent_dir.join("resources").join("archive")
315315
}
316316

317-
/// Resolve an inbox by stable identity. A proven catalog-less root retains the legacy flat bus.
318-
/// Inside a catalog, an absent identity fails closed unless a real flat inbox was explicitly
319-
/// provisioned, as eval does for its external requester.
317+
/// Eval-owned authority for one external flat requester mailbox. General catalog routing remains
318+
/// declaration-only; possessing this value is the explicit exception at message call sites.
319+
#[derive(Debug, Clone, PartialEq, Eq)]
320+
pub struct ExternalInbox {
321+
root: PathBuf,
322+
identity: String,
323+
inbox: PathBuf,
324+
}
325+
326+
impl ExternalInbox {
327+
pub fn new(root: &Path, identity: &str) -> anyhow::Result<Self> {
328+
let mut components = Path::new(identity).components();
329+
let safe = matches!(components.next(), Some(Component::Normal(component)) if component == identity)
330+
&& components.next().is_none();
331+
anyhow::ensure!(safe, "external requester identity must be one non-empty relative path component");
332+
Ok(Self {
333+
root: root.to_path_buf(),
334+
identity: identity.to_owned(),
335+
inbox: root.join(identity).join("inbox"),
336+
})
337+
}
338+
339+
pub fn provision(root: &Path, identity: &str) -> anyhow::Result<Self> {
340+
let external = Self::new(root, identity)?;
341+
fs::create_dir_all(&external.inbox).map_err(|error| {
342+
anyhow::anyhow!(
343+
"provisioning external requester {identity:?} inbox {}: {error}",
344+
external.inbox.display()
345+
)
346+
})?;
347+
Ok(external)
348+
}
349+
}
350+
351+
/// Resolve an inbox by stable identity. A proven catalog-less root retains the legacy flat bus;
352+
/// inside a catalog an absent identity always fails closed.
320353
pub fn resolve_inbox(root: &Path, id: &str, host: &str) -> anyhow::Result<PathBuf> {
321-
match resolve_list_box(root, id, host, false, false) {
354+
resolve_list_box(root, id, host, false, false)
355+
}
356+
357+
/// Resolve a normal declared inbox or one exact eval-owned external requester capability.
358+
pub fn resolve_inbox_with_external(
359+
root: &Path,
360+
id: &str,
361+
host: &str,
362+
external: Option<&ExternalInbox>,
363+
) -> anyhow::Result<PathBuf> {
364+
match resolve_inbox(root, id, host) {
322365
Ok(inbox) => Ok(inbox),
323-
Err(error) => {
324-
let flat = root.join(id).join("inbox");
325-
match fs::symlink_metadata(&flat) {
326-
Ok(metadata) if metadata.file_type().is_dir() => Ok(flat),
327-
_ => Err(error),
366+
Err(error) => match external {
367+
Some(external)
368+
if external.root == root
369+
&& external.identity == id
370+
&& external.inbox.is_dir() =>
371+
{
372+
Ok(external.inbox.clone())
328373
}
329-
}
374+
_ => Err(error),
375+
},
330376
}
331377
}
332378

@@ -675,8 +721,27 @@ mod tests {
675721
);
676722
assert!(resolve_inbox(root, "Shared Worker", "h").is_err());
677723

724+
let external = ExternalInbox::new(root, "requester").unwrap();
725+
assert!(resolve_inbox_with_external(root, "requester", "h", Some(&external)).is_err());
726+
678727
let requester = root.join("requester").join("inbox");
679728
std::fs::create_dir_all(&requester).unwrap();
680-
assert_eq!(resolve_inbox(root, "requester", "h").unwrap(), requester);
729+
assert!(resolve_inbox(root, "requester", "h").is_err());
730+
assert_eq!(
731+
resolve_inbox_with_external(root, "requester", "h", Some(&external)).unwrap(),
732+
requester
733+
);
734+
assert!(resolve_inbox_with_external(root, "missing", "h", Some(&external)).is_err());
735+
}
736+
737+
#[test]
738+
fn external_inbox_rejects_unsafe_or_nested_identities() {
739+
let tmp = tempfile::tempdir().unwrap();
740+
for identity in ["", ".", "..", "nested/requester", "../requester", "/requester"] {
741+
assert!(
742+
ExternalInbox::new(tmp.path(), identity).is_err(),
743+
"accepted unsafe external identity {identity:?}"
744+
);
745+
}
681746
}
682747
}

tests/eval_run_e2e.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,7 @@ eval {
271271
fixture.join("agents/evalhost/worker/agent.kdl"),
272272
r#"agent "worker" {
273273
identity "worker"
274+
name "requester"
274275
host "evalhost"
275276
workspace "$CATALOG/worker"
276277
supervisor "sup"
@@ -802,14 +803,6 @@ fn canonical_agents_fail_closed_matrix_is_pre_spawn_and_non_vacuous() {
802803
r#"agent "worker" { identity "worker"; host "evalhost"; pty "agent" { id ""; command "touch \"$CATALOG/SPAWNED\"; sleep 60" } }"#,
803804
)],
804805
),
805-
(
806-
"presentation name",
807-
"evalhost.worker",
808-
vec![(
809-
"worker",
810-
r#"agent "worker" { identity "worker"; name "requester"; host "evalhost"; argv "sh" "-c" "touch \"$CATALOG/SPAWNED\"; sleep 60" }"#,
811-
)],
812-
),
813806
(
814807
"duplicate canonical route",
815808
"worker",

tests/message_cli.rs

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -196,16 +196,19 @@ fn known_empty_native_and_catalog_less_flat_boxes_remain_valid() {
196196

197197
#[test]
198198
fn send_routes_only_by_stable_identity_in_a_catalog_and_preserves_catalogless_bus() {
199-
let send = |root: &Path, recipient: &str, root_flag: &str| {
200-
let mut child = Command::new(env!("CARGO_BIN_EXE_st2"))
199+
let send = |root: &Path, recipient: &str, root_flag: &str, external: Option<&str>| {
200+
let mut command = Command::new(env!("CARGO_BIN_EXE_st2"));
201+
command
201202
.args(["message", "send", recipient, root_flag])
202203
.arg(root)
203204
.args(["--host", "h", "--as", "h.sender"])
204205
.stdin(Stdio::piped())
205206
.stdout(Stdio::piped())
206-
.stderr(Stdio::piped())
207-
.spawn()
208-
.unwrap();
207+
.stderr(Stdio::piped());
208+
if let Some(identity) = external {
209+
command.env("ST2_EVAL_REQUESTER", identity);
210+
}
211+
let mut child = command.spawn().unwrap();
209212
child.stdin.take().unwrap().write_all(b"work\n").unwrap();
210213
child.wait_with_output().unwrap()
211214
};
@@ -222,15 +225,15 @@ fn send_routes_only_by_stable_identity_in_a_catalog_and_preserves_catalogless_bu
222225
)
223226
.unwrap();
224227

225-
let display = send(catalog.path(), "Shared Worker", "--catalog");
228+
let display = send(catalog.path(), "Shared Worker", "--catalog", None);
226229
assert!(!display.status.success());
227230
assert!(
228231
String::from_utf8_lossy(&display.stderr)
229232
.contains("no agent 'Shared Worker' found in catalog")
230233
);
231234
assert!(!catalog.path().join("Shared Worker").exists());
232235

233-
let stable = send(catalog.path(), "h.worker", "--catalog");
236+
let stable = send(catalog.path(), "h.worker", "--catalog", None);
234237
assert!(
235238
stable.status.success(),
236239
"{}",
@@ -243,8 +246,26 @@ fn send_routes_only_by_stable_identity_in_a_catalog_and_preserves_catalogless_bu
243246
1
244247
);
245248

249+
fs::create_dir_all(catalog.path().join("requester/inbox")).unwrap();
250+
assert!(!send(catalog.path(), "requester", "--catalog", None).status.success());
251+
assert!(!send(catalog.path(), "requester", "--catalog", Some("other"))
252+
.status
253+
.success());
254+
let external = send(catalog.path(), "requester", "--catalog", Some("requester"));
255+
assert!(
256+
external.status.success(),
257+
"{}",
258+
String::from_utf8_lossy(&external.stderr)
259+
);
260+
assert_eq!(
261+
fs::read_dir(catalog.path().join("requester/inbox"))
262+
.unwrap()
263+
.count(),
264+
1
265+
);
266+
246267
let flat = tempfile::tempdir().unwrap();
247-
let raw = send(flat.path(), "requester", "--root");
268+
let raw = send(flat.path(), "requester", "--root", None);
248269
assert!(
249270
raw.status.success(),
250271
"{}",

0 commit comments

Comments
 (0)