diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index 3078be9818..644973f894 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/backend/windmill-api-settings/src/lib.rs @@ -63,7 +63,7 @@ use windmill_common::{ server::Smtp, worker::is_cloud_production_host, }; -use windmill_common::{error::to_anyhow, PgDatabase}; +use windmill_common::{error::to_anyhow, worker::CLOUD_HOSTED, PgDatabase}; /// Unauthenticated settings routes. /// @@ -588,7 +588,10 @@ async fn run_setting_pre_write_hook( ))); }; - if !*workspaced_route { + // Cloud always scopes app custom paths by workspace_id (see + // `custom_path_exists` in apps.rs), so duplicates across workspaces + // are expected and this setting has no runtime effect on cloud. + if !*workspaced_route && !*CLOUD_HOSTED { #[derive(Debug, Deserialize, Serialize)] #[allow(unused)] struct DuplicateApp { @@ -651,7 +654,11 @@ async fn run_setting_pre_write_hook( ))); }; - if !*workspaced_route { + // Cloud always scopes routes by workspace_id (see + // `route_path_key_exists` in windmill-trigger-http), so duplicates + // across workspaces are expected and this setting has no runtime + // effect on cloud. + if !*workspaced_route && !*CLOUD_HOSTED { #[derive(Debug, Deserialize, Serialize)] #[allow(unused)] struct DuplicateRoute { diff --git a/backend/windmill-common/src/error.rs b/backend/windmill-common/src/error.rs index 49dcf25450..f202bf27f9 100644 --- a/backend/windmill-common/src/error.rs +++ b/backend/windmill-common/src/error.rs @@ -72,7 +72,7 @@ pub enum Error { ExecutionRawError(Box), #[error("Error: {error:#} @{location:#}")] Anyhow { error: anyhow::Error, location: String }, - #[error("Error: {0:#?}")] + #[error("{}", format_json_err_message(.0))] JsonErr(serde_json::Value), #[error("{0}")] AIError(String), @@ -256,6 +256,7 @@ impl IntoResponse for Error { Self::SqlErr { .. } | Self::BadRequest(_) | Self::AIError(_) + | Self::JsonErr(_) | Self::QuotaExceeded(_) => axum::http::StatusCode::BAD_REQUEST, Self::BadGateway(_) => axum::http::StatusCode::BAD_GATEWAY, Self::Generic(status_code, _) => status_code, @@ -280,6 +281,59 @@ impl IntoResponse for Error { } } +/// Render a `JsonErr` payload as a readable message suitable for direct +/// display in a toast: surface the `error` field as the headline, append a +/// short summary of `details` (e.g. duplicate paths) when present, and fall +/// back to pretty JSON for unknown shapes. Avoids the Rust `Debug` output +/// (`Object { "error": String("..."), ... }`) that previously leaked to users. +fn format_json_err_message(v: &serde_json::Value) -> String { + if let Some(obj) = v.as_object() { + let headline = obj + .get("error") + .and_then(|e| e.as_str()) + .map(|s| s.to_string()); + let details_summary = obj.get("details").and_then(|d| { + let arr = d.as_array()?; + if arr.is_empty() { + return None; + } + let preview = arr + .iter() + .take(5) + .map(|item| match item { + serde_json::Value::Object(o) => { + let parts: Vec = o + .iter() + .map(|(k, val)| match val { + serde_json::Value::String(s) => format!("{k}={s}"), + _ => format!("{k}={val}"), + }) + .collect(); + format!("- {}", parts.join(", ")) + } + serde_json::Value::String(s) => format!("- {s}"), + other => format!("- {other}"), + }) + .collect::>() + .join("\n"); + let suffix = if arr.len() > 5 { + format!("\n... ({} more)", arr.len() - 5) + } else { + String::new() + }; + Some(format!("{preview}{suffix}")) + }); + + match (headline, details_summary) { + (Some(h), Some(d)) => return format!("{h}\n{d}"), + (Some(h), None) => return h, + (None, Some(d)) => return d, + (None, None) => {} + } + } + serde_json::to_string_pretty(v).unwrap_or_else(|_| v.to_string()) +} + pub trait OrElseNotFound { fn or_else_not_found(self, s: impl ToString) -> Result; } @@ -316,3 +370,53 @@ where Self(err.into()) } } + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn json_err_message_error_and_details() { + let v = json!({ + "error": "Duplicate HTTP route paths detected", + "details": [ + { "route_path": "a", "workspace_id": "admins", "http_method": "post" }, + { "route_path": "a", "workspace_id": "starter", "http_method": "post" }, + ], + }); + let rendered = Error::JsonErr(v).to_string(); + assert_eq!( + rendered, + "Duplicate HTTP route paths detected\n\ + - route_path=a, workspace_id=admins, http_method=post\n\ + - route_path=a, workspace_id=starter, http_method=post" + ); + } + + #[test] + fn json_err_message_error_only() { + let v = json!({ "error": "Something went wrong" }); + assert_eq!(Error::JsonErr(v).to_string(), "Something went wrong"); + } + + #[test] + fn json_err_message_truncates_long_details() { + let details: Vec<_> = (0..8).map(|i| json!({ "k": i })).collect(); + let v = json!({ "error": "boom", "details": details }); + let rendered = Error::JsonErr(v).to_string(); + assert!(rendered.starts_with("boom\n- k=0\n- k=1\n- k=2\n- k=3\n- k=4")); + assert!(rendered.ends_with("... (3 more)")); + // Items beyond the cap aren't enumerated. + assert!(!rendered.contains("- k=5")); + } + + #[test] + fn json_err_message_fallback_to_pretty_json() { + let v = json!([1, 2, 3]); + // Non-object payload falls back to pretty JSON instead of leaking + // Rust `Debug` syntax. + let rendered = Error::JsonErr(v).to_string(); + assert_eq!(rendered, "[\n 1,\n 2,\n 3\n]"); + } +} diff --git a/frontend/src/lib/components/toast.ts b/frontend/src/lib/components/toast.ts index 6a68f76f26..d013d20aca 100644 --- a/frontend/src/lib/components/toast.ts +++ b/frontend/src/lib/components/toast.ts @@ -1,12 +1,28 @@ const pathRegex = /\b(u|f)(\/[^\/\s]+){2,}\b/g +function escapeHtml(s: string): string { + return s + .replaceAll('&', '&') + .replaceAll('<', '<') + .replaceAll('>', '>') + .replaceAll('"', '"') + .replaceAll("'", ''') +} + export function processMessage(message: string | undefined): string { let msg = !message ? 'Error without message' : typeof message != 'string' ? JSON.stringify(message, null, 2) : message - return msg.replaceAll(pathRegex, (path) => { + // Toast renders via {@html}, so escape unconditionally — server error + // bodies can contain arbitrary content (e.g. `<` from a SQL fragment in + // an error message), and the path regex below only ever inserts a safe + // `` around a `u/...` or `f/...` capture that itself cannot match + // any HTML metacharacter. Convert `\n` to `
` so multi-line errors + // stay readable in the toast (without this, HTML collapses newlines). + let html = escapeHtml(msg).replaceAll('\n', '
') + return html.replaceAll(pathRegex, (path) => { return `${path}` }) }