diff --git a/codex-rs/rmcp-client/src/http_client_adapter.rs b/codex-rs/rmcp-client/src/http_client_adapter.rs index 2564aef3a1..c2327735f8 100644 --- a/codex-rs/rmcp-client/src/http_client_adapter.rs +++ b/codex-rs/rmcp-client/src/http_client_adapter.rs @@ -56,6 +56,7 @@ use sse_stream::SseStream; use tokio::sync::oneshot; use crate::event_notification_transport::MAX_EVENT_NOTIFICATION_BYTES; +use crate::http_client_redirect::SameOriginRedirectHttpClient; use crate::incoming_jsonrpc::deserialize_incoming_jsonrpc_message; use crate::incoming_jsonrpc::normalize_sse_jsonrpc_message; use crate::local_stdio_transport::MAX_MCP_STDIO_LINE_BYTES; @@ -124,7 +125,7 @@ impl StreamableHttpClientAdapter { initialize_deadline: Arc>>, ) -> Self { Self { - http_client, + http_client: Arc::new(SameOriginRedirectHttpClient::new(http_client)), default_headers, auth_provider, event_stream_cancellations: Arc::default(), diff --git a/codex-rs/rmcp-client/src/http_client_redirect.rs b/codex-rs/rmcp-client/src/http_client_redirect.rs new file mode 100644 index 0000000000..fa73b66b36 --- /dev/null +++ b/codex-rs/rmcp-client/src/http_client_redirect.rs @@ -0,0 +1,221 @@ +//! Restricts MCP HTTP redirects to the configured server's origin. +//! +//! MCP requests can carry sensitive headers and tool-call bodies. Following a +//! cross-origin redirect would send them to another server or an internal +//! service, so this transport validates each redirect before following it. + +use std::sync::Arc; +use std::time::Duration; +use std::time::Instant; + +use codex_exec_server::ExecServerError; +use codex_exec_server::HttpClient; +use codex_exec_server::HttpHeader; +use codex_exec_server::HttpRedirectPolicy; +use codex_exec_server::HttpRequestParams; +use codex_exec_server::HttpRequestResponse; +use codex_exec_server::HttpResponseBodyStream; +use futures::FutureExt; +use futures::future::BoxFuture; +use http::HeaderValue; +use http::StatusCode; +use url::Url; + +const MAX_REDIRECTS: usize = 10; + +pub(crate) struct SameOriginRedirectHttpClient { + inner: Arc, +} + +enum RedirectResponse { + Buffered(HttpRequestResponse), + Streaming(HttpRequestResponse, HttpResponseBodyStream), +} + +impl SameOriginRedirectHttpClient { + pub(crate) fn new(inner: Arc) -> Self { + Self { inner } + } + + async fn execute( + &self, + mut params: HttpRequestParams, + ) -> Result { + // Inspect each Location ourselves before the underlying client can send + // sensitive request data to the redirect destination. + params.redirect_policy = HttpRedirectPolicy::Stop; + let mut current_url = Url::parse(¶ms.url) + .map_err(|error| ExecServerError::HttpRequest(error.to_string()))?; + let original_origin = current_url.origin(); + // Redirect hops share the original timeout instead of restarting it. + let deadline = params + .timeout_ms + .map(|timeout_ms| Instant::now() + Duration::from_millis(timeout_ms)); + let mut redirects = 0; + + loop { + if let Some(deadline) = deadline { + let Some(remaining) = deadline.checked_duration_since(Instant::now()) else { + return Err(ExecServerError::HttpRequest( + "MCP HTTP request timed out".to_string(), + )); + }; + params.timeout_ms = Some( + u64::try_from(remaining.as_millis()) + .unwrap_or(u64::MAX) + .max(1), + ); + } + + let result = if params.stream_response { + let (response, stream) = self.inner.http_request_stream(params.clone()).await?; + RedirectResponse::Streaming(response, stream) + } else { + RedirectResponse::Buffered(self.inner.http_request(params.clone()).await?) + }; + let response = match &result { + RedirectResponse::Buffered(response) | RedirectResponse::Streaming(response, _) => { + response + } + }; + let status = StatusCode::from_u16(response.status).ok(); + if !matches!( + status, + Some( + StatusCode::MOVED_PERMANENTLY + | StatusCode::FOUND + | StatusCode::SEE_OTHER + | StatusCode::TEMPORARY_REDIRECT + | StatusCode::PERMANENT_REDIRECT + ) + ) { + return Ok(result); + } + + let Some(location) = response + .headers + .iter() + .find(|header| header.name.eq_ignore_ascii_case("location")) + .map(|header| header.value.as_str()) + else { + return Ok(result); + }; + let next_url = current_url + .join(location) + .map_err(|error| ExecServerError::HttpRequest(error.to_string()))?; + // Compare every hop with the configured origin so redirect chains + // cannot escape it through a later cross-origin Location. + if next_url.origin() != original_origin { + return Err(ExecServerError::HttpRequest( + "MCP HTTP redirect to a different origin is not allowed".to_string(), + )); + } + // A hostname can resolve to a different address on the next HTTP + // connection. HTTPS authenticates the hostname; localhost and IP + // literals do not depend on attacker-controlled DNS. + if next_url.scheme() == "http" + && matches!(next_url.host(), Some(url::Host::Domain(host)) if host != "localhost") + { + return Err(ExecServerError::HttpRequest( + "MCP HTTP redirects for non-loopback hostnames require HTTPS".to_string(), + )); + } + if redirects >= MAX_REDIRECTS { + return Err(ExecServerError::HttpRequest( + "MCP HTTP request exceeded the redirect limit".to_string(), + )); + } + + // Preserve normal HTTP behavior: 307/308 retain the method and body; + // 303 and legacy 301/302 POST redirects switch to a bodyless GET. + let drop_body = matches!( + status, + Some(StatusCode::MOVED_PERMANENTLY | StatusCode::FOUND) + ) && params.method.eq_ignore_ascii_case("POST") + || status == Some(StatusCode::SEE_OTHER); + if drop_body { + if !params.method.eq_ignore_ascii_case("HEAD") { + params.method = "GET".to_string(); + } + params.body = None; + params.headers.retain(|header| { + ![ + "content-type", + "content-length", + "content-encoding", + "transfer-encoding", + ] + .iter() + .any(|name| header.name.eq_ignore_ascii_case(name)) + }); + } + + // HTTPS keeps application proxy credentials inside the tunnel to + // the already-validated origin. Plaintext routes can change across + // requests, so their proxy credentials must never be replayed. + params.headers.retain(|header| { + (current_url.scheme() == "https" + || !header.name.eq_ignore_ascii_case("proxy-authorization")) + && !header.name.eq_ignore_ascii_case("referer") + }); + let mut referer = current_url.clone(); + let _ = referer.set_username(""); + let _ = referer.set_password(None); + referer.set_fragment(None); + if HeaderValue::from_str(referer.as_str()).is_ok() { + params.headers.push(HttpHeader { + name: "referer".to_string(), + value: referer.to_string(), + }); + } + + params.url = next_url.to_string(); + current_url = next_url; + redirects += 1; + } + } +} + +impl HttpClient for SameOriginRedirectHttpClient { + fn http_request( + &self, + mut params: HttpRequestParams, + ) -> BoxFuture<'_, Result> { + async move { + params.stream_response = false; + if params.redirect_policy == HttpRedirectPolicy::Stop { + return self.inner.http_request(params).await; + } + match self.execute(params).await? { + RedirectResponse::Buffered(response) => Ok(response), + RedirectResponse::Streaming(_, _) => Err(ExecServerError::Protocol( + "MCP buffered HTTP request returned a streamed response".to_string(), + )), + } + } + .boxed() + } + + fn http_request_stream( + &self, + mut params: HttpRequestParams, + ) -> BoxFuture<'_, Result<(HttpRequestResponse, HttpResponseBodyStream), ExecServerError>> { + async move { + params.stream_response = true; + if params.redirect_policy == HttpRedirectPolicy::Stop { + return self.inner.http_request_stream(params).await; + } + match self.execute(params).await? { + RedirectResponse::Streaming(response, stream) => Ok((response, stream)), + RedirectResponse::Buffered(_) => Err(ExecServerError::Protocol( + "MCP streamed HTTP request returned a buffered response".to_string(), + )), + } + } + .boxed() + } +} + +#[cfg(test)] +#[path = "http_client_redirect_tests.rs"] +mod tests; diff --git a/codex-rs/rmcp-client/src/http_client_redirect_tests.rs b/codex-rs/rmcp-client/src/http_client_redirect_tests.rs new file mode 100644 index 0000000000..f3bb42cfdb --- /dev/null +++ b/codex-rs/rmcp-client/src/http_client_redirect_tests.rs @@ -0,0 +1,390 @@ +use std::sync::Arc; +use std::sync::Mutex; + +use codex_exec_server::Environment; +use codex_exec_server::HttpHeader; +use pretty_assertions::assert_eq; +use wiremock::Mock; +use wiremock::MockServer; +use wiremock::ResponseTemplate; +use wiremock::matchers::path; + +use crate::http_headers::with_http_headers_helper; + +use super::*; + +const PROXY_HEADERS_HELPER: &str = if cfg!(windows) { + r#"echo {"Proxy-Authorization":"Bearer proxy-token"}"# +} else { + r#"printf '{"Proxy-Authorization":"Bearer proxy-token"}'"# +}; + +fn request(url: impl Into) -> HttpRequestParams { + HttpRequestParams { + method: "POST".to_string(), + url: url.into(), + headers: Vec::new(), + body: Some(b"sensitive-body".to_vec().into()), + timeout_ms: Some(5_000), + redirect_policy: HttpRedirectPolicy::Follow, + request_id: "redirect-test".to_string(), + stream_response: false, + } +} + +fn headers(headers: [(&str, &str); N]) -> Vec { + headers + .into_iter() + .map(|(name, value)| HttpHeader { + name: name.to_string(), + value: value.to_string(), + }) + .collect() +} + +#[derive(Default)] +struct RecordingRedirectHttpClient { + requests: Mutex>, + delay: Duration, + loop_redirects: bool, +} + +impl HttpClient for RecordingRedirectHttpClient { + fn http_request( + &self, + params: HttpRequestParams, + ) -> BoxFuture<'_, Result> { + let mut requests = self.requests.lock().expect("request recorder lock"); + let redirect = requests.is_empty() || self.loop_redirects; + let delay = self.delay; + requests.push(params); + async move { + if !delay.is_zero() { + tokio::time::sleep(delay).await; + } + Ok(HttpRequestResponse { + status: if redirect { 307 } else { 200 }, + headers: if redirect { + headers([("location", "/final")]) + } else { + Vec::new() + }, + body: Vec::new().into(), + }) + } + .boxed() + } + + fn http_request_stream( + &self, + _params: HttpRequestParams, + ) -> BoxFuture<'_, Result<(HttpRequestResponse, HttpResponseBodyStream), ExecServerError>> { + async { + Err(ExecServerError::HttpRequest( + "unexpected streaming request".to_string(), + )) + } + .boxed() + } +} + +#[tokio::test] +async fn plaintext_hostname_redirects_are_rejected_before_dns_can_rebind() -> anyhow::Result<()> { + for (url, expected_error) in [ + ( + "http://mcp.example/start", + Some("non-loopback hostnames require HTTPS"), + ), + ("http://localhost/start", None), + ("http://127.0.0.1/start", None), + ("http://[::1]/start", None), + ("https://mcp.example/start", None), + ] { + let recorder = Arc::new(RecordingRedirectHttpClient::default()); + let client = SameOriginRedirectHttpClient::new(recorder.clone()); + let response = client + .http_request(HttpRequestParams { + headers: headers([("authorization", "Bearer sensitive-token")]), + ..request(url) + }) + .await; + + if let Some(expected_error) = expected_error { + let error = response.expect_err("plaintext hostname redirects must fail"); + assert!(error.to_string().contains(expected_error), "{error}"); + } else { + assert_eq!(response?.status, 200); + } + let requests = recorder.requests.lock().expect("request recorder lock"); + assert_eq!(requests.len(), if expected_error.is_some() { 1 } else { 2 }); + } + + Ok(()) +} + +#[tokio::test] +async fn same_origin_redirects_enforce_shared_timeout_and_hop_limit() { + for (delay, timeout_ms, expected_error, expected_requests) in [ + (Duration::from_millis(100), 175, "timed out", 2), + (Duration::ZERO, 5_000, "redirect limit", MAX_REDIRECTS + 1), + ] { + let recorder = Arc::new(RecordingRedirectHttpClient { + delay, + loop_redirects: true, + ..Default::default() + }); + let client = SameOriginRedirectHttpClient::new(recorder.clone()); + let error = client + .http_request(HttpRequestParams { + method: "GET".to_string(), + body: None, + timeout_ms: Some(timeout_ms), + ..request("https://mcp.example/loop") + }) + .await + .expect_err("redirect loops must respect both limits"); + + assert!(error.to_string().contains(expected_error), "{error}"); + let requests = recorder.requests.lock().expect("request recorder lock"); + assert_eq!(requests.len(), expected_requests); + } +} + +#[tokio::test] +async fn same_origin_redirects_preserve_method_body_and_headers() -> anyhow::Result<()> { + for (method, redirect_status, redirected_method) in [ + ("POST", 301, "GET"), + ("POST", 302, "GET"), + ("POST", 303, "GET"), + ("HEAD", 303, "HEAD"), + ("POST", 307, "POST"), + ("GET", 307, "GET"), + ("DELETE", 307, "DELETE"), + ("POST", 308, "POST"), + ] { + let server = MockServer::start().await; + Mock::given(path("/start")) + .respond_with( + ResponseTemplate::new(redirect_status).insert_header("location", "/final"), + ) + .expect(1) + .mount(&server) + .await; + Mock::given(path("/final")) + .respond_with(ResponseTemplate::new(200)) + .expect(1) + .mount(&server) + .await; + + let client = + SameOriginRedirectHttpClient::new(Environment::default_for_tests().get_http_client()); + let expected_content_length = b"sensitive-body".len().to_string(); + let mut params = HttpRequestParams { + method: method.to_string(), + headers: headers([ + ("x-api-key", "sensitive-key"), + ("proxy-authorization", "sensitive-proxy-credentials"), + ("referer", "https://stale.example/"), + ]), + body: (method == "POST").then(|| b"sensitive-body".to_vec().into()), + stream_response: method != "DELETE", + ..request(format!("{}/start", server.uri())) + }; + if method == "POST" { + params.headers.extend(headers([ + ("content-type", "application/json"), + ("content-encoding", "identity"), + ("content-length", &expected_content_length), + ])); + } + let status = if method == "DELETE" { + client.http_request(params).await?.status + } else { + client.http_request_stream(params).await?.0.status + }; + assert_eq!(status, 200); + + let requests = server.received_requests().await.expect("recorded requests"); + let redirected = &requests[1]; + let expected_referer = format!("{}/start", server.uri()); + let body_preserved = redirected_method == "POST"; + for (name, expected) in [ + ("x-api-key", Some("sensitive-key")), + ("proxy-authorization", None), + ("referer", Some(expected_referer.as_str())), + ("content-type", body_preserved.then_some("application/json")), + ("content-encoding", body_preserved.then_some("identity")), + ( + "content-length", + body_preserved.then_some(expected_content_length.as_str()), + ), + ] { + assert_eq!( + redirected + .headers + .get(name) + .and_then(|value| value.to_str().ok()), + expected, + "unexpected redirected {name} header" + ); + } + assert_eq!( + (redirected.method.as_str(), redirected.body.as_slice()), + ( + redirected_method, + if body_preserved { + b"sensitive-body".as_slice() + } else { + b"".as_slice() + } + ) + ); + } + + Ok(()) +} + +#[tokio::test] +async fn https_redirects_preserve_configured_and_helper_proxy_authorization() -> anyhow::Result<()> +{ + for helper_enabled in [false, true] { + let recorder = Arc::new(RecordingRedirectHttpClient::default()); + let inner: Arc = recorder.clone(); + let directory = tempfile::tempdir()?; + let url = "https://mcp.example/start"; + let inner = if helper_enabled { + with_http_headers_helper( + inner, + url, + PROXY_HEADERS_HELPER, + directory.path().to_path_buf(), + )? + } else { + inner + }; + let client = SameOriginRedirectHttpClient::new(inner); + let response = client + .http_request(HttpRequestParams { + headers: if helper_enabled { + Vec::new() + } else { + headers([("Proxy-Authorization", "Bearer proxy-token")]) + }, + ..request(url) + }) + .await?; + + let requests = recorder.requests.lock().expect("request recorder lock"); + assert_eq!( + ( + response.status, + requests.len(), + requests[1].url.as_str(), + requests[1] + .headers + .iter() + .find(|header| header.name.eq_ignore_ascii_case("proxy-authorization")) + .map(|header| header.value.as_str()), + ), + ( + 200, + 2, + "https://mcp.example/final", + Some("Bearer proxy-token"), + ), + ); + } + + Ok(()) +} + +#[tokio::test] +async fn plaintext_helper_redirects_block_mcp_but_preserve_oauth_stop() -> anyhow::Result<()> { + for (request_id, redirect_policy) in [ + ("mcp-request-1", HttpRedirectPolicy::Follow), + ("oauth-request-1", HttpRedirectPolicy::Stop), + ] { + let server = MockServer::start().await; + Mock::given(path("/start")) + .respond_with(ResponseTemplate::new(307).insert_header("location", "/final")) + .expect(1) + .mount(&server) + .await; + + let directory = tempfile::tempdir()?; + let url = format!("{}/start", server.uri()); + let helper = with_http_headers_helper( + Environment::default_for_tests().get_http_client(), + &url, + PROXY_HEADERS_HELPER, + directory.path().to_path_buf(), + )?; + let client = SameOriginRedirectHttpClient::new(helper); + let response = client + .http_request_stream(HttpRequestParams { + request_id: request_id.to_string(), + redirect_policy, + stream_response: true, + ..request(url) + }) + .await; + + match (redirect_policy, response) { + (HttpRedirectPolicy::Stop, Ok((response, _))) => assert_eq!(response.status, 307), + (HttpRedirectPolicy::Follow, Err(error)) => { + assert!(error.to_string().contains("Proxy-Authorization")); + } + _ => panic!("unexpected plaintext proxy-credential redirect behavior"), + } + assert_eq!(server.received_requests().await.unwrap().len(), 1); + } + Ok(()) +} + +#[tokio::test] +async fn cross_origin_redirects_never_reach_their_destination() -> anyhow::Result<()> { + for status in [307, 308] { + for method in ["POST", "GET", "DELETE"] { + let destination = MockServer::start().await; + let server = MockServer::start().await; + Mock::given(path("/start")) + .respond_with( + ResponseTemplate::new(status) + .insert_header("location", format!("{}/private", destination.uri())), + ) + .expect(1) + .mount(&server) + .await; + + let client = SameOriginRedirectHttpClient::new( + Environment::default_for_tests().get_http_client(), + ); + let params = HttpRequestParams { + method: method.to_string(), + headers: headers([("x-api-key", "sensitive-key")]), + body: (method == "POST").then(|| b"sensitive-body".to_vec().into()), + stream_response: method != "DELETE", + ..request(format!("{}/start", server.uri())) + }; + let error = if method == "DELETE" { + client + .http_request(params) + .await + .expect_err("cross-origin DELETE redirect must fail") + } else { + match client.http_request_stream(params).await { + Ok(_) => panic!("cross-origin {method} redirect must fail"), + Err(error) => error, + } + }; + + assert!( + error.to_string().contains("different origin"), + "cross-origin {method} redirect must explain its rejection: {error}" + ); + assert!(destination.received_requests().await.unwrap().is_empty()); + } + } + + Ok(()) +} diff --git a/codex-rs/rmcp-client/src/http_headers.rs b/codex-rs/rmcp-client/src/http_headers.rs index 66ae1bd8bf..230443cd1d 100644 --- a/codex-rs/rmcp-client/src/http_headers.rs +++ b/codex-rs/rmcp-client/src/http_headers.rs @@ -197,6 +197,23 @@ impl HttpHeadersClient { } Ok(params) } + + fn reject_proxy_authorization_redirect( + response: &HttpRequestResponse, + ) -> Result<(), ExecServerError> { + if (300..400).contains(&response.status) + && response + .headers + .iter() + .any(|header| header.name.eq_ignore_ascii_case("location")) + { + return Err(ExecServerError::HttpRequest( + "MCP HTTP redirect cannot safely replay Proxy-Authorization credentials" + .to_string(), + )); + } + Ok(()) + } } impl HttpClient for HttpHeadersClient { @@ -205,8 +222,21 @@ impl HttpClient for HttpHeadersClient { params: HttpRequestParams, ) -> BoxFuture<'_, Result> { async move { + // OAuth validates redirect responses itself; only intercept MCP replay. + let mcp_redirect_was_stopped = params.redirect_policy == HttpRedirectPolicy::Stop + && !params.request_id.starts_with("oauth-request-"); let params = self.prepare_request(params).await?; - self.inner.http_request(params).await + let prevent_proxy_authorization_redirect = mcp_redirect_was_stopped + && Url::parse(¶ms.url).is_ok_and(|url| url.scheme() == "http") + && params + .headers + .iter() + .any(|header| header.name.eq_ignore_ascii_case("proxy-authorization")); + let response = self.inner.http_request(params).await?; + if prevent_proxy_authorization_redirect { + Self::reject_proxy_authorization_redirect(&response)?; + } + Ok(response) } .boxed() } @@ -216,8 +246,20 @@ impl HttpClient for HttpHeadersClient { params: HttpRequestParams, ) -> BoxFuture<'_, Result<(HttpRequestResponse, HttpResponseBodyStream), ExecServerError>> { async move { + let mcp_redirect_was_stopped = params.redirect_policy == HttpRedirectPolicy::Stop + && !params.request_id.starts_with("oauth-request-"); let params = self.prepare_request(params).await?; - self.inner.http_request_stream(params).await + let prevent_proxy_authorization_redirect = mcp_redirect_was_stopped + && Url::parse(¶ms.url).is_ok_and(|url| url.scheme() == "http") + && params + .headers + .iter() + .any(|header| header.name.eq_ignore_ascii_case("proxy-authorization")); + let (response, stream) = self.inner.http_request_stream(params).await?; + if prevent_proxy_authorization_redirect { + Self::reject_proxy_authorization_redirect(&response)?; + } + Ok((response, stream)) } .boxed() } diff --git a/codex-rs/rmcp-client/src/lib.rs b/codex-rs/rmcp-client/src/lib.rs index 379fa8d5b7..4203788638 100644 --- a/codex-rs/rmcp-client/src/lib.rs +++ b/codex-rs/rmcp-client/src/lib.rs @@ -3,6 +3,7 @@ mod elicitation_client_service; mod event_notification_transport; mod executor_process_transport; mod http_client_adapter; +mod http_client_redirect; mod http_headers; mod in_process_transport; mod incoming_jsonrpc; diff --git a/codex-rs/rmcp-client/tests/mcp_2026_discovery.rs b/codex-rs/rmcp-client/tests/mcp_2026_discovery.rs index 72968a75d3..9e660d1f1c 100644 --- a/codex-rs/rmcp-client/tests/mcp_2026_discovery.rs +++ b/codex-rs/rmcp-client/tests/mcp_2026_discovery.rs @@ -230,8 +230,9 @@ async fn modern_discovery_does_not_follow_redirects_with_sensitive_headers() -> } #[tokio::test] -async fn legacy_mcp_requests_follow_redirects_with_configured_headers() -> anyhow::Result<()> { - let redirect_target = MockServer::start().await; +async fn legacy_mcp_requests_follow_same_origin_redirects_with_configured_headers() +-> anyhow::Result<()> { + let server = MockServer::start().await; Mock::given(method("POST")) .and(path("/forwarded")) .and(header("x-api-key", "sensitive-key")) @@ -244,23 +245,19 @@ async fn legacy_mcp_requests_follow_redirects_with_configured_headers() -> anyho } }) .expect(2) - .mount(&redirect_target) + .mount(&server) .await; - let resource_server = MockServer::start().await; Mock::given(method("POST")) .and(path("/mcp")) - .respond_with( - ResponseTemplate::new(307) - .insert_header("location", format!("{}/forwarded", redirect_target.uri())), - ) + .respond_with(ResponseTemplate::new(307).insert_header("location", "/forwarded")) .expect(2) - .mount(&resource_server) + .mount(&server) .await; let client = RmcpClient::new_streamable_http_client_with_protocol_mode( "legacy-redirect-test", - &format!("{}/mcp", resource_server.uri()), + &format!("{}/mcp", server.uri()), /*bearer_token*/ None, Some(HashMap::from([( "x-api-key".to_string(), @@ -275,12 +272,66 @@ async fn legacy_mcp_requests_follow_redirects_with_configured_headers() -> anyho ) .await?; initialize_client(&client).await?; - redirect_target.verify().await; - resource_server.verify().await; + server.verify().await; client.shutdown().await; Ok(()) } +#[tokio::test] +async fn legacy_fallback_does_not_follow_cross_origin_tool_redirects() -> anyhow::Result<()> { + for mode in [McpProtocolMode::Legacy, McpProtocolMode::V20260728] { + let destination = MockServer::start().await; + let server = MockServer::start().await; + let destination_url = format!("{}/private", destination.uri()); + Mock::given(method("POST")) + .and(path("/redirected")) + .respond_with(ResponseTemplate::new(307).insert_header("location", destination_url)) + .expect(1) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path("/mcp")) + .respond_with(move |request: &Request| { + let body: Value = request.body_json().expect("valid JSON-RPC request"); + match body["method"].as_str() { + Some("server/discover") => ResponseTemplate::new(200).set_body_json(json!({ + "jsonrpc": "2.0", + "id": body["id"], + "error": {"code": -32601, "message": "method not found"}, + })), + Some("initialize") => legacy_initialize_response(&body), + Some("notifications/initialized") => ResponseTemplate::new(202), + Some("tools/call") => { + ResponseTemplate::new(307).insert_header("location", "/redirected") + } + other => panic!("unexpected legacy MCP method: {other:?}"), + } + }) + .mount(&server) + .await; + + let client = create_client(&server, mode).await?; + initialize_client(&client).await?; + let error = client + .call_tool( + "query_logs_sql".to_string(), + Some(json!({"query": "sensitive-query"})), + /*meta*/ None, + Some(Duration::from_secs(5)), + ) + .await + .expect_err("cross-origin tools/call redirect must fail"); + assert!( + format!("{error:#}").contains("different origin"), + "cross-origin tools/call redirect must explain its rejection: {error:#}" + ); + assert!(destination.received_requests().await.unwrap().is_empty()); + client.shutdown().await; + } + + Ok(()) +} + #[tokio::test] async fn modern_discovery_accepts_metadata_namespaced_server_identity() -> anyhow::Result<()> { let server = MockServer::start().await;