Repository navigation
fix(server): reject server-to-client requests on discover sessions #1397
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,8 +51,15 @@ impl ServiceRole for RoleServer { | |
| } | ||
| } | ||
|
|
||
| fn enforce_outbound_request(peer: &Peer<Self>) -> Result<(), ServiceError> { | ||
| // Same order as `RequestContext::protocol_version`: the version the | ||
| // current request names wins. Outside a handler, fall back to the version | ||
| // an inline session (e.g. discover on stdio) opened on, since it never | ||
| // stores peer info, then to peer info. | ||
| let version = originating_protocol_version() | ||
| .or_else(|| peer.inline_protocol_version().cloned()) | ||
| .or_else(|| peer.peer_info().map(|info| info.protocol_version.clone())); | ||
| if version.is_some_and(|version| version >= ProtocolVersion::V_2026_07_28) { | ||
| return Err(ServiceError::McpError(ErrorData::invalid_request( | ||
| "server-to-client requests are not allowed on protocol 2026-07-28 or later; return InputRequiredResult instead", | ||
| None, | ||
|
|
@@ -650,7 +657,7 @@ where | |
| })?; | ||
| continue; | ||
| } | ||
| peer.require_request_metadata(requested_version); | ||
| // Dispatch the request from inside the service loop rather than | ||
| // inline: its handler may send notifications through `peer`, which | ||
| // only complete once the loop drains `peer_rx`. | ||
|
|
@@ -1237,6 +1244,10 @@ impl Peer<RoleServer> { | |
| where | ||
| T: ElicitationSafe + for<'de> serde::Deserialize<'de>, | ||
| { | ||
| // Reject on 2026-07-28 before the capability check, which would | ||
| // otherwise answer `CapabilityNotSupported` first. | ||
| RoleServer::enforce_outbound_request(self)?; | ||
|
|
||
| // Check if client supports form elicitation capability | ||
| if !self | ||
| .supported_elicitation_modes() | ||
|
|
@@ -1388,6 +1399,10 @@ impl Peer<RoleServer> { | |
| elicitation_id: impl Into<String>, | ||
| timeout: Option<std::time::Duration>, | ||
| ) -> Result<ElicitationAction, ElicitationError> { | ||
| // Reject on 2026-07-28 before the capability check, which would | ||
| // otherwise answer `CapabilityNotSupported` first. | ||
| RoleServer::enforce_outbound_request(self)?; | ||
|
|
||
| // Check if client supports url elicitation | ||
| if !self | ||
| .supported_elicitation_modes() | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,8 @@ | |
| #![expect(deprecated, reason = "This test exercises the deprecated sampling API")] | ||
|
|
||
| use rmcp::{ | ||
| ClientHandler, ClientLifecycleMode, ClientServiceExt, RoleClient, RoleServer, ServerHandler, | ||
| ServiceError, ServiceExt, | ||
| model::{ | ||
| CallToolRequestParams, CallToolResponse, CallToolResult, ClientConfig, ContentBlock, | ||
| CreateMessageRequest, CreateMessageRequestParams, CreateMessageResult, ErrorCode, | ||
|
|
@@ -35,6 +36,14 @@ impl ServerHandler for SamplingServer { | |
| CreateMessageRequestParams::new(vec![SamplingMessage::user_text("nested")], 16); | ||
| let outcome = match request.name.as_ref() { | ||
| "sample" => context.peer.create_message(params).await.map(|_| ()), | ||
| // A spawned task loses the originating request, so the guard falls | ||
| // back to the session's version. | ||
| "sample_spawned" => { | ||
| let peer = context.peer.clone(); | ||
| tokio::spawn(async move { peer.create_message(params).await.map(|_| ()) }) | ||
| .await | ||
| .unwrap() | ||
| } | ||
| "sample_generic" => context | ||
| .peer | ||
| .send_request(ServerRequest::CreateMessageRequest( | ||
|
|
@@ -47,6 +56,20 @@ impl ServerHandler for SamplingServer { | |
| .send_request(ServerRequest::PingRequest(PingRequest::default())) | ||
| .await | ||
| .map(|_| ()), | ||
| #[cfg(feature = "elicitation")] | ||
| "elicit_url" => match context | ||
| .peer | ||
| .elicit_url( | ||
| "visit", | ||
| url::Url::parse("https://example.com/").unwrap(), | ||
| "elicitation-1", | ||
| ) | ||
| .await | ||
| { | ||
| Ok(_) => Ok(()), | ||
| Err(rmcp::service::ElicitationError::Service(e)) => Err(e), | ||
| Err(e) => return Err(rmcp::ErrorData::internal_error(e.to_string(), None)), | ||
| }, | ||
| other => panic!("unexpected tool {other}"), | ||
| }; | ||
| let text = match outcome { | ||
|
|
@@ -130,6 +153,46 @@ async fn call_tool_on_modern_pair(tool: &'static str) -> anyhow::Result<String> | |
| Ok(text) | ||
| } | ||
|
|
||
| /// Starts the pair the way stdio does: the server through plain `serve()`, the | ||
| /// client through the discover lifecycle on `version`. Unlike `serve_directly`, | ||
| /// this leaves the server's `peer_info` empty. | ||
| async fn call_tool_on_discover_pair( | ||
| tool: &'static str, | ||
| version: ProtocolVersion, | ||
| ) -> anyhow::Result<String> { | ||
| let (server_transport, client_transport) = tokio::io::duplex(4096); | ||
| let server_handle = tokio::spawn(async move { | ||
| let server = SamplingServer.serve(server_transport).await?; | ||
| server.waiting().await?; | ||
| anyhow::Ok(()) | ||
| }); | ||
| let client = SamplingClient | ||
| .serve_with_lifecycle( | ||
| client_transport, | ||
| ClientLifecycleMode::Discover { | ||
| preferred_versions: vec![version], | ||
| }, | ||
| ) | ||
| .await?; | ||
|
|
||
| let result = client | ||
| .peer() | ||
| .call_tool(CallToolRequestParams::new(tool)) | ||
| .await?; | ||
| let text = result | ||
| .content | ||
| .first() | ||
| .unwrap() | ||
| .as_text() | ||
| .unwrap() | ||
| .text | ||
| .clone(); | ||
|
|
||
| client.cancel().await?; | ||
| let _ = server_handle.await?; | ||
| Ok(text) | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn sampling_from_handler_rejected_on_modern_protocol() -> anyhow::Result<()> { | ||
| assert_eq!(call_tool_on_modern_pair("sample").await?, "rejected"); | ||
|
|
@@ -151,6 +214,139 @@ async fn ping_rejected_on_modern_protocol() -> anyhow::Result<()> { | |
| Ok(()) | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn sampling_from_handler_rejected_on_discover_lifecycle() -> anyhow::Result<()> { | ||
| assert_eq!( | ||
| call_tool_on_discover_pair("sample", ProtocolVersion::V_2026_07_28).await?, | ||
| "rejected" | ||
| ); | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn generic_send_request_rejected_on_discover_lifecycle() -> anyhow::Result<()> { | ||
| assert_eq!( | ||
| call_tool_on_discover_pair("sample_generic", ProtocolVersion::V_2026_07_28).await?, | ||
| "rejected" | ||
| ); | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn ping_rejected_on_discover_lifecycle() -> anyhow::Result<()> { | ||
| assert_eq!( | ||
| call_tool_on_discover_pair("ping", ProtocolVersion::V_2026_07_28).await?, | ||
| "rejected" | ||
| ); | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn spawned_sampling_rejected_on_discover_lifecycle() -> anyhow::Result<()> { | ||
| assert_eq!( | ||
| call_tool_on_discover_pair("sample_spawned", ProtocolVersion::V_2026_07_28).await?, | ||
| "rejected" | ||
| ); | ||
| Ok(()) | ||
| } | ||
|
|
||
| /// Plays a raw client that skips discover and names a version in each request's | ||
| /// `_meta`, then calls the `sample` tool once per entry in `versions` on one | ||
| /// connection. Answers any `sampling/createMessage` the server sends. | ||
| async fn raw_inline_sample_calls(versions: &[&str]) -> anyhow::Result<Vec<String>> { | ||
| let (server_io, client_io) = tokio::io::duplex(4096); | ||
| tokio::spawn(async move { | ||
| let server = SamplingServer.serve(server_io).await?; | ||
| server.waiting().await?; | ||
| anyhow::Ok(()) | ||
| }); | ||
| let (read, mut write) = tokio::io::split(client_io); | ||
| let mut lines = BufReader::new(read).lines(); | ||
| let mut texts = Vec::new(); | ||
| for (id, version) in versions.iter().enumerate() { | ||
| let call = json!({ | ||
| "jsonrpc": "2.0", | ||
| "id": id, | ||
| "method": "tools/call", | ||
| "params": { | ||
| "name": "sample", | ||
| "_meta": { | ||
| "io.modelcontextprotocol/protocolVersion": version, | ||
| "io.modelcontextprotocol/clientCapabilities": {} | ||
| } | ||
| } | ||
| }); | ||
| write.write_all(format!("{call}\n").as_bytes()).await?; | ||
| loop { | ||
| let line = tokio::time::timeout(std::time::Duration::from_secs(5), lines.next_line()) | ||
| .await?? | ||
| .expect("server message"); | ||
| let message: Value = serde_json::from_str(&line)?; | ||
| if message["method"] == "sampling/createMessage" { | ||
| let reply = json!({ | ||
| "jsonrpc": "2.0", | ||
| "id": message["id"], | ||
| "result": { | ||
| "role": "assistant", | ||
| "content": { "type": "text", "text": "pong" }, | ||
| "model": "test-model" | ||
| } | ||
| }); | ||
| write.write_all(format!("{reply}\n").as_bytes()).await?; | ||
| continue; | ||
| } | ||
| assert_eq!(message["id"], id, "unexpected message {message}"); | ||
| let text = message["result"]["content"][0]["text"] | ||
| .as_str() | ||
| .unwrap_or_else(|| panic!("tool result expected, got {message}")); | ||
| texts.push(text.to_owned()); | ||
| break; | ||
| } | ||
| } | ||
| Ok(texts) | ||
| } | ||
|
|
||
| /// Each inline request names its own version, so the guard follows the request | ||
| /// a handler is serving, not the one that opened the session. | ||
| #[tokio::test] | ||
| async fn guard_follows_each_request_version_on_inline_session() -> anyhow::Result<()> { | ||
| assert_eq!( | ||
| raw_inline_sample_calls(&["2026-07-28", "2025-11-25"]).await?, | ||
| ["rejected", "sent"] | ||
| ); | ||
| assert_eq!( | ||
| raw_inline_sample_calls(&["2025-11-25", "2026-07-28"]).await?, | ||
| ["sent", "rejected"] | ||
| ); | ||
| Ok(()) | ||
| } | ||
|
|
||
| /// A discover session can still pick an older version, where these requests | ||
| /// are allowed. | ||
| #[tokio::test] | ||
| async fn server_requests_allowed_on_older_discover_session() -> anyhow::Result<()> { | ||
| for tool in ["sample", "ping"] { | ||
| assert_eq!( | ||
| call_tool_on_discover_pair(tool, ProtocolVersion::V_2025_11_25).await?, | ||
| "sent" | ||
| ); | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| /// The client declares no elicitation modes, so this also pins that the | ||
| /// protocol check runs before the capability check. | ||
| #[cfg(feature = "elicitation")] | ||
| #[tokio::test] | ||
| async fn elicit_url_rejected_on_modern_protocol() -> anyhow::Result<()> { | ||
| assert_eq!(call_tool_on_modern_pair("elicit_url").await?, "rejected"); | ||
| assert_eq!( | ||
| call_tool_on_discover_pair("elicit_url", ProtocolVersion::V_2026_07_28).await?, | ||
| "rejected" | ||
| ); | ||
| Ok(()) | ||
| } | ||
|
|
||
| // A compliant rmcp server cannot produce a server-to-client request at | ||
| // >= 2026-07-28 (send-side enforcement blocks it), so the client's | ||
| // receive-side enforcement is exercised with a raw JSON-RPC server. | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that this records the session's initial version instead of flipping a flag,
require_request_metadatasounds a bit odd next toinline_protocol_version(). What aboutset_inline_protocol_versioninstead?