diff --git a/src/openhuman/tinyagents/middleware.rs b/src/openhuman/tinyagents/middleware.rs index b0a0537067..a3b30cfa58 100644 --- a/src/openhuman/tinyagents/middleware.rs +++ b/src/openhuman/tinyagents/middleware.rs @@ -1403,6 +1403,25 @@ impl Middleware<()> for ToolOutcomeCaptureMiddleware { _state: &(), result: &mut TaToolResult, ) -> TaResult<()> { + // Enrich a raw security-policy / autonomy block (issue #4094): the ~20 + // `[policy-blocked]` denials emitted deep in `SecurityPolicy` / the tools + // return a bare marker line with no workaround and no relay directive, so + // the agent dead-ends. Rewrite the content into the structured + // `Blocked / Reason / Workaround / relay` shape here — the last `after_tool` + // hook, so the enriched text is what the transcript keeps. The marker is + // preserved, and already-structured `ToolPolicyMiddleware` denials (which + // carry a `Workaround:` suffix) are left untouched. This runs before + // classification below, which still recognises the preserved marker. + if let Some(enriched) = + super::policy_denial::maybe_enrich_policy_block(&result.name, &result.content) + { + tracing::debug!( + tool = result.name.as_str(), + "[tinyagents::mw] enriched raw security-policy block with workaround + relay" + ); + result.content = enriched; + } + let success = result.error.is_none(); // Classify the failure so the live `ToolCallCompleted` event and the // persisted timeline can explain it in plain language. A hard @@ -1923,6 +1942,52 @@ mod tests { } } + // ── ToolOutcomeCaptureMiddleware policy-block enrichment (issue #4094) ─── + + fn outcome_capture_mw() -> ToolOutcomeCaptureMiddleware { + ToolOutcomeCaptureMiddleware::new( + std::sync::Arc::new(std::sync::Mutex::new(Vec::new())), + std::sync::Arc::new(std::sync::Mutex::new(std::collections::HashMap::new())), + ) + } + + #[tokio::test] + async fn raw_security_policy_block_is_enriched_with_workaround_and_relay() { + let mw = outcome_capture_mw(); + let mut result = tool_result( + "run_command", + "[policy-blocked] Security policy: read-only mode — only read commands are allowed", + ); + result.error = Some(result.content.clone()); + mw.after_tool(&mut ctx(), &(), &mut result).await.unwrap(); + // The bare denial now carries a workaround + relay directive, and keeps the + // marker so classification / the loop-breaker still recognise it. + assert!(result.content.contains("Workaround:"), "{}", result.content); + assert!(result.content.contains("Relay this to the user")); + assert!(result + .content + .contains(crate::openhuman::security::POLICY_BLOCKED_MARKER)); + assert!(result.content.contains("read-only mode")); + } + + #[tokio::test] + async fn already_structured_denial_is_not_double_wrapped() { + // A ToolPolicyMiddleware-style denial already has "Workaround:"; the capture + // middleware must leave it untouched (no second Workaround block). + let mw = outcome_capture_mw(); + let structured = + "Blocked: Tool 'x' denied. Reason: nope. Workaround: do y. Relay this to the user: ..."; + let mut result = tool_result("x", structured); + result.error = Some(result.content.clone()); + mw.after_tool(&mut ctx(), &(), &mut result).await.unwrap(); + assert_eq!( + result.content.matches("Workaround:").count(), + 1, + "must not double-wrap: {}", + result.content + ); + } + // ── TurnContextMiddleware config ──────────────────────────────────────── #[test] diff --git a/src/openhuman/tinyagents/policy_denial.rs b/src/openhuman/tinyagents/policy_denial.rs index 33db2f7596..ccf70d9371 100644 --- a/src/openhuman/tinyagents/policy_denial.rs +++ b/src/openhuman/tinyagents/policy_denial.rs @@ -9,11 +9,29 @@ //! flows back into the turn the same way the unknown-tool corrective error is //! surfaced to the model (see PR #4360). +use crate::openhuman::security::POLICY_BLOCKED_MARKER; use crate::openhuman::tools::PermissionLevel; +/// Generic workaround for a raw security-policy / autonomy block. These denials +/// originate deep in the tools / `SecurityPolicy` layer (autonomy tier, command +/// classification, path checks) rather than the pluggable `ToolPolicy`, so there +/// is no single structured reason to lean on — point the agent at the levers that +/// actually unblock the family. +const SECURITY_POLICY_WORKAROUND: &str = "Raise the agent's access tier / autonomy \ + (Settings → Agent access, or the `config.update_autonomy_settings` RPC / \ + `[autonomy]` config) if this action should be allowed; otherwise reach the goal \ + with a permitted (e.g. read-only) alternative, or report that it can't be done \ + here."; + /// The boundary that blocked a tool call, with the context needed to explain it /// and suggest a way forward. pub(super) enum PolicyDenial<'a> { + /// A raw security-policy / autonomy denial emitted by the tools / + /// `SecurityPolicy` layer, recognised by its [`POLICY_BLOCKED_MARKER`] prefix. + /// It already states *what* and *why* but carries no workaround or relay + /// directive; [`render`](Self::render) keeps the marker and reason and appends + /// the missing guidance. `raw_reason` is the full marker-bearing tool output. + SecurityPolicyBlocked { tool: &'a str, raw_reason: &'a str }, /// The session tool policy forbids this tool for the channel's permission /// tier (it is not in the allowed set). SessionForbidden { @@ -57,6 +75,24 @@ impl PolicyDenial<'_> { /// message for the model. pub(super) fn render(&self) -> String { let (blocked, reason, workaround) = match self { + PolicyDenial::SecurityPolicyBlocked { tool, raw_reason } => { + // Strip the marker prefix from the reason (it is re-added on the + // `blocked` line so downstream `contains(POLICY_BLOCKED_MARKER)` + // checks — classification, the loop-breaker — keep matching). + let reason = raw_reason + .trim() + .strip_prefix(POLICY_BLOCKED_MARKER) + .unwrap_or(raw_reason) + .trim() + .to_string(); + ( + format!( + "{POLICY_BLOCKED_MARKER} Tool '{tool}' was blocked by the security policy" + ), + reason, + SECURITY_POLICY_WORKAROUND.to_string(), + ) + } PolicyDenial::SessionForbidden { tool, required, @@ -125,6 +161,33 @@ impl PolicyDenial<'_> { } } +/// Enrich a raw security-policy / autonomy tool result (issue #4094). +/// +/// The pluggable-`ToolPolicy` and channel-permission denials are already rendered +/// with a `Blocked / Reason / Workaround / relay` shape by [`ToolPolicyMiddleware`] +/// (PR #4443). But the ~20 `[policy-blocked]` denials emitted deep in +/// `SecurityPolicy` / the tools themselves still return a bare marker line with no +/// workaround and no relay directive, so the agent dead-ends. This wraps such a +/// result into the structured form, appending the workaround + relay while keeping +/// the marker (so classification and the repeated-failure breaker still match). +/// +/// Returns `None` — leaving the content untouched — when the result is not a raw +/// policy block, i.e. it lacks the marker, or it already carries a `Workaround:` +/// suffix (an already-structured `ToolPolicyMiddleware` denial that must not be +/// double-wrapped). +pub(super) fn maybe_enrich_policy_block(tool: &str, content: &str) -> Option { + if !content.contains(POLICY_BLOCKED_MARKER) || content.contains("Workaround:") { + return None; + } + Some( + PolicyDenial::SecurityPolicyBlocked { + tool, + raw_reason: content, + } + .render(), + ) +} + /// 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( @@ -215,6 +278,50 @@ mod tests { assert!(msg.contains("Relay this to the user")); } + #[test] + fn security_policy_block_keeps_marker_and_adds_workaround_and_relay() { + let raw = + "[policy-blocked] Security policy: read-only mode — only read commands are allowed"; + let msg = PolicyDenial::SecurityPolicyBlocked { + tool: "run_command", + raw_reason: raw, + } + .render(); + + // The marker survives so classification + the loop-breaker still match. + assert!(msg.contains(POLICY_BLOCKED_MARKER)); + assert!(msg.starts_with("Blocked:")); + // The original reason is preserved (without a duplicated marker in it). + assert!(msg.contains("read-only mode — only read commands are allowed")); + assert!(msg.contains("Workaround:")); + assert!(msg.contains("agent-access tier / autonomy") || msg.contains("Agent access")); + assert!(msg.contains("Relay this to the user")); + } + + #[test] + fn maybe_enrich_only_touches_raw_marker_results() { + // A raw marker line with no workaround → enriched. + let raw = "[policy-blocked] Command not allowed by security policy: rm -rf /"; + let enriched = maybe_enrich_policy_block("run_command", raw) + .expect("a raw policy block should be enriched"); + assert!(enriched.contains("Workaround:")); + assert!(enriched.contains("Relay this to the user")); + assert!(enriched.contains(POLICY_BLOCKED_MARKER)); + + // An already-structured ToolPolicyMiddleware denial (has "Workaround:") is + // left alone — no double-wrapping. + let already = PolicyDenial::PolicyDenied { + tool: "run_script", + policy: "sandbox", + reason: "sandbox restriction", + } + .render(); + assert!(maybe_enrich_policy_block("run_script", &already).is_none()); + + // A plain non-policy error is untouched. + assert!(maybe_enrich_policy_block("read_file", "Error: file not found").is_none()); + } + #[test] fn approval_required_suggests_approval_then_retry() { let msg = PolicyDenial::ApprovalRequired { diff --git a/tests/config_auth_app_state_connectivity_e2e.rs b/tests/config_auth_app_state_connectivity_e2e.rs index 266d46584c..802855f2fe 100644 --- a/tests/config_auth_app_state_connectivity_e2e.rs +++ b/tests/config_auth_app_state_connectivity_e2e.rs @@ -2802,6 +2802,7 @@ async fn worker_a_controller_schemas_are_fully_exposed() { "openhuman.config_get_meet_settings", "openhuman.config_get_memory_sync_settings", "openhuman.config_get_onboarding_completed", + "openhuman.config_get_privacy_mode", "openhuman.config_get_runtime_flags", "openhuman.config_get_sandbox_settings", "openhuman.config_get_search_settings", @@ -2811,6 +2812,7 @@ async fn worker_a_controller_schemas_are_fully_exposed() { "openhuman.config_resolve_api_url", "openhuman.config_set_browser_allow_all", "openhuman.config_set_onboarding_completed", + "openhuman.config_set_privacy_mode", "openhuman.config_set_super_context_enabled", "openhuman.config_update_activity_level_settings", "openhuman.config_update_agent_paths",