From 80e75c6aa0c0a7eb1a0549e8f2168134cc818acd Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Wed, 1 Jul 2026 23:43:17 +0530 Subject: [PATCH 1/3] fix(agent-harness): structured policy-block messages with workaround Policy/permission denials in the session tool executor now return a structured message (what was blocked, why, and a concrete workaround / how-to-enable / permitted alternative) plus an explicit instruction to relay it to the user, instead of a bare denial line the agent could dead-end on. Covers the session-policy, per-call permission, pluggable policy Deny, and RequireApproval branches. The message is returned as the failed tool result so it flows back into the turn. Closes #4094 --- .../agent/harness/session/agent_tool_exec.rs | 232 ++++++++++++++++-- src/openhuman/agent/harness/session/mod.rs | 1 + .../agent/harness/session/policy_denial.rs | 225 +++++++++++++++++ 3 files changed, 435 insertions(+), 23 deletions(-) create mode 100644 src/openhuman/agent/harness/session/policy_denial.rs diff --git a/src/openhuman/agent/harness/session/agent_tool_exec.rs b/src/openhuman/agent/harness/session/agent_tool_exec.rs index f7f165423f..aeeede61e4 100644 --- a/src/openhuman/agent/harness/session/agent_tool_exec.rs +++ b/src/openhuman/agent/harness/session/agent_tool_exec.rs @@ -14,6 +14,7 @@ use crate::core::event_bus::{publish_global, DomainEvent}; use crate::openhuman::agent::dispatcher::{ParsedToolCall, ToolExecutionResult}; use crate::openhuman::agent::harness::engine::ProgressReporter; use crate::openhuman::agent::harness::payload_summarizer::PayloadSummarizer; +use crate::openhuman::agent::harness::session::policy_denial::PolicyDenial; use crate::openhuman::agent::harness::tool_result_artifacts::{ apply_per_result_persistence, ToolResultArtifactStore, }; @@ -105,15 +106,14 @@ pub(super) async fn run_agent_tool_call( } else if let Some(tool) = ctx.tools.iter().find(|t| t.name() == call.name) { let session_decision = ctx.tool_policy_session.decision_for(&call.name); if session_decision.is_denied() { - let required = session_decision - .required_permission - .map(|permission| permission.to_string()) - .unwrap_or_else(|| "unknown".to_string()); ( - format!( - "Tool '{}' blocked by tool policy: requires {}, channel '{}' allows {}", - call.name, required, ctx.event_channel, session_decision.allowed_permission - ), + PolicyDenial::SessionForbidden { + tool: &call.name, + required: session_decision.required_permission, + allowed: session_decision.allowed_permission, + channel: ctx.event_channel, + } + .render(), false, ) } else { @@ -126,13 +126,13 @@ pub(super) async fn run_agent_tool_call( "[agent_loop] tool action blocked by per-call permission check" ); ( - format!( - "Tool '{}' action requires {} permission, channel '{}' allows {}", - call.name, - call_required, - ctx.event_channel, - session_decision.allowed_permission - ), + PolicyDenial::PermissionTooLow { + tool: &call.name, + required: call_required, + allowed: session_decision.allowed_permission, + channel: ctx.event_channel, + } + .render(), false, ) } else { @@ -168,14 +168,21 @@ pub(super) async fn run_agent_tool_call( reason = %reason, "[agent_loop] tool blocked by policy" ); - ( - format!( - "Tool '{}' {blocked_action} by policy '{}': {reason}", - call.name, - ctx.tool_policy.name() - ), - false, - ) + let denial = match &policy_decision { + ToolPolicyDecision::RequireApproval { .. } => { + PolicyDenial::ApprovalRequired { + tool: &call.name, + policy: ctx.tool_policy.name(), + reason, + } + } + _ => PolicyDenial::PolicyDenied { + tool: &call.name, + policy: ctx.tool_policy.name(), + reason, + }, + }; + (denial.render(), false) } else { let options = ToolCallOptions { prefer_markdown: ctx.prefer_markdown, @@ -502,4 +509,183 @@ mod tests { assert_eq!(progress.completed.load(Ordering::Relaxed), 1); assert_eq!(progress.timeout_completions.load(Ordering::Relaxed), 1); } + + /// A tool whose required permission is configurable, used to trip the + /// session / per-call permission gates. + struct PermissionedTool { + name: &'static str, + permission: crate::openhuman::tools::PermissionLevel, + } + + #[async_trait] + impl Tool for PermissionedTool { + fn name(&self) -> &str { + self.name + } + + fn description(&self) -> &str { + "test tool with a fixed permission level" + } + + fn parameters_schema(&self) -> serde_json::Value { + json!({ "type": "object", "properties": {} }) + } + + async fn execute(&self, _args: serde_json::Value) -> anyhow::Result { + Ok(ToolResult::success("ran")) + } + + fn permission_level(&self) -> crate::openhuman::tools::PermissionLevel { + self.permission + } + } + + /// A pluggable policy that denies every call with a fixed reason, standing + /// in for a sandbox / permission-boundary block. + struct DenyingToolPolicy; + + #[async_trait] + impl ToolPolicy for DenyingToolPolicy { + fn name(&self) -> &str { + "sandbox" + } + + async fn check(&self, _request: &ToolPolicyRequest) -> ToolPolicyDecision { + ToolPolicyDecision::deny("sandbox restriction") + } + } + + /// Progress reporter that captures the last completion so tests can assert + /// the block was surfaced (not silently dropped). + struct CapturingProgress { + completed: AtomicUsize, + } + + #[async_trait] + impl ProgressReporter for CapturingProgress { + async fn tool_completed( + &self, + _call_id: &str, + _tool_name: &str, + _success: bool, + _output: &str, + _elapsed_ms: u64, + _iteration: u32, + ) { + self.completed.fetch_add(1, Ordering::Relaxed); + } + } + + #[tokio::test(flavor = "current_thread")] + async fn session_policy_denial_returns_structured_message_with_workaround() { + // Channel capped at read-only; an Execute tool is denied by the session + // policy rather than allowed to run. + let tools: Vec> = vec![Box::new(PermissionedTool { + name: "run_script", + permission: crate::openhuman::tools::PermissionLevel::Execute, + })]; + let visible_tool_names = HashSet::new(); + let mut channel_permissions = HashMap::new(); + channel_permissions.insert("web".to_string(), "read_only".to_string()); + let policy_session = ToolPolicyEngine::build_session( + "context_scout", + "web", + "test", + &channel_permissions, + &tools, + &visible_tool_names, + ); + assert!(policy_session.decision_for("run_script").is_denied()); + + let tool_policy = AllowAllToolPolicy; + let ctx = AgentToolExecCtx { + tools: &tools, + visible_tool_names: &visible_tool_names, + tool_policy_session: &policy_session, + tool_policy: &tool_policy, + payload_summarizer: None, + event_session_id: "session-1", + event_channel: "web", + agent_definition_id: "context_scout", + prefer_markdown: false, + budget_bytes: 4096, + compaction_enabled: false, + tokenjuice_compression: crate::openhuman::tokenjuice::AgentTokenjuiceCompression::Off, + artifact_store: None, + }; + let call = ParsedToolCall { + name: "run_script".to_string(), + arguments: json!({}), + tool_call_id: Some("call-1".to_string()), + }; + let progress = CapturingProgress { + completed: AtomicUsize::new(0), + }; + + let (result, record) = run_agent_tool_call(&ctx, &progress, &call, 0).await; + + assert!(!result.success); + assert!(!record.success); + // Structured: what was blocked, why, and a concrete workaround. + assert!(result.output.contains("Blocked: Tool 'run_script'")); + assert!(result.output.contains("requires Execute permission")); + assert!(result.output.contains("Workaround:")); + assert!(result.output.contains("agent-access tier")); + // The agent is told to relay rather than halt silently — and the block + // was surfaced through progress, not dropped. + assert!(result.output.contains("Relay this to the user")); + assert_eq!(progress.completed.load(Ordering::Relaxed), 1); + } + + #[tokio::test(flavor = "current_thread")] + async fn pluggable_policy_denial_returns_structured_message_with_alternative() { + // Tool passes the session/permission gates but a pluggable policy denies + // it — matching the issue's `run_script → blocked by sandbox` example. + let tools: Vec> = vec![Box::new(PermissionedTool { + name: "run_script", + permission: crate::openhuman::tools::PermissionLevel::ReadOnly, + })]; + let visible_tool_names = HashSet::new(); + let policy_session = ToolPolicyEngine::build_session( + "context_scout", + "web", + "test", + &HashMap::new(), + &tools, + &visible_tool_names, + ); + let tool_policy = DenyingToolPolicy; + let ctx = AgentToolExecCtx { + tools: &tools, + visible_tool_names: &visible_tool_names, + tool_policy_session: &policy_session, + tool_policy: &tool_policy, + payload_summarizer: None, + event_session_id: "session-1", + event_channel: "web", + agent_definition_id: "context_scout", + prefer_markdown: false, + budget_bytes: 4096, + compaction_enabled: false, + tokenjuice_compression: crate::openhuman::tokenjuice::AgentTokenjuiceCompression::Off, + artifact_store: None, + }; + let call = ParsedToolCall { + name: "run_script".to_string(), + arguments: json!({}), + tool_call_id: Some("call-1".to_string()), + }; + let progress = CapturingProgress { + completed: AtomicUsize::new(0), + }; + + let (result, _record) = run_agent_tool_call(&ctx, &progress, &call, 0).await; + + assert!(!result.success); + assert!(result.output.contains("denied by policy 'sandbox'")); + assert!(result.output.contains("sandbox restriction")); + assert!(result.output.contains("permitted alternative")); + assert!(result.output.contains("Relay this to the user")); + assert_eq!(progress.completed.load(Ordering::Relaxed), 1); + } } diff --git a/src/openhuman/agent/harness/session/mod.rs b/src/openhuman/agent/harness/session/mod.rs index 02e75b1e1a..5b001c4cb0 100644 --- a/src/openhuman/agent/harness/session/mod.rs +++ b/src/openhuman/agent/harness/session/mod.rs @@ -22,6 +22,7 @@ mod agent_tool_exec; mod builder; +mod policy_denial; pub mod migration; mod runtime; pub(crate) mod transcript; diff --git a/src/openhuman/agent/harness/session/policy_denial.rs b/src/openhuman/agent/harness/session/policy_denial.rs new file mode 100644 index 0000000000..5d5e0036b6 --- /dev/null +++ b/src/openhuman/agent/harness/session/policy_denial.rs @@ -0,0 +1,225 @@ +//! Structured, actionable messages for policy / permission denials. +//! +//! When the harness blocks a tool call at a policy or permission boundary, the +//! agent must not dead-end with a bare "blocked" line. Each denial is rendered +//! into a structured message — **what** was blocked, **why**, and a concrete +//! **workaround** (how to enable it, or a permitted alternative) — followed by +//! an explicit instruction to relay it to the user rather than halting +//! silently. The rendered string is returned as the (failed) tool result, so it +//! flows back into the turn the same way the unknown-tool corrective error is +//! surfaced to the model (see PR #4360). + +use crate::openhuman::tools::PermissionLevel; + +/// The boundary that blocked a tool call, with the context needed to explain it +/// and suggest a way forward. +pub(super) enum PolicyDenial<'a> { + /// The session tool policy forbids this tool for the channel's permission + /// tier (it is not in the allowed set). + SessionForbidden { + tool: &'a str, + required: Option, + allowed: PermissionLevel, + channel: &'a str, + }, + /// The tool is allowed in general, but *this call's* arguments require a + /// higher permission than the channel grants. + PermissionTooLow { + tool: &'a str, + required: PermissionLevel, + allowed: PermissionLevel, + channel: &'a str, + }, + /// A pluggable [`ToolPolicy`](crate::openhuman::agent::tool_policy::ToolPolicy) + /// denied the call outright. + PolicyDenied { + tool: &'a str, + policy: &'a str, + reason: &'a str, + }, + /// A pluggable [`ToolPolicy`](crate::openhuman::agent::tool_policy::ToolPolicy) + /// requires an approval handoff this executor cannot complete inline. + ApprovalRequired { + tool: &'a str, + policy: &'a str, + reason: &'a str, + }, +} + +/// Suffix appended to every denial so the agent relays the block instead of +/// silently stopping. +const RELAY_INSTRUCTION: &str = "Relay this to the user: explain what was \ + blocked and why, then offer the workaround as the next step. Do not stop \ + silently."; + +impl PolicyDenial<'_> { + /// Render the denial as a structured `Blocked / Reason / Workaround / relay` + /// message for the model. + pub(super) fn render(&self) -> String { + let (blocked, reason, workaround) = match self { + PolicyDenial::SessionForbidden { + tool, + required, + allowed, + channel, + } => { + let reason = match required { + Some(required) => format!( + "it requires {required} permission, but the '{channel}' channel only \ + grants {allowed} access" + ), + None => format!( + "it is not permitted at the '{channel}' channel's {allowed} access tier" + ), + }; + ( + format!("Tool '{tool}' is blocked by the session tool policy"), + reason, + raise_tier_workaround( + required.map(|p| p.to_string()).as_deref(), + *allowed, + channel, + ), + ) + } + PolicyDenial::PermissionTooLow { + tool, + required, + allowed, + channel, + } => ( + format!("Tool '{tool}' is blocked by a per-call permission check"), + format!( + "this call needs {required} permission, but the '{channel}' channel only \ + grants {allowed} access" + ), + raise_tier_workaround(Some(&required.to_string()), *allowed, channel), + ), + PolicyDenial::PolicyDenied { + tool, + policy, + reason, + } => ( + format!("Tool '{tool}' was denied by policy '{policy}'"), + (*reason).to_string(), + "Address the reason above, or reach the goal with a permitted alternative tool / \ + path. If this action is genuinely required, ask the user to adjust the policy." + .to_string(), + ), + PolicyDenial::ApprovalRequired { + tool, + policy, + reason, + } => ( + format!("Tool '{tool}' requires approval under policy '{policy}'"), + (*reason).to_string(), + "Ask the user to approve this action, then retry — or choose an alternative that \ + does not require approval." + .to_string(), + ), + }; + + format!("Blocked: {blocked}. Reason: {reason}. Workaround: {workaround} {RELAY_INSTRUCTION}") + } +} + +/// Workaround shared by the permission-tier denials: raise the channel's +/// agent-access tier, or fall back to a lower-permission tool. +fn raise_tier_workaround(required: Option<&str>, allowed: PermissionLevel, channel: &str) -> String { + match required { + Some(required) => format!( + "Raise the '{channel}' channel's agent-access tier to at least {required} \ + (Settings → Agent access, or the `config.update_autonomy_settings` RPC / \ + `[autonomy]` config), or accomplish the goal with a tool that needs only \ + {allowed} access." + ), + None => format!( + "Raise the '{channel}' channel's agent-access tier (Settings → Agent access, or the \ + `config.update_autonomy_settings` RPC / `[autonomy]` config), or accomplish the goal \ + with a tool that needs only {allowed} access." + ), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn session_forbidden_with_required_lists_reason_and_workaround() { + let msg = PolicyDenial::SessionForbidden { + tool: "run_script", + required: Some(PermissionLevel::Execute), + allowed: PermissionLevel::ReadOnly, + channel: "web", + } + .render(); + + assert!(msg.starts_with("Blocked: Tool 'run_script'")); + assert!(msg.contains("Reason:")); + assert!(msg.contains("requires Execute permission")); + assert!(msg.contains("Workaround:")); + assert!(msg.contains("agent-access tier")); + // The relay instruction is what keeps the agent from halting silently. + assert!(msg.contains("Relay this to the user")); + } + + #[test] + fn session_forbidden_without_required_still_has_workaround() { + let msg = PolicyDenial::SessionForbidden { + tool: "run_script", + required: None, + allowed: PermissionLevel::ReadOnly, + channel: "cron", + } + .render(); + + assert!(msg.contains("not permitted")); + assert!(msg.contains("Workaround:")); + assert!(msg.contains("Relay this to the user")); + } + + #[test] + fn permission_too_low_names_both_levels() { + let msg = PolicyDenial::PermissionTooLow { + tool: "shell", + required: PermissionLevel::Write, + allowed: PermissionLevel::ReadOnly, + channel: "web", + } + .render(); + + assert!(msg.contains("needs Write permission")); + assert!(msg.contains("only grants ReadOnly")); + assert!(msg.contains("Workaround:")); + } + + #[test] + fn policy_denied_carries_reason_and_alternative() { + let msg = PolicyDenial::PolicyDenied { + tool: "run_script", + policy: "sandbox", + reason: "sandbox restriction", + } + .render(); + + assert!(msg.contains("denied by policy 'sandbox'")); + assert!(msg.contains("sandbox restriction")); + assert!(msg.contains("permitted alternative")); + assert!(msg.contains("Relay this to the user")); + } + + #[test] + fn approval_required_suggests_approval_then_retry() { + let msg = PolicyDenial::ApprovalRequired { + tool: "send_email", + policy: "approval_gate", + reason: "outbound message needs sign-off", + } + .render(); + + assert!(msg.contains("requires approval under policy 'approval_gate'")); + assert!(msg.contains("approve this action")); + assert!(msg.contains("Relay this to the user")); + } +} From 71abfe23d4cd78e5d3e788cec448addd1b951449 Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Wed, 1 Jul 2026 23:48:56 +0530 Subject: [PATCH 2/3] style: rustfmt policy_denial module (mod ordering + fmt) --- src/openhuman/agent/harness/session/mod.rs | 2 +- src/openhuman/agent/harness/session/policy_denial.rs | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/openhuman/agent/harness/session/mod.rs b/src/openhuman/agent/harness/session/mod.rs index 5b001c4cb0..87799dff3f 100644 --- a/src/openhuman/agent/harness/session/mod.rs +++ b/src/openhuman/agent/harness/session/mod.rs @@ -22,8 +22,8 @@ mod agent_tool_exec; mod builder; -mod policy_denial; pub mod migration; +mod policy_denial; mod runtime; pub(crate) mod transcript; mod turn; diff --git a/src/openhuman/agent/harness/session/policy_denial.rs b/src/openhuman/agent/harness/session/policy_denial.rs index 5d5e0036b6..a2d196ab73 100644 --- a/src/openhuman/agent/harness/session/policy_denial.rs +++ b/src/openhuman/agent/harness/session/policy_denial.rs @@ -119,13 +119,19 @@ impl PolicyDenial<'_> { ), }; - format!("Blocked: {blocked}. Reason: {reason}. Workaround: {workaround} {RELAY_INSTRUCTION}") + format!( + "Blocked: {blocked}. Reason: {reason}. Workaround: {workaround} {RELAY_INSTRUCTION}" + ) } } /// Workaround shared by the permission-tier denials: raise the channel's /// agent-access tier, or fall back to a lower-permission tool. -fn raise_tier_workaround(required: Option<&str>, allowed: PermissionLevel, channel: &str) -> String { +fn raise_tier_workaround( + required: Option<&str>, + allowed: PermissionLevel, + channel: &str, +) -> String { match required { Some(required) => format!( "Raise the '{channel}' channel's agent-access tier to at least {required} \ From 6450b9419559002d0338f115b08f3e8ab9efda0e Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Thu, 2 Jul 2026 00:13:47 +0530 Subject: [PATCH 3/3] fix(agent-harness): route structured denials through tinyagents turn path; point workaround at channel_permissions Address review on #4390: - P1: the real turn flow runs through the tinyagents ToolPolicyMiddleware, which duplicated the denial branches with the old bare strings, so the structured message never reached production turns. Share PolicyDenial (now pub(crate)) and render it in the middleware's channel-permission and pluggable-policy blocks. Add a middleware regression test. - P2: permission-ceiling denials are capped by [agent].channel_permissions, not autonomy. Point the workaround at that config with valid level tokens (read_only/write/execute/dangerous) instead of update_autonomy_settings. --- .../agent/harness/session/agent_tool_exec.rs | 2 +- src/openhuman/agent/harness/session/mod.rs | 2 +- .../agent/harness/session/policy_denial.rs | 53 ++++--- src/openhuman/tinyagents/middleware.rs | 133 +++++++++++++++--- 4 files changed, 148 insertions(+), 42 deletions(-) diff --git a/src/openhuman/agent/harness/session/agent_tool_exec.rs b/src/openhuman/agent/harness/session/agent_tool_exec.rs index aeeede61e4..3237d382fa 100644 --- a/src/openhuman/agent/harness/session/agent_tool_exec.rs +++ b/src/openhuman/agent/harness/session/agent_tool_exec.rs @@ -630,7 +630,7 @@ mod tests { assert!(result.output.contains("Blocked: Tool 'run_script'")); assert!(result.output.contains("requires Execute permission")); assert!(result.output.contains("Workaround:")); - assert!(result.output.contains("agent-access tier")); + assert!(result.output.contains("channel_permissions")); // The agent is told to relay rather than halt silently — and the block // was surfaced through progress, not dropped. assert!(result.output.contains("Relay this to the user")); diff --git a/src/openhuman/agent/harness/session/mod.rs b/src/openhuman/agent/harness/session/mod.rs index 87799dff3f..fda715212b 100644 --- a/src/openhuman/agent/harness/session/mod.rs +++ b/src/openhuman/agent/harness/session/mod.rs @@ -23,7 +23,7 @@ mod agent_tool_exec; mod builder; pub mod migration; -mod policy_denial; +pub(crate) mod policy_denial; mod runtime; pub(crate) mod transcript; mod turn; diff --git a/src/openhuman/agent/harness/session/policy_denial.rs b/src/openhuman/agent/harness/session/policy_denial.rs index a2d196ab73..4469ba5093 100644 --- a/src/openhuman/agent/harness/session/policy_denial.rs +++ b/src/openhuman/agent/harness/session/policy_denial.rs @@ -13,7 +13,7 @@ use crate::openhuman::tools::PermissionLevel; /// The boundary that blocked a tool call, with the context needed to explain it /// and suggest a way forward. -pub(super) enum PolicyDenial<'a> { +pub(crate) enum PolicyDenial<'a> { /// The session tool policy forbids this tool for the channel's permission /// tier (it is not in the allowed set). SessionForbidden { @@ -55,7 +55,7 @@ const RELAY_INSTRUCTION: &str = "Relay this to the user: explain what was \ impl PolicyDenial<'_> { /// Render the denial as a structured `Blocked / Reason / Workaround / relay` /// message for the model. - pub(super) fn render(&self) -> String { + pub(crate) fn render(&self) -> String { let (blocked, reason, workaround) = match self { PolicyDenial::SessionForbidden { tool, @@ -75,11 +75,7 @@ impl PolicyDenial<'_> { ( format!("Tool '{tool}' is blocked by the session tool policy"), reason, - raise_tier_workaround( - required.map(|p| p.to_string()).as_deref(), - *allowed, - channel, - ), + raise_ceiling_workaround(*required, *allowed, channel), ) } PolicyDenial::PermissionTooLow { @@ -93,7 +89,7 @@ impl PolicyDenial<'_> { "this call needs {required} permission, but the '{channel}' channel only \ grants {allowed} access" ), - raise_tier_workaround(Some(&required.to_string()), *allowed, channel), + raise_ceiling_workaround(Some(*required), *allowed, channel), ), PolicyDenial::PolicyDenied { tool, @@ -125,28 +121,43 @@ impl PolicyDenial<'_> { } } -/// Workaround shared by the permission-tier denials: raise the channel's -/// agent-access tier, or fall back to a lower-permission tool. -fn raise_tier_workaround( - required: Option<&str>, +/// Workaround shared by the permission-ceiling denials. The blocked `allowed` +/// level is the channel's cap from `[agent].channel_permissions` (see +/// `ToolPolicyEngine::build_session`), so the fix is to raise *that* channel's +/// entry — not the autonomy tier, which is a separate `readonly|supervised|full` +/// knob. Alternatively, fall back to a tool that fits the current ceiling. +fn raise_ceiling_workaround( + required: Option, allowed: PermissionLevel, channel: &str, ) -> String { match required { Some(required) => format!( - "Raise the '{channel}' channel's agent-access tier to at least {required} \ - (Settings → Agent access, or the `config.update_autonomy_settings` RPC / \ - `[autonomy]` config), or accomplish the goal with a tool that needs only \ - {allowed} access." + "Raise the '{channel}' channel's permission ceiling in the `[agent].channel_permissions` \ + config to at least `{}` (levels: read_only, write, execute, dangerous), or accomplish \ + the goal with a tool that needs only {allowed} access.", + permission_config_token(required) ), None => format!( - "Raise the '{channel}' channel's agent-access tier (Settings → Agent access, or the \ - `config.update_autonomy_settings` RPC / `[autonomy]` config), or accomplish the goal \ - with a tool that needs only {allowed} access." + "Raise the '{channel}' channel's permission ceiling in the `[agent].channel_permissions` \ + config (levels: read_only, write, execute, dangerous), or accomplish the goal with a \ + tool that needs only {allowed} access." ), } } +/// The lowercase token `[agent].channel_permissions` accepts for a level +/// (mirrors `agent_tool_policy::engine::parse_permission_level`). +fn permission_config_token(level: PermissionLevel) -> &'static str { + match level { + PermissionLevel::None => "none", + PermissionLevel::ReadOnly => "read_only", + PermissionLevel::Write => "write", + PermissionLevel::Execute => "execute", + PermissionLevel::Dangerous => "dangerous", + } +} + #[cfg(test)] mod tests { use super::*; @@ -165,7 +176,9 @@ mod tests { assert!(msg.contains("Reason:")); assert!(msg.contains("requires Execute permission")); assert!(msg.contains("Workaround:")); - assert!(msg.contains("agent-access tier")); + // Points at the channel-permission ceiling (not the autonomy tier). + assert!(msg.contains("channel_permissions")); + assert!(msg.contains("`execute`")); // The relay instruction is what keeps the agent from halting silently. assert!(msg.contains("Relay this to the user")); } diff --git a/src/openhuman/tinyagents/middleware.rs b/src/openhuman/tinyagents/middleware.rs index a6ea26b9dd..2c99c90c96 100644 --- a/src/openhuman/tinyagents/middleware.rs +++ b/src/openhuman/tinyagents/middleware.rs @@ -38,6 +38,7 @@ use tinyagents::harness::tool::{ToolCall as TaToolCall, ToolResult as TaToolResu use super::tools::UNKNOWN_TOOL_SENTINEL; use crate::openhuman::agent::harness::payload_summarizer::PayloadSummarizer; +use crate::openhuman::agent::harness::session::policy_denial::PolicyDenial; use crate::openhuman::approval::{ redact_args, summarize_action, ApprovalGate, ExecutionOutcome, GateOutcome, }; @@ -662,9 +663,11 @@ impl ToolMiddleware<()> for CliRpcOnlyMiddleware { /// `agent_tool_exec` (`ctx.tool_policy.check(...)`); the tinyagents path bypassed /// it, so a `.tool_policy()` deny/require-approval silently no-opped and the tool /// executed anyway — a security regression. This middleware restores it: a -/// blocking decision short-circuits with a model-consumable result carrying the -/// same `"Tool '' by policy '': "` -/// wording the engine produced. +/// blocking decision short-circuits with a model-consumable result. The message +/// is rendered by [`PolicyDenial`] — a structured `Blocked / Reason / Workaround` +/// line with an explicit instruction to relay it to the user (issue #4094) — +/// shared with the engine's `agent_tool_exec` path so both surface the same +/// wording. pub struct ToolPolicyMiddleware { policy: Arc, /// The session's channel-permission snapshot — enforces the per-channel deny @@ -718,22 +721,28 @@ impl ToolPolicyMiddleware { fn channel_permission_block(&self, call: &TaToolCall) -> Option { let decision = self.session.decision_for(&call.name); if decision.is_denied() { - let required = decision - .required_permission - .map(|permission| permission.to_string()) - .unwrap_or_else(|| "unknown".to_string()); - return Some(format!( - "Tool '{}' blocked by tool policy: requires {}, channel '{}' allows {}", - call.name, required, self.channel, decision.allowed_permission - )); + return Some( + PolicyDenial::SessionForbidden { + tool: &call.name, + required: decision.required_permission, + allowed: decision.allowed_permission, + channel: &self.channel, + } + .render(), + ); } let tool = self.resolve_tool(&call.name)?; let call_required = tool.permission_level_with_args(&call.arguments); if call_required > decision.allowed_permission { - return Some(format!( - "Tool '{}' action requires {} permission, channel '{}' allows {}", - call.name, call_required, self.channel, decision.allowed_permission - )); + return Some( + PolicyDenial::PermissionTooLow { + tool: &call.name, + required: call_required, + allowed: decision.allowed_permission, + channel: &self.channel, + } + .render(), + ); } None } @@ -849,11 +858,19 @@ impl ToolMiddleware<()> for ToolPolicyMiddleware { reason = %reason, "[tinyagents::mw] tool blocked by policy" ); - let content = format!( - "Tool '{}' {blocked_action} by policy '{}': {reason}", - call.name, - self.policy.name() - ); + let content = match &decision { + ToolPolicyDecision::RequireApproval { .. } => PolicyDenial::ApprovalRequired { + tool: &call.name, + policy: self.policy.name(), + reason, + }, + _ => PolicyDenial::PolicyDenied { + tool: &call.name, + policy: self.policy.name(), + reason, + }, + } + .render(); return Ok(MiddlewareToolOutcome::Result(TaToolResult { call_id: call.id, name: call.name, @@ -1577,6 +1594,82 @@ mod tests { assert_eq!(call.name, UNKNOWN_TOOL_SENTINEL); } + // ── ToolPolicyMiddleware (real turn path) ─────────────────────────────── + + /// A tool with a configurable required permission, to trip the channel + /// permission ceiling the middleware enforces on the tinyagents turn path. + struct PermTool { + name: &'static str, + permission: crate::openhuman::tools::PermissionLevel, + } + + #[async_trait] + impl Tool for PermTool { + fn name(&self) -> &str { + self.name + } + fn description(&self) -> &str { + "perm" + } + fn parameters_schema(&self) -> serde_json::Value { + json!({ "type": "object" }) + } + async fn execute( + &self, + _args: serde_json::Value, + ) -> anyhow::Result { + Ok(crate::openhuman::tools::ToolResult::success("ok")) + } + fn permission_level(&self) -> crate::openhuman::tools::PermissionLevel { + self.permission + } + } + + /// The real turn path (tinyagents middleware) must surface the structured + /// denial with a workaround, not the old bare string — otherwise the #4094 + /// fix never reaches production turns (only direct `execute_tool_call`). + #[test] + fn channel_permission_block_returns_structured_denial() { + let tools: Arc>> = Arc::new(vec![Box::new(PermTool { + name: "run_script", + permission: crate::openhuman::tools::PermissionLevel::Execute, + })]); + let visible = HashSet::new(); + let mut channel_permissions = std::collections::HashMap::new(); + channel_permissions.insert("web".to_string(), "read_only".to_string()); + let session = crate::openhuman::agent_tool_policy::ToolPolicyEngine::build_session( + "context_scout", + "web", + "test", + &channel_permissions, + tools.as_ref(), + &visible, + ); + let mw = ToolPolicyMiddleware::new( + Arc::new(crate::openhuman::agent::tool_policy::AllowAllToolPolicy), + session, + vec![tools], + visible, + "session-1".to_string(), + "web".to_string(), + "context_scout".to_string(), + ); + let call = TaToolCall { + id: "1".into(), + name: "run_script".into(), + arguments: json!({}), + }; + + let msg = mw + .channel_permission_block(&call) + .expect("execute tool must be blocked at the read_only channel ceiling"); + assert!(msg.contains("Blocked: Tool 'run_script'")); + assert!(msg.contains("requires Execute permission")); + assert!(msg.contains("Workaround:")); + assert!(msg.contains("channel_permissions")); + assert!(msg.contains("Relay this to the user")); + } + // ── CostBudgetMiddleware ──────────────────────────────────────────────── #[tokio::test]