diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index 22fa54ab2f..57447853ae 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -355,9 +355,7 @@ 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, @@ -702,6 +700,7 @@ 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) @@ -1058,8 +1057,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(()) } @@ -2543,6 +2542,7 @@ 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, @@ -2633,6 +2633,7 @@ 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, @@ -2740,6 +2741,7 @@ 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, @@ -2963,6 +2965,7 @@ 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 @@ -3257,10 +3260,7 @@ 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 { @@ -3652,6 +3652,7 @@ pub fn parse_setting_value( value } + #[cfg(feature = "prometheus")] pub async fn monitor_pool(db: &DB) { if METRICS_ENABLED.load(Ordering::Relaxed) { @@ -6311,8 +6312,10 @@ pub async fn reload_http_route_default_allowed_origins_setting(conn: &DB) -> err 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. + // A bad value leaves whatever is already loaded in place rather than + // reverting to no restriction. On the boot path that is still the empty + // default, so what keeps a stored typo from widening CORS instance-wide is + // write-time validation, not this. let origins = match parse_allowed_origins_setting(value.as_ref()) { Ok(origins) => origins, Err(err) => { @@ -6393,6 +6396,7 @@ 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-common/src/global_settings.rs b/backend/windmill-common/src/global_settings.rs index 91fa831dd1..76e997302a 100644 --- a/backend/windmill-common/src/global_settings.rs +++ b/backend/windmill-common/src/global_settings.rs @@ -294,7 +294,10 @@ pub fn validate_allowed_origins(allowed_origins: &[String]) -> crate::error::Res let Some((scheme, rest)) = origin.split_once("://") else { return Err(invalid("missing scheme")); }; - if scheme.is_empty() + if !scheme + .chars() + .next() + .is_some_and(|first| first.is_ascii_alphabetic()) || !scheme .chars() .all(|c| c.is_ascii_alphanumeric() || c == '.' || c == '+' || c == '-') @@ -313,6 +316,30 @@ pub fn validate_allowed_origins(allowed_origins: &[String]) -> crate::error::Res if rest.contains('@') { return Err(invalid("must not contain userinfo")); } + + // An IPv6 literal is bracketed, so its own colons are not the port + // separator: splitting on the last colon would read `http://[::1]` as + // host `[:` and reject an origin a browser really does send. + let (host, port) = match rest.strip_prefix('[') { + Some(after_bracket) => match after_bracket.split_once(']') { + Some((host, "")) => (host, None), + Some((host, tail)) => match tail.strip_prefix(':') { + Some(port) => (host, Some(port)), + None => return Err(invalid("invalid port")), + }, + None => return Err(invalid("missing host")), + }, + None => match rest.split_once(':') { + Some((host, port)) => (host, Some(port)), + None => (rest, None), + }, + }; + if host.is_empty() { + return Err(invalid("missing host")); + } + if port.is_some_and(|port| port.is_empty() || !port.chars().all(|c| c.is_ascii_digit())) { + return Err(invalid("invalid port")); + } } Ok(()) diff --git a/backend/windmill-trigger-http/src/lib.rs b/backend/windmill-trigger-http/src/lib.rs index 6a43db2e92..eda7e9fa90 100644 --- a/backend/windmill-trigger-http/src/lib.rs +++ b/backend/windmill-trigger-http/src/lib.rs @@ -688,6 +688,8 @@ mod tests { let allowed = vec![ "https://app.example.com".to_string(), "http://localhost:3000".to_string(), + "http://[::1]".to_string(), + "http://[::1]:8080".to_string(), "*".to_string(), ]; assert!(validate_allowed_origins(&allowed).is_ok()); @@ -709,6 +711,10 @@ mod tests { // Every sandboxed iframe sends `Origin: null`, so allowing it would // grant access to any page that can open one. "null", + "1://app.example.com", + "https://app.example.com:not-a-port", + "https://app.example.com:", + "https://:3000", ] { assert!( validate_allowed_origins(&[invalid.to_string()]).is_err(), diff --git a/frontend/src/lib/components/triggers/http/RouteCorsOption.svelte b/frontend/src/lib/components/triggers/http/RouteCorsOption.svelte index 1a79eb8f63..cd59f94728 100644 --- a/frontend/src/lib/components/triggers/http/RouteCorsOption.svelte +++ b/frontend/src/lib/components/triggers/http/RouteCorsOption.svelte @@ -40,15 +40,25 @@ // whitespace and a non-punycoded IDN, which the backend rejects too. if (!/^[\x21-\x7e]+$/.test(origin)) return `'${origin}' must contain only visible ASCII, with no whitespace` - const [scheme, ...rest] = origin.split('://') - if (rest.length !== 1) return `'${origin}' is missing a scheme, such as https://` - const host = rest[0] - if (host === '') return `'${origin}' is missing a host` - if (host.includes('/')) return `'${origin}' must not contain a path or trailing slash` - if (host.includes('?') || host.includes('#')) + const separator = origin.indexOf('://') + if (separator < 0) return `'${origin}' is missing a scheme, such as https://` + const scheme = origin.slice(0, separator) + const rest = origin.slice(separator + 3) + if (!/^[A-Za-z][A-Za-z0-9.+-]*$/.test(scheme)) return `'${origin}' has an invalid scheme` + if (rest === '') return `'${origin}' is missing a host` + if (rest.includes('/')) return `'${origin}' must not contain a path or trailing slash` + if (rest.includes('?') || rest.includes('#')) return `'${origin}' must not contain a query or fragment` - if (host.includes('@')) return `'${origin}' must not contain userinfo` - if (!/^[A-Za-z0-9.+-]+$/.test(scheme)) return `'${origin}' has an invalid scheme` + if (rest.includes('@')) return `'${origin}' must not contain userinfo` + // An IPv6 literal is bracketed, so its own colons are not the port + // separator, and `http://[::1]` must not be read as host '[:'. + const authority = rest.startsWith('[') + ? /^\[([^\]]*)\](?::(.*))?$/.exec(rest) + : /^([^:]*)(?::(.*))?$/.exec(rest) + if (!authority) return `'${origin}' is missing a host` + const [, host, port] = authority + if (host === '') return `'${origin}' is missing a host` + if (port !== undefined && !/^[0-9]+$/.test(port)) return `'${origin}' has an invalid port` return undefined } @@ -86,6 +96,16 @@ allowed_origins = restricted ? origins : undefined }) + // This option renders on one tab only, so an unusable list must not outlive + // it: `error` alone would keep Save disabled from a screen that cannot show + // why, and clearing it alone would let a half-typed list save as a real + // restriction. Leaving restores what was there on arrival, never widening. + const mountedWith = allowed_origins + $effect(() => () => { + if (error !== undefined) allowed_origins = mountedWith + error = undefined + }) + // Re-seed the text field when the value is replaced from outside — applying // a draft, or resetting to deployed, both write the prop while this // component stays mounted. Comparing against what this component would diff --git a/frontend/src/lib/components/triggers/http/RouteEditorInner.svelte b/frontend/src/lib/components/triggers/http/RouteEditorInner.svelte index 41cf5d2304..b38a8a8f0a 100644 --- a/frontend/src/lib/components/triggers/http/RouteEditorInner.svelte +++ b/frontend/src/lib/components/triggers/http/RouteEditorInner.svelte @@ -51,7 +51,7 @@ HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING, HUB_SCRIPT_ID, isOriginRestricted, - parseAllowedOrigins, + parseAllowedOriginsSetting, saveHttpRouteFromCfg, SECRET_KEY_PATH } from './utils' @@ -129,7 +129,7 @@ const setting = await SettingService.getGlobal({ key: HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING }) - instanceDefaultOrigins = typeof setting === 'string' ? parseAllowedOrigins(setting) : [] + instanceDefaultOrigins = parseAllowedOriginsSetting(setting) } catch { instanceDefaultOrigins = [] } diff --git a/frontend/src/lib/components/triggers/http/utils.ts b/frontend/src/lib/components/triggers/http/utils.ts index 762cbe2c61..c1ee7d3110 100644 --- a/frontend/src/lib/components/triggers/http/utils.ts +++ b/frontend/src/lib/components/triggers/http/utils.ts @@ -19,18 +19,34 @@ export function parseAllowedOrigins(raw: string): string[] { .filter((origin) => origin !== '') } +/** + * Read the instance-default setting, mirroring `parse_allowed_origins_setting` + * in windmill-common: the settings UI writes a comma-separated string, but the + * API accepts an array too. + */ +export function parseAllowedOriginsSetting(setting: unknown): string[] { + if (typeof setting === 'string') return parseAllowedOrigins(setting) + if (Array.isArray(setting)) + return setting + .filter((origin): origin is string => typeof origin === 'string') + .map((origin) => origin.trim()) + .filter((origin) => origin !== '') + return [] +} + /** * Whether a route is restricted to specific origins, mirroring - * `effective_allowed_origins` in windmill-trigger-http: the route's own list - * when it has one, otherwise the instance default, and `*` in either means no - * restriction at all. + * `effective_allowed_origins` in windmill-trigger-http: a route with its own + * list restricts, an empty one included since it then matches no origin at all; + * only a route without one falls back to the instance default, and `*` in + * either is the opt-out. */ export function isOriginRestricted( allowed_origins: string[] | undefined, instanceDefaultOrigins: string[] ): boolean { - const effective = allowed_origins ?? instanceDefaultOrigins - return effective.length > 0 && !effective.includes('*') + if (allowed_origins !== undefined) return !allowed_origins.includes('*') + return instanceDefaultOrigins.length > 0 && !instanceDefaultOrigins.includes('*') } export const SECRET_KEY_PATH = 'secret_key_path'