Skip to content

Commit c13a148

Browse files
committed
Fix delegated task permission propagation
1 parent abfc28d commit c13a148

3 files changed

Lines changed: 119 additions & 19 deletions

File tree

core/src/agent_api/agent_binding.rs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ fn apply_permissions(opts: &mut SessionOptions, def: &AgentDefinition) {
2727
}
2828

2929
fn has_defined_permissions(def: &AgentDefinition) -> bool {
30-
!def.permissions.allow.is_empty() || !def.permissions.deny.is_empty()
30+
!def.permissions.is_default_policy()
3131
}
3232

3333
fn apply_step_budget(opts: &mut SessionOptions, def: &AgentDefinition) {
@@ -129,4 +129,25 @@ mod tests {
129129

130130
assert_eq!(opts.model.as_deref(), Some("anthropic/claude-opus"));
131131
}
132+
133+
#[test]
134+
fn applies_default_allow_agent_permissions() {
135+
use crate::permissions::PermissionDecision;
136+
137+
let permissions = PermissionPolicy {
138+
default_decision: PermissionDecision::Allow,
139+
..PermissionPolicy::new()
140+
};
141+
let def = AgentDefinition::new("worker", "Allowed worker").with_permissions(permissions);
142+
143+
let opts = apply_agent_definition(SessionOptions::new(), &def);
144+
145+
let checker = opts
146+
.permission_checker
147+
.expect("non-default permission policy should be applied");
148+
assert_eq!(
149+
checker.check("bash", &serde_json::json!({"command": "echo ok"})),
150+
PermissionDecision::Allow
151+
);
152+
}
132153
}

core/src/permissions/policy.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use super::{MatchingRules, PermissionChecker, PermissionDecision, PermissionRule
99
/// 2. Allow rules - any match results in auto-approval
1010
/// 3. Ask rules - any match requires user confirmation
1111
/// 4. Default - falls back to default_decision
12-
#[derive(Debug, Clone, Serialize, Deserialize)]
12+
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
1313
pub struct PermissionPolicy {
1414
/// Rules that always deny (checked first)
1515
#[serde(default)]
@@ -58,6 +58,11 @@ impl PermissionPolicy {
5858
Self::default()
5959
}
6060

61+
/// Return true when this policy is still the implicit default Ask policy.
62+
pub fn is_default_policy(&self) -> bool {
63+
self == &Self::default()
64+
}
65+
6166
/// Create a strict policy that asks for everything
6267
pub fn strict() -> Self {
6368
Self {

core/src/tools/task.rs

Lines changed: 91 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,10 @@
1515
//! ```
1616
1717
use crate::agent::{AgentConfig, AgentEvent, AgentLoop};
18-
use crate::llm::LlmClient;
18+
use crate::llm::{LlmClient, ToolDefinition};
1919
use crate::mcp::manager::McpManager;
20-
use crate::subagent::AgentRegistry;
20+
use crate::permissions::PermissionChecker;
21+
use crate::subagent::{AgentDefinition, AgentRegistry};
2122
use crate::tools::types::{Tool, ToolContext, ToolOutput};
2223
use anyhow::{Context, Result};
2324
use async_trait::async_trait;
@@ -128,6 +129,39 @@ fn format_task_result_for_context(result: &TaskResult) -> (String, bool) {
128129
(formatted, truncated)
129130
}
130131

132+
fn has_defined_permissions(agent: &AgentDefinition) -> bool {
133+
!agent.permissions.is_default_policy()
134+
}
135+
136+
fn agent_permission_checker(agent: &AgentDefinition) -> Option<Arc<dyn PermissionChecker>> {
137+
if has_defined_permissions(agent) {
138+
Some(Arc::new(agent.permissions.clone()) as Arc<dyn PermissionChecker>)
139+
} else {
140+
None
141+
}
142+
}
143+
144+
fn build_child_config(
145+
agent: &AgentDefinition,
146+
params: &TaskParams,
147+
tools: Vec<ToolDefinition>,
148+
) -> AgentConfig {
149+
let mut prompt_slots = crate::prompts::SystemPromptSlots::default();
150+
if let Some(ref p) = agent.prompt {
151+
prompt_slots.extra = Some(p.clone());
152+
}
153+
154+
AgentConfig {
155+
prompt_slots,
156+
tools,
157+
max_tool_rounds: params
158+
.max_steps
159+
.unwrap_or_else(|| agent.max_steps.unwrap_or(20)),
160+
permission_checker: agent_permission_checker(agent),
161+
..AgentConfig::default()
162+
}
163+
}
164+
131165
/// Task executor for delegated child runs.
132166
pub struct TaskExecutor {
133167
/// Agent registry for looking up agent definitions
@@ -217,26 +251,13 @@ impl TaskExecutor {
217251
}
218252
}
219253

220-
if !agent.permissions.allow.is_empty() || !agent.permissions.deny.is_empty() {
254+
if has_defined_permissions(&agent) {
221255
child_executor.set_guard_policy(Arc::new(agent.permissions.clone())
222256
as Arc<dyn crate::permissions::PermissionChecker>);
223257
}
224258
let child_executor = Arc::new(child_executor);
225259

226-
// Inject the agent system prompt via the extra slot.
227-
let mut prompt_slots = crate::prompts::SystemPromptSlots::default();
228-
if let Some(ref p) = agent.prompt {
229-
prompt_slots.extra = Some(p.clone());
230-
}
231-
232-
let child_config = AgentConfig {
233-
prompt_slots,
234-
tools: child_executor.definitions(),
235-
max_tool_rounds: params
236-
.max_steps
237-
.unwrap_or_else(|| agent.max_steps.unwrap_or(20)),
238-
..AgentConfig::default()
239-
};
260+
let child_config = build_child_config(&agent, &params, child_executor.definitions());
240261

241262
let tool_context =
242263
ToolContext::new(PathBuf::from(&self.workspace)).with_session_id(session_id.clone());
@@ -1256,4 +1277,57 @@ mod tests {
12561277

12571278
assert!(props.get("permissive").is_none());
12581279
}
1280+
1281+
#[test]
1282+
fn test_child_config_inherits_agent_permissions() {
1283+
use crate::permissions::{PermissionDecision, PermissionPolicy};
1284+
1285+
let agent = AgentDefinition::new("worker", "Worker")
1286+
.with_permissions(PermissionPolicy::new().allow("bash(echo:*)"));
1287+
let params = TaskParams {
1288+
agent: "worker".to_string(),
1289+
description: "Run allowed command".to_string(),
1290+
prompt: "Use bash".to_string(),
1291+
background: false,
1292+
max_steps: None,
1293+
};
1294+
1295+
let config = build_child_config(&agent, &params, Vec::new());
1296+
1297+
let checker = config
1298+
.permission_checker
1299+
.expect("agent permission policy should be installed");
1300+
assert_eq!(
1301+
checker.check("bash", &serde_json::json!({"command": "echo ok"})),
1302+
PermissionDecision::Allow
1303+
);
1304+
}
1305+
1306+
#[test]
1307+
fn test_child_config_inherits_default_allow_permissions() {
1308+
use crate::permissions::{PermissionDecision, PermissionPolicy};
1309+
1310+
let permissions = PermissionPolicy {
1311+
default_decision: PermissionDecision::Allow,
1312+
..PermissionPolicy::new()
1313+
};
1314+
let agent = AgentDefinition::new("worker", "Worker").with_permissions(permissions);
1315+
let params = TaskParams {
1316+
agent: "worker".to_string(),
1317+
description: "Run any command".to_string(),
1318+
prompt: "Use bash".to_string(),
1319+
background: false,
1320+
max_steps: None,
1321+
};
1322+
1323+
let config = build_child_config(&agent, &params, Vec::new());
1324+
1325+
let checker = config
1326+
.permission_checker
1327+
.expect("non-default permission policy should be installed");
1328+
assert_eq!(
1329+
checker.check("bash", &serde_json::json!({"command": "echo ok"})),
1330+
PermissionDecision::Allow
1331+
);
1332+
}
12591333
}

0 commit comments

Comments
 (0)