mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
fix(web): sanitize routine trigger errors
This commit is contained in:
@@ -14,8 +14,7 @@ use crate::agent::routine::{Trigger, next_cron_fire};
|
|||||||
use crate::channels::web::auth::AuthenticatedUser;
|
use crate::channels::web::auth::AuthenticatedUser;
|
||||||
use crate::channels::web::server::GatewayState;
|
use crate::channels::web::server::GatewayState;
|
||||||
use crate::channels::web::types::*;
|
use crate::channels::web::types::*;
|
||||||
use crate::channels::web::util::sanitized_db_error;
|
use crate::channels::web::util::{sanitized_db_error, sanitized_routine_error};
|
||||||
use crate::error::RoutineError;
|
|
||||||
|
|
||||||
pub async fn routines_list_handler(
|
pub async fn routines_list_handler(
|
||||||
State(state): State<Arc<GatewayState>>,
|
State(state): State<Arc<GatewayState>>,
|
||||||
@@ -164,7 +163,7 @@ pub async fn routines_trigger_handler(
|
|||||||
let run_id = engine
|
let run_id = engine
|
||||||
.fire_manual(routine_id, Some(&user.user_id))
|
.fire_manual(routine_id, Some(&user.user_id))
|
||||||
.await
|
.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!({
|
Ok(Json(serde_json::json!({
|
||||||
"status": "triggered",
|
"status": "triggered",
|
||||||
@@ -337,15 +336,3 @@ pub async fn routines_runs_handler(
|
|||||||
"runs": run_infos,
|
"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,
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ use subtle::ConstantTimeEq;
|
|||||||
|
|
||||||
use crate::agent::routine::Trigger;
|
use crate::agent::routine::Trigger;
|
||||||
use crate::channels::web::server::GatewayState;
|
use crate::channels::web::server::GatewayState;
|
||||||
|
use crate::channels::web::util::{sanitized_db_error, sanitized_routine_error};
|
||||||
|
|
||||||
/// Validate the webhook secret for a routine.
|
/// Validate the webhook secret for a routine.
|
||||||
///
|
///
|
||||||
@@ -103,7 +104,7 @@ async fn fire_webhook_inner(
|
|||||||
let routine = store
|
let routine = store
|
||||||
.get_webhook_routine_by_path(path, user_id)
|
.get_webhook_routine_by_path(path, user_id)
|
||||||
.await
|
.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((
|
.ok_or((
|
||||||
StatusCode::NOT_FOUND,
|
StatusCode::NOT_FOUND,
|
||||||
"No routine matches this webhook path".to_string(),
|
"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 run_id = engine
|
||||||
let status = match &e {
|
.fire_webhook(routine.id, path)
|
||||||
crate::error::RoutineError::NotFound { .. } => StatusCode::NOT_FOUND,
|
.await
|
||||||
crate::error::RoutineError::Disabled { .. }
|
.map_err(|e| sanitized_routine_error(e, "trigger routine from webhook"))?;
|
||||||
| crate::error::RoutineError::Cooldown { .. }
|
|
||||||
| crate::error::RoutineError::MaxConcurrent { .. } => StatusCode::CONFLICT,
|
|
||||||
_ => StatusCode::INTERNAL_SERVER_ERROR,
|
|
||||||
};
|
|
||||||
(status, e.to_string())
|
|
||||||
})?;
|
|
||||||
|
|
||||||
Ok(Json(serde_json::json!({
|
Ok(Json(serde_json::json!({
|
||||||
"status": "triggered",
|
"status": "triggered",
|
||||||
|
|||||||
+11
-33
@@ -3068,8 +3068,10 @@ mod tests {
|
|||||||
|
|
||||||
// --- OAuth callback handler tests ---
|
// --- OAuth callback handler tests ---
|
||||||
|
|
||||||
/// Build a minimal `GatewayState` for testing the OAuth callback handler.
|
fn test_gateway_state_inner(
|
||||||
fn test_gateway_state(ext_mgr: Option<Arc<ExtensionManager>>) -> Arc<GatewayState> {
|
ext_mgr: Option<Arc<ExtensionManager>>,
|
||||||
|
store: Option<Arc<dyn crate::db::Database>>,
|
||||||
|
) -> Arc<GatewayState> {
|
||||||
Arc::new(GatewayState {
|
Arc::new(GatewayState {
|
||||||
msg_tx: tokio::sync::RwLock::new(None),
|
msg_tx: tokio::sync::RwLock::new(None),
|
||||||
sse: Arc::new(SseManager::new()),
|
sse: Arc::new(SseManager::new()),
|
||||||
@@ -3080,7 +3082,7 @@ mod tests {
|
|||||||
log_level_handle: None,
|
log_level_handle: None,
|
||||||
extension_manager: ext_mgr,
|
extension_manager: ext_mgr,
|
||||||
tool_registry: None,
|
tool_registry: None,
|
||||||
store: None,
|
store,
|
||||||
job_manager: None,
|
job_manager: None,
|
||||||
prompt_queue: None,
|
prompt_queue: None,
|
||||||
owner_id: "test".to_string(),
|
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<ExtensionManager>>) -> Arc<GatewayState> {
|
||||||
|
test_gateway_state_inner(ext_mgr, None)
|
||||||
|
}
|
||||||
|
|
||||||
fn test_gateway_state_with_store(
|
fn test_gateway_state_with_store(
|
||||||
store: Arc<dyn crate::db::Database>,
|
store: Arc<dyn crate::db::Database>,
|
||||||
ext_mgr: Option<Arc<ExtensionManager>>,
|
ext_mgr: Option<Arc<ExtensionManager>>,
|
||||||
) -> Arc<GatewayState> {
|
) -> Arc<GatewayState> {
|
||||||
Arc::new(GatewayState {
|
test_gateway_state_inner(ext_mgr, Some(store))
|
||||||
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(),
|
|
||||||
})
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(feature = "libsql")]
|
#[cfg(feature = "libsql")]
|
||||||
|
|||||||
@@ -38,6 +38,34 @@ pub fn sanitized_internal_error_response<E: Display>(
|
|||||||
sanitized_internal_error(error, context, "Internal error")
|
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.
|
/// Parse tool call summary JSON objects into `ToolCallInfo` structs.
|
||||||
fn parse_tool_call_infos(calls: &[serde_json::Value]) -> Vec<ToolCallInfo> {
|
fn parse_tool_call_infos(calls: &[serde_json::Value]) -> Vec<ToolCallInfo> {
|
||||||
calls
|
calls
|
||||||
@@ -170,6 +198,17 @@ mod tests {
|
|||||||
assert_eq!(body, "Internal error");
|
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 ----
|
// ---- build_turns_from_db_messages tests ----
|
||||||
|
|
||||||
fn make_msg(role: &str, content: &str, offset_ms: i64) -> crate::history::ConversationMessage {
|
fn make_msg(role: &str, content: &str, offset_ms: i64) -> crate::history::ConversationMessage {
|
||||||
|
|||||||
Reference in New Issue
Block a user