fix: fail closed on cold router cache and invalid origin input

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

backend/windmill-api/src/triggers/http/handler.rs

@@ -64,24 +64,57 @@ fn cors_lookup_method(req: &axum::extract::Request) -> Option<HttpMethod> {
     }
 }
 
+enum CorsRouteLookup {
+    /// The routers were readable: `Some` when a trigger matched the request.
+    Resolved(Option<CorsRoute>),
+    /// The routers could not be loaded, so whether this path is restricted is
+    /// unknown.
+    Unavailable,
+}
+
 /// Resolve the trigger a request targets, for CORS purposes only.
 ///
-/// Reads the cache without ever refreshing it from the DB:
-/// `refresh_routers_loop` keeps it current, and a cold miss falls back to the
-/// permissive default rather than putting a query on the CORS path.
-async fn resolve_cors_route(http_method: HttpMethod, requested_path: &str) -> Option<CorsRoute> {
+/// Loads the routers when the cache is cold, the way `get_http_route_trigger`
+/// does. Without that, a failed startup load would leave the middleware
+/// resolving nothing while `route_job` refreshes and serves a restricted route
+/// behind it, and the response would carry the permissive default.
+async fn resolve_cors_route(
+    db: &DB,
+    http_method: HttpMethod,
+    requested_path: &str,
+) -> CorsRouteLookup {
     let routers_cache = HTTP_ROUTERS_CACHE.read().await;
-    let trigger = routers_cache
-        .routers
-        .get(&http_method)?
-        .at(requested_path.trim_end_matches('/'))
-        .ok()?
-        .value;
 
-    Some(CorsRoute { allowed_origins: trigger.allowed_origins.clone(), http_method })
+    let routers_cache = if routers_cache.routers.is_empty() {
+        drop(routers_cache);
+        match refresh_routers(db).await {
+            Ok((_, routers_cache)) => routers_cache,
+            Err(err) => {
+                tracing::error!("Could not load HTTP routers to resolve CORS: {err:#}");
+                return CorsRouteLookup::Unavailable;
+            }
+        }
+    } else {
+        routers_cache
+    };
+
+    let Some(router) = routers_cache.routers.get(&http_method) else {
+        return CorsRouteLookup::Unavailable;
+    };
+
+    CorsRouteLookup::Resolved(
+        router
+            .at(requested_path.trim_end_matches('/'))
+            .ok()
+            .map(|trigger| CorsRoute {
+                allowed_origins: trigger.value.allowed_origins.clone(),
+                http_method,
+            }),
+    )
 }
 
 async fn conditional_cors_middleware(
+    Extension(db): Extension<DB>,
     req: axum::extract::Request,
     next: axum::middleware::Next,
 ) -> Response {
@@ -91,8 +124,10 @@ async fn conditional_cors_middleware(
     // borrow of the request itself.
     let lookup = cors_lookup_method(&req).map(|method| (method, req.uri().path().to_string()));
     let route = match lookup {
-        Some((method, path)) => resolve_cors_route(method, &path).await,
-        None => None,
+        Some((method, path)) => resolve_cors_route(&db, method, &path).await,
+        // Nothing to look up: not a preflight, and not a method any route can
+        // be registered under.
+        None => CorsRouteLookup::Resolved(None),
     };
 
     let mut response = next.run(req).await;
@@ -121,10 +156,16 @@ async fn conditional_cors_middleware(
         }
     }
 
-    if let Some(allowed_origins) = route
-        .as_ref()
-        .and_then(|route| route.allowed_origins.as_ref())
-    {
+    let resolved = match &route {
+        CorsRouteLookup::Resolved(resolved) => resolved.as_ref(),
+        // Whether this path is restricted could not be determined, and
+        // `route_job` may still load the routers and serve a restricted route
+        // behind this middleware. Emitting the permissive default here would
+        // hand that response to any origin, so emit nothing at all.
+        CorsRouteLookup::Unavailable => None,
+    };
+
+    if let Some(allowed_origins) = resolved.and_then(|route| route.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
@@ -142,7 +183,7 @@ async fn conditional_cors_middleware(
         if !allowed_origins.iter().any(|allowed| allowed == "*") {
             headers.append(http::header::VARY, http::HeaderValue::from_static("origin"));
         }
-    } else if !not_insert_origin {
+    } else if !not_insert_origin && !matches!(route, CorsRouteLookup::Unavailable) {
         headers.insert(
             http::header::ACCESS_CONTROL_ALLOW_ORIGIN,
             http::HeaderValue::from_static("*"),
@@ -154,7 +195,7 @@ async fn conditional_cors_middleware(
         // overstates it. Unresolved requests keep the historical list.
         headers.insert(
             http::header::ACCESS_CONTROL_ALLOW_METHODS,
-            http::HeaderValue::from_static(match route.as_ref().map(|route| route.http_method) {
+            http::HeaderValue::from_static(match resolved.map(|route| route.http_method) {
                 Some(HttpMethod::Get) => "GET, OPTIONS",
                 Some(HttpMethod::Post) => "POST, OPTIONS",
                 Some(HttpMethod::Put) => "PUT, OPTIONS",

frontend/src/lib/components/triggers/http/RouteCorsOption.svelte

@@ -7,19 +7,22 @@
 
 	interface Props {
 		allowed_origins: string[] | undefined
+		/** Bound so the editor can block saving while the list is unusable. */
+		error?: string | undefined
 		disabled?: boolean
 		testingBadge?: Snippet | undefined
 	}
 
 	let {
 		allowed_origins = $bindable(),
+		error = $bindable(),
 		disabled = false,
 		testingBadge = undefined
 	}: Props = $props()
 
 	// The text field is the editing surface, `allowed_origins` the saved value.
 	// Keeping them separate lets a half-typed entry stay on screen while the
-	// trigger config holds only what parses.
+	// trigger config holds the parsed list.
 	let raw = $state(allowed_origins?.join(', ') ?? '')
 	let restricted = $state(allowed_origins !== undefined)
 
@@ -47,10 +50,25 @@
 	}
 
 	let origins = $derived(parse(raw))
-	let error = $derived(origins.map(originError).find((error) => error !== undefined))
+	// 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.
+	let malformed = $derived(
+		restricted ? origins.map(originError).find((message) => message !== undefined) : undefined
+	)
 
 	$effect(() => {
-		allowed_origins = restricted && !error && origins.length > 0 ? origins : undefined
+		error = restricted
+			? (malformed ?? (origins.length === 0 ? 'Enter at least one origin' : undefined))
+			: undefined
+	})
+
+	// While the toggle is on, whatever is typed is what gets saved — a rejected
+	// entry must never collapse to `undefined`, because `undefined` is stored as
+	// NULL and NULL means "any origin". A typo would otherwise lift the
+	// restriction while the toggle still reads as on. `error` blocks the save,
+	// and the backend rejects the same values if one ever gets past it.
+	$effect(() => {
+		allowed_origins = restricted ? origins : undefined
 	})
 </script>
 
@@ -80,13 +98,15 @@
 		<TextInput
 			bind:value={raw}
 			inputProps={{ autocomplete: 'off', disabled, placeholder: 'https://app.example.com' }}
-			error={error !== undefined}
+			error={malformed !== undefined}
 		/>
 		<div class="text-2xs text-secondary">
 			Separate origins with commas. Use * to allow any origin.
 		</div>
-		{#if error}
-			<div class="text-2xs text-red-600 dark:text-red-400">{error}</div>
+		{#if malformed}
+			<div class="text-2xs text-red-600 dark:text-red-400">{malformed}</div>
+		{:else if error}
+			<div class="text-2xs text-hint">{error}</div>
 		{/if}
 	{/if}
 </Label>

frontend/src/lib/components/triggers/http/RouteEditorInner.svelte

@@ -112,6 +112,7 @@
 	let raw_string = $state(false)
 	let wrap_body = $state(false)
 	let allowed_origins = $state<string[] | undefined>(undefined)
+	let allowedOriginsError = $state<string | undefined>(undefined)
 	let drawerLoading = $state(true)
 	let showLoader = $state(false)
 	let authentication_resource_path = $state('')
@@ -162,6 +163,7 @@
 			!can_write ||
 			pathError != '' ||
 			!isValid ||
+			allowedOriginsError != undefined ||
 			(!static_asset_config && emptyString(script_path)) ||
 			!hasChanged
 	)
@@ -967,7 +969,12 @@
 										{testingBadge}
 									/>
 
-									<RouteCorsOption bind:allowed_origins disabled={!can_write} {testingBadge} />
+									<RouteCorsOption
+										bind:allowed_origins
+										bind:error={allowedOriginsError}
+										disabled={!can_write}
+										{testingBadge}
+									/>
 								</div>
 							{:else}
 								<TriggerRetriesAndErrorHandler