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: parse real IPv6 hosts and refuse a newly emptied allowlist

Browse Source
This commit is contained in:
hugocasa
2026-08-28 17:36:25 +02:00
parent 027302d93b
commit 192dfc746c
5 changed files with 55 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/windmill-common/src/global_settings.rs
+11 -6
View File
@@ -339,10 +339,9 @@ pub fn validate_allowed_origins(allowed_origins: &[String]) -> crate::error::Res
return Err(invalid("missing host"));
}
let host_ok = if bracketed {
host.contains(':')
&& host
.chars()
.all(|c| c.is_ascii_hexdigit() || c == ':' || c == '.')
// Brackets promise an IPv6 literal, so parse one: a charset check
// would pass `[:::]`, which no browser can ever send.
host.parse::<std::net::Ipv6Addr>().is_ok()
} else {
host.chars()
.all(|c| c.is_ascii_alphanumeric() || c == '.' || c == '-' || c == '_')
@@ -350,8 +349,14 @@ pub fn validate_allowed_origins(allowed_origins: &[String]) -> crate::error::Res
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()) {
// `u16` gives the range. The digit and length checks are what keep `+80`
// and `000080` out, which it would otherwise accept as 80 and no browser
// sends; they also keep this identical to the editor's own check.
if port.is_some_and(|port| {
port.len() > 5
|| !port.chars().all(|c| c.is_ascii_digit())
|| port.parse::<u16>().is_err()
}) {
return Err(invalid("invalid port"));
}
}
backend/windmill-trigger-http/src/lib.rs
+4
View File
@@ -719,6 +719,10 @@ mod tests {
"https://[notipv6]",
"https://exa[mple.com",
"https://[::1",
"https://[:::]",
"https://[1:2:3:4:5:6:7:8:9]",
"https://app.example.com:+80",
"https://app.example.com:000080",
] {
assert!(
validate_allowed_origins(&[invalid.to_string()]).is_err(),
frontend/src/lib/components/triggers/http/RouteCorsOption.svelte
+11 -9
View File
@@ -3,11 +3,17 @@
import Toggle from '$lib/components/Toggle.svelte'
import Tooltip from '$lib/components/Tooltip.svelte'
import TextInput from '$lib/components/text_input/TextInput.svelte'
import { allowedOriginError, parseAllowedOrigins } from './utils'
import { parseAllowedOrigins } from './utils'
import type { Snippet } from 'svelte'
interface Props {
allowed_origins: string[] | undefined
/**
* Why the list cannot be saved, owned and derived by the editor. Passed
* one way: this component displays it and never writes it back, so it
* cannot outlive the tab or go stale against the stored value.
*/
error?: string | undefined
/** Fetched once by the editor, so the badge and this field agree. */
instanceDefaultOrigins?: string[]
disabled?: boolean
@@ -16,6 +22,7 @@
let {
allowed_origins = $bindable(),
error = undefined,
instanceDefaultOrigins = [],
disabled = false,
testingBadge = undefined
@@ -37,11 +44,6 @@
let inheritsInstanceDefault = $derived(!restricted && hasInstanceDefault)
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(allowedOriginError).find((message) => message !== 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
@@ -97,13 +99,13 @@
<TextInput
bind:value={raw}
inputProps={{ autocomplete: 'off', disabled, placeholder: 'https://app.example.com' }}
error={malformed !== undefined}
error={error !== undefined}
/>
<!-- One line, per the form guideline's single Input -> Validation/Hint
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>
{#if error}
<div class="text-2xs text-red-600 dark:text-red-400">{error}</div>
{:else if origins.length === 0}
<div class="text-2xs text-secondary">
Allows no origin. Add one, comma-separated, or * to allow any.
frontend/src/lib/components/triggers/http/RouteEditorInner.svelte
+16 -4
View File
@@ -121,10 +121,6 @@
let raw_string = $state(false)
let wrap_body = $state(false)
let allowed_origins = $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[]>([])
@@ -167,6 +163,21 @@
let suspendedJobsModal = $state<TriggerSuspendedJobsModal | null>(null)
let originalConfig = $state<NewHttpTrigger | 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.
//
// An empty list denies every origin, which a route can legitimately be in
// and must stay editable in. Only turning the toggle on and saving without
// naming one is refused, since that reads as "I restricted this", not "I
// locked every browser out". Both sides come from editor state, so nothing
// here can go stale the way a component-held snapshot did.
const originsError = $derived(
allowedOriginsError(allowed_origins) ??
(allowed_origins?.length === 0 && originalConfig?.allowed_origins?.length !== 0
? 'Enter at least one origin, or turn this off'
: undefined)
)
let userSettings = $state<UserSettings | undefined>(undefined)
let hasChanged = $derived(!deepEqual(getRouteConfig(), originalConfig ?? {}))
@@ -1001,6 +1012,7 @@
<RouteCorsOption
bind:allowed_origins
error={originsError}
{instanceDefaultOrigins}
disabled={!can_write}
{testingBadge}
frontend/src/lib/components/triggers/http/utils.ts
+13 -2
View File
@@ -42,12 +42,23 @@ export function allowedOriginError(origin: string): string | undefined {
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)
const bracketed = rest.startsWith('[')
const authority = bracketed
? /^\[([^\]]*)\](?::(.*))?$/.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`
// Brackets promise an IPv6 literal. The URL parser is the same one that
// decides what a browser can put in an Origin, and it agrees with the
// backend's `Ipv6Addr` parse, so `[:::]` is rejected on both sides.
if (bracketed) {
try {
new URL(`http://[${host}]`)
} catch {
return `'${origin}' has an invalid host`
}
}
if (port !== undefined && !(/^[0-9]{1,5}$/.test(port) && Number(port) <= 65535))
return `'${origin}' has an invalid port`
return undefined