From 35b64ab2fde9fa10f032a2c4f4e0378d9b39fe97 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Mon, 31 Aug 2026 13:59:04 +0200 Subject: [PATCH] feat: treat an empty allowlist as unset at both levels --- backend/windmill-trigger-http/src/lib.rs | 18 +++++++++--------- .../src/lib/components/instanceSettings.ts | 2 +- .../triggers/http/RouteCorsOption.svelte | 11 +++++++---- .../src/lib/components/triggers/http/utils.ts | 11 ++++++----- 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/backend/windmill-trigger-http/src/lib.rs b/backend/windmill-trigger-http/src/lib.rs index 9b4ed67ddf..9eb1f2300e 100644 --- a/backend/windmill-trigger-http/src/lib.rs +++ b/backend/windmill-trigger-http/src/lib.rs @@ -231,12 +231,12 @@ pub fn effective_allowed_origins<'a>( route_allowed_origins: Option<&'a [String]>, instance_default: &'a [String], ) -> Option<&'a [String]> { - match route_allowed_origins { + // An empty list is not a configuration. It reads exactly as never having set + // one, so such a route still inherits the instance default rather than + // skipping it, which is what would make `[]` more permissive than `NULL`. + match route_allowed_origins.filter(|list| !list.is_empty()) { // `*` is the opt-out, including out of a stricter instance default. Some(list) if allows_any_origin(list) => None, - // Any other stored list restricts, an empty one included: it allows no - // origin at all. Falling back to the default here would make `[]` more - // permissive than `NULL`, which is the wrong direction to fail in. Some(list) => Some(list), None => (!instance_default.is_empty() && !allows_any_origin(instance_default)) .then_some(instance_default), @@ -708,14 +708,14 @@ mod tests { effective_allowed_origins(Some(&["*".to_string()]), &default), None ); - // An empty route list is a restriction that matches nothing, distinct - // from `NULL` which inherits the instance default. It must never come - // back as `None`, which the middleware reads as "any origin". + // An empty route list is not a configuration: it resolves exactly as + // `NULL` does, so it inherits the instance default rather than skipping + // it and becoming more permissive than an unset one. assert_eq!( effective_allowed_origins(Some(&[]), &default), - Some(&[][..]) + Some(&default[..]) ); - assert_eq!(match_origin(&[], Some(&origin("https://a.com"))), None); + assert_eq!(effective_allowed_origins(Some(&[]), &[]), None); } #[test] diff --git a/frontend/src/lib/components/instanceSettings.ts b/frontend/src/lib/components/instanceSettings.ts index d0528ec640..1ab5ad7201 100644 --- a/frontend/src/lib/components/instanceSettings.ts +++ b/frontend/src/lib/components/instanceSettings.ts @@ -266,7 +266,7 @@ export const settings: Record = { { 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.', + '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 unset for no instance-wide default, so every route is callable from any origin unless it restricts itself.', key: 'http_route_default_allowed_origins', fieldType: 'text', placeholder: 'https://app.example.com, https://admin.example.com', diff --git a/frontend/src/lib/components/triggers/http/RouteCorsOption.svelte b/frontend/src/lib/components/triggers/http/RouteCorsOption.svelte index 87f39bcf87..e1c4b5ed22 100644 --- a/frontend/src/lib/components/triggers/http/RouteCorsOption.svelte +++ b/frontend/src/lib/components/triggers/http/RouteCorsOption.svelte @@ -107,16 +107,19 @@ error={error !== undefined} /> + slot. Red is what the API refuses, yellow is what saves but can never + match, and an empty field is neither: it reads as no list at all. --> {#if error}
{error}
{:else if warning}
{warning}
+ {:else if origins.length === 0 && hasInstanceDefault} +
+ Empty, so the instance default still applies: {instanceDefaultOrigins.join(', ')}. +
{:else if origins.length === 0}
- Allows no origin. Add one, comma-separated, or * to allow any. + Empty, so any origin may call this route. Add one, comma-separated.
{:else}
Comma-separated. Use * to allow any.
diff --git a/frontend/src/lib/components/triggers/http/utils.ts b/frontend/src/lib/components/triggers/http/utils.ts index 2c94abd333..492e3a94e9 100644 --- a/frontend/src/lib/components/triggers/http/utils.ts +++ b/frontend/src/lib/components/triggers/http/utils.ts @@ -102,16 +102,17 @@ export function parseAllowedOriginsSetting(setting: unknown): string[] { /** * Whether a route is restricted to specific origins, mirroring - * `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. + * `effective_allowed_origins` in windmill-trigger-http: a route with a non-empty + * list of its own restricts, `*` in it is the opt-out, and anything else — an + * empty list included, since that is not a configuration — falls back to the + * instance default. */ export function isOriginRestricted( allowed_origins: string[] | undefined, instanceDefaultOrigins: string[] ): boolean { - if (allowed_origins !== undefined) return !allowed_origins.includes('*') + if (allowed_origins !== undefined && allowed_origins.length > 0) + return !allowed_origins.includes('*') return instanceDefaultOrigins.length > 0 && !instanceDefaultOrigins.includes('*') }