From d8a81a0d0b4447bb1603dfc696fd3cc0f7fdf30e Mon Sep 17 00:00:00 2001 From: Henry Park Date: Fri, 27 Mar 2026 14:59:43 -0700 Subject: [PATCH] fix(security): sanitize internal error details in API responses (#1702) Replace 8 instances of `.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))` with logging + generic error messages. DB errors (SQL details, connection info) were being returned directly to API clients in chat history, thread listing, routine, extension, and pairing endpoints. Matches the existing pattern at line 1706 which already used a generic "Database error" message. Applied the same fix to all remaining instances in server.rs. Closes #1702 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/channels/web/server.rs | 69 +++++++++++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 13 deletions(-) diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs index 73e183d2..033bfb0c 100644 --- a/src/channels/web/server.rs +++ b/src/channels/web/server.rs @@ -1717,7 +1717,13 @@ async fn chat_history_handler( let (messages, has_more) = store .list_conversation_messages_paginated(thread_id, before_cursor, limit as i64) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + tracing::error!(error = %e, "DB error listing paginated messages"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + "Database error".to_string(), + ) + })?; let oldest_timestamp = messages.first().map(|m| m.created_at.to_rfc3339()); let turns = build_turns_from_db_messages(&messages); @@ -1790,7 +1796,13 @@ async fn chat_history_handler( let (messages, has_more) = store .list_conversation_messages_paginated(thread_id, None, limit as i64) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + tracing::error!(error = %e, "DB error listing messages"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + "Database error".to_string(), + ) + })?; if !messages.is_empty() { let oldest_timestamp = messages.first().map(|m| m.created_at.to_rfc3339()); @@ -1833,7 +1845,13 @@ async fn chat_threads_handler( let assistant_id = store .get_or_create_assistant_conversation(&user.user_id, "gateway") .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + tracing::error!(error = %e, "DB error getting assistant conversation"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + "Database error".to_string(), + ) + })?; match store .list_conversations_all_channels(&user.user_id, 50) @@ -2055,7 +2073,13 @@ async fn extensions_list_handler( let installed = ext_mgr .list(None, false, &user.user_id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + tracing::error!(error = %e, "Error listing extensions"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + "Internal error".to_string(), + ) + })?; let pairing_store = crate::pairing::PairingStore::new(); let mut owner_bound_channels = std::collections::HashSet::new(); @@ -2485,7 +2509,13 @@ async fn extensions_setup_handler( let setup = ext_mgr .get_setup_schema(&name, &user.user_id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + tracing::error!(error = %e, "Error getting extension setup schema"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + "Internal error".to_string(), + ) + })?; let kind = ext_mgr .list(None, false, &user.user_id) @@ -2559,9 +2589,13 @@ async fn pairing_list_handler( Path(channel): Path, ) -> Result, (StatusCode, String)> { let store = crate::pairing::PairingStore::new(); - let requests = store - .list_pending(&channel) - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + let requests = store.list_pending(&channel).map_err(|e| { + tracing::error!(error = %e, "Error listing pairing requests"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + "Internal error".to_string(), + ) + })?; let infos = requests .into_iter() @@ -2617,17 +2651,26 @@ 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| { + tracing::error!(error = %e, "DB error getting routine"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + "Database error".to_string(), + ) + })? .ok_or((StatusCode::NOT_FOUND, "Routine not found".to_string()))?; if routine.user_id != user.user_id { return Err((StatusCode::NOT_FOUND, "Routine not found".to_string())); } - let runs = store - .list_routine_runs(routine_id, 50) - .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + let runs = store.list_routine_runs(routine_id, 50).await.map_err(|e| { + tracing::error!(error = %e, "DB error listing routine runs"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + "Database error".to_string(), + ) + })?; let run_infos: Vec = runs .iter()