fix(security): preserve sub-agent approval boundaries
This commit is contained in:
@@ -747,18 +747,17 @@ impl SubAgentRuntime {
|
||||
}
|
||||
|
||||
/// Build a child runtime cloning this one, incrementing `spawn_depth`,
|
||||
/// deriving a child cancellation token, and forcing `auto_approve` on
|
||||
/// the child's `ToolContext`. Used at spawn entry to construct the
|
||||
/// runtime the new sub-agent will see.
|
||||
/// and deriving a child cancellation token. Used at spawn entry to
|
||||
/// construct the runtime the new sub-agent will see.
|
||||
///
|
||||
/// The `auto_approve` override is deliberate: spawning IS the approval.
|
||||
/// Per-tool prompts inside a child would break delegation, so children
|
||||
/// inherit a YOLO-equivalent context regardless of the parent's mode.
|
||||
/// The workspace boundary + sandbox profile still apply.
|
||||
/// Children inherit the parent's approval state. A non-auto parent can
|
||||
/// still delegate read-only investigation, but approval-gated child tools
|
||||
/// are blocked by the sub-agent registry instead of being silently run
|
||||
/// without a prompt.
|
||||
#[must_use]
|
||||
pub fn child_runtime(&self) -> Self {
|
||||
let mut child_context = self.context.clone();
|
||||
child_context.auto_approve = true;
|
||||
child_context.auto_approve = self.context.auto_approve;
|
||||
Self {
|
||||
client: self.client.clone(),
|
||||
model: self.model.clone(),
|
||||
@@ -798,7 +797,8 @@ pub struct SubAgent {
|
||||
pub result: Option<String>,
|
||||
pub steps_taken: u32,
|
||||
pub started_at: Instant,
|
||||
/// `None` = full registry inheritance (v0.6.6 default).
|
||||
/// `None` = full registry inheritance, with approval-gated tools still
|
||||
/// blocked unless the parent runtime is auto-approved.
|
||||
/// `Some(list)` = explicit narrow allowlist (Custom agents, legacy).
|
||||
pub allowed_tools: Option<Vec<String>>,
|
||||
/// Stable id of the manager that spawned this agent (#405). Compared
|
||||
@@ -1634,7 +1634,7 @@ impl ToolSpec for AgentSpawnTool {
|
||||
"allowed_tools": {
|
||||
"type": "array",
|
||||
"items": { "type": "string" },
|
||||
"description": "Explicit tool allowlist (required for custom type). Default behavior is full registry inheritance from the parent."
|
||||
"description": "Explicit tool allowlist (required for custom type). Default behavior is full registry inheritance from the parent; approval-gated tools still require an auto-approved parent."
|
||||
},
|
||||
"model": {
|
||||
"type": "string",
|
||||
@@ -1712,9 +1712,9 @@ impl ToolSpec for AgentSpawnTool {
|
||||
};
|
||||
|
||||
// Derive the child's runtime as a durable background job: it keeps
|
||||
// its own cancellation token, forces auto_approve, and optionally
|
||||
// overrides cwd if the caller passed one (used for the parallel-
|
||||
// worktree pattern).
|
||||
// its own cancellation token, inherits the parent approval state, and
|
||||
// optionally overrides cwd if the caller passed one (used for the
|
||||
// parallel-worktree pattern).
|
||||
let mut child_runtime = self.runtime.background_runtime();
|
||||
if let Some(cwd) = validated_cwd {
|
||||
child_runtime.context.workspace = cwd;
|
||||
@@ -2705,6 +2705,7 @@ struct SubAgentTask {
|
||||
prompt: String,
|
||||
assignment: SubAgentAssignment,
|
||||
/// `None` = full registry inheritance. `Some(list)` = explicit narrow.
|
||||
/// Approval-gated tools still require an auto-approved parent runtime.
|
||||
allowed_tools: Option<Vec<String>>,
|
||||
fork_context: bool,
|
||||
started_at: Instant,
|
||||
@@ -3775,14 +3776,16 @@ fn emit_agent_progress(
|
||||
/// Two modes:
|
||||
/// - **Full inheritance** (`allowed_tools = None`): the child sees the same
|
||||
/// tool surface as the parent's Agent mode — every tool family including
|
||||
/// `with_subagent_tools` (so it can recurse). This is the v0.6.6 default.
|
||||
/// `with_subagent_tools` (so it can recurse). Approval-gated tools are
|
||||
/// callable only when the parent runtime is auto-approved.
|
||||
/// - **Explicit narrow** (`allowed_tools = Some(list)`): legacy / Custom
|
||||
/// path. The registry still builds the full surface, but only the listed
|
||||
/// tool names are visible to the model and callable.
|
||||
struct SubAgentToolRegistry {
|
||||
/// `None` → full inheritance (no filter applied). `Some(list)` →
|
||||
/// `None` → full inheritance (no allowlist filter applied). `Some(list)` →
|
||||
/// only the listed tools are visible to the model and callable.
|
||||
allowed_tools: Option<Vec<String>>,
|
||||
auto_approve: bool,
|
||||
registry: ToolRegistry,
|
||||
}
|
||||
|
||||
@@ -3812,6 +3815,7 @@ impl SubAgentToolRegistry {
|
||||
|
||||
Self {
|
||||
allowed_tools: explicit_allowed_tools,
|
||||
auto_approve: runtime.context.auto_approve,
|
||||
registry,
|
||||
}
|
||||
}
|
||||
@@ -3851,6 +3855,16 @@ impl SubAgentToolRegistry {
|
||||
if !self.is_tool_allowed(name) {
|
||||
return Err(anyhow!("Tool {name} not allowed for this sub-agent"));
|
||||
}
|
||||
if !self.auto_approve {
|
||||
let Some(spec) = self.registry.get(name) else {
|
||||
return Err(anyhow!("Tool {name} is not registered"));
|
||||
};
|
||||
if spec.approval_requirement() != ApprovalRequirement::Auto {
|
||||
return Err(anyhow!(
|
||||
"Tool {name} requires approval and cannot run inside this sub-agent unless the parent session is auto-approved"
|
||||
));
|
||||
}
|
||||
}
|
||||
reject_subagent_terminal_takeover(name, &input)?;
|
||||
self.registry
|
||||
.execute(name, input)
|
||||
@@ -3914,10 +3928,10 @@ fn build_allowed_tools(
|
||||
));
|
||||
}
|
||||
|
||||
// Default: full registry inheritance from the parent. The child sees
|
||||
// every tool the parent has, including the sub-agent management family
|
||||
// (so it can recurse). Sandbox + workspace + depth cap remain the
|
||||
// safety net.
|
||||
// Default: full registry inheritance from the parent. The child sees every
|
||||
// tool the parent has, including the sub-agent management family. The
|
||||
// registry execution guard still blocks approval-gated tools unless the
|
||||
// parent runtime is auto-approved.
|
||||
Ok(None)
|
||||
}
|
||||
|
||||
|
||||
@@ -1066,18 +1066,46 @@ fn would_exceed_depth_at_boundary() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn child_runtime_increments_depth_and_forces_auto_approve() {
|
||||
fn child_runtime_increments_depth_and_preserves_auto_approve() {
|
||||
let mut parent = stub_runtime();
|
||||
parent.spawn_depth = 1;
|
||||
parent.context.auto_approve = false; // parent in suggest mode
|
||||
let child = parent.child_runtime();
|
||||
assert_eq!(child.spawn_depth, 2, "child depth = parent + 1");
|
||||
assert!(
|
||||
child.context.auto_approve,
|
||||
"child must auto-approve regardless of parent mode (spawning IS the approval)"
|
||||
!child.context.auto_approve,
|
||||
"child must inherit parent approval state"
|
||||
);
|
||||
// Parent mode is unchanged — the override is on the child only.
|
||||
assert!(!parent.context.auto_approve);
|
||||
|
||||
parent.context.auto_approve = true;
|
||||
let auto_child = parent.child_runtime();
|
||||
assert!(
|
||||
auto_child.context.auto_approve,
|
||||
"auto-approved parents should still create auto-approved children"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn subagent_registry_blocks_approval_tools_without_parent_auto_approve() {
|
||||
let mut runtime = stub_runtime();
|
||||
runtime.context.auto_approve = false;
|
||||
let registry = SubAgentToolRegistry::new(
|
||||
runtime,
|
||||
Some(vec!["exec_shell".to_string()]),
|
||||
Arc::new(Mutex::new(TodoList::new())),
|
||||
Arc::new(Mutex::new(PlanState::default())),
|
||||
);
|
||||
|
||||
let err = registry
|
||||
.execute("agent_test", "exec_shell", json!({"command": "echo hi"}))
|
||||
.await
|
||||
.expect_err("approval-gated child tool should be blocked");
|
||||
|
||||
assert!(
|
||||
err.to_string().contains("requires approval"),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user