From 71da7d4f1fc028885457b580ace5b50ca35aec76 Mon Sep 17 00:00:00 2001 From: Henry Park Date: Fri, 27 Mar 2026 16:52:21 -0700 Subject: [PATCH] fix(web): sanitize routine trigger errors --- src/channels/web/handlers/routines.rs | 17 ++--------- src/channels/web/handlers/webhooks.rs | 17 ++++------- src/channels/web/server.rs | 44 +++++++-------------------- src/channels/web/util.rs | 39 ++++++++++++++++++++++++ 4 files changed, 58 insertions(+), 59 deletions(-) diff --git a/src/channels/web/handlers/routines.rs b/src/channels/web/handlers/routines.rs index fc914a35..5f5041a7 100644 --- a/src/channels/web/handlers/routines.rs +++ b/src/channels/web/handlers/routines.rs @@ -14,8 +14,7 @@ use crate::agent::routine::{Trigger, next_cron_fire}; use crate::channels::web::auth::AuthenticatedUser; use crate::channels::web::server::GatewayState; use crate::channels::web::types::*; -use crate::channels::web::util::sanitized_db_error; -use crate::error::RoutineError; +use crate::channels::web::util::{sanitized_db_error, sanitized_routine_error}; pub async fn routines_list_handler( State(state): State>, @@ -164,7 +163,7 @@ pub async fn routines_trigger_handler( let run_id = engine .fire_manual(routine_id, Some(&user.user_id)) .await - .map_err(|e| (routine_error_status(&e), e.to_string()))?; + .map_err(|e| sanitized_routine_error(e, "trigger routine manually"))?; Ok(Json(serde_json::json!({ "status": "triggered", @@ -337,15 +336,3 @@ pub async fn routines_runs_handler( "runs": run_infos, }))) } - -/// Map `RoutineError` variants to appropriate HTTP status codes. -fn routine_error_status(err: &RoutineError) -> StatusCode { - match err { - RoutineError::NotFound { .. } => StatusCode::NOT_FOUND, - RoutineError::NotAuthorized { .. } => StatusCode::FORBIDDEN, - RoutineError::Disabled { .. } - | RoutineError::Cooldown { .. } - | RoutineError::MaxConcurrent { .. } => StatusCode::CONFLICT, - _ => StatusCode::INTERNAL_SERVER_ERROR, - } -} diff --git a/src/channels/web/handlers/webhooks.rs b/src/channels/web/handlers/webhooks.rs index 1fd78c66..4b416c93 100644 --- a/src/channels/web/handlers/webhooks.rs +++ b/src/channels/web/handlers/webhooks.rs @@ -15,6 +15,7 @@ use subtle::ConstantTimeEq; use crate::agent::routine::Trigger; use crate::channels::web::server::GatewayState; +use crate::channels::web::util::{sanitized_db_error, sanitized_routine_error}; /// Validate the webhook secret for a routine. /// @@ -103,7 +104,7 @@ async fn fire_webhook_inner( let routine = store .get_webhook_routine_by_path(path, user_id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))? + .map_err(|e| sanitized_db_error(e, "get webhook routine by path"))? .ok_or(( StatusCode::NOT_FOUND, "No routine matches this webhook path".to_string(), @@ -126,16 +127,10 @@ async fn fire_webhook_inner( ))? }; - let run_id = engine.fire_webhook(routine.id, path).await.map_err(|e| { - let status = match &e { - crate::error::RoutineError::NotFound { .. } => StatusCode::NOT_FOUND, - crate::error::RoutineError::Disabled { .. } - | crate::error::RoutineError::Cooldown { .. } - | crate::error::RoutineError::MaxConcurrent { .. } => StatusCode::CONFLICT, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - (status, e.to_string()) - })?; + let run_id = engine + .fire_webhook(routine.id, path) + .await + .map_err(|e| sanitized_routine_error(e, "trigger routine from webhook"))?; Ok(Json(serde_json::json!({ "status": "triggered", diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs index 4f0c6cd8..15608dca 100644 --- a/src/channels/web/server.rs +++ b/src/channels/web/server.rs @@ -3068,8 +3068,10 @@ mod tests { // --- OAuth callback handler tests --- - /// Build a minimal `GatewayState` for testing the OAuth callback handler. - fn test_gateway_state(ext_mgr: Option>) -> Arc { + fn test_gateway_state_inner( + ext_mgr: Option>, + store: Option>, + ) -> Arc { Arc::new(GatewayState { msg_tx: tokio::sync::RwLock::new(None), sse: Arc::new(SseManager::new()), @@ -3080,7 +3082,7 @@ mod tests { log_level_handle: None, extension_manager: ext_mgr, tool_registry: None, - store: None, + store, job_manager: None, prompt_queue: None, owner_id: "test".to_string(), @@ -3102,40 +3104,16 @@ mod tests { }) } + /// Build a minimal `GatewayState` for testing the OAuth callback handler. + fn test_gateway_state(ext_mgr: Option>) -> Arc { + test_gateway_state_inner(ext_mgr, None) + } + fn test_gateway_state_with_store( store: Arc, ext_mgr: Option>, ) -> Arc { - Arc::new(GatewayState { - msg_tx: tokio::sync::RwLock::new(None), - sse: Arc::new(SseManager::new()), - workspace: None, - workspace_pool: None, - session_manager: None, - log_broadcaster: None, - log_level_handle: None, - extension_manager: ext_mgr, - tool_registry: None, - store: Some(store), - job_manager: None, - prompt_queue: None, - owner_id: "test".to_string(), - default_sender_id: "test".to_string(), - shutdown_tx: tokio::sync::RwLock::new(None), - ws_tracker: None, - llm_provider: None, - skill_registry: None, - skill_catalog: None, - scheduler: None, - chat_rate_limiter: PerUserRateLimiter::new(30, 60), - oauth_rate_limiter: RateLimiter::new(10, 60), - webhook_rate_limiter: RateLimiter::new(10, 60), - registry_entries: vec![], - cost_guard: None, - routine_engine: Arc::new(tokio::sync::RwLock::new(None)), - startup_time: std::time::Instant::now(), - active_config: ActiveConfigSnapshot::default(), - }) + test_gateway_state_inner(ext_mgr, Some(store)) } #[cfg(feature = "libsql")] diff --git a/src/channels/web/util.rs b/src/channels/web/util.rs index 62197e36..297a0e2a 100644 --- a/src/channels/web/util.rs +++ b/src/channels/web/util.rs @@ -38,6 +38,34 @@ pub fn sanitized_internal_error_response( sanitized_internal_error(error, context, "Internal error") } +/// Return safe client responses for `RoutineError` while preserving user-actionable variants. +pub fn sanitized_routine_error( + error: crate::error::RoutineError, + context: &str, +) -> (StatusCode, String) { + use crate::error::RoutineError; + + match error { + err @ RoutineError::NotFound { .. } => (StatusCode::NOT_FOUND, err.to_string()), + err @ RoutineError::NotAuthorized { .. } => (StatusCode::FORBIDDEN, err.to_string()), + err @ RoutineError::Disabled { .. } + | err @ RoutineError::Cooldown { .. } + | err @ RoutineError::MaxConcurrent { .. } => (StatusCode::CONFLICT, err.to_string()), + err @ RoutineError::Database { .. } => sanitized_db_error(err, context), + err @ RoutineError::LlmFailed { .. } + | err @ RoutineError::JobDispatchFailed { .. } + | err @ RoutineError::EmptyResponse + | err @ RoutineError::TruncatedResponse + | err @ RoutineError::UnknownTriggerType { .. } + | err @ RoutineError::UnknownActionType { .. } + | err @ RoutineError::MissingField { .. } + | err @ RoutineError::InvalidCron { .. } + | err @ RoutineError::UnknownRunStatus { .. } => { + sanitized_internal_error_response(err, context) + } + } +} + /// Parse tool call summary JSON objects into `ToolCallInfo` structs. fn parse_tool_call_infos(calls: &[serde_json::Value]) -> Vec { calls @@ -170,6 +198,17 @@ mod tests { assert_eq!(body, "Internal error"); } + #[test] + fn test_sanitized_routine_error_hides_database_details() { + let (_, body) = sanitized_routine_error( + crate::error::RoutineError::Database { + reason: "sqlite: no such table: routine_runs".to_string(), + }, + "trigger routine", + ); + assert_eq!(body, "Database error"); + } + // ---- build_turns_from_db_messages tests ---- fn make_msg(role: &str, content: &str, offset_ms: i64) -> crate::history::ConversationMessage {