diff --git a/backend/src/main.rs b/backend/src/main.rs index 10d1434f98..74c2148309 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -46,7 +46,8 @@ use windmill_common::{ CUSTOM_TAGS_SETTING, DEFAULT_TAGS_PER_WORKSPACE_SETTING, DEFAULT_TAGS_WORKSPACES_SETTING, DISABLE_PASSWORD_LOGIN_SETTING, EMAIL_DOMAIN_SETTING, ENV_SETTINGS, EXPOSE_DEBUG_METRICS_SETTING, EXPOSE_METRICS_SETTING, EXTRA_PIP_INDEX_URL_SETTING, - FORK_WORKSPACE_TAG_APPEND_FORK_SUFFIX_SETTING, HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, + FORK_WORKSPACE_TAG_APPEND_FORK_SUFFIX_SETTING, + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING, HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, HUB_API_SECRET_SETTING, HUB_BASE_URL_SETTING, INDEXER_SETTING, INSTANCE_EVENTS_WEBHOOK_SETTING, INSTANCE_PYTHON_VERSION_SETTING, JOB_DEFAULT_TIMEOUT_SECS_SETTING, JOB_ISOLATION_SETTING, JWT_SECRET_SETTING, @@ -134,7 +135,8 @@ use crate::monitor::{ reload_bun_install_min_release_age_setting, reload_bunfig_install_scopes_setting, reload_critical_alert_mute_ui_setting, reload_critical_alert_mute_zombie_job_restart_setting, reload_critical_alerts_on_token_expiry_setting, reload_critical_error_channels_setting, - reload_extra_pip_index_url_setting, reload_http_route_workspaced_route_setting, + reload_extra_pip_index_url_setting, reload_http_route_default_allowed_origins_setting, + reload_http_route_workspaced_route_setting, reload_hub_api_secret_setting, reload_hub_base_url_setting, reload_instance_events_webhook_setting, reload_job_default_timeout_setting, reload_job_isolation_setting, reload_jwt_secret_setting, reload_license_key, @@ -2054,6 +2056,11 @@ async fn process_notify_event( tracing::error!(error = %e, "Could not reload app workspaced route setting"); } } + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING => { + if let Err(e) = reload_http_route_default_allowed_origins_setting(db).await { + tracing::error!(error = %e, "Could not reload http route default allowed origins setting"); + } + } HTTP_ROUTE_WORKSPACED_ROUTE_SETTING => { if let Err(e) = reload_http_route_workspaced_route_setting(db).await { tracing::error!(error = %e, "Could not reload http route workspaced route setting"); diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index 430f3659c3..22fa54ab2f 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -106,8 +106,9 @@ use windmill_common::{ use windmill_common::{ client::AuthedClient, global_settings::{ - APP_WORKSPACED_ROUTE_SETTING, HTTP_ROUTE_WORKSPACED_ROUTE, - HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, + parse_allowed_origins_setting, APP_WORKSPACED_ROUTE_SETTING, + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS, HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING, + HTTP_ROUTE_WORKSPACED_ROUTE, HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, }, }; #[cfg(feature = "parquet")] @@ -354,7 +355,9 @@ pub async fn initial_load( ) } }); - pass.action(windmill_common::min_version::store_min_keep_alive_version(db)); + pass.action(windmill_common::min_version::store_min_keep_alive_version( + db, + )); pass.setting( windmill_common::global_settings::INSTANCE_EVENTS_WEBHOOK_SETTING, false, @@ -420,6 +423,18 @@ pub async fn initial_load( pass.setting(APP_WORKSPACED_ROUTE_SETTING, false, |v| async move { apply_app_workspaced_route_setting(v) }); + pass.setting( + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING, + false, + |v| async move { + if let Err(e) = apply_http_route_default_allowed_origins_setting(v) { + tracing::error!( + "Error reloading http route default allowed origins: {:?}", + e + ) + } + }, + ); pass.setting( HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, false, @@ -687,7 +702,6 @@ pub async fn initial_load( pass.run(conn).await; } - pub fn apply_metrics_enabled(value: Option) { if let Some(serde_json::Value::Bool(t)) = value { METRICS_ENABLED.store(t, Ordering::Relaxed) @@ -1044,8 +1058,8 @@ pub fn apply_fork_workspace_tag_append_fork_suffix(value: Option error::Result<()> { - let v = - load_value_from_global_settings_with_conn(conn, CRITICAL_ALERT_MUTE_UI_SETTING, true).await?; + let v = load_value_from_global_settings_with_conn(conn, CRITICAL_ALERT_MUTE_UI_SETTING, true) + .await?; apply_critical_alert_mute_ui_setting(v); Ok(()) } @@ -2529,7 +2543,6 @@ pub async fn reload_timeout_wait_result_setting(conn: &Connection) { .await; } - pub async fn reload_extra_pip_index_url_setting(conn: &Connection) { reload_option_setting_with_tracing( conn, @@ -2620,7 +2633,6 @@ pub async fn reload_bunfig_install_scopes_setting(conn: &Connection) { .await; } - pub async fn reload_nuget_config_setting(conn: &Connection) { reload_option_setting_with_tracing( conn, @@ -2728,7 +2740,6 @@ pub async fn reload_ruby_repos_setting(conn: &Connection) { .await; } - pub async fn reload_workspace_registries_setting(conn: &Connection) { match load_value_from_global_settings_with_conn( conn, @@ -2952,7 +2963,6 @@ pub async fn apply_job_isolation_setting(value: Option) { } } - async fn resolve_license_key_value(conn: &Connection, quiet: bool) -> anyhow::Result { let q = load_value_from_global_settings_with_conn(conn, LICENSE_KEY_SETTING, true) .await @@ -3247,7 +3257,10 @@ impl<'a> SettingsPass<'a> { // on compile-time defaults until the next full reload. Only the single-query transport // can fail this way; over HTTP the batch already is the per-setting read. if matches!(conn, Connection::Sql(_)) && values.is_empty() && !names.is_empty() { - tracing::warn!("Falling back to per-setting reads for {} settings", names.len()); + tracing::warn!( + "Falling back to per-setting reads for {} settings", + names.len() + ); values = fetch_settings_individually(conn, &names).await; } for (name, http) in &declared { @@ -3639,7 +3652,6 @@ pub fn parse_setting_value( value } - #[cfg(feature = "prometheus")] pub async fn monitor_pool(db: &DB) { if METRICS_ENABLED.load(Ordering::Relaxed) { @@ -6290,6 +6302,32 @@ pub fn apply_app_workspaced_route_setting(app_workspaced_route: Option error::Result<()> { + let v = + load_value_from_global_settings(conn, HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING).await?; + apply_http_route_default_allowed_origins_setting(v) +} + +pub fn apply_http_route_default_allowed_origins_setting( + value: Option, +) -> error::Result<()> { + // A bad value leaves the previous list in place rather than falling back to + // no restriction: silently widening CORS instance-wide is the worse failure. + let origins = match parse_allowed_origins_setting(value.as_ref()) { + Ok(origins) => origins, + Err(err) => { + tracing::error!( + "Invalid {} setting, keeping the previous value: {err:#}", + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING + ); + return Ok(()); + } + }; + + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS.store(std::sync::Arc::new(origins)); + Ok(()) +} + pub async fn reload_http_route_workspaced_route_setting(conn: &DB) -> error::Result<()> { let v = load_value_from_global_settings(conn, HTTP_ROUTE_WORKSPACED_ROUTE_SETTING).await?; apply_http_route_workspaced_route_setting(conn, v).await @@ -6355,7 +6393,6 @@ pub async fn reload_critical_alerts_on_db_oversize(conn: &DB) -> error::Result<( Ok(()) } - pub async fn reload_jwt_secret_setting(db: &DB) -> error::Result<()> { let v = load_value_from_global_settings(db, JWT_SECRET_SETTING).await?; apply_jwt_secret_setting(db, v).await diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index cead2f4b27..aa4a2043c5 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/backend/windmill-api-settings/src/lib.rs @@ -58,12 +58,12 @@ use windmill_common::{ AI_CONFIG_SETTING, APP_WORKSPACED_ROUTE_SETTING, AUTOMATE_USERNAME_CREATION_SETTING, CRITICAL_ALERT_MUTE_UI_SETTING, CUSTOM_TAGS_SETTING, DEFAULT_TAGS_WORKSPACES_SETTING, DISABLE_HUB_SETTING, EMAIL_DOMAIN_SETTING, ENV_SETTINGS, - GITHUB_APP_WEBHOOK_BASE_URL_SETTING, HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, - HUB_ACCESSIBLE_URL_SETTING, HUB_BASE_URL_SETTING, MAX_RETENTION_OVERRIDE_WORKSPACES, - RETENTION_PERIOD_SECS_OVERRIDES_SETTING, RUFF_CONFIG_SETTING, - WORKSPACE_FAIRNESS_DURATION_SECS_SETTING, WORKSPACE_FAIRNESS_ENABLED_SETTING, - WORKSPACE_FAIRNESS_MAX_PERCENT_SETTING, WORKSPACE_FAIRNESS_MIN_TOTAL_SETTING, - WS_BASE_URL_SETTING, + GITHUB_APP_WEBHOOK_BASE_URL_SETTING, HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING, + HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, HUB_ACCESSIBLE_URL_SETTING, HUB_BASE_URL_SETTING, + MAX_RETENTION_OVERRIDE_WORKSPACES, RETENTION_PERIOD_SECS_OVERRIDES_SETTING, + RUFF_CONFIG_SETTING, WORKSPACE_FAIRNESS_DURATION_SECS_SETTING, + WORKSPACE_FAIRNESS_ENABLED_SETTING, WORKSPACE_FAIRNESS_MAX_PERCENT_SETTING, + WORKSPACE_FAIRNESS_MIN_TOTAL_SETTING, WS_BASE_URL_SETTING, }, instance_config::{self, ApplyMode, InstanceConfig}, server::Smtp, @@ -1014,6 +1014,12 @@ async fn run_setting_pre_write_hook( } } } + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING => { + // Rejected at write time rather than at boot: a mistyped origin + // matches no request, so it would silently block the very app it + // names with nothing but a log line to go on. + windmill_common::global_settings::parse_allowed_origins_setting(Some(value))?; + } HTTP_ROUTE_WORKSPACED_ROUTE_SETTING => { let serde_json::Value::Bool(workspaced_route) = value else { return Err(error::Error::BadRequest(format!( diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index c2fc950c65..2a4eec2da1 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -30201,7 +30201,7 @@ components: nullable: true items: type: string - description: "Origins allowed to call this route cross-origin, matched against the request's Origin header (ignoring case) and echoed back on a match. Use ['*'] to allow any origin. When null, the route answers Access-Control-Allow-Origin: * and the runnable's wm_headers can override it; when set, the configured list governs both the preflight and the response." + description: "Origins allowed to call this route cross-origin, matched against the request's Origin header (ignoring case) and echoed back on a match. When set, the list governs both the preflight and the response, overriding any Access-Control-Allow-Origin the runnable returns via wm_headers. Use ['*'] to opt out of any restriction, including the http_route_default_allowed_origins instance setting. When null, the instance setting applies, or Access-Control-Allow-Origin: * if it is unset." error_handler_path: type: string description: Path to a script or flow to run when the triggered job fails @@ -30296,7 +30296,7 @@ components: nullable: true items: type: string - description: "Origins allowed to call this route cross-origin, matched against the request's Origin header (ignoring case) and echoed back on a match. Use ['*'] to allow any origin. When null, the route answers Access-Control-Allow-Origin: * and the runnable's wm_headers can override it; when set, the configured list governs both the preflight and the response." + description: "Origins allowed to call this route cross-origin, matched against the request's Origin header (ignoring case) and echoed back on a match. When set, the list governs both the preflight and the response, overriding any Access-Control-Allow-Origin the runnable returns via wm_headers. Use ['*'] to opt out of any restriction, including the http_route_default_allowed_origins instance setting. When null, the instance setting applies, or Access-Control-Allow-Origin: * if it is unset." error_handler_path: type: string description: Path to a script or flow to run when the triggered job fails @@ -30398,7 +30398,7 @@ components: nullable: true items: type: string - description: "Origins allowed to call this route cross-origin, matched against the request's Origin header (ignoring case) and echoed back on a match. Use ['*'] to allow any origin. When null, the route answers Access-Control-Allow-Origin: * and the runnable's wm_headers can override it; when set, the configured list governs both the preflight and the response." + description: "Origins allowed to call this route cross-origin, matched against the request's Origin header (ignoring case) and echoed back on a match. When set, the list governs both the preflight and the response, overriding any Access-Control-Allow-Origin the runnable returns via wm_headers. Use ['*'] to opt out of any restriction, including the http_route_default_allowed_origins instance setting. When null, the instance setting applies, or Access-Control-Allow-Origin: * if it is unset." error_handler_path: type: string description: Path to a script or flow to run when the triggered job fails diff --git a/backend/windmill-api/src/triggers/http/handler.rs b/backend/windmill-api/src/triggers/http/handler.rs index 3847d26f1d..20f2f96add 100644 --- a/backend/windmill-api/src/triggers/http/handler.rs +++ b/backend/windmill-api/src/triggers/http/handler.rs @@ -1,6 +1,7 @@ use super::{ - http_trigger_args::RawHttpTriggerArgs, match_origin, refresh_routers, AuthenticationMethod, - HttpMethod, RequestType, TriggerRoute, HTTP_ACCESS_CACHE, HTTP_AUTH_CACHE, HTTP_ROUTERS_CACHE, + effective_allowed_origins, http_trigger_args::RawHttpTriggerArgs, match_origin, + refresh_routers, AuthenticationMethod, HttpMethod, RequestType, TriggerRoute, + HTTP_ACCESS_CACHE, HTTP_AUTH_CACHE, HTTP_ROUTERS_CACHE, }; use crate::{ auth::{AuthCache, OptTokened}, @@ -175,12 +176,26 @@ async fn conditional_cors_middleware( CorsRouteLookup::Unavailable => None, }; - if let Some(allowed_origins) = resolved.and_then(|route| route.allowed_origins.as_ref()) { + // The route's own list, or the instance-wide default when it has none. + // `None` means neither is configured, or one of them opted out with `*`. + // Nothing is applied when the routers were unreadable: the instance default + // is not necessarily what the route this request lands on would have used. + let allowed_origins = match &route { + CorsRouteLookup::Unavailable => None, + CorsRouteLookup::Resolved(resolved) => effective_allowed_origins( + resolved + .as_ref() + .and_then(|route| route.allowed_origins.as_ref()), + ), + }; + + if let Some(allowed_origins) = allowed_origins.as_ref() { // A configured allowlist decides, overriding any `wm_headers` value the // runnable set. The preflight is answered before any code runs, so // config is the only thing it can consult; letting the response widen // what the preflight advertised would make the two disagree and leave - // the allowlist bounding nothing. + // the allowlist bounding nothing. A route escapes a stricter instance + // default — `wm_headers` included — by setting its own list to `*`. match match_origin(allowed_origins, origin.as_ref()) { Some(value) => headers.insert(http::header::ACCESS_CONTROL_ALLOW_ORIGIN, value), // No match: omit the header entirely so the browser blocks the @@ -190,9 +205,7 @@ async fn conditional_cors_middleware( // Appended, not inserted: the answer now depends on the request's // Origin, and a shared cache that ignores it would hand one origin's // response to another. - if !allowed_origins.iter().any(|allowed| allowed == "*") { - headers.append(http::header::VARY, http::HeaderValue::from_static("origin")); - } + headers.append(http::header::VARY, http::HeaderValue::from_static("origin")); } else if !not_insert_origin && !matches!(route, CorsRouteLookup::Unavailable) { headers.insert( http::header::ACCESS_CONTROL_ALLOW_ORIGIN, @@ -202,10 +215,10 @@ async fn conditional_cors_middleware( if !not_insert_methods { // A route accepts exactly one method, so advertising all seven - // overstates it — but only routes that opted into an allowlist get the - // narrower answer. A route with no allowlist must respond exactly as it - // did before this existed. - let restricted_route = resolved.filter(|route| route.allowed_origins.is_some()); + // overstates it — but only routes under an allowlist get the narrower + // answer. An unrestricted route must respond exactly as it did before + // this existed. + let restricted_route = resolved.filter(|_| allowed_origins.is_some()); headers.insert( http::header::ACCESS_CONTROL_ALLOW_METHODS, http::HeaderValue::from_static(match restricted_route.map(|route| route.http_method) { diff --git a/backend/windmill-common/src/global_settings.rs b/backend/windmill-common/src/global_settings.rs index b539c1c6b3..91fa831dd1 100644 --- a/backend/windmill-common/src/global_settings.rs +++ b/backend/windmill-common/src/global_settings.rs @@ -109,6 +109,7 @@ pub const OTEL_SETTING: &str = "otel"; pub const OTEL_TRACING_PROXY_SETTING: &str = "otel_tracing_proxy"; pub const APP_WORKSPACED_ROUTE_SETTING: &str = "app_workspaced_route"; pub const HTTP_ROUTE_WORKSPACED_ROUTE_SETTING: &str = "http_route_workspaced_route"; +pub const HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING: &str = "http_route_default_allowed_origins"; pub const SECRET_BACKEND_SETTING: &str = "secret_backend"; pub const MIN_KEEP_ALIVE_VERSION_SETTING: &str = "min_keep_alive_version"; pub const GITHUB_ENTERPRISE_APP_SETTING: &str = "github_enterprise_app"; @@ -247,6 +248,113 @@ use std::sync::atomic::AtomicBool; lazy_static::lazy_static! { pub static ref HTTP_ROUTE_WORKSPACED_ROUTE: AtomicBool = AtomicBool::new(false); pub static ref DISABLE_PASSWORD_LOGIN: AtomicBool = AtomicBool::new(false); + /// Origins HTTP routes allow cross-origin when they configure none of their + /// own. Empty means unset, which keeps the historical `*`. + pub static ref HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS: arc_swap::ArcSwap> = + arc_swap::ArcSwap::from_pointee(vec![]); +} + +/// Whether an allowlist places no restriction at all. +/// +/// `*` is the explicit "open on purpose" entry, and a route carrying it behaves +/// exactly as an unconfigured one: it is how a route opts out of a stricter +/// instance default, including back into the `wm_headers` escape hatch. +pub fn allows_any_origin(allowed_origins: &[String]) -> bool { + allowed_origins.iter().any(|allowed| allowed == "*") +} + +/// Reject allowlist entries a browser would silently ignore. +/// +/// An origin is a bare `scheme://host[:port]`: anything with a path, query, +/// fragment, userinfo or whitespace never equals the `Origin` header a browser +/// sends, so it would look configured while matching nothing. `null` is rejected +/// outright — every sandboxed iframe sends `Origin: null`, so allowing it grants +/// access to any page that can open one. +pub fn validate_allowed_origins(allowed_origins: &[String]) -> crate::error::Result<()> { + for origin in allowed_origins { + if origin == "*" { + continue; + } + + let invalid = |reason: &str| { + crate::error::Error::BadRequest(format!( + "Invalid allowed origin '{}': {}. Expected an origin such as https://app.example.com, or * to allow any origin.", + origin, reason + )) + }; + + // An Origin header is always visible ASCII — browsers punycode IDNs — + // so this is both the header-value check and a whitespace check. + if !origin.chars().all(|c| c.is_ascii_graphic()) { + return Err(invalid( + "must contain only visible ASCII, with no whitespace", + )); + } + + let Some((scheme, rest)) = origin.split_once("://") else { + return Err(invalid("missing scheme")); + }; + if scheme.is_empty() + || !scheme + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '.' || c == '+' || c == '-') + { + return Err(invalid("invalid scheme")); + } + if rest.is_empty() { + return Err(invalid("missing host")); + } + if rest.contains('/') { + return Err(invalid("must not contain a path or trailing slash")); + } + if rest.contains('?') || rest.contains('#') { + return Err(invalid("must not contain a query or fragment")); + } + if rest.contains('@') { + return Err(invalid("must not contain userinfo")); + } + } + + Ok(()) +} + +/// Read [`HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING`] from its stored value. +/// +/// Accepts the comma-separated string the settings UI writes, or a JSON array +/// for anything setting it through the API directly. +pub fn parse_allowed_origins_setting( + value: Option<&serde_json::Value>, +) -> crate::error::Result> { + let origins = match value { + None | Some(serde_json::Value::Null) => vec![], + Some(serde_json::Value::String(raw)) => raw + .split(',') + .map(|origin| origin.trim().to_string()) + .filter(|origin| !origin.is_empty()) + .collect(), + Some(serde_json::Value::Array(entries)) => entries + .iter() + .map(|entry| match entry { + serde_json::Value::String(origin) => Ok(origin.trim().to_string()), + _ => Err(crate::error::Error::BadRequest(format!( + "{} entries must be strings", + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING + ))), + }) + .collect::>>()? + .into_iter() + .filter(|origin| !origin.is_empty()) + .collect(), + Some(_) => { + return Err(crate::error::Error::BadRequest(format!( + "{} expected to be a comma-separated string or an array of strings", + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING + ))) + } + }; + + validate_allowed_origins(&origins)?; + Ok(origins) } pub const ENV_SETTINGS: &[&str] = &[ diff --git a/backend/windmill-trigger-http/src/handler.rs b/backend/windmill-trigger-http/src/handler.rs index 8d3c3170e3..1150424c64 100644 --- a/backend/windmill-trigger-http/src/handler.rs +++ b/backend/windmill-trigger-http/src/handler.rs @@ -1,6 +1,6 @@ use super::{ - validate_allowed_origins, validate_authentication_method, HttpConfig, HttpConfigRequest, - HttpMethod, HttpTrigger, RouteExists, ROUTE_PATH_KEY_RE, VALID_ROUTE_PATH_RE, + validate_authentication_method, HttpConfig, HttpConfigRequest, HttpMethod, HttpTrigger, + RouteExists, ROUTE_PATH_KEY_RE, VALID_ROUTE_PATH_RE, }; use async_trait::async_trait; use axum::{extract::Path, routing::post, Extension, Json, Router}; @@ -9,7 +9,7 @@ use sqlx::PgConnection; use std::collections::HashSet; use windmill_api_auth::{check_scopes, ApiAuthed}; use windmill_audit::{audit_oss::audit_log, ActionKind}; -use windmill_common::global_settings::HTTP_ROUTE_WORKSPACED_ROUTE; +use windmill_common::global_settings::{validate_allowed_origins, HTTP_ROUTE_WORKSPACED_ROUTE}; use windmill_common::{ db::UserDB, error::{Error, Result}, @@ -468,7 +468,7 @@ impl TriggerCrud for HttpTrigger { validate_authentication_method(new.authentication_method, new.raw_string)?; - validate_allowed_origins(new.allowed_origins.as_ref())?; + validate_allowed_origins(new.allowed_origins.as_deref().unwrap_or_default())?; Ok(()) } @@ -488,7 +488,7 @@ impl TriggerCrud for HttpTrigger { validate_authentication_method(edit.authentication_method, edit.raw_string)?; - validate_allowed_origins(edit.allowed_origins.as_ref())?; + validate_allowed_origins(edit.allowed_origins.as_deref().unwrap_or_default())?; Ok(()) } diff --git a/backend/windmill-trigger-http/src/lib.rs b/backend/windmill-trigger-http/src/lib.rs index 58fa02a062..e61495c0e3 100644 --- a/backend/windmill-trigger-http/src/lib.rs +++ b/backend/windmill-trigger-http/src/lib.rs @@ -7,7 +7,9 @@ use tokio::sync::{RwLock, RwLockReadGuard}; use windmill_common::{ error::{Error, Result}, flows::Retry, - global_settings::HTTP_ROUTE_WORKSPACED_ROUTE, + global_settings::{ + allows_any_origin, HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS, HTTP_ROUTE_WORKSPACED_ROUTE, + }, utils::ExpiringCacheEntry, worker::CLOUD_HOSTED, DB, @@ -193,7 +195,7 @@ impl<'de> Deserialize<'de> for HttpConfigRequest { workspaced_route: helper.workspaced_route, wrap_body: helper.wrap_body, raw_string: helper.raw_string, - allowed_origins: normalize_allowed_origins(helper.allowed_origins), + allowed_origins: helper.allowed_origins, }) } } @@ -213,12 +215,21 @@ pub struct RouteExists { pub workspaced_route: Option, } -/// Collapse an empty list to `None` so "no allowlist configured" has a single -/// representation. `NULL` is the only value that means "keep the historical -/// `Access-Control-Allow-Origin: *`", and an empty array reaching the column -/// would be a second, silently different one. -pub fn normalize_allowed_origins(allowed_origins: Option>) -> Option> { - allowed_origins.filter(|origins| !origins.is_empty()) +/// The allowlist that governs a route: its own when it has one, otherwise the +/// instance-wide default. `None` means nothing is configured at either level, so +/// the route keeps the historical permissive behaviour. +/// +/// A list containing `*` is treated as no restriction, which is how a route opts +/// out of a stricter instance default. +pub fn effective_allowed_origins( + route_allowed_origins: Option<&Vec>, +) -> Option> { + let effective = match route_allowed_origins { + Some(route_allowed_origins) => route_allowed_origins.clone(), + None => HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS.load().as_ref().clone(), + }; + + (!effective.is_empty() && !allows_any_origin(&effective)).then_some(effective) } /// Resolve the `Access-Control-Allow-Origin` value for a request, or `None` to @@ -235,10 +246,6 @@ pub fn match_origin( allowed_origins: &[String], origin: Option<&http::HeaderValue>, ) -> Option { - if allowed_origins.iter().any(|allowed| allowed == "*") { - return Some(http::HeaderValue::from_static("*")); - } - let origin = origin?; let origin_str = origin.to_str().ok()?; allowed_origins @@ -247,66 +254,6 @@ pub fn match_origin( .then(|| origin.clone()) } -/// Reject allowlist entries a browser would silently ignore. -/// -/// An origin is a bare `scheme://host[:port]`: anything with a path, query, -/// fragment or userinfo never equals the `Origin` header a browser sends, so it -/// would look configured while matching nothing. `null` is rejected outright — -/// every sandboxed iframe sends `Origin: null`, so allowing it grants access to -/// any page that can open one. -pub fn validate_allowed_origins(allowed_origins: Option<&Vec>) -> Result<()> { - let Some(allowed_origins) = allowed_origins else { - return Ok(()); - }; - - for origin in allowed_origins { - if origin == "*" { - continue; - } - - let invalid = |reason: &str| { - Error::BadRequest(format!( - "Invalid allowed origin '{}': {}. Expected an origin such as https://app.example.com, or * to allow any origin.", - origin, reason - )) - }; - - if http::HeaderValue::from_str(origin).is_err() { - return Err(invalid("not a valid header value")); - } - - let Some((scheme, rest)) = origin.split_once("://") else { - return Err(invalid("missing scheme")); - }; - if scheme.is_empty() - || !scheme - .chars() - .all(|c| c.is_ascii_alphanumeric() || c == '.' || c == '+' || c == '-') - { - return Err(invalid("invalid scheme")); - } - if rest.is_empty() { - return Err(invalid("missing host")); - } - if rest.contains('/') { - return Err(invalid("must not contain a path or trailing slash")); - } - if rest.contains('?') || rest.contains('#') { - return Err(invalid("must not contain a query or fragment")); - } - if rest.contains('@') { - return Err(invalid("must not contain userinfo")); - } - // A space is a legal header-value byte, so this survives - // `HeaderValue::from_str` and would sit in the list matching nothing. - if rest.contains(|c: char| c.is_whitespace()) { - return Err(invalid("must not contain whitespace")); - } - } - - Ok(()) -} - pub fn validate_authentication_method( authentication_method: AuthenticationMethod, raw_string: Option, @@ -690,14 +637,34 @@ mod tests { } #[test] - fn test_match_origin_wildcard_allows_any() { - let allowed = vec!["*".to_string()]; + fn test_wildcard_entry_means_unrestricted() { + // `*` is handled before matching: it means "no restriction", which is + // how a route opts out of a stricter instance default. + assert!(allows_any_origin(&["*".to_string()])); + assert!(allows_any_origin(&[ + "https://a.com".to_string(), + "*".to_string() + ])); + assert!(!allows_any_origin(&["https://a.com".to_string()])); assert_eq!( - match_origin(&allowed, Some(&origin("https://evil.com"))), - Some(origin("*")) + effective_allowed_origins(Some(&vec!["*".to_string()])), + None ); - // `*` holds even with no Origin header, matching the historical default. - assert_eq!(match_origin(&allowed, None), Some(origin("*"))); + } + + #[test] + fn test_effective_allowed_origins_prefers_the_route() { + let route = vec!["https://a.com".to_string()]; + assert_eq!( + effective_allowed_origins(Some(&route)), + Some(vec!["https://a.com".to_string()]) + ); + // No route list and no instance default: nothing is restricted, so the + // historical permissive behaviour is kept. + assert_eq!(effective_allowed_origins(None), None); + // An empty route list is a restriction that matches nothing, distinct + // from `NULL` which inherits the instance default. + assert_eq!(effective_allowed_origins(Some(&vec![])), None); } #[test] @@ -713,8 +680,8 @@ mod tests { "http://localhost:3000".to_string(), "*".to_string(), ]; - assert!(validate_allowed_origins(Some(&allowed)).is_ok()); - assert!(validate_allowed_origins(None).is_ok()); + assert!(validate_allowed_origins(&allowed).is_ok()); + assert!(validate_allowed_origins(&[]).is_ok()); } #[test] @@ -734,22 +701,12 @@ mod tests { "null", ] { assert!( - validate_allowed_origins(Some(&vec![invalid.to_string()])).is_err(), + validate_allowed_origins(&[invalid.to_string()]).is_err(), "expected {invalid} to be rejected" ); } } - #[test] - fn test_normalize_allowed_origins_collapses_empty_to_none() { - assert_eq!(normalize_allowed_origins(Some(vec![])), None); - assert_eq!(normalize_allowed_origins(None), None); - assert_eq!( - normalize_allowed_origins(Some(vec!["*".to_string()])), - Some(vec!["*".to_string()]) - ); - } - // --- Route path regex --- #[test] diff --git a/cli/src/guidance/skills.gen.ts b/cli/src/guidance/skills.gen.ts index c3c5f0c285..43a4dca1ee 100644 --- a/cli/src/guidance/skills.gen.ts +++ b/cli/src/guidance/skills.gen.ts @@ -8368,10 +8368,11 @@ properties: items: type: string description: 'Origins allowed to call this route cross-origin, matched against - the request''s Origin header (ignoring case) and echoed back on a match. Use - [''*''] to allow any origin. When null, the route answers Access-Control-Allow-Origin: - * and the runnable''s wm_headers can override it; when set, the configured list - governs both the preflight and the response.' + the request''s Origin header (ignoring case) and echoed back on a match. When + set, the list governs both the preflight and the response, overriding any Access-Control-Allow-Origin + the runnable returns via wm_headers. Use [''*''] to opt out of any restriction, + including the http_route_default_allowed_origins instance setting. When null, + the instance setting applies, or Access-Control-Allow-Origin: * if it is unset.' error_handler_path: type: string description: Path to a script or flow to run when the triggered job fails diff --git a/frontend/src/lib/components/copilot/chat/workspaceToolsZod.gen.ts b/frontend/src/lib/components/copilot/chat/workspaceToolsZod.gen.ts index 83a117c734..85e961b8bb 100644 --- a/frontend/src/lib/components/copilot/chat/workspaceToolsZod.gen.ts +++ b/frontend/src/lib/components/copilot/chat/workspaceToolsZod.gen.ts @@ -69,7 +69,7 @@ export const httpTriggerRequestSchema = z.object({ "wrap_body": z.boolean().describe("If true, wraps the request body in a 'body' parameter").optional(), "mode": z.enum(["enabled", "disabled", "suspended"]).describe("job trigger mode").optional(), "raw_string": z.boolean().describe("If true, passes the request body as a raw string instead of parsing as JSON").optional(), - "allowed_origins": z.array(z.string()).describe("Origins allowed to call this route cross-origin, matched against the request's Origin header (ignoring case) and echoed back on a match. Use ['*'] to allow any origin. When null, the route answers Access-Control-Allow-Origin: * and the runnable's wm_headers can override it; when set, the configured list governs both the preflight and the response.").nullable().optional(), + "allowed_origins": z.array(z.string()).describe("Origins allowed to call this route cross-origin, matched against the request's Origin header (ignoring case) and echoed back on a match. When set, the list governs both the preflight and the response, overriding any Access-Control-Allow-Origin the runnable returns via wm_headers. Use ['*'] to opt out of any restriction, including the http_route_default_allowed_origins instance setting. When null, the instance setting applies, or Access-Control-Allow-Origin: * if it is unset.").nullable().optional(), "error_handler_path": z.string().describe("Path to a script or flow to run when the triggered job fails").optional(), "error_handler_args": z.record(z.string(), z.any()).describe("Arguments to pass to the error handler").optional(), "retry": z.object({ diff --git a/frontend/src/lib/components/instanceSettings.ts b/frontend/src/lib/components/instanceSettings.ts index 6c66af7d84..3c60ac990d 100644 --- a/frontend/src/lib/components/instanceSettings.ts +++ b/frontend/src/lib/components/instanceSettings.ts @@ -262,6 +262,17 @@ export const settings: Record = { ee_only: '', hideInQuickSetup: true }, + { + label: 'HTTP route default allowed origins', + description: + 'Origins that HTTP routes allow to call them from a browser when the route sets none of its own. A route overrides this with its own list, and opts out entirely by setting its allowed origins to *. Leave empty to let every route be called from any origin.', + key: 'http_route_default_allowed_origins', + fieldType: 'text', + placeholder: 'https://app.example.com, https://admin.example.com', + storage: 'setting', + ee_only: '', + hideInQuickSetup: true + }, { label: 'Audit log retention (days)', key: 'audit_log_retention_days', diff --git a/frontend/src/lib/components/triggers/http/RouteCorsOption.svelte b/frontend/src/lib/components/triggers/http/RouteCorsOption.svelte index de63fc19ba..3225fbe995 100644 --- a/frontend/src/lib/components/triggers/http/RouteCorsOption.svelte +++ b/frontend/src/lib/components/triggers/http/RouteCorsOption.svelte @@ -3,6 +3,7 @@ import Toggle from '$lib/components/Toggle.svelte' import Tooltip from '$lib/components/Tooltip.svelte' import TextInput from '$lib/components/text_input/TextInput.svelte' + import { SettingService } from '$lib/gen' import type { Snippet } from 'svelte' interface Props { @@ -49,6 +50,23 @@ return undefined } + // Without this the toggle reading "off" would look like "callable from + // anywhere" on an instance that has narrowed the default. + let instanceDefault = $state('') + async function loadInstanceDefault() { + try { + const setting = await SettingService.getGlobal({ key: 'http_route_default_allowed_origins' }) + instanceDefault = typeof setting === 'string' ? setting.trim() : '' + } catch { + instanceDefault = '' + } + } + loadInstanceDefault() + + let inheritsInstanceDefault = $derived( + !restricted && instanceDefault !== '' && instanceDefault !== '*' + ) + let origins = $derived(parse(raw)) // Split from `error` so an entry that is merely still empty reads as a hint // rather than a rejection: both block the save, only one is a mistake. @@ -122,5 +140,10 @@ {:else if error}
{error}
{/if} + {:else if inheritsInstanceDefault} +
+ Inherits the instance default: {instanceDefault}. Turn this on to set origins for this route, + or enter * to allow any. +
{/if} diff --git a/system_prompts/auto-generated/schemas/http_trigger.schema.yaml b/system_prompts/auto-generated/schemas/http_trigger.schema.yaml index 28708a9f91..09ab46b99e 100644 --- a/system_prompts/auto-generated/schemas/http_trigger.schema.yaml +++ b/system_prompts/auto-generated/schemas/http_trigger.schema.yaml @@ -100,10 +100,11 @@ properties: items: type: string description: 'Origins allowed to call this route cross-origin, matched against - the request''s Origin header (ignoring case) and echoed back on a match. Use - [''*''] to allow any origin. When null, the route answers Access-Control-Allow-Origin: - * and the runnable''s wm_headers can override it; when set, the configured list - governs both the preflight and the response.' + the request''s Origin header (ignoring case) and echoed back on a match. When + set, the list governs both the preflight and the response, overriding any Access-Control-Allow-Origin + the runnable returns via wm_headers. Use [''*''] to opt out of any restriction, + including the http_route_default_allowed_origins instance setting. When null, + the instance setting applies, or Access-Control-Allow-Origin: * if it is unset.' error_handler_path: type: string description: Path to a script or flow to run when the triggered job fails