From 9f565d84fc83541dbaea32c373c316c231848345 Mon Sep 17 00:00:00 2001 From: CodeGhost21 <164498022+CodeGhost21@users.noreply.github.com> Date: Thu, 14 May 2026 02:55:48 +0530 Subject: [PATCH] fix(triage): defer instead of error on prompt-guard rejection (OPENHUMAN-TAURI-X) (#1678) --- src/openhuman/agent/triage/evaluator.rs | 119 ++++++++++-- src/openhuman/agent/triage/evaluator_tests.rs | 177 ++++++++++++++++++ 2 files changed, 280 insertions(+), 16 deletions(-) diff --git a/src/openhuman/agent/triage/evaluator.rs b/src/openhuman/agent/triage/evaluator.rs index 39c70946b..375c58ce7 100644 --- a/src/openhuman/agent/triage/evaluator.rs +++ b/src/openhuman/agent/triage/evaluator.rs @@ -210,6 +210,12 @@ where // eventual Deferred reason explains *why* we're sitting idle rather // than the generic "both arms failed" copy. let mut cloud_budget_exhausted: Option = None; + // Track whether the cloud arm bailed because the prompt-guard + // flagged the content (OPENHUMAN-TAURI-X). The guard runs before + // dispatch and is shared across arms, so a flag on cloud will + // repeat on local — surface the verdict in the Deferred reason + // for operator-facing telemetry. + let mut cloud_safety_flagged: Option = None; // ── Cloud arm ────────────────────────────────────────────────── match try_arm(&cloud, envelope, TriageResolutionPath::Cloud).await { @@ -227,6 +233,18 @@ where ); cloud_budget_exhausted = Some(err); } + Err(ArmError::SafetyFlagged(err)) => { + tracing::warn!( + source = %envelope.source.slug(), + label = %envelope.display_label, + external_id = %envelope.external_id, + path = TriageResolutionPath::Cloud.as_str(), + error = %err, + "[triage::evaluator] cloud rejected by prompt-guard; \ + skipping retry and falling back to local arm" + ); + cloud_safety_flagged = Some(err); + } Err(ArmError::Retryable { retry_after_ms, .. }) => { // Sleep before the cloud retry. Honour Retry-After when // present; otherwise use a short backoff so the second @@ -257,6 +275,18 @@ where ); cloud_budget_exhausted = Some(err); } + Err(ArmError::SafetyFlagged(err)) => { + tracing::warn!( + source = %envelope.source.slug(), + label = %envelope.display_label, + external_id = %envelope.external_id, + path = TriageResolutionPath::CloudAfterRetry.as_str(), + error = %err, + "[triage::evaluator] cloud rejected by prompt-guard on retry; \ + falling back to local arm" + ); + cloud_safety_flagged = Some(err); + } Err(ArmError::Retryable { .. }) => { // Exhausted cloud budget — fall through to local. tracing::warn!( @@ -278,19 +308,28 @@ where // forwarded into telemetry / UI, so it must stay a stable, // scrubbed string. Raw upstream error text goes to the debug // log instead, where it is operator-visible but not surfaced. - let reason = match &cloud_budget_exhausted { - Some(err) => { - tracing::debug!( - target: "[triage::evaluator]", - source = %envelope.source.slug(), - label = %envelope.display_label, - external_id = %envelope.external_id, - error = %err, - "cloud budget exhausted; no local arm — full upstream error" - ); - "cloud budget exhausted; local arm unavailable".to_string() - } - None => "cloud retry exhausted; local arm unavailable".to_string(), + let reason = if let Some(err) = cloud_safety_flagged.as_ref() { + tracing::debug!( + target: "[triage::evaluator]", + source = %envelope.source.slug(), + label = %envelope.display_label, + external_id = %envelope.external_id, + error = %err, + "prompt-guard rejected on cloud; no local arm — full guard verdict" + ); + "prompt-guard rejection; local arm unavailable".to_string() + } else if let Some(err) = cloud_budget_exhausted.as_ref() { + tracing::debug!( + target: "[triage::evaluator]", + source = %envelope.source.slug(), + label = %envelope.display_label, + external_id = %envelope.external_id, + error = %err, + "cloud budget exhausted; no local arm — full upstream error" + ); + "cloud budget exhausted; local arm unavailable".to_string() + } else { + "cloud retry exhausted; local arm unavailable".to_string() }; return Ok(TriageOutcome::Deferred { defer_until_ms: now_ms().saturating_add(DEFER_WAKEUP_MS), @@ -306,6 +345,7 @@ where Ok(run) => Ok(TriageOutcome::Decision(run)), Err(ArmError::Fatal(err)) | Err(ArmError::BudgetExhausted(err)) + | Err(ArmError::SafetyFlagged(err)) | Err(ArmError::Retryable { source: err, .. }) => { // Local also failed — defer rather than surface a hard // error. Today's "hard fail" is the wrong default for a @@ -315,9 +355,12 @@ where // flow into telemetry / UI, so keep it scrubbed. Raw error // text from cloud + local lives in the structured warn // fields below — visible to operators, not callers. - let reason = match cloud_budget_exhausted.as_ref() { - Some(_) => "cloud budget exhausted; local arm also failed".to_string(), - None => "cloud retry exhausted; local arm also failed".to_string(), + let reason = if cloud_safety_flagged.is_some() { + "prompt-guard rejection; local arm also failed".to_string() + } else if cloud_budget_exhausted.is_some() { + "cloud budget exhausted; local arm also failed".to_string() + } else { + "cloud retry exhausted; local arm also failed".to_string() }; tracing::warn!( target: "[triage::evaluator]", @@ -327,6 +370,7 @@ where local_error = %err, cloud_error = cloud_budget_exhausted .as_ref() + .or(cloud_safety_flagged.as_ref()) .map(|e| e.to_string()) .unwrap_or_default(), defer_ms = DEFER_WAKEUP_MS, @@ -361,6 +405,17 @@ enum ArmError { /// This is **not** a fatal error: the user takes an explicit /// action (top up) to fix it, so it must not page Sentry. BudgetExhausted(anyhow::Error), + /// Our prompt-injection guard (`agent::bus`'s `enforce_prompt_input`, + /// also used by `agent::harness::session::runtime`) flagged the + /// incoming content as adversarial / unsafe and refused to dispatch + /// the turn. The guard runs *before* either model is contacted, so + /// trying the same prompt again — on cloud or local — produces the + /// same verdict. This is **not** a fatal error: the guard is doing + /// its job (OPENHUMAN-TAURI-X regression: an adversarial Gmail + /// message reliably trips the guard, and every fire paged Sentry). + /// Route the same way as `BudgetExhausted` so the local-arm fallthrough + /// lands in `TriageOutcome::Deferred` rather than `Err(_)`. + SafetyFlagged(anyhow::Error), } /// Run a single arm: dispatch the agent turn through the native bus @@ -519,9 +574,41 @@ fn classify_error(message: String) -> ArmError { if is_inference_budget_exceeded(&message) { return ArmError::BudgetExhausted(err); } + // Prompt-guard rejection (`agent::bus::enforce_prompt_input` → + // `ReviewBlocked` / `Blocked`). The guard fires *before* either + // arm contacts a model, so the verdict is identical on cloud and + // local — no point retrying. Treat as Deferred-eligible so the + // chain ends in `TriageOutcome::Deferred` rather than Fatal, + // which was paging Sentry for adversarial-email triage attempts + // (OPENHUMAN-TAURI-X regression). + if is_prompt_guard_rejection(&message) { + return ArmError::SafetyFlagged(err); + } ArmError::Fatal(err) } +/// Returns `true` when `message` is the verbatim string our prompt-injection +/// guard returns when it rejects a turn before dispatch. +/// +/// Canonical sources: +/// - `src/openhuman/agent/bus.rs` — `Blocked` / `ReviewBlocked` arms of the +/// `enforce_prompt_input` decision (the path the triage evaluator hits via +/// `agent.run_turn`). +/// - `src/openhuman/agent/harness/session/runtime.rs` — same strings in the +/// tool-call loop, kept identical so this classifier covers both. +/// - `src/openhuman/local_ai/ops.rs` — user-facing variants with the +/// `"Please rephrase clearly."` suffix; we match the leading phrase so +/// either form classifies. +/// +/// Kept narrow on purpose: the guard's full output strings are private to +/// our code, so a substring match against the leading phrase will not collide +/// with anything coming back from upstream providers. +fn is_prompt_guard_rejection(message: &str) -> bool { + let lower = message.to_ascii_lowercase(); + lower.contains("prompt flagged for security review") + || lower.contains("prompt blocked by security policy") +} + /// Returns `true` when `message` signals that the upstream rejected the /// call because the user's inference budget or credit balance is empty — /// meaning a retry would hit the same wall. diff --git a/src/openhuman/agent/triage/evaluator_tests.rs b/src/openhuman/agent/triage/evaluator_tests.rs index f2b54bd2c..b9d9b7453 100644 --- a/src/openhuman/agent/triage/evaluator_tests.rs +++ b/src/openhuman/agent/triage/evaluator_tests.rs @@ -163,6 +163,7 @@ fn classify_string_recognises_top_up_and_out_of_credits_as_budget_exhausted() { ArmError::Retryable { .. } => "Retryable", ArmError::Fatal(_) => "Fatal", ArmError::BudgetExhausted(_) => "BudgetExhausted", + ArmError::SafetyFlagged(_) => "SafetyFlagged", }; assert!( matches!(&err, ArmError::BudgetExhausted(_)), @@ -171,6 +172,60 @@ fn classify_string_recognises_top_up_and_out_of_credits_as_budget_exhausted() { } } +fn arm_error_label(err: &ArmError) -> &'static str { + match err { + ArmError::Retryable { .. } => "Retryable", + ArmError::Fatal(_) => "Fatal", + ArmError::BudgetExhausted(_) => "BudgetExhausted", + ArmError::SafetyFlagged(_) => "SafetyFlagged", + } +} + +#[test] +fn classify_string_recognises_prompt_guard_rejection_as_safety_flagged() { + // OPENHUMAN-TAURI-X regression: the exact phrase our prompt-injection + // guard emits from `agent::bus::enforce_prompt_input` / + // `agent::harness::session::runtime` when it refuses to dispatch a + // turn. Was previously classified as Fatal, which paged Sentry for + // every adversarial Gmail message the triage agent saw. + for raw in [ + "Prompt flagged for security review and was not processed.", + "Prompt flagged for security review and was not processed. Please rephrase clearly.", + "Prompt blocked by security policy.", + // Wrapped in caller context (the agent bus surfaces the + // verdict via `NativeRequestError::HandlerFailed` whose + // message may carry additional prefix) — substring match + // must still classify. + "[agent.run_turn dispatch] HandlerFailed: Prompt flagged for security review and was not processed.", + ] { + let err = classify_error(raw.to_string()); + let label = arm_error_label(&err); + assert!( + matches!(&err, ArmError::SafetyFlagged(_)), + "expected SafetyFlagged for {raw:?}, got {label}" + ); + } +} + +#[test] +fn classify_string_does_not_misclassify_unrelated_security_phrases() { + // Conservative: only the verbatim guard phrases match. A doc-string + // mentioning "security review" generically must NOT classify, or + // we'd silently hide real issues from Sentry. + for raw in [ + "scheduling a quarterly security review", + "the runbook covers security policy violations", + "ai security report attached", + ] { + let err = classify_error(raw.to_string()); + let label = arm_error_label(&err); + assert!( + matches!(&err, ArmError::Fatal(_)), + "expected Fatal for {raw:?}, got {label}" + ); + } +} + // ── Tiered fallback integration tests ─────────────────────────── // // These drive `run_triage_with_arms` end-to-end through the agent @@ -557,6 +612,128 @@ async fn cloud_budget_exhausted_without_local_returns_deferred_not_err() { ); } +#[tokio::test] +async fn cloud_safety_flagged_then_local_flagged_defers_not_errs() { + // Regression for OPENHUMAN-TAURI-X (regressed): the prompt-injection + // guard fires the same verdict on cloud and local arms (the guard + // runs in `agent::bus::run_turn` before either model is contacted), + // so the realistic path is cloud-flagged → local-flagged → defer. + // Previously this paged Sentry every time an adversarial Gmail + // triage attempt fired (118 hits in 6 days). + AgentDefinitionRegistry::init_global_builtins().expect("init_global_builtins"); + let counter = StdArc::new(AtomicUsize::new(0)); + let counter_for_stub = StdArc::clone(&counter); + + let _guard = mock_agent_run_turn(move |_req| { + let counter = StdArc::clone(&counter_for_stub); + async move { + counter.fetch_add(1, Ordering::SeqCst); + // Verbatim string our prompt-injection guard emits. + Err("Prompt flagged for security review and was not processed.".to_string()) + } + }) + .await; + + let outcome = run_triage_with_arms_for_test(cloud_arm(), Some(local_arm()), &envelope()) + .await + .expect("safety-flagged must not surface as Err — must Defer"); + + match outcome { + TriageOutcome::Deferred { reason, .. } => { + assert!( + reason.to_lowercase().contains("prompt-guard"), + "deferral reason should name the prompt-guard cause: {reason}" + ); + } + TriageOutcome::Decision(_) => panic!("expected Deferred, got Decision"), + } + assert_eq!( + counter.load(Ordering::SeqCst), + 2, + "1 cloud (safety-flagged, no retry) + 1 local (same verdict) = 2" + ); +} + +#[tokio::test] +async fn cloud_safety_flagged_then_local_recovers_decides_on_local() { + // Defense in depth: while in practice cloud + local share the + // guard verdict, the chain must still cleanly dispatch to local + // if cloud is the only arm that flagged. Locks in the + // skip-retry-and-fall-through semantics independently of whether + // local also flags. + AgentDefinitionRegistry::init_global_builtins().expect("init_global_builtins"); + let counter = StdArc::new(AtomicUsize::new(0)); + let counter_for_stub = StdArc::clone(&counter); + + let _guard = mock_agent_run_turn(move |req| { + let counter = StdArc::clone(&counter_for_stub); + async move { + let n = counter.fetch_add(1, Ordering::SeqCst); + if n == 0 { + assert_eq!(req.provider_name, "stub-cloud", "first call must hit cloud"); + Err("Prompt flagged for security review and was not processed.".to_string()) + } else { + assert_eq!( + req.provider_name, "stub-local", + "safety-flagged on cloud must skip cloud retry and dispatch to local" + ); + Ok(AgentTurnResponse { + text: VALID_JSON_REPLY.to_string(), + }) + } + } + }) + .await; + + let outcome = run_triage_with_arms_for_test(cloud_arm(), Some(local_arm()), &envelope()) + .await + .expect("safety-flagged must not surface as Err"); + + let run = outcome.into_decision().expect("decision"); + assert_eq!(run.resolution_path, TriageResolutionPath::LocalFallback); + assert!(run.used_local); + assert_eq!( + counter.load(Ordering::SeqCst), + 2, + "1 cloud (safety-flagged) + 1 local = 2 (no cloud retry)" + ); +} + +#[tokio::test] +async fn cloud_safety_flagged_without_local_returns_deferred_not_err() { + AgentDefinitionRegistry::init_global_builtins().expect("init_global_builtins"); + let counter = StdArc::new(AtomicUsize::new(0)); + let counter_for_stub = StdArc::clone(&counter); + + let _guard = mock_agent_run_turn(move |_req| { + let counter = StdArc::clone(&counter_for_stub); + async move { + counter.fetch_add(1, Ordering::SeqCst); + Err("Prompt flagged for security review and was not processed.".to_string()) + } + }) + .await; + + let outcome = run_triage_with_arms_for_test(cloud_arm(), None, &envelope()) + .await + .expect("safety-flagged with no local must Defer, not Err"); + + match outcome { + TriageOutcome::Deferred { reason, .. } => { + assert!( + reason.to_lowercase().contains("prompt-guard"), + "deferral reason should name the prompt-guard cause: {reason}" + ); + } + TriageOutcome::Decision(_) => panic!("expected Deferred"), + } + assert_eq!( + counter.load(Ordering::SeqCst), + 1, + "no retry — guard would block the second cloud call too" + ); +} + #[tokio::test] async fn no_local_arm_returns_deferred_after_cloud_exhaustion() { AgentDefinitionRegistry::init_global_builtins().expect("init_global_builtins");