fix(web): redact database error details from API responses

[anthhub]

Summary

The jobs API handlers were formatting full database exception messages into HTTP responses via format!("Database error: {}", e), potentially exposing internal implementation details (table names, query structure, connection errors) to API clients.

• Replaced all 8 instances with a db_error() helper that logs the full error via tracing::error! and returns a generic "Internal database error" to the client
• No change to server.rs which already handled this correctly

Fixes #1702

Test plan

• test_db_error_does_not_leak_details — verifies returned body contains no DB-specific strings
• cargo check
• cargo fmt

[gemini-code-assist]

Code Review

This pull request introduces a centralized db_error helper function to prevent leaking internal database details in API responses and adds a corresponding unit test. While this is a positive change, the implementation is incomplete as several other locations in the file still convert database errors directly to strings, potentially exposing sensitive information. It is recommended to apply this masking consistently across all error handlers and consider using specific error variants for better semantic clarity.

[gemini-code-assist]

This is a great addition to prevent leaking database error details. However, the fix is incomplete as there are several other places in this file where database errors are directly converted to strings and returned in the API response (e.g., lines 301, 343, 420, 460, 664, 708, 776). This can still leak sensitive implementation details. Please ensure all database error conversions are handled by masking internal details. Consider creating specific error variants for different failure modes to provide semantically correct messages. Note: Avoid including function names or source code layout in error messages. Additionally, ensure that sensitive commands like process restarts are restricted to securely authenticated channels, and verify that the authenticated user ID matches the resource owner's ID before performing operations.

References

1. Avoid coupling log messages to implementation details like configuration interfaces or source code layout. The underlying error message should provide sufficient context on its own.
2. Create specific error variants for different failure modes (e.g., DownloadFailed with a URL string vs. ManifestRead with a file path) to provide semantically correct and clear error messages.
3. Tools that interact with user-owned resources, such as sandbox jobs, must verify that the authenticated user ID in the tool context matches the resource owner's ID (e.g., by using a ContextManager) before performing any read or write operations to prevent unauthorized cross-user access.
4. Restrict highly sensitive commands, like process restart, to a specific, securely authenticated channel (e.g., a web gateway with a single admin token) to prevent unauthorized use in multi-user environments.

[zmanian]

Approve. Correct fix for DB error detail leakage. All 8 instances in jobs.rs replaced with generic message + tracing::error. Minor suggestion: use more specific context per handler (e.g., "jobs_list", "jobs_cancel") instead of "jobs_handler" for all 8.

[claude]

Code review

Found 3 issues:

1. [CRITICAL:100] Incorrect tracing macro syntax: context parameter needs a field name

The macro on line 19 passes context as a bare value without a field assignment. The tracing crate requires structured fields to be assigned. This will cause a compile error.

ironclaw/src/channels/web/handlers/jobs.rs

Lines 18 to 24 in 9bb19a9

	fn db_error(context: &str, e: impl std::fmt::Display) -> (StatusCode, String) {
	tracing::error!(%e, context, "Database error in jobs handler");
	(
	StatusCode::INTERNAL_SERVER_ERROR,
	"Internal database error".to_string(),
	)
	}

Correct syntax:

tracing::error!(error = %e, context = context, "Database error in jobs handler");

---

2. [HIGH:90] Incomplete pattern fix: other database error paths still leak details

Per .claude/rules/review-discipline.md ("Fix the pattern, not just the instance"), this PR only redacts errors from database query operations (wrapped via db_error()) but leaves database mutation operations unpatched. These still call .to_string() on errors, exposing full database error details:

• Line 300: update_sandbox_job_status() error in jobs_cancel_handler
• Line 343: update_job_status() error in jobs_cancel_handler
• Line 420: save_sandbox_job() error in jobs_restart_handler
• Line 460: update_sandbox_job_status() error in jobs_restart_handler
• Line 517: dispatch_job() error in jobs_restart_handler
• Line 618: send_message() error in jobs_prompt_handler
• Line 664: list_job_events() error in jobs_events_handler
• Lines 708, 776: get_sandbox_job() errors in file handlers

All database error paths should use the db_error() helper for consistency and defense-in-depth.

ironclaw/src/channels/web/handlers/jobs.rs

Line 300 in 9bb19a9

.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;

---

3. [MEDIUM:70] Generic context parameter reduces debuggability

All calls to db_error() use hardcoded "jobs_handler" context (lines 224, 265, 309, 352, 470, 526, 602, 657). This makes it harder to trace which handler/operation failed when errors occur in production logs. Consider passing handler-specific context strings like "jobs_detail_handler" or including operation details.

ironclaw/src/channels/web/handlers/jobs.rs

Lines 224 to 226 in 9bb19a9

	return Err(db_error("jobs_handler", e));
	}
	}