Compare commits

..
Author SHA1 Message Date
Henry Park 71da7d4f1f fix(web): sanitize routine trigger errors 2026-03-27 16:52:21 -07:00
Henry Park 169ee62b08 fix(web): sanitize live gateway error responses 2026-03-27 15:47:05 -07:00
Henry ParkandClaude Opus 4.6 d8a81a0d0b 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) <[email protected]>
2026-03-27 14:59:43 -07:00
8 changed files with 262 additions and 341 deletions
+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 {
+12 -24
View File
@@ -14,7 +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::error::RoutineError;
use crate::channels::web::util::{sanitized_db_error, sanitized_routine_error};
pub async fn routines_list_handler(
State(state): State<Arc<GatewayState>>,
@@ -28,7 +28,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 +47,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 +95,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 +105,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()
@@ -163,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",
@@ -194,7 +194,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 +228,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 +259,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 +269,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 +305,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 +315,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()
@@ -336,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,
}
}
+6 -11
View File
@@ -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",
+127 -16
View File
@@ -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<String>,
) -> Result<Json<PairingListResponse>, (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<RoutineRunInfo> = runs
.iter()
@@ -3025,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<ExtensionManager>>) -> Arc<GatewayState> {
fn test_gateway_state_inner(
ext_mgr: Option<Arc<ExtensionManager>>,
store: Option<Arc<dyn crate::db::Database>>,
) -> Arc<GatewayState> {
Arc::new(GatewayState {
msg_tx: tokio::sync::RwLock::new(None),
sse: Arc::new(SseManager::new()),
@@ -3037,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(),
@@ -3059,6 +3104,31 @@ 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(
store: Arc<dyn crate::db::Database>,
ext_mgr: Option<Arc<ExtensionManager>>,
) -> Arc<GatewayState> {
test_gateway_state_inner(ext_mgr, Some(store))
}
#[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()
@@ -3300,6 +3370,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;
+81
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,59 @@ 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")
}
/// 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<ToolCallInfo> {
calls
@@ -128,6 +185,30 @@ 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");
}
#[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 {
-32
View File
@@ -117,13 +117,6 @@ impl Tool for ReadFileTool {
let path = validate_path(path_str, self.base_dir.as_deref())?;
if super::path_utils::is_sensitive_path(&path) {
return Err(ToolError::NotAuthorized(format!(
"Access denied: '{}' is a sensitive path. Use the appropriate secrets management tool instead.",
path_str
)));
}
// Check file size
let metadata = fs::metadata(&path)
.await
@@ -263,13 +256,6 @@ impl Tool for WriteFileTool {
let path = validate_path(path_str, self.base_dir.as_deref())?;
if super::path_utils::is_sensitive_path(&path) {
return Err(ToolError::NotAuthorized(format!(
"Access denied: '{}' is a sensitive path",
path_str
)));
}
// Create parent directories
if let Some(parent) = path.parent() {
fs::create_dir_all(parent).await.map_err(|e| {
@@ -378,13 +364,6 @@ impl Tool for ListDirTool {
let path = validate_path(path_str, self.base_dir.as_deref())?;
if super::path_utils::is_sensitive_path(&path) {
return Err(ToolError::NotAuthorized(format!(
"Access denied: '{}' is a sensitive directory",
path_str
)));
}
let mut entries = Vec::new();
list_dir_inner(&path, &path, recursive, max_depth, 0, &mut entries).await?;
@@ -468,10 +447,6 @@ async fn list_dir_inner(
entries.push(display);
if recursive && is_dir && current_depth < max_depth {
// Skip sensitive directories during recursive traversal
if super::path_utils::is_sensitive_path(&entry_path) {
continue;
}
// Skip common non-essential directories
let name = entry.file_name();
let name_str = name.to_string_lossy();
@@ -586,13 +561,6 @@ impl Tool for ApplyPatchTool {
let path = validate_path(path_str, self.base_dir.as_deref())?;
if super::path_utils::is_sensitive_path(&path) {
return Err(ToolError::NotAuthorized(format!(
"Access denied: '{}' is a sensitive path",
path_str
)));
}
// Read current content
let content = fs::read_to_string(&path)
.await
-201
View File
@@ -4,119 +4,9 @@
//! attacks and ensure paths stay within allowed sandboxes.
use std::path::{Path, PathBuf};
use std::sync::LazyLock;
use crate::tools::tool::ToolError;
/// Paths that contain credentials, secrets, or private keys.
/// Used by both file tools (exact path check) and shell tool (substring scan).
/// Keep sorted by category for readability.
static SENSITIVE_PATH_PATTERNS: LazyLock<Vec<&'static str>> = LazyLock::new(|| {
vec![
// SSH
"/.ssh/",
"/id_rsa",
"/id_ed25519",
"/id_ecdsa",
"/id_dsa",
"/authorized_keys",
"/known_hosts",
// GPG
"/.gnupg/",
// AWS
"/.aws/credentials",
"/.aws/config",
// Kubernetes
"/.kube/config",
// Cloud providers
"/.azure/",
"/.gcloud/",
"/.config/gcloud/",
// Terraform
"/.terraform.d/credentials.tfrc.json",
// GitHub CLI
"/.config/gh/hosts.yml",
// Docker
"/.docker/config.json",
// Vault
"/.vault-token",
// Shell history
"/.bash_history",
"/.zsh_history",
"/.histfile",
// Env files (may contain secrets)
"/.env",
// Git credentials
"/.git-credentials",
"/.netrc",
"/.pgpass",
// IronClaw's own secrets
"/.ironclaw/secrets/",
// System
"/etc/shadow",
"/etc/gshadow",
]
});
/// File extensions that are always sensitive regardless of location.
static SENSITIVE_EXTENSIONS: LazyLock<Vec<&'static str>> =
LazyLock::new(|| vec![".pem", ".key", ".p12", ".pfx", ".jks", ".keystore"]);
/// Suffixes that indicate a file is safe despite matching a sensitive pattern
/// (e.g., `.env.example`, `.env.sample`).
static SAFE_SUFFIXES: LazyLock<Vec<&'static str>> =
LazyLock::new(|| vec![".example", ".sample", ".template", ".dist", ".bak.example"]);
/// Check if a resolved file path points to a sensitive location.
/// Used by file tools (read, write, list_dir, apply_patch).
pub fn is_sensitive_path(path: &Path) -> bool {
let path_str = match path.canonicalize() {
Ok(p) => p.to_string_lossy().to_string(),
Err(_) => path.to_string_lossy().to_string(),
};
// Safe suffixes override sensitive patterns
let lower = path_str.to_lowercase();
if SAFE_SUFFIXES.iter().any(|s| lower.ends_with(s)) {
return false;
}
// Check sensitive path patterns
if SENSITIVE_PATH_PATTERNS.iter().any(|p| path_str.contains(p)) {
return true;
}
// Check sensitive file extensions
if let Some(ext) = path.extension().and_then(|e| e.to_str()) {
let dot_ext = format!(".{}", ext.to_lowercase());
if SENSITIVE_EXTENSIONS.iter().any(|e| *e == dot_ext) {
return true;
}
}
false
}
/// Scan a shell command string for references to sensitive paths.
/// Returns the first matched pattern, or None if the command is clean.
/// Used by the shell tool to block `cat ~/.ssh/id_rsa` etc.
pub fn command_references_sensitive_path(command: &str) -> Option<&'static str> {
let normalized = command.to_lowercase();
for pattern in SENSITIVE_PATH_PATTERNS.iter() {
// For path patterns, check case-insensitively
if normalized.contains(&pattern.to_lowercase()) {
return Some(pattern);
}
}
// Check for sensitive extensions in file arguments
SENSITIVE_EXTENSIONS
.iter()
.find(|ext| normalized.contains(*ext))
.copied()
}
/// Normalize a path by resolving `.` and `..` components lexically (no filesystem access).
///
/// This is critical for security: `std::fs::canonicalize` only works on paths that exist,
@@ -346,95 +236,4 @@ mod tests {
let result = validate_path("a/b/../c.txt", Some(dir.path()));
assert!(result.is_ok());
}
// ── sensitive path tests ──
#[test]
fn test_is_sensitive_path_blocks_ssh() {
assert!(is_sensitive_path(Path::new("/home/user/.ssh/id_rsa")));
assert!(is_sensitive_path(Path::new(
"/home/user/.ssh/authorized_keys"
)));
assert!(is_sensitive_path(Path::new("/root/.ssh/config")));
}
#[test]
fn test_is_sensitive_path_blocks_cloud_credentials() {
assert!(is_sensitive_path(Path::new("/home/user/.aws/credentials")));
assert!(is_sensitive_path(Path::new("/home/user/.kube/config")));
assert!(is_sensitive_path(Path::new("/home/user/.azure/some_token")));
assert!(is_sensitive_path(Path::new(
"/home/user/.config/gh/hosts.yml"
)));
}
#[test]
fn test_is_sensitive_path_blocks_system_secrets() {
assert!(is_sensitive_path(Path::new("/etc/shadow")));
assert!(is_sensitive_path(Path::new("/etc/gshadow")));
}
#[test]
fn test_is_sensitive_path_blocks_key_files_by_extension() {
assert!(is_sensitive_path(Path::new("/tmp/server.pem")));
assert!(is_sensitive_path(Path::new("/app/certs/private.key")));
assert!(is_sensitive_path(Path::new("/home/user/keystore.p12")));
}
#[test]
fn test_is_sensitive_path_allows_safe_suffixes() {
assert!(!is_sensitive_path(Path::new("/app/.env.example")));
assert!(!is_sensitive_path(Path::new("/app/.env.sample")));
assert!(!is_sensitive_path(Path::new("/app/.env.template")));
}
#[test]
fn test_is_sensitive_path_allows_normal_files() {
assert!(!is_sensitive_path(Path::new("/app/src/main.rs")));
assert!(!is_sensitive_path(Path::new("/home/user/README.md")));
assert!(!is_sensitive_path(Path::new("/tmp/output.json")));
}
#[test]
fn test_is_sensitive_path_blocks_env_files() {
assert!(is_sensitive_path(Path::new("/app/.env")));
assert!(is_sensitive_path(Path::new("/app/.env.local")));
assert!(is_sensitive_path(Path::new("/app/.env.production")));
}
// ── command scanning tests ──
#[test]
fn test_command_references_sensitive_path_catches_cat_ssh() {
assert!(command_references_sensitive_path("cat ~/.ssh/id_rsa").is_some());
assert!(
command_references_sensitive_path("head -n 5 /home/user/.ssh/authorized_keys")
.is_some()
);
}
#[test]
fn test_command_references_sensitive_path_catches_aws() {
assert!(command_references_sensitive_path("cat ~/.aws/credentials").is_some());
assert!(command_references_sensitive_path("grep key ~/.aws/config").is_some());
}
#[test]
fn test_command_references_sensitive_path_catches_etc_shadow() {
assert!(command_references_sensitive_path("cat /etc/shadow").is_some());
}
#[test]
fn test_command_references_sensitive_path_catches_key_extensions() {
assert!(command_references_sensitive_path("cp server.pem /tmp/").is_some());
assert!(command_references_sensitive_path("cat private.key").is_some());
}
#[test]
fn test_command_references_sensitive_path_allows_safe_commands() {
assert!(command_references_sensitive_path("ls -la").is_none());
assert!(command_references_sensitive_path("cargo build").is_none());
assert!(command_references_sensitive_path("git status").is_none());
assert!(command_references_sensitive_path("cat README.md").is_none());
}
}
+13 -9
View File
@@ -83,12 +83,21 @@ static BLOCKED_COMMANDS: LazyLock<HashSet<&'static str>> = LazyLock::new(|| {
});
/// Patterns that indicate potentially dangerous commands.
/// Note: sensitive file paths (/.ssh/, /etc/shadow, etc.) are now handled by
/// `command_references_sensitive_path` in path_utils.rs for consistency with
/// file tool protections. This list covers command-level dangers only.
static DANGEROUS_PATTERNS: LazyLock<Vec<&'static str>> = LazyLock::new(|| {
vec![
"sudo ", "doas ", " | sh", " | bash", " | zsh", "eval ", "$(curl", "$(wget",
"sudo ",
"doas ",
" | sh",
" | bash",
" | zsh",
"eval ",
"$(curl",
"$(wget",
"/etc/passwd",
"/etc/shadow",
"~/.ssh",
".bash_history",
"id_rsa",
]
});
@@ -613,11 +622,6 @@ impl ShellTool {
}
}
// Block commands that reference sensitive file paths (shared with file tools)
if super::path_utils::command_references_sensitive_path(cmd).is_some() {
return Some("Command references sensitive file path");
}
None
}