From 2a7f03c86facb9e790fe7c3c741974b2012c9447 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Fri, 28 Aug 2026 19:37:57 +0200 Subject: [PATCH] fix: strip runnable cors headers when the routers are unavailable --- backend/windmill-api/src/triggers/http/handler.rs | 9 ++++++--- backend/windmill-common/src/global_settings.rs | 9 +++++---- backend/windmill-trigger-http/src/lib.rs | 14 ++++++++++++++ 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/backend/windmill-api/src/triggers/http/handler.rs b/backend/windmill-api/src/triggers/http/handler.rs index 0f9135121f..c87847c96c 100644 --- a/backend/windmill-api/src/triggers/http/handler.rs +++ b/backend/windmill-api/src/triggers/http/handler.rs @@ -255,9 +255,12 @@ async fn conditional_cors_middleware( } // Whether this path is restricted could not be determined, and // `route_job` may still have served a restricted route behind this - // middleware. Emitting the permissive default would hand that response - // to any origin, so emit nothing at all. - CorsDecision::Unavailable => {} + // middleware. Emitting nothing is not enough: the runnable may have set + // its own `Access-Control-Allow-Origin` through `wm_headers`, and + // leaving that in place would hand the response to whatever it names. + CorsDecision::Unavailable => { + headers.remove(http::header::ACCESS_CONTROL_ALLOW_ORIGIN); + } CorsDecision::Unrestricted => { if !not_insert_origin { headers.insert( diff --git a/backend/windmill-common/src/global_settings.rs b/backend/windmill-common/src/global_settings.rs index b1c7c913e4..0eeb536582 100644 --- a/backend/windmill-common/src/global_settings.rs +++ b/backend/windmill-common/src/global_settings.rs @@ -352,10 +352,11 @@ pub fn parse_allowed_origins_setting( HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING ))), }) - .collect::>>()? - .into_iter() - .filter(|origin| !origin.is_empty()) - .collect(), + // Not filtered for empties, unlike the string form: there a + // trailing separator naturally yields an empty token, whereas an + // empty array entry is something the caller wrote and validation + // should reject rather than silently drop. + .collect::>>()?, Some(_) => { return Err(crate::error::Error::BadRequest(format!( "{} expected to be a comma-separated string or an array of strings", diff --git a/backend/windmill-trigger-http/src/lib.rs b/backend/windmill-trigger-http/src/lib.rs index 2be2ce7e68..9b4ed67ddf 100644 --- a/backend/windmill-trigger-http/src/lib.rs +++ b/backend/windmill-trigger-http/src/lib.rs @@ -744,6 +744,20 @@ mod tests { assert!(validate_allowed_origins(&[]).is_ok()); } + #[test] + fn test_parse_allowed_origins_setting_rejects_empty_array_entries() { + use windmill_common::global_settings::parse_allowed_origins_setting; + // A trailing separator in the string form is a typing artifact and is + // dropped; an empty array entry is something the caller wrote, so it + // must reach validation rather than be filtered away into an empty + // (and therefore unrestricted) default. + assert!(parse_allowed_origins_setting(Some(&serde_json::json!("https://a.com,"))).is_ok()); + assert!(parse_allowed_origins_setting(Some(&serde_json::json!([""]))).is_err()); + assert!( + parse_allowed_origins_setting(Some(&serde_json::json!(["https://a.com", ""]))).is_err() + ); + } + #[test] fn test_validate_allowed_origins_bounds_the_list() { // An allowlist is scanned on every request to a restricted route, the