feat: allow private MCP server URLs (#9470)

* feat: allow private MCP server URLs

* docs: remove private MCP server URL doc

* fix: apply MCP URL opt-in to OAuth handlers

* fix: update EE ref for MCP OAuth redirects

* fix: preserve MCP OAuth client timeout

* chore: update ee-repo-ref to 481ea7f28dc5af6b72390c82f494f34cb9809546

This commit updates the EE repository reference after PR #608 was merged in windmill-ee-private.

Previous ee-repo-ref: 6c7da03fb994be23ed6aca59bece94d257a641b5

New ee-repo-ref: 481ea7f28dc5af6b72390c82f494f34cb9809546

Automated by sync-ee-ref workflow.

---------

Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>

backend/THREAT_MODEL.md

@@ -97,7 +97,7 @@ published advisory history (73 GHSA advisories, several rated 9.9 critical).
 | id | threat | actor | surface | asset | impact | likelihood | status | controls | evidence |
 |---|---|---|---|---|---|---|---|---|---|
 | T1 | SQL injection in app/internal query builders and trigger clauses compromises the metadata DB and connected databases | remote_auth | EP8 | Database, downstream connected systems | critical | almost_certain | partially_mitigated | sqlx parameterized queries elsewhere; query-builder safety reviews | GHSA-225c-j3xq-g6x6, GHSA-78p7-jc72-gv66, GHSA-hvc7-f67h-jx3g, GHSA-wrrg-f89m-f84q, GHSA-79vf-3qwm-2w64, GHSA-55p6-fxj4-v983, GHSA-5g4v-49rj-r52r, GHSA-x6cq-7xr8-53x3, 2cf4bb180b |
-| T2 | Server-side request forgery via proxies/executors reaches cloud metadata, internal network, and downstream credentials | remote_auth | EP6, EP7 | Cloud metadata, internal network, downstream connected systems, resource creds | critical | almost_certain | partially_mitigated | SSRF URL validation + redirect-following disabled added piecemeal; outbound network isolation (`clone_newnet`) is opt-in and off by default | GHSA-3ggp-h37f-5qfw, GHSA-98qq-g8rh-xhff, GHSA-hfw8-27mx-63jm, GHSA-3r59-qvvc-774j, GHSA-4pj9-w5jc-g8w7, GHSA-8hh3-jf25-78j5, GHSA-3pjm-4w7f-3r2w, GHSA-f44c-x9hq-h68r, GHSA-j4h4-f8fj-3m3c, 4b06881918, 96a8eb63d4, dbd3942ef3 |
+| T2 | Server-side request forgery via proxies/executors reaches cloud metadata, internal network, and downstream credentials | remote_auth | EP6, EP7 | Cloud metadata, internal network, downstream connected systems, resource creds | critical | almost_certain | partially_mitigated | SSRF URL validation + redirect-following disabled added piecemeal; MCP private URL access requires the instance-wide `ALLOW_PRIVATE_MCP_SERVER_URLS` opt-in; outbound network isolation (`clone_newnet`) is opt-in and off by default | GHSA-3ggp-h37f-5qfw, GHSA-98qq-g8rh-xhff, GHSA-hfw8-27mx-63jm, GHSA-3r59-qvvc-774j, GHSA-4pj9-w5jc-g8w7, GHSA-8hh3-jf25-78j5, GHSA-3pjm-4w7f-3r2w, GHSA-f44c-x9hq-h68r, GHSA-j4h4-f8fj-3m3c, 4b06881918, 96a8eb63d4, dbd3942ef3 |
 | T3 | Broken authorization / IDOR lets a scoped token or low-privilege member read scripts, job data, and secrets across folders and workspaces | remote_auth | EP5, EP2, EP1 | Scripts, job data, secrets, isolation | critical | almost_certain | partially_mitigated | RLS, token scopes, folder ACLs, view-token HMAC (added incrementally); on managed, sensitive tenants can opt into dedicated DB/worker/namespace, but the shared tier IS the software boundary | GHSA-qfg7-x243-5hg4, GHSA-8x8x-88qc-qp4r, GHSA-2ppx-66jv-wpw5, GHSA-x3x7-g97v-mp59, GHSA-j276-g4h8-g6h5, GHSA-8mv7-hmrg-96xv, GHSA-x2wf-f962-7frq, GHSA-qc7c-gcw6-h4xp, GHSA-vxc5-w28p-m9xw, GHSA-2g34-wfvr-5qqj, GHSA-w7p6-wpxm-pp66, 7edf3f0212, 89a7a37776, ab11c7747a, 664edcdfb7 |
 | T4 | Remote code execution by injecting attacker-controlled identifiers into generated worker wrappers | remote_auth | EP10 | Worker host, isolation, downstream | critical | likely | partially_mitigated | entrypoint/env-var-name validation added | GHSA-wxjq-w5pj-jqhx, GHSA-5f5q-2vg2-r2x4, GHSA-8q8j-mm3g-5c2q (CVE-2026-33881), bf93657fee, bd05bcadde, 22ec4da5f0 |
 | T5 | Worker compromise & cross-tenant access via weak-by-default isolation (nsjail off by default → user code runs with only PID-ns `unshare`); sandbox escape where nsjail/dind/podman is enabled | remote_auth | EP9, EP15 | Worker host, isolation, downstream | critical | likely | unmitigated | nsjail off by default everywhere (`DISABLE_NSJAIL=true`); shipped compose gives PID-ns `unshare` only (`FAVOR_UNSHARE_PID=true`), bare installs get no isolation. Where nsjail enabled: read-only remounts, jail-tmp refusal, podman socket gating | GHSA-6qr8-xhg4-453q, GHSA-3vpp-vf62-wqp6, f8467f38c8, df5aec0f5d, f1b6746e0e |

backend/ee-repo-ref.txt

@@ -1 +1 @@
-2c7964460327fab5e3a27c0f74b8d6f26ab7f79a
+481ea7f28dc5af6b72390c82f494f34cb9809546

backend/windmill-common/src/ssrf.rs

@@ -2,6 +2,8 @@ use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
 
 use crate::error::Error;
 
+pub const ALLOW_PRIVATE_MCP_SERVER_URLS_ENV: &str = "ALLOW_PRIVATE_MCP_SERVER_URLS";
+
 /// Why a URL failed SSRF validation.
 ///
 /// The distinction matters for callers that gate private endpoints behind a
@@ -116,6 +118,49 @@ pub async fn validate_url_for_ssrf(url: &str) -> Result<(), SsrfValidationError>
     Ok(())
 }
 
+pub fn allow_private_mcp_server_urls() -> bool {
+    std::env::var(ALLOW_PRIVATE_MCP_SERVER_URLS_ENV)
+        .ok()
+        .is_some_and(|v| v == "true" || v == "1")
+}
+
+pub async fn validate_mcp_server_url(url: &str) -> Result<(), SsrfValidationError> {
+    let parsed =
+        url::Url::parse(url).map_err(|e| SsrfValidationError::InvalidUrl(e.to_string()))?;
+
+    match parsed.scheme() {
+        "http" | "https" => {}
+        scheme => return Err(SsrfValidationError::DisallowedScheme(scheme.to_string())),
+    }
+
+    parsed.host_str().ok_or(SsrfValidationError::MissingHost)?;
+
+    if allow_private_mcp_server_urls() {
+        return Ok(());
+    }
+
+    validate_url_for_ssrf(url).await
+}
+
+pub async fn validate_mcp_server_url_for_bad_request(url: &str, label: &str) -> Result<(), Error> {
+    validate_mcp_server_url(url).await.map_err(|e| {
+        Error::BadRequest(format!(
+            "{label} is not allowed: {}",
+            mcp_ssrf_error_message(&e)
+        ))
+    })
+}
+
+pub fn mcp_ssrf_error_message(e: &SsrfValidationError) -> String {
+    match e {
+        SsrfValidationError::Private { .. } => format!(
+            "{e}. If you need to use private/internal MCP server URLs, \
+             set the {ALLOW_PRIVATE_MCP_SERVER_URLS_ENV}=true environment variable"
+        ),
+        _ => e.to_string(),
+    }
+}
+
 fn is_private_ip(ip: &IpAddr) -> bool {
     match ip {
         IpAddr::V4(ipv4) => is_private_ipv4(ipv4),
@@ -152,6 +197,32 @@ fn is_private_ipv6(ip: &Ipv6Addr) -> bool {
 mod tests {
     use super::*;
 
+    static TEST_ENV_LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(());
+
+    struct PrivateMcpServerUrlsEnvGuard {
+        previous: Option<String>,
+    }
+
+    impl PrivateMcpServerUrlsEnvGuard {
+        fn set(value: Option<&str>) -> Self {
+            let previous = std::env::var(ALLOW_PRIVATE_MCP_SERVER_URLS_ENV).ok();
+            match value {
+                Some(value) => std::env::set_var(ALLOW_PRIVATE_MCP_SERVER_URLS_ENV, value),
+                None => std::env::remove_var(ALLOW_PRIVATE_MCP_SERVER_URLS_ENV),
+            }
+            Self { previous }
+        }
+    }
+
+    impl Drop for PrivateMcpServerUrlsEnvGuard {
+        fn drop(&mut self) {
+            match &self.previous {
+                Some(value) => std::env::set_var(ALLOW_PRIVATE_MCP_SERVER_URLS_ENV, value),
+                None => std::env::remove_var(ALLOW_PRIVATE_MCP_SERVER_URLS_ENV),
+            }
+        }
+    }
+
     #[test]
     fn test_private_ipv4() {
         assert!(is_private_ipv4(&"127.0.0.1".parse().unwrap()));
@@ -227,4 +298,66 @@ mod tests {
             Err(SsrfValidationError::Private { resolved: false })
         ));
     }
+
+    #[tokio::test]
+    async fn validate_mcp_server_url_blocks_private_by_default() {
+        let _lock = TEST_ENV_LOCK.lock().await;
+        let _guard = PrivateMcpServerUrlsEnvGuard::set(None);
+
+        assert!(matches!(
+            validate_mcp_server_url("http://127.0.0.1/foo").await,
+            Err(SsrfValidationError::Private { resolved: false })
+        ));
+    }
+
+    #[tokio::test]
+    async fn validate_mcp_server_url_allows_private_when_env_is_enabled() {
+        let _lock = TEST_ENV_LOCK.lock().await;
+        let _guard = PrivateMcpServerUrlsEnvGuard::set(Some("true"));
+
+        assert!(validate_mcp_server_url("http://127.0.0.1/foo")
+            .await
+            .is_ok());
+    }
+
+    #[tokio::test]
+    async fn validate_mcp_server_url_allows_private_when_env_is_one() {
+        let _lock = TEST_ENV_LOCK.lock().await;
+        let _guard = PrivateMcpServerUrlsEnvGuard::set(Some("1"));
+
+        assert!(validate_mcp_server_url("http://10.0.0.1/foo").await.is_ok());
+    }
+
+    #[tokio::test]
+    async fn validate_mcp_server_url_keeps_syntax_guards_when_private_urls_are_allowed() {
+        let _lock = TEST_ENV_LOCK.lock().await;
+        let _guard = PrivateMcpServerUrlsEnvGuard::set(Some("true"));
+
+        assert!(matches!(
+            validate_mcp_server_url("localhost:11434/v1").await,
+            Err(SsrfValidationError::DisallowedScheme(_))
+        ));
+        assert!(matches!(
+            validate_mcp_server_url("file:///tmp/socket").await,
+            Err(SsrfValidationError::DisallowedScheme(_))
+        ));
+    }
+
+    #[tokio::test]
+    async fn private_mcp_error_message_includes_env_hint_only_for_private_urls() {
+        let _lock = TEST_ENV_LOCK.lock().await;
+        let _guard = PrivateMcpServerUrlsEnvGuard::set(None);
+
+        let private_error = validate_mcp_server_url("http://127.0.0.1/foo")
+            .await
+            .unwrap_err();
+        assert!(
+            mcp_ssrf_error_message(&private_error).contains("ALLOW_PRIVATE_MCP_SERVER_URLS=true")
+        );
+
+        let invalid_error = validate_mcp_server_url("localhost:11434/v1")
+            .await
+            .unwrap_err();
+        assert!(!mcp_ssrf_error_message(&invalid_error).contains(ALLOW_PRIVATE_MCP_SERVER_URLS_ENV));
+    }
 }

backend/windmill-mcp/src/client/mod.rs

@@ -43,9 +43,14 @@ impl McpClient {
         // The resource URL is author-controlled and we send a (potentially
         // secret) bearer token to it, so it must be validated against SSRF
         // before we connect (e.g. cloud metadata endpoints, internal services).
-        windmill_common::ssrf::validate_url_for_ssrf(&resource.url)
+        windmill_common::ssrf::validate_mcp_server_url(&resource.url)
             .await
-            .map_err(|e| anyhow::anyhow!("MCP server URL is not allowed: {}", e))?;
+            .map_err(|e| {
+                anyhow::anyhow!(
+                    "MCP server URL is not allowed: {}",
+                    windmill_common::ssrf::mcp_ssrf_error_message(&e)
+                )
+            })?;
 
         // Build custom reqwest client with headers if provided
         let mut headers = HeaderMap::new();
@@ -230,6 +235,33 @@ impl McpClient {
 mod tests {
     use super::*;
 
+    struct PrivateMcpServerUrlsEnvGuard {
+        previous: Option<String>,
+    }
+
+    impl PrivateMcpServerUrlsEnvGuard {
+        fn unset() -> Self {
+            let previous =
+                std::env::var(windmill_common::ssrf::ALLOW_PRIVATE_MCP_SERVER_URLS_ENV).ok();
+            std::env::remove_var(windmill_common::ssrf::ALLOW_PRIVATE_MCP_SERVER_URLS_ENV);
+            Self { previous }
+        }
+    }
+
+    impl Drop for PrivateMcpServerUrlsEnvGuard {
+        fn drop(&mut self) {
+            match &self.previous {
+                Some(value) => std::env::set_var(
+                    windmill_common::ssrf::ALLOW_PRIVATE_MCP_SERVER_URLS_ENV,
+                    value,
+                ),
+                None => {
+                    std::env::remove_var(windmill_common::ssrf::ALLOW_PRIVATE_MCP_SERVER_URLS_ENV)
+                }
+            }
+        }
+    }
+
     /// Regression test: `from_resource` must refuse to connect to a URL that
     /// targets a private/internal address (here the AWS
     /// instance-metadata endpoint), so a resource author cannot use the MCP
@@ -237,6 +269,8 @@ mod tests {
     /// before any connection attempt, so this fails fast without network access.
     #[tokio::test]
     async fn from_resource_rejects_ssrf_url() {
+        let _guard = PrivateMcpServerUrlsEnvGuard::unset();
+
         let resource = McpResource {
             name: "evil".to_string(),
             url: "http://169.254.169.254".to_string(),

backend/windmill-mcp/src/client_registration.rs

@@ -17,7 +17,7 @@ use windmill_common::db::DB;
 use windmill_common::error;
 use windmill_common::variables::{build_crypt, decrypt, encrypt};
 
-use crate::oauth::AuthorizationManager;
+use crate::oauth::{no_redirect_http_client, AuthorizationManager};
 
 /// MCP client credentials returned by [`get_or_refresh_mcp_client`].
 pub struct McpClientCredentials {
@@ -77,7 +77,14 @@ async fn register_client(
     redirect_uri: &str,
     client_name: &str,
 ) -> Result<DcrResponse, error::Error> {
-    let client = reqwest::Client::new();
+    windmill_common::ssrf::validate_mcp_server_url_for_bad_request(
+        registration_endpoint,
+        "MCP server registration endpoint URL",
+    )
+    .await?;
+
+    let client = no_redirect_http_client()
+        .map_err(|e| error::Error::BadRequest(format!("Failed to build DCR client: {e}")))?;
     let request = DcrRequest {
         client_name: client_name.to_string(),
         redirect_uris: vec![redirect_uri.to_string()],
@@ -121,6 +128,12 @@ pub async fn get_or_refresh_mcp_client(
     let base_url = (**windmill_common::BASE_URL.load()).clone();
     let redirect_uri = format!("{}/api/mcp/oauth/callback", base_url);
 
+    windmill_common::ssrf::validate_mcp_server_url_for_bad_request(
+        mcp_server_url,
+        "MCP server URL",
+    )
+    .await?;
+
     let cached_client: Option<McpOAuthClient> =
         sqlx::query_as("SELECT mcp_server_url, client_id, client_secret, client_secret_expires_at, token_endpoint FROM mcp_oauth_client WHERE mcp_server_url = $1")
             .bind(mcp_server_url)
@@ -131,6 +144,11 @@ pub async fn get_or_refresh_mcp_client(
     if let Some(client) = cached_client {
         if !client.is_expired() {
             tracing::debug!("Using cached MCP client for {}", mcp_server_url);
+            windmill_common::ssrf::validate_mcp_server_url_for_bad_request(
+                &client.token_endpoint,
+                "MCP server token endpoint URL",
+            )
+            .await?;
             let decrypted_secret = if let Some(ref encrypted_secret) = client.client_secret {
                 Some(decrypt_client_secret(db, encrypted_secret).await?)
             } else {
@@ -145,17 +163,27 @@ pub async fn get_or_refresh_mcp_client(
         tracing::debug!("Cached MCP client expired, re-registering");
     }
 
-    windmill_common::ssrf::validate_url_for_ssrf(mcp_server_url).await?;
-
-    let manager = AuthorizationManager::new(mcp_server_url)
+    let mut manager = AuthorizationManager::new(mcp_server_url)
         .await
         .map_err(|e| error::Error::BadRequest(format!("Failed to create auth manager: {e}")))?;
+    let discovery_client = no_redirect_http_client().map_err(|e| {
+        error::Error::BadRequest(format!("Failed to build MCP OAuth discovery client: {e}"))
+    })?;
+    manager
+        .with_client(discovery_client)
+        .map_err(|e| error::Error::BadRequest(format!("Failed to configure auth manager: {e}")))?;
 
     let metadata = manager
         .discover_metadata()
         .await
         .map_err(|e| error::Error::BadRequest(format!("OAuth discovery failed: {e}")))?;
 
+    windmill_common::ssrf::validate_mcp_server_url_for_bad_request(
+        &metadata.token_endpoint,
+        "MCP server token endpoint URL",
+    )
+    .await?;
+
     let supports_dynamic_registration = metadata.registration_endpoint.is_some();
 
     let (client_id, client_secret, expires_at) = if supports_dynamic_registration {

backend/windmill-mcp/src/lib.rs

@@ -38,11 +38,70 @@ pub mod client_registration;
 pub mod oauth {
     //! Re-exports of rmcp auth and oauth2 types for MCP OAuth implementations
 
+    use std::time::Duration;
+
     pub use rmcp::transport::auth::AuthorizationManager;
 
+    const DEFAULT_OAUTH_HTTP_TIMEOUT: Duration = Duration::from_secs(30);
+
+    pub fn no_redirect_http_client() -> Result<reqwest::Client, reqwest::Error> {
+        no_redirect_http_client_with_timeout(DEFAULT_OAUTH_HTTP_TIMEOUT)
+    }
+
+    pub(crate) fn no_redirect_http_client_with_timeout(
+        timeout: Duration,
+    ) -> Result<reqwest::Client, reqwest::Error> {
+        reqwest::Client::builder()
+            .timeout(timeout)
+            .redirect(reqwest::redirect::Policy::none())
+            .build()
+    }
+
     // Re-export oauth2 types needed for MCP OAuth flow
     pub use oauth2::{
         basic::BasicClient, AuthUrl, ClientId, ClientSecret, CsrfToken, PkceCodeChallenge,
         RedirectUrl, Scope, TokenUrl,
     };
+
+    #[cfg(test)]
+    mod tests {
+        use super::*;
+        use std::{
+            io::Read,
+            net::TcpListener,
+            thread,
+            time::{Duration, Instant},
+        };
+
+        #[tokio::test]
+        async fn no_redirect_http_client_times_out_stalled_responses() {
+            let listener = TcpListener::bind("127.0.0.1:0").unwrap();
+            let addr = listener.local_addr().unwrap();
+
+            let handle = thread::spawn(move || {
+                if let Ok((mut stream, _)) = listener.accept() {
+                    let _ = stream.set_read_timeout(Some(Duration::from_millis(200)));
+                    let mut buffer = [0; 1024];
+                    let _ = stream.read(&mut buffer);
+                    thread::sleep(Duration::from_millis(300));
+                }
+            });
+
+            let client = no_redirect_http_client_with_timeout(Duration::from_millis(50)).unwrap();
+            let started = Instant::now();
+            let err = client
+                .get(format!("http://{addr}/stall"))
+                .send()
+                .await
+                .expect_err("stalled response should time out");
+
+            assert!(err.is_timeout(), "expected timeout error, got: {err}");
+            assert!(
+                started.elapsed() < Duration::from_secs(2),
+                "stalled request should fail promptly"
+            );
+
+            handle.join().unwrap();
+        }
+    }
 }