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
7 changed files with 255 additions and 187 deletions
+5 -12
View File
@@ -6,7 +6,7 @@
## Change Type
<!-- Check all that apply. Refactor-only PRs are for core team or maintainer-requested work. -->
<!-- Check one -->
- [ ] Bug fix
- [ ] New feature
@@ -18,19 +18,16 @@
## Linked Issue
<!-- Closes #N, Fixes #N, Related #N, or "None". New feature PRs must link an approved issue. -->
<!-- Closes #N, or "None" -->
## Validation
<!-- How did you verify this works? -->
- [ ] `cargo fmt --all -- --check`
- [ ] `cargo clippy --all --benches --tests --examples --all-features -- -D warnings`
- [ ] `cargo build`
- [ ] `cargo fmt`
- [ ] `cargo clippy --all --benches --tests --examples --all-features`
- [ ] Relevant tests pass: <!-- list specific tests -->
- [ ] `cargo test --features integration` if database-backed or integration behavior changed
- [ ] Manual testing: <!-- describe what you tested -->
- [ ] If a coding agent was used and supports it, `review-pr` or `pr-shepherd --fix` was run before requesting review
## Security Impact
@@ -48,10 +45,6 @@
<!-- How to revert if this causes problems? For Track C changes, this is mandatory. -->
## Review Follow-Through
<!-- Review conversations are author-owned. Summarize any known follow-up or areas where reviewer judgment is still needed. -->
---
**Review track**: <!-- A (docs/tests/chore) | B (feature/maintainer-requested refactor) | C (security/runtime/DB/CI) -->
**Review track**: <!-- A (docs/tests/chore) | B (feature/refactor) | C (security/runtime/DB/CI) -->
+1 -76
View File
@@ -10,42 +10,6 @@ cd ironclaw
This installs the Rust toolchain, WASM targets, git hooks, and runs initial checks.
## How to Contribute
- Bug fixes, docs improvements, and focused cleanup tied to a concrete problem are welcome.
- Search existing issues and PRs before opening a new one to avoid duplicates.
- Keep changes scoped. One bug, one feature, or one documentation improvement per PR.
### Creating Issues
Open an issue when you are reporting a bug, proposing a feature, or documenting a gap in behavior.
For bug reports, include:
- What you expected to happen
- What actually happened
- Clear reproduction steps
- Relevant logs, screenshots, or error output
- Environment details when they matter (OS, database backend, feature flags, commit/branch)
For feature requests:
- Open an issue first before writing code
- Explain the problem being solved, not just the implementation idea
- Wait for maintainer feedback before investing in a large PR
We require an issue for new features so maintainers can prioritize the work and confirm it fits the roadmap before anyone spends time implementing it.
### Fixing Bugs
- Small, targeted bug-fix PRs are welcome
- If there is already an issue, link it in your PR
- If the bug is non-trivial, security-sensitive, or changes behavior across subsystems, open or confirm an issue first so the approach can be aligned before implementation
### Refactor-Only PRs
Refactor-only PRs are not accepted from contributors outside the core team. If a refactor is necessary to land a bug fix or approved feature, keep it minimal and clearly tied to that change.
## Development Workflow
```bash
@@ -55,45 +19,6 @@ cargo test # unit tests
cargo test --features integration # + PostgreSQL tests
```
These commands are for day-to-day iteration while you are developing locally. The pre-submission checks below are intentionally stricter and use CI-style flags so you can catch formatting drift and clippy warnings before requesting review.
## Before You Open a PR
Run the local validation checks required before requesting a review. These are stricter than the commands for iterative development:
```bash
cargo fmt --all -- --check
cargo clippy --all --benches --tests --examples --all-features -- -D warnings
cargo build
cargo test
```
Also run this when your change touches database-backed or integration behavior:
```bash
cargo test --features integration
```
Before asking for review:
- Build and exercise the changed path locally, not just the narrowest unit test
- Keep the PR focused and avoid mixing unrelated concerns
- Fill out the PR template with a clear summary, validation notes, and impact assessment
- If your change affects tracked behavior, update `FEATURE_PARITY.md` in the same branch
- If onboarding or setup behavior changes, update the relevant setup docs in the same branch
- If you are using a coding agent and it supports them, run `review-pr` or `pr-shepherd --fix` before opening or updating the PR
- `codex review --base origin/main` is also encouraged before requesting review
## Review Follow-Through
Review conversations are author-owned.
- Address each review comment with a code change or a clear explanation
- Resolve conversations you have handled; leave them open only when reviewer judgment is still needed
- Do not leave review cleanup for maintainers when the follow-through belongs to the author
If a PR is stale for more than 48 hours after review feedback is posted, maintainers may take over the follow-up work and land the changes needed to accomplish the original PR or issue intent.
## Code Style
- Zero clippy warnings policy
@@ -121,7 +46,7 @@ All PRs follow a risk-based review process:
| Track | Scope | Requirements |
|-------|-------|-------------|
| **A** | Docs, tests, chore, dependency bumps | 1 approval + CI green |
| **B** | Features, maintainer-requested refactors, new tools/channels | 1 approval + CI green + test evidence |
| **B** | Features, refactors, new tools/channels | 1 approval + CI green + test evidence |
| **C** | Security (`src/safety/`, `src/secrets/`), runtime (`src/agent/`, `src/worker/`), database schema, CI workflows | 2 approvals + rollback plan documented |
Select the appropriate track in the PR template based on what your changes touch.
+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 {