gitops/optimclaw
1
0
Fork 0
mirror of https://github.com/outbackdingo/optimclaw.git synced 2026-08-26 23:50:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(web): sanitize live gateway error responses

Browse Source
This commit is contained in:
Henry Park
2026-03-27 15:47:05 -07:00
parent d8a81a0d0b
commit 169ee62b08
4 changed files with 166 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/channels/web/handlers/jobs.rs
+23 -48
View File
@@ -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<Arc<GatewayState>>,
@@ -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<serde_json::Value> = 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 {
src/channels/web/handlers/routines.rs
+11 -10
View File
@@ -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<RoutineInfo> = 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<RoutineRunInfo> = 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<RoutineRunInfo> = runs
.iter()
src/channels/web/server.rs
+90
View File
@@ -3102,6 +3102,55 @@ mod tests {
})
}
fn test_gateway_state_with_store(
store: Arc<dyn crate::db::Database>,
ext_mgr: Option<Arc<ExtensionManager>>,
) -> Arc<GatewayState> {
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<dyn crate::db::Database>, 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<dyn crate::db::Database> = Arc::new(backend);
(db, temp_dir)
}
/// Build a test router with just the OAuth callback route.
fn test_oauth_router(state: Arc<GatewayState>) -> 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::<axum::http::Request<Body>>::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;
src/channels/web/util.rs
+42
View File
@@ -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<E: Display>(
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<E: Display>(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<E: Display>(
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<ToolCallInfo> {
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 {