diff --git a/src/channels/web/handlers/jobs.rs b/src/channels/web/handlers/jobs.rs index 35adeec6..06b4462a 100644 --- a/src/channels/web/handlers/jobs.rs +++ b/src/channels/web/handlers/jobs.rs @@ -14,6 +14,7 @@ use uuid::Uuid; 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, sanitized_internal_error_response}; pub async fn jobs_list_handler( State(state): State>, @@ -213,10 +214,7 @@ pub async fn jobs_detail_handler( } Ok(None) => {} Err(e) => { - return Err(( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Database error: {}", e), - )); + return Err(sanitized_db_error(e, "get sandbox job detail")); } } @@ -257,10 +255,7 @@ pub async fn jobs_detail_handler( })) } Ok(None) => Err((StatusCode::NOT_FOUND, "Job not found".to_string())), - Err(e) => Err(( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Database error: {}", e), - )), + Err(e) => Err(sanitized_db_error(e, "get agent job detail")), } } @@ -295,7 +290,7 @@ pub async fn jobs_cancel_handler( Some(chrono::Utc::now()), ) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| sanitized_db_error(e, "persist sandbox job cancellation"))?; } return Ok(Json(serde_json::json!({ "status": "cancelled", @@ -304,10 +299,7 @@ pub async fn jobs_cancel_handler( } Ok(None) => {} Err(e) => { - return Err(( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Database error: {}", e), - )); + return Err(sanitized_db_error(e, "get sandbox job for cancellation")); } } } @@ -341,7 +333,7 @@ pub async fn jobs_cancel_handler( Some("Cancelled by user"), ) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| sanitized_db_error(e, "persist agent job cancellation"))?; } return Ok(Json(serde_json::json!({ "status": "cancelled", @@ -350,10 +342,7 @@ pub async fn jobs_cancel_handler( } Ok(None) => {} Err(e) => { - return Err(( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Database error: {}", e), - )); + return Err(sanitized_db_error(e, "get agent job for cancellation")); } } } @@ -421,7 +410,7 @@ pub async fn jobs_restart_handler( store .save_sandbox_job(&record) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| sanitized_db_error(e, "persist restarted sandbox job"))?; let mode = match store.get_sandbox_job_mode(old_job_id).await { Ok(Some(m)) if m == "claude_code" => { @@ -452,16 +441,13 @@ pub async fn jobs_restart_handler( ) .await .map_err(|e| { - ( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Failed to create container: {}", e), - ) + sanitized_internal_error_response(e, "create restarted sandbox container") })?; store .update_sandbox_job_status(new_job_id, "running", None, None, Some(now), None) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| sanitized_db_error(e, "mark restarted sandbox job running"))?; return Ok(Json(serde_json::json!({ "status": "restarted", @@ -471,10 +457,7 @@ pub async fn jobs_restart_handler( } Ok(None) => {} Err(e) => { - return Err(( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Database error: {}", e), - )); + return Err(sanitized_db_error(e, "get sandbox job for restart")); } } @@ -521,7 +504,9 @@ pub async fn jobs_restart_handler( let new_job_id = scheduler .dispatch_job(&old_job.user_id, &title, &old_job.description, None) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + sanitized_internal_error_response(e, "dispatch restarted agent job") + })?; Ok(Json(serde_json::json!({ "status": "restarted", @@ -530,10 +515,7 @@ pub async fn jobs_restart_handler( }))) } Ok(None) => Err((StatusCode::NOT_FOUND, "Job not found".to_string())), - Err(e) => Err(( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Database error: {}", e), - )), + Err(e) => Err(sanitized_db_error(e, "get agent job for restart")), } } @@ -609,10 +591,7 @@ pub async fn jobs_prompt_handler( return Err((StatusCode::NOT_FOUND, "Job not found".to_string())); } Err(e) => { - return Err(( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Database error: {}", e), - )); + return Err(sanitized_db_error(e, "get agent job for prompt")); } } } @@ -625,10 +604,9 @@ pub async fn jobs_prompt_handler( if let Some(ref scheduler) = *scheduler_guard && scheduler.is_running(job_id).await { - scheduler - .send_message(job_id, content) - .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + scheduler.send_message(job_id, content).await.map_err(|e| { + sanitized_internal_error_response(e, "send prompt to running agent job") + })?; return Ok(Json(serde_json::json!({ "status": "sent", "job_id": job_id.to_string(), @@ -667,17 +645,14 @@ pub async fn jobs_events_handler( return Err((StatusCode::NOT_FOUND, "Job not found".to_string())); } Err(e) => { - return Err(( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Database error: {}", e), - )); + return Err(sanitized_db_error(e, "get sandbox job events")); } } let events = store .list_job_events(job_id, None) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| sanitized_db_error(e, "list job events"))?; let events_json: Vec = events .into_iter() @@ -721,7 +696,7 @@ pub async fn job_files_list_handler( let job = store .get_sandbox_job(job_id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))? + .map_err(|e| sanitized_db_error(e, "get sandbox job file list"))? .ok_or((StatusCode::NOT_FOUND, "Job not found".to_string()))?; if job.user_id != user.user_id { @@ -789,7 +764,7 @@ pub async fn job_files_read_handler( let job = store .get_sandbox_job(job_id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))? + .map_err(|e| sanitized_db_error(e, "get sandbox job file read"))? .ok_or((StatusCode::NOT_FOUND, "Job not found".to_string()))?; if job.user_id != user.user_id { diff --git a/src/channels/web/handlers/routines.rs b/src/channels/web/handlers/routines.rs index fc56b187..fc914a35 100644 --- a/src/channels/web/handlers/routines.rs +++ b/src/channels/web/handlers/routines.rs @@ -14,6 +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; pub async fn routines_list_handler( @@ -28,7 +29,7 @@ pub async fn routines_list_handler( let routines = store .list_routines(&user.user_id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| sanitized_db_error(e, "list routines"))?; let items: Vec = routines.iter().map(RoutineInfo::from_routine).collect(); @@ -47,7 +48,7 @@ pub async fn routines_summary_handler( let routines = store .list_routines(&user.user_id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| sanitized_db_error(e, "list routines summary"))?; let total = routines.len() as u64; let enabled = routines.iter().filter(|r| r.enabled).count() as u64; @@ -95,7 +96,7 @@ pub async fn routines_detail_handler( let routine = store .get_routine(routine_id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))? + .map_err(|e| sanitized_db_error(e, "get routine detail"))? .ok_or((StatusCode::NOT_FOUND, "Routine not found".to_string()))?; if routine.user_id != user.user_id { @@ -105,7 +106,7 @@ pub async fn routines_detail_handler( let runs = store .list_routine_runs(routine_id, 20) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| sanitized_db_error(e, "list routine detail runs"))?; let recent_runs: Vec = runs .iter() @@ -194,7 +195,7 @@ pub async fn routines_toggle_handler( let mut routine = store .get_routine(routine_id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))? + .map_err(|e| sanitized_db_error(e, "get routine for toggle"))? .ok_or((StatusCode::NOT_FOUND, "Routine not found".to_string()))?; if routine.user_id != user.user_id { @@ -228,7 +229,7 @@ pub async fn routines_toggle_handler( store .update_routine(&routine) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| sanitized_db_error(e, "update routine toggle state"))?; // Refresh the in-memory event trigger cache so event/system_event // routines reflect the new enabled state immediately (issue #1076). @@ -259,7 +260,7 @@ pub async fn routines_delete_handler( let routine = store .get_routine(routine_id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))? + .map_err(|e| sanitized_db_error(e, "get routine for delete"))? .ok_or((StatusCode::NOT_FOUND, "Routine not found".to_string()))?; if routine.user_id != user.user_id { @@ -269,7 +270,7 @@ pub async fn routines_delete_handler( let deleted = store .delete_routine(routine_id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| sanitized_db_error(e, "delete routine"))?; if deleted { // Refresh the in-memory event trigger cache so deleted event/system_event @@ -305,7 +306,7 @@ pub async fn routines_runs_handler( let routine = store .get_routine(routine_id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))? + .map_err(|e| sanitized_db_error(e, "get routine runs"))? .ok_or((StatusCode::NOT_FOUND, "Routine not found".to_string()))?; if routine.user_id != user.user_id { @@ -315,7 +316,7 @@ pub async fn routines_runs_handler( let runs = store .list_routine_runs(routine_id, 50) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| sanitized_db_error(e, "list routine runs"))?; let run_infos: Vec = runs .iter() diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs index 033bfb0c..4f0c6cd8 100644 --- a/src/channels/web/server.rs +++ b/src/channels/web/server.rs @@ -3102,6 +3102,55 @@ mod tests { }) } + 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(), + }) + } + + #[cfg(feature = "libsql")] + async fn create_unmigrated_test_db() -> (Arc, tempfile::TempDir) { + use crate::db::libsql::LibSqlBackend; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let db_path = temp_dir.path().join("test.db"); + let backend = LibSqlBackend::new_local(&db_path) + .await + .expect("LibSqlBackend"); + let db: Arc = Arc::new(backend); + (db, temp_dir) + } + /// Build a test router with just the OAuth callback route. fn test_oauth_router(state: Arc) -> Router { Router::new() @@ -3343,6 +3392,47 @@ mod tests { ); } + #[cfg(feature = "libsql")] + #[tokio::test] + async fn test_routines_list_sanitizes_database_errors() { + use axum::body::Body; + use tower::ServiceExt; + + let (db, _tmp) = create_unmigrated_test_db().await; + let state = test_gateway_state_with_store(db, None); + let app = Router::new() + .route( + "/api/routines", + get(crate::channels::web::handlers::routines::routines_list_handler), + ) + .with_state(state); + + let mut req = axum::http::Request::builder() + .method("GET") + .uri("/api/routines") + .body(Body::empty()) + .expect("request"); + req.extensions_mut().insert(UserIdentity { + user_id: "test".to_string(), + workspace_read_scopes: Vec::new(), + }); + + let resp = ServiceExt::>::oneshot(app, req) + .await + .expect("response"); + assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); + + let body = axum::body::to_bytes(resp.into_body(), 1024 * 64) + .await + .expect("body"); + let text = String::from_utf8(body.to_vec()).expect("utf8 body"); + assert_eq!(text, "Database error"); + assert!( + !text.contains("no such table"), + "client response should not leak backend error details" + ); + } + #[tokio::test] async fn test_extensions_setup_submit_telegram_verification_does_not_broadcast_auth_required() { use axum::body::Body; diff --git a/src/channels/web/util.rs b/src/channels/web/util.rs index 1ee8e229..62197e36 100644 --- a/src/channels/web/util.rs +++ b/src/channels/web/util.rs @@ -1,5 +1,9 @@ //! Shared utility functions for the web gateway. +use std::fmt::Display; + +use axum::http::StatusCode; + use crate::channels::web::types::{ToolCallInfo, TurnInfo}; pub use ironclaw_common::truncate_preview; @@ -9,6 +13,31 @@ pub fn tool_error_for_display(error: &str) -> String { ironclaw_safety::SafetyLayer::unwrap_tool_output(error).unwrap_or_else(|| error.to_string()) } +fn sanitized_internal_error( + error: E, + context: &str, + client_message: &str, +) -> (StatusCode, String) { + tracing::error!(error = %error, context, "Web gateway request failed"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + client_message.to_string(), + ) +} + +/// Log a detailed backend error while returning a generic DB message to the client. +pub fn sanitized_db_error(error: E, context: &str) -> (StatusCode, String) { + sanitized_internal_error(error, context, "Database error") +} + +/// Log a detailed backend error while returning a generic internal message to the client. +pub fn sanitized_internal_error_response( + error: E, + context: &str, +) -> (StatusCode, String) { + sanitized_internal_error(error, context, "Internal error") +} + /// Parse tool call summary JSON objects into `ToolCallInfo` structs. fn parse_tool_call_infos(calls: &[serde_json::Value]) -> Vec { calls @@ -128,6 +157,19 @@ mod tests { use super::*; use uuid::Uuid; + #[test] + fn test_sanitized_db_error_hides_internal_details() { + let (_, body) = sanitized_db_error("sqlite: no such table: routines", "list routines"); + assert_eq!(body, "Database error"); + } + + #[test] + fn test_sanitized_internal_error_hides_internal_details() { + let (_, body) = + sanitized_internal_error_response("container launch failed: timeout", "restart job"); + assert_eq!(body, "Internal error"); + } + // ---- build_turns_from_db_messages tests ---- fn make_msg(role: &str, content: &str, offset_ms: i64) -> crate::history::ConversationMessage {