From 4c8edd5e944d77ed2d41c2b87171c1115c0fdcdc Mon Sep 17 00:00:00 2001
From: Ruben Fiszel <ruben@windmill.dev>
Date: Wed, 25 Mar 2026 14:51:13 +0000
Subject: [PATCH] fix: restrict logout redirect to whitelisted domains (#8524)

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
---
 backend/Cargo.lock                            |  1 +
 backend/windmill-api-users/Cargo.toml         |  1 +
 backend/windmill-api-users/src/users.rs       | 36 +++++++++++++++++--
 frontend/src/lib/logoutRedirect.ts            | 23 ++++++++++++
 .../(logged)/user/(user)/logout/+page@.svelte |  7 +++-
 5 files changed, 65 insertions(+), 3 deletions(-)
 create mode 100644 frontend/src/lib/logoutRedirect.ts

diff --git a/backend/Cargo.lock b/backend/Cargo.lock
index 34532bc173..c903490c8d 100644
--- a/backend/Cargo.lock
+++ b/backend/Cargo.lock
@@ -16386,6 +16386,7 @@ dependencies = [
  "tokio",
  "tower-cookies",
  "tracing",
+ "url",
  "windmill-api-auth",
  "windmill-audit",
  "windmill-common",
diff --git a/backend/windmill-api-users/Cargo.toml b/backend/windmill-api-users/Cargo.toml
index e720b37eb7..13ab8143d8 100644
--- a/backend/windmill-api-users/Cargo.toml
+++ b/backend/windmill-api-users/Cargo.toml
@@ -34,3 +34,4 @@ time.workspace = true
 tokio.workspace = true
 tower-cookies.workspace = true
 tracing.workspace = true
+url.workspace = true
diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs
index 8f7bec1398..75db0e2d24 100644
--- a/backend/windmill-api-users/src/users.rs
+++ b/backend/windmill-api-users/src/users.rs
@@ -49,13 +49,13 @@ use windmill_common::users::truncate_token;
 use windmill_common::users::COOKIE_NAME;
 use windmill_common::utils::paginate;
 use windmill_common::worker::CLOUD_HOSTED;
-use windmill_common::BASE_URL;
 use windmill_common::{
     auth::{get_folders_for_user, get_groups_for_user},
     db::UserDB,
     error::{self, Error, JsonResult, Result},
     utils::{not_found_if_none, rd_string, require_admin, Pagination, StripPath},
 };
+use windmill_common::{BASE_URL, HUB_BASE_URL};
 use windmill_git_sync::handle_deployment_metadata;
 
 const COOKIE_PATH: &str = "/";
@@ -577,12 +577,44 @@ async fn logout(
     }
     tx.commit().await?;
     if let Some(rd) = rd {
-        Ok((StatusCode::TEMPORARY_REDIRECT, [(LOCATION, rd)]).into_response())
+        if is_valid_logout_redirect(&rd).await {
+            Ok((StatusCode::TEMPORARY_REDIRECT, [(LOCATION, rd)]).into_response())
+        } else {
+            tracing::warn!("Blocked logout redirect to non-whitelisted URL: {}", rd);
+            Ok((StatusCode::OK, "logged out successfully".to_string()).into_response())
+        }
     } else {
         Ok((StatusCode::OK, "logged out successfully".to_string()).into_response())
     }
 }
 
+async fn is_valid_logout_redirect(rd: &str) -> bool {
+    // Allow relative paths (same-origin redirects)
+    if rd.starts_with('/') && !rd.starts_with("//") {
+        return true;
+    }
+    let parsed = match url::Url::parse(rd) {
+        Ok(u) => u,
+        Err(_) => return false,
+    };
+    let host: &str = match parsed.host_str() {
+        Some(h) => h,
+        None => return false,
+    };
+    if host == "windmill.dev" || host.ends_with(".windmill.dev") {
+        return true;
+    }
+    let hub_url = HUB_BASE_URL.read().await.clone();
+    if let Ok(hub_parsed) = url::Url::parse(&hub_url) {
+        if let Some(hub_host) = hub_parsed.host_str() {
+            if host == hub_host {
+                return true;
+            }
+        }
+    }
+    false
+}
+
 async fn whoami(
     Extension(db): Extension<DB>,
     Path(w_id): Path<String>,
diff --git a/frontend/src/lib/logoutRedirect.ts b/frontend/src/lib/logoutRedirect.ts
new file mode 100644
index 0000000000..9b09880fd2
--- /dev/null
+++ b/frontend/src/lib/logoutRedirect.ts
@@ -0,0 +1,23 @@
+import { get } from 'svelte/store'
+import { hubBaseUrlStore } from './stores'
+
+export function isValidLogoutRedirect(url: string): boolean {
+	if (url.startsWith('/') && !url.startsWith('//')) {
+		return true
+	}
+	try {
+		const parsed = new URL(url)
+		const host = parsed.hostname
+		if (host === 'windmill.dev' || host.endsWith('.windmill.dev')) {
+			return true
+		}
+		const hubBaseUrl = get(hubBaseUrlStore)
+		try {
+			const hubHost = new URL(hubBaseUrl).hostname
+			if (host === hubHost) {
+				return true
+			}
+		} catch {}
+	} catch {}
+	return false
+}
diff --git a/frontend/src/routes/(root)/(logged)/user/(user)/logout/+page@.svelte b/frontend/src/routes/(root)/(logged)/user/(user)/logout/+page@.svelte
index e328c37f5a..75e31a9a00 100644
--- a/frontend/src/routes/(root)/(logged)/user/(user)/logout/+page@.svelte
+++ b/frontend/src/routes/(root)/(logged)/user/(user)/logout/+page@.svelte
@@ -2,6 +2,7 @@
 	import { page } from '$app/state'
 	import CenteredModal from '$lib/components/CenteredModal.svelte'
 	import { clearUser } from '$lib/logout'
+	import { isValidLogoutRedirect } from '$lib/logoutRedirect'
 	import { userStore } from '$lib/stores'
 	import { onMount } from 'svelte'
 
@@ -29,7 +30,11 @@
 			return
 		}
 
-		window.location.href = rd ?? '/user/login'
+		if (rd && isValidLogoutRedirect(rd)) {
+			window.location.href = rd
+		} else {
+			window.location.href = '/user/login'
+		}
 	})
 </script>