starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-09-21 00:02:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: derive the origins error from the stored list and tighten host validation

Browse Source
This commit is contained in:
hugocasa
2026-08-28 16:56:00 +02:00
parent 8a9a0287f3
commit 027302d93b
5 changed files with 86 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/windmill-common/src/global_settings.rs
+16 -2
View File
@@ -320,6 +320,7 @@ pub fn validate_allowed_origins(allowed_origins: &[String]) -> crate::error::Res
// 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 bracketed = rest.starts_with('[');
let (host, port) = match rest.strip_prefix('[') {
Some(after_bracket) => match after_bracket.split_once(']') {
Some((host, "")) => (host, None),
@@ -327,7 +328,7 @@ pub fn validate_allowed_origins(allowed_origins: &[String]) -> crate::error::Res
Some(port) => (host, Some(port)),
None => return Err(invalid("invalid port")),
},
None => return Err(invalid("missing host")),
None => return Err(invalid("invalid host")),
},
None => match rest.split_once(':') {
Some((host, port)) => (host, Some(port)),
@@ -337,7 +338,20 @@ pub fn validate_allowed_origins(allowed_origins: &[String]) -> crate::error::Res
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())) {
let host_ok = if bracketed {
host.contains(':')
&& host
.chars()
.all(|c| c.is_ascii_hexdigit() || c == ':' || c == '.')
} else {
host.chars()
.all(|c| c.is_ascii_alphanumeric() || c == '.' || c == '-' || c == '_')
};
if !host_ok {
return Err(invalid("invalid host"));
}
// `u16` is exactly the rule: digits only, and no port above 65535.
if port.is_some_and(|port| port.parse::<u16>().is_err()) {
return Err(invalid("invalid port"));
}
}
backend/windmill-trigger-http/src/lib.rs
+4
View File
@@ -715,6 +715,10 @@ mod tests {
"https://app.example.com:not-a-port",
"https://app.example.com:",
"https://:3000",
"https://app.example.com:99999",
"https://[notipv6]",
"https://exa[mple.com",
"https://[::1",
] {
assert!(
validate_allowed_origins(&[invalid.to_string()]).is_err(),
frontend/src/lib/components/triggers/http/RouteCorsOption.svelte
+14 -67
View File
@@ -3,13 +3,11 @@
import Toggle from '$lib/components/Toggle.svelte'
import Tooltip from '$lib/components/Tooltip.svelte'
import TextInput from '$lib/components/text_input/TextInput.svelte'
import { parseAllowedOrigins } from './utils'
import { allowedOriginError, parseAllowedOrigins } from './utils'
import type { Snippet } from 'svelte'
interface Props {
allowed_origins: string[] | undefined
/** Bound so the editor can block saving while the list is unusable. */
error?: string | undefined
/** Fetched once by the editor, so the badge and this field agree. */
instanceDefaultOrigins?: string[]
disabled?: boolean
@@ -18,7 +16,6 @@
let {
allowed_origins = $bindable(),
error = $bindable(),
instanceDefaultOrigins = [],
disabled = false,
testingBadge = undefined
@@ -30,38 +27,6 @@
let raw = $state(allowed_origins?.join(', ') ?? '')
let restricted = $state(allowed_origins !== undefined)
const parse = parseAllowedOrigins
// Mirrors `validate_allowed_origins` in windmill-trigger-http so the error
// shows before saving rather than as a 400 from the API.
function originError(origin: string): string | undefined {
if (origin === '*') return undefined
// An Origin header is always visible ASCII, so this covers both embedded
// 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 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 (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
}
// Independent of the toggle: it decides what the toggle is called, which must
// not change as it is flipped.
let hasInstanceDefault = $derived(
@@ -71,41 +36,21 @@
// anywhere" on an instance that has narrowed the default.
let inheritsInstanceDefault = $derived(!restricted && hasInstanceDefault)
let origins = $derived(parse(raw))
// Only a typed entry that cannot work is shown as an error. A list that is
// merely still empty is where the reader has just arrived, not a mistake to
// report back at them.
let origins = $derived(parseAllowedOrigins(raw))
// Shown here; the editor gates the save on the same check applied to the
// stored list, so leaving this tab cannot strand a save.
let malformed = $derived(
restricted ? origins.map(originError).find((message) => message !== undefined) : undefined
restricted ? origins.map(allowedOriginError).find((message) => message !== undefined) : undefined
)
// Gates the save; only `malformed` is rendered. An empty list still blocks
// saving, because storing it as NULL would mean "any origin".
$effect(() => {
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
// 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.
// NULL and NULL means "any origin", so a typo would lift the restriction
// while the toggle still reads as on.
$effect(() => {
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
@@ -155,14 +100,16 @@
error={malformed !== undefined}
/>
<!-- One line, per the form guideline's single Input -> Validation/Hint
slot. The format stays on screen until something is actually wrong,
so it is there when the field first appears empty. -->
slot. An empty list is a real state rather than a mistake: it allows
no origin at all, so say that instead of demanding an entry. -->
{#if malformed}
<div class="text-2xs text-red-600 dark:text-red-400">{malformed}</div>
{:else}
{:else if origins.length === 0}
<div class="text-2xs text-secondary">
At least one origin, comma-separated. Use * to allow any.
Allows no origin. Add one, comma-separated, or * to allow any.
</div>
{:else}
<div class="text-2xs text-secondary">Comma-separated. Use * to allow any.</div>
{/if}
{:else if inheritsInstanceDefault}
<div class="text-2xs text-secondary">
frontend/src/lib/components/triggers/http/RouteEditorInner.svelte
+6 -8
View File
@@ -50,6 +50,7 @@
import {
HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING,
HUB_SCRIPT_ID,
allowedOriginsError,
isOriginRestricted,
parseAllowedOriginsSetting,
saveHttpRouteFromCfg,
@@ -120,7 +121,10 @@
let raw_string = $state(false)
let wrap_body = $state(false)
let allowed_origins = $state<string[] | undefined>(undefined)
let allowedOriginsError = $state<string | undefined>(undefined)
// Derived from the stored list, not reported by the field: the field only
// exists on the request-options tab, so an error owned by it would keep Save
// disabled from a screen that cannot show why.
const originsError = $derived(allowedOriginsError(allowed_origins))
// Fetched once here rather than in RouteCorsOption so the Advanced badge can
// show an inherited restriction without the section being expanded.
let instanceDefaultOrigins = $state<string[]>([])
@@ -185,7 +189,7 @@
!can_write ||
pathError != '' ||
!isValid ||
allowedOriginsError != undefined ||
originsError != undefined ||
(!static_asset_config && emptyString(script_path)) ||
!hasChanged
)
@@ -322,7 +326,6 @@
raw_string = defaultValues?.raw_string ?? false
wrap_body = defaultValues?.wrap_body ?? false
allowed_origins = defaultValues?.allowed_origins ?? undefined
allowedOriginsError = undefined
summary = defaultValues?.summary ?? ''
routeDescription = defaultValues?.description ?? ''
error_handler_path = defaultValues?.error_handler_path ?? undefined
@@ -352,10 +355,6 @@
wrap_body = cfg?.wrap_body ?? false
raw_string = cfg?.raw_string ?? false
allowed_origins = cfg?.allowed_origins ?? undefined
// RouteCorsOption only lives on the request-options tab, so its error
// would otherwise outlive the trigger it came from and block saving one
// that has nothing wrong with it.
allowedOriginsError = undefined
summary = cfg?.summary ?? ''
mode = cfg?.mode ?? 'enabled'
routeDescription = cfg?.description ?? ''
@@ -1002,7 +1001,6 @@
<RouteCorsOption
bind:allowed_origins
bind:error={allowedOriginsError}
{instanceDefaultOrigins}
disabled={!can_write}
{testingBadge}
frontend/src/lib/components/triggers/http/utils.ts
+46
View File
@@ -19,6 +19,52 @@ export function parseAllowedOrigins(raw: string): string[] {
.filter((origin) => origin !== '')
}
/**
* Mirrors `validate_allowed_origins` in windmill-common, so an entry that could
* never equal a browser `Origin` is caught in the editor rather than coming
* back as a 400.
*/
export function allowedOriginError(origin: string): string | undefined {
if (origin === '*') return undefined
// An Origin header is always visible ASCII, so this covers both embedded
// 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 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 (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('[')
? /^\[([0-9A-Fa-f.]*:[0-9A-Fa-f:.]*)\](?::(.*))?$/.exec(rest)
: /^([A-Za-z0-9._-]*)(?::(.*))?$/.exec(rest)
if (!authority) return `'${origin}' has an invalid host`
const [, host, port] = authority
if (host === '') return `'${origin}' is missing a host`
if (port !== undefined && !(/^[0-9]{1,5}$/.test(port) && Number(port) <= 65535))
return `'${origin}' has an invalid port`
return undefined
}
/**
* The first entry of a route's allowlist that could never match, if any.
*
* Derived from the stored list rather than from the field, so it stays correct
* while the editor is on another tab and the field is not even mounted. An
* empty list is not an error: it is the deny-every-origin state the backend
* accepts.
*/
export function allowedOriginsError(allowed_origins: string[] | undefined): string | undefined {
return allowed_origins?.map(allowedOriginError).find((message) => message !== undefined)
}
/**
* Read the instance-default setting, mirroring `parse_allowed_origins_setting`
* in windmill-common: the settings UI writes a comma-separated string, but the