Skip to content

Commit 17b2ed8

Browse files
committed
fix(runtime): tighten delegate boundary, clarify SubAgent allowlist
DelegateTool::policy_for_target now refuses narrowing in addition to escalation. The spawned agentic loop reuses the caller's parent_tools registry, so a narrower target policy never reaches those tool calls; catching the narrowing at the delegate boundary turns a silent over-grant into a loud refusal that points operators at spawn_subagent for genuinely narrowed runs. SubAgent allowlist fields renamed to make explicit that they carry config aliases (the [agents.<alias>] keys) rather than backend storage identifiers: SubAgentOverrides.allowed_agent_aliases, SubAgentContext.parent_alias / allowed_agent_aliases, SubAgentSpawn.parent_alias / parent_allowed_agent_aliases. Module doc spells out that consumers building an AgentScopedMemory must resolve via Memory::ensure_agent_uuid first (SQL backends use UUIDs from the agents table; Markdown / Qdrant / None use the alias verbatim per the trait default). The in-tree consumer today is zeroclaw_memory::create_memory_for_agent, which already does the resolution. Drop a stale `let _ = agent_config;` in build_enriched_system_prompt that was claiming the parameter was unused; it is used several lines above to resolve the agent's skill bundles.
1 parent 0185dfe commit 17b2ed8

4 files changed

Lines changed: 182 additions & 52 deletions

File tree

crates/zeroclaw-runtime/src/cron/scheduler.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ async fn run_agent_job(
383383

384384
let subagent_span = tracing::info_span!(
385385
"subagent",
386-
parent_alias = %subagent_ctx.agent_id,
386+
parent_alias = %subagent_ctx.parent_alias,
387387
run_id = %run_session_id,
388388
spawn_site = "cron",
389389
);

crates/zeroclaw-runtime/src/subagent/mod.rs

Lines changed: 69 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,19 @@
1212
//! [`SubAgentOverrides`]; [`SubAgentSpawn::build`] validates each
1313
//! override as a subset of the parent (using
1414
//! [`SecurityPolicy::ensure_no_escalation_beyond`] for the policy and
15-
//! a UUID-set containment check for the memory allowlist) and returns
16-
//! `Err` with the originating violation chained on any escalation.
15+
//! an alias-set containment check for the memory allowlist) and
16+
//! returns `Err` with the originating violation chained on any
17+
//! escalation.
18+
//!
19+
//! The memory allowlist is carried as a set of agent **aliases** (the
20+
//! `[agents.<alias>]` config keys), not backend storage identifiers.
21+
//! Consumers that build an [`AgentScopedMemory`] must resolve aliases
22+
//! to backend identifiers via
23+
//! [`zeroclaw_memory::Memory::ensure_agent_uuid`] first — SQL-backed
24+
//! stores use UUIDs from the `agents` table; Markdown / Qdrant / None
25+
//! use the alias verbatim per the trait default. Holding aliases at
26+
//! this layer means [`SubAgentSpawn::for_agent`] does not need a
27+
//! backend handle to construct.
1728
1829
use anyhow::{Context, Result};
1930
use std::collections::HashSet;
@@ -36,41 +47,57 @@ pub struct SubAgentOverrides {
3647
/// [`SecurityPolicy::ensure_no_escalation_beyond`].
3748
pub policy: Option<SecurityPolicy>,
3849
/// Override the SubAgent's memory allowlist (the set of sibling
39-
/// agent UUIDs the SubAgent may recall from). Validated as a
40-
/// subset of the parent's allowlist; any UUID present here that
41-
/// is not on the parent's list is rejected.
42-
pub allowed_agent_ids: Option<HashSet<String>>,
50+
/// agent **aliases** the SubAgent may recall from, as written in
51+
/// `[agents.<alias>]` keys). Validated as a subset of the
52+
/// parent's allowlist; any alias here that is not on the parent's
53+
/// list is rejected.
54+
///
55+
/// These are config-layer aliases, not backend storage
56+
/// identifiers. Consumers that build an [`AgentScopedMemory`]
57+
/// must resolve aliases to backend identifiers via
58+
/// [`zeroclaw_memory::Memory::ensure_agent_uuid`] before passing
59+
/// them to the wrapper (SQL backends use UUIDs; Markdown / Qdrant
60+
/// / None use the alias verbatim per the trait default). The
61+
/// in-tree consumer today is `zeroclaw_memory::create_memory_for_agent`,
62+
/// which performs the resolution.
63+
pub allowed_agent_aliases: Option<HashSet<String>>,
4364
}
4465

4566
/// Constructed SubAgent context: bound parent identity, validated
4667
/// child policy, and the resolved memory allowlist.
4768
#[derive(Debug, Clone)]
4869
pub struct SubAgentContext {
49-
/// The parent agent's identifier. SubAgents share the parent's
50-
/// identity at the data layer (no separate row in the agents
51-
/// table); the distinction between parent and sub-run is captured
52-
/// at the tracing-span level (`agent.<alias>.subagent.<run_id>`).
53-
pub agent_id: String,
70+
/// The parent agent's alias (e.g. `"researcher"`). SubAgents share
71+
/// the parent's identity at the data layer (no separate row in the
72+
/// `agents` table); the distinction between parent and sub-run is
73+
/// captured at the tracing-span level
74+
/// (`agent.<alias>.subagent.<run_id>`).
75+
pub parent_alias: String,
5476
/// The validated [`SecurityPolicy`] this SubAgent operates under.
5577
/// Identical to the parent's when `SubAgentOverrides::policy` is
5678
/// `None`; otherwise a narrowed copy that passed
5779
/// [`SecurityPolicy::ensure_no_escalation_beyond`].
5880
pub policy: Arc<SecurityPolicy>,
59-
/// Resolved memory allowlist. The bound `agent_id` is always
60-
/// included so the SubAgent always sees the parent's own rows;
61-
/// the rest is either the parent's allowlist verbatim or a
62-
/// validated subset.
63-
pub allowed_agent_ids: HashSet<String>,
81+
/// Resolved memory allowlist as a set of agent **aliases**. The
82+
/// bound `parent_alias` is always included so the SubAgent always
83+
/// sees the parent's own rows; the rest is either the parent's
84+
/// allowlist verbatim or a validated subset.
85+
///
86+
/// See [`SubAgentOverrides::allowed_agent_aliases`] for the
87+
/// alias-vs-backend-identifier distinction; consumers that build
88+
/// an [`AgentScopedMemory`] must resolve to backend identifiers
89+
/// before passing the set to the wrapper.
90+
pub allowed_agent_aliases: HashSet<String>,
6491
}
6592

6693
/// Builder for a SubAgent spawn. The caller resolves a parent agent
6794
/// from the loaded config; [`Self::build`] applies any narrowing
6895
/// overrides and validates the result.
6996
#[derive(Debug)]
7097
pub struct SubAgentSpawn {
71-
pub parent_agent_id: String,
98+
pub parent_alias: String,
7299
pub parent_policy: Arc<SecurityPolicy>,
73-
pub parent_allowed_agent_ids: HashSet<String>,
100+
pub parent_allowed_agent_aliases: HashSet<String>,
74101
}
75102

76103
impl SubAgentSpawn {
@@ -91,18 +118,18 @@ impl SubAgentSpawn {
91118
format!("could not resolve security policy for agent {agent_alias:?}")
92119
})?;
93120

94-
let mut parent_allowed_agent_ids: HashSet<String> = agent
121+
let mut parent_allowed_agent_aliases: HashSet<String> = agent
95122
.workspace
96123
.read_memory_from
97124
.iter()
98125
.map(|alias| alias.as_str().to_string())
99126
.collect();
100-
parent_allowed_agent_ids.insert(agent_alias.to_string());
127+
parent_allowed_agent_aliases.insert(agent_alias.to_string());
101128

102129
Ok(Self {
103-
parent_agent_id: agent_alias.to_string(),
130+
parent_alias: agent_alias.to_string(),
104131
parent_policy,
105-
parent_allowed_agent_ids,
132+
parent_allowed_agent_aliases,
106133
})
107134
}
108135

@@ -137,26 +164,26 @@ impl SubAgentSpawn {
137164
self.parent_policy.clone()
138165
};
139166

140-
let allowed_agent_ids = if let Some(child_allowed) = overrides.allowed_agent_ids {
141-
for id in &child_allowed {
142-
if !self.parent_allowed_agent_ids.contains(id) {
167+
let allowed_agent_aliases = if let Some(child_allowed) = overrides.allowed_agent_aliases {
168+
for alias in &child_allowed {
169+
if !self.parent_allowed_agent_aliases.contains(alias) {
143170
anyhow::bail!(
144-
"subagent allowlist override contains agent_id {id:?} not present on \
171+
"subagent allowlist override contains alias {alias:?} not present on \
145172
parent's memory allowlist; SubAgent overrides may only narrow"
146173
);
147174
}
148175
}
149176
let mut resolved = child_allowed;
150-
resolved.insert(self.parent_agent_id.clone());
177+
resolved.insert(self.parent_alias.clone());
151178
resolved
152179
} else {
153-
self.parent_allowed_agent_ids
180+
self.parent_allowed_agent_aliases
154181
};
155182

156183
Ok(SubAgentContext {
157-
agent_id: self.parent_agent_id,
184+
parent_alias: self.parent_alias,
158185
policy,
159-
allowed_agent_ids,
186+
allowed_agent_aliases,
160187
})
161188
}
162189
}
@@ -189,9 +216,9 @@ mod tests {
189216
.expect("for_agent must succeed for a configured agent")
190217
.build(SubAgentOverrides::default())
191218
.expect("inherits-verbatim build must succeed");
192-
assert_eq!(ctx.agent_id, "alpha");
219+
assert_eq!(ctx.parent_alias, "alpha");
193220
assert!(
194-
ctx.allowed_agent_ids.contains("alpha"),
221+
ctx.allowed_agent_aliases.contains("alpha"),
195222
"an agent always sees its own rows"
196223
);
197224
}
@@ -211,11 +238,11 @@ mod tests {
211238
let config = config_with_agent("alpha");
212239
let spawn = SubAgentSpawn::for_agent(&config, "alpha").unwrap();
213240
let parent_policy = spawn.parent_policy.clone();
214-
let parent_allowlist = spawn.parent_allowed_agent_ids.clone();
241+
let parent_allowlist = spawn.parent_allowed_agent_aliases.clone();
215242

216243
let ctx = spawn.build(SubAgentOverrides::default()).unwrap();
217244
assert!(Arc::ptr_eq(&ctx.policy, &parent_policy));
218-
assert_eq!(ctx.allowed_agent_ids, parent_allowlist);
245+
assert_eq!(ctx.allowed_agent_aliases, parent_allowlist);
219246
}
220247

221248
#[test]
@@ -240,7 +267,7 @@ mod tests {
240267
}
241268

242269
#[test]
243-
fn build_rejects_allowlist_override_with_id_not_on_parent() {
270+
fn build_rejects_allowlist_override_with_alias_not_on_parent() {
244271
let config = config_with_agent("alpha");
245272
let spawn = SubAgentSpawn::for_agent(&config, "alpha").unwrap();
246273

@@ -249,13 +276,13 @@ mod tests {
249276

250277
let err = spawn
251278
.build(SubAgentOverrides {
252-
allowed_agent_ids: Some(rogue),
279+
allowed_agent_aliases: Some(rogue),
253280
..SubAgentOverrides::default()
254281
})
255-
.expect_err("allowlist override with foreign UUID must be rejected");
282+
.expect_err("allowlist override with foreign alias must be rejected");
256283
assert!(
257284
err.to_string().contains("rogue-agent"),
258-
"expected the rogue UUID in the error chain, got: {err}"
285+
"expected the rogue alias in the error chain, got: {err}"
259286
);
260287
}
261288

@@ -264,15 +291,15 @@ mod tests {
264291
let config = config_with_agent("alpha");
265292
let spawn = SubAgentSpawn::for_agent(&config, "alpha").unwrap();
266293

267-
// Empty subset is still allowed; the bound agent_id is added back.
294+
// Empty subset is still allowed; the bound parent alias is added back.
268295
let ctx = spawn
269296
.build(SubAgentOverrides {
270-
allowed_agent_ids: Some(HashSet::new()),
297+
allowed_agent_aliases: Some(HashSet::new()),
271298
..SubAgentOverrides::default()
272299
})
273300
.expect("narrowing to {} is a valid subset");
274-
assert_eq!(ctx.allowed_agent_ids.len(), 1);
275-
assert!(ctx.allowed_agent_ids.contains("alpha"));
301+
assert_eq!(ctx.allowed_agent_aliases.len(), 1);
302+
assert!(ctx.allowed_agent_aliases.contains("alpha"));
276303
}
277304

278305
#[test]

crates/zeroclaw-runtime/src/tools/delegate.rs

Lines changed: 111 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -306,19 +306,31 @@ impl DelegateTool {
306306
}
307307

308308
/// Build a `SecurityPolicy` for the delegated target agent
309-
/// validated as a subset of the caller's policy with shared
310-
/// action / cost tracker.
309+
/// validated as **mutually equivalent** to the caller's policy
310+
/// (neither escalates nor narrows), with the caller's action /
311+
/// cost tracker shared into the returned policy.
311312
///
312313
/// Returns:
313314
/// - `Ok(target_policy)` when `root_config` is set, the target
314-
/// resolves, and its policy is a subset of the caller's
315-
/// (`ensure_no_escalation_beyond`). The returned policy's
316-
/// `tracker` field is the caller's `Arc`-shared tracker so
317-
/// delegated actions count against the caller's
318-
/// `max_actions_per_hour` / `max_cost_per_day_cents`.
315+
/// resolves, and the target's policy is equivalent to the
316+
/// caller's under [`SecurityPolicy::ensure_no_escalation_beyond`]
317+
/// in both directions. The returned policy's `tracker` field is
318+
/// the caller's `Arc`-shared tracker so delegated actions count
319+
/// against the caller's `max_actions_per_hour` /
320+
/// `max_cost_per_day_cents`.
319321
/// - `Err(_)` on escalation: the target's risk profile or
320322
/// workspace.access map would widen permissions beyond the
321323
/// caller. The originating `EscalationViolation` is chained.
324+
/// - `Err(_)` on narrowing: the target's policy is strictly
325+
/// tighter than the caller's. `DelegateTool` reuses the
326+
/// caller's `parent_tools` registry whose tools each hold the
327+
/// caller's `Arc<SecurityPolicy>` from registration time, so a
328+
/// narrower target would silently inherit the caller's broader
329+
/// allowlist — an over-grant the validator catches loudly here
330+
/// instead of letting it ship as an enforcement gap. The error
331+
/// message names `spawn_subagent` as the supported path for
332+
/// narrowed runs (it re-enters `agent::run`, which rebuilds the
333+
/// tool registry under the validated child policy).
322334
/// - `Ok(self.security)` (caller's policy) when `root_config`
323335
/// is `None`. This branch only fires for the legacy unit-test
324336
/// constructors that don't plumb root config.
@@ -338,6 +350,28 @@ impl DelegateTool {
338350
"delegate target {target_alias:?} policy escalates beyond caller: {violation}"
339351
)
340352
})?;
353+
// Refuse strict narrowing. `DelegateTool::execute_agentic`
354+
// reuses the caller's `parent_tools` registry (every tool
355+
// there holds the caller's `Arc<SecurityPolicy>` from
356+
// registration time); a narrower target policy would not
357+
// reach those tools, so the target would silently inherit
358+
// the caller's broader allowlist. Catching the narrowing
359+
// here turns a silent over-grant into a loud refusal.
360+
// Operators with truly narrowed sub-agents should use
361+
// `spawn_subagent`, which re-enters `agent::run` with the
362+
// validated child policy and rebuilds the tool registry
363+
// under it.
364+
self.security
365+
.ensure_no_escalation_beyond(&target_policy)
366+
.map_err(|violation| {
367+
anyhow::anyhow!(
368+
"delegate target {target_alias:?} policy narrows the caller's ({violation}); \
369+
DelegateTool reuses the caller's tool registry, so narrowing is not enforced \
370+
by the spawned tool calls. Either align caller and target risk_profile / \
371+
workspace.access so the policies are equivalent, or use `spawn_subagent` for \
372+
a narrowed run."
373+
)
374+
})?;
341375
target_policy.tracker = self.security.tracker.clone();
342376
Ok(Arc::new(target_policy))
343377
}
@@ -1393,7 +1427,6 @@ impl DelegateTool {
13931427
// sub-agent's own workspace dir. Each missing file is silently
13941428
// skipped — the operator may not have authored every file.
13951429
let target_workspace = self.agent_workspace(agent_alias);
1396-
let _ = agent_config; // kept for callers; no longer used here.
13971430
let identity_files = [
13981431
"AGENTS.md",
13991432
"SOUL.md",
@@ -3270,4 +3303,74 @@ mod tests {
32703303
"without root_config the helper returns the caller's Arc verbatim"
32713304
);
32723305
}
3306+
3307+
/// Build a config where `caller` has a strictly broader policy
3308+
/// than `target`. The caller-only command is the narrowing axis
3309+
/// the validator should catch.
3310+
fn config_with_narrowed_target() -> Arc<zeroclaw_config::schema::Config> {
3311+
use zeroclaw_config::schema::{AliasedAgentConfig, Config, RiskProfileConfig};
3312+
let mut config = Config::default();
3313+
config.risk_profiles.insert(
3314+
"broad".to_string(),
3315+
RiskProfileConfig {
3316+
allowed_commands: vec!["git".into(), "cargo".into()],
3317+
..RiskProfileConfig::default()
3318+
},
3319+
);
3320+
config.risk_profiles.insert(
3321+
"narrow".to_string(),
3322+
RiskProfileConfig {
3323+
allowed_commands: vec!["git".into()],
3324+
..RiskProfileConfig::default()
3325+
},
3326+
);
3327+
config.agents.insert(
3328+
"caller".to_string(),
3329+
AliasedAgentConfig {
3330+
risk_profile: "broad".to_string(),
3331+
model_provider: "ollama.caller".into(),
3332+
..AliasedAgentConfig::default()
3333+
},
3334+
);
3335+
config.agents.insert(
3336+
"target".to_string(),
3337+
AliasedAgentConfig {
3338+
risk_profile: "narrow".to_string(),
3339+
model_provider: "ollama.target".into(),
3340+
..AliasedAgentConfig::default()
3341+
},
3342+
);
3343+
Arc::new(config)
3344+
}
3345+
3346+
#[tokio::test]
3347+
async fn delegate_rejects_target_whose_policy_narrows_caller() {
3348+
// DelegateTool's spawned agentic loop reuses the caller's
3349+
// parent_tools registry — a narrower target would silently
3350+
// inherit the caller's broader allowlist. The validator
3351+
// must catch the narrowing at the delegate boundary and
3352+
// refuse to dispatch.
3353+
let config = config_with_narrowed_target();
3354+
let caller_policy =
3355+
Arc::new(SecurityPolicy::for_agent(&config, "caller").expect("caller policy resolves"));
3356+
let mut delegate_agents = HashMap::new();
3357+
for (name, agent) in &config.agents {
3358+
delegate_agents.insert(name.clone(), agent.clone());
3359+
}
3360+
let tool = DelegateTool::new(delegate_agents, None, caller_policy)
3361+
.with_root_config(config.clone());
3362+
3363+
let err = tool
3364+
.policy_for_target("target")
3365+
.expect_err("narrowing target must be rejected at delegate boundary");
3366+
let chain = format!("{err:#}");
3367+
assert!(
3368+
chain.contains("narrows the caller"),
3369+
"expected narrowing error, got: {chain}"
3370+
);
3371+
assert!(
3372+
chain.contains("spawn_subagent"),
3373+
"error must point operators at spawn_subagent for narrowed runs, got: {chain}"
3374+
);
3375+
}
32733376
}

crates/zeroclaw-runtime/src/tools/spawn_subagent.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ impl Tool for SpawnSubagentTool {
100100
let run_id = uuid::Uuid::new_v4().to_string();
101101
let span = tracing::info_span!(
102102
"subagent",
103-
parent_alias = %subagent_ctx.agent_id,
103+
parent_alias = %subagent_ctx.parent_alias,
104104
run_id = %run_id,
105105
spawn_site = "tool",
106106
);

0 commit comments

Comments
 (0)