Skip to content

Commit da64ad3

Browse files
committed
feat: Implement SEP-2260 require server requests to associate with client requests
1 parent 07abfdf commit da64ad3

4 files changed

Lines changed: 343 additions & 3 deletions

File tree

crates/rmcp/src/service.rs

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,24 @@ pub trait ServiceRole: std::fmt::Debug + Send + Sync + 'static + Copy + Clone {
136136
fn peer_cancelled_params(_notification: &Self::PeerNot) -> Option<&CancelledNotificationParam> {
137137
None
138138
}
139+
140+
/// SEP-2260 hook: enforce that outbound requests which MUST be associated
141+
/// with an originating peer request are only sent from within a request
142+
/// handler scope.
143+
///
144+
/// This is invoked at the single transport chokepoint
145+
/// ([`Peer::send_request_with_option_and_subscription`]) so the guarantee
146+
/// holds regardless of which public sending API is used (including the
147+
/// generic [`Peer::send_request`]). The default is a no-op; only
148+
/// [`RoleServer`] restricts anything.
149+
#[doc(hidden)]
150+
fn enforce_request_association(
151+
_request: &Self::Req,
152+
_peer_info: Option<&Self::PeerInfo>,
153+
_in_request_handler_scope: bool,
154+
) -> Result<(), ServiceError> {
155+
Ok(())
156+
}
139157
}
140158

141159
pub(crate) fn uses_legacy_lifecycle(
@@ -146,6 +164,14 @@ pub(crate) fn uses_legacy_lifecycle(
146164
&& protocol_version.is_none_or(|version| version < &ProtocolVersion::V_2026_07_28)
147165
}
148166

167+
tokio::task_local! {
168+
pub(crate) static ORIGINATING_REQUEST: RequestId;
169+
}
170+
171+
pub(crate) fn in_request_handler_scope() -> bool {
172+
ORIGINATING_REQUEST.try_with(|_| ()).is_ok()
173+
}
174+
149175
pub type TxJsonRpcMessage<R> =
150176
JsonRpcMessage<<R as ServiceRole>::Req, <R as ServiceRole>::Resp, <R as ServiceRole>::Not>;
151177
pub type RxJsonRpcMessage<R> = JsonRpcMessage<
@@ -706,6 +732,15 @@ impl<R: ServiceRole> Peer<R> {
706732
options: PeerRequestOptions,
707733
subscription_sender: Option<SubscriptionChannel<R::PeerNot>>,
708734
) -> Result<RequestHandle<R>, ServiceError> {
735+
// SEP-2260: enforce request-association at the single transport
736+
// chokepoint. This covers every public sending API (typed convenience
737+
// methods and the generic `send_request`/`send_cancellable_request`),
738+
// so the guarantee cannot be bypassed.
739+
R::enforce_request_association(
740+
&request,
741+
self.peer_info().as_deref(),
742+
in_request_handler_scope(),
743+
)?;
709744
let id = self.request_id_provider.next_request_id();
710745
let progress_token = self.progress_token_provider.next_progress_token();
711746
if let Some(metadata) = self.client_request_metadata.get() {
@@ -1379,9 +1414,10 @@ where
13791414
extensions,
13801415
};
13811416
let current_span = tracing::Span::current();
1417+
let handler_id = id.clone();
13821418
spawn_service_task(async move {
1383-
let result = service
1384-
.handle_request(request, context)
1419+
let result = ORIGINATING_REQUEST
1420+
.scope(handler_id, service.handle_request(request, context))
13851421
.await;
13861422
let response = match result {
13871423
Ok(result) => {

crates/rmcp/src/service/server.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,40 @@ impl ServiceRole for RoleServer {
4949
_ => None,
5050
}
5151
}
52+
53+
/// SEP-2260: `sampling/createMessage`, `roots/list`, and
54+
/// `elicitation/create` MUST be associated with an originating
55+
/// client-to-server request. We enforce this for peers negotiating
56+
/// protocol `2026-07-28` or newer by requiring an active request-handler
57+
/// scope (see [`crate::service::ORIGINATING_REQUEST`]).
58+
///
59+
/// `ping` is explicitly excepted, and `CustomRequest` is left to the
60+
/// implementor. Older peers are not enforced, preserving backward
61+
/// compatibility.
62+
fn enforce_request_association(
63+
request: &Self::Req,
64+
peer_info: Option<&Self::PeerInfo>,
65+
in_request_handler_scope: bool,
66+
) -> Result<(), ServiceError> {
67+
let restricted = matches!(
68+
request,
69+
ServerRequest::CreateMessageRequest(_)
70+
| ServerRequest::ListRootsRequest(_)
71+
| ServerRequest::ElicitRequest(_)
72+
);
73+
if !restricted {
74+
return Ok(());
75+
}
76+
let strict =
77+
peer_info.is_some_and(|info| info.protocol_version >= ProtocolVersion::V_2026_07_28);
78+
if strict && !in_request_handler_scope {
79+
return Err(ServiceError::McpError(ErrorData::invalid_request(
80+
"SEP-2260: server-to-client requests must be associated with an originating client request",
81+
None,
82+
)));
83+
}
84+
Ok(())
85+
}
5286
}
5387

5488
/// It represents the error that may occur when serving the server.

crates/rmcp/src/task_manager.rs

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,8 +349,18 @@ impl TaskManager {
349349
let future = make_future(context);
350350
let inner = self.inner.clone();
351351
let id_for_task = task_id.clone();
352+
// SEP-2260: a task operation runs on a detached `tokio::spawn`, which
353+
// does not inherit the caller's task-local `ORIGINATING_REQUEST`
354+
// scope. Capture the originating client request id (if we are inside a
355+
// request handler, which is the normal case for `tools/call`) and
356+
// re-establish it inside the spawned task. Without this, legitimate
357+
// nested sampling/elicitation/roots requests surfaced mid-task would
358+
// be wrongly rejected as unassociated.
359+
let originating_request = crate::service::ORIGINATING_REQUEST
360+
.try_with(|id| id.clone())
361+
.ok();
352362
let handle = tokio::spawn(async move {
353-
let result = future.await;
363+
let result = run_task_operation(originating_request, future).await;
354364
let mut inner = inner.lock().expect("task manager lock poisoned");
355365
if let Some(entry) = inner.tasks.get_mut(&id_for_task) {
356366
if entry.terminal.is_none() {
@@ -538,6 +548,24 @@ fn unknown_task(task_id: &str) -> McpError {
538548
McpError::invalid_params(format!("unknown task: {task_id}"), None)
539549
}
540550

551+
/// Run a task operation future, re-establishing the originating request's
552+
/// SEP-2260 association scope if one was captured at spawn time.
553+
///
554+
/// When the operation was started from within a request handler (the normal
555+
/// `tools/call` path), `originating_request` carries that request's id and the
556+
/// future runs inside an `ORIGINATING_REQUEST` scope so nested
557+
/// sampling/elicitation/roots requests are recognized as associated. When
558+
/// there is no originating request, the future runs unscoped.
559+
async fn run_task_operation(
560+
originating_request: Option<crate::model::RequestId>,
561+
future: TaskFuture,
562+
) -> Result<CallToolResult, TaskExit> {
563+
match originating_request {
564+
Some(id) => crate::service::ORIGINATING_REQUEST.scope(id, future).await,
565+
None => future.await,
566+
}
567+
}
568+
541569
fn result_to_object(result: &CallToolResult) -> JsonObject {
542570
match serde_json::to_value(result) {
543571
Ok(serde_json::Value::Object(map)) => map,
@@ -917,4 +945,77 @@ mod tests {
917945
}
918946
panic!("task did not complete after input response");
919947
}
948+
949+
// SEP-2260 (GAP 2): a task operation runs on a detached `tokio::spawn`,
950+
// which does not inherit the caller's `ORIGINATING_REQUEST` task-local.
951+
// Verify the originating request's association scope is re-established
952+
// inside the spawned task so nested sampling/elicitation/roots requests
953+
// are recognized as associated.
954+
#[tokio::test]
955+
async fn task_operation_reestablishes_request_association_scope() {
956+
use crate::{
957+
model::RequestId,
958+
service::{ORIGINATING_REQUEST, in_request_handler_scope},
959+
};
960+
961+
let manager = TaskManager::new();
962+
let observed = Arc::new(Mutex::new(None::<bool>));
963+
let observed_in_task = observed.clone();
964+
965+
// Spawn from within a request-handler scope (as `tools/call` does).
966+
ORIGINATING_REQUEST
967+
.scope(RequestId::Number(7), async {
968+
manager.spawn(TaskOptions::default(), move |_ctx| {
969+
let observed_in_task = observed_in_task.clone();
970+
Box::pin(async move {
971+
*observed_in_task.lock().unwrap() = Some(in_request_handler_scope());
972+
Ok(ok_result("done"))
973+
})
974+
})
975+
})
976+
.await;
977+
978+
for _ in 0..100 {
979+
tokio::time::sleep(std::time::Duration::from_millis(10)).await;
980+
if let Some(scoped) = *observed.lock().unwrap() {
981+
assert!(
982+
scoped,
983+
"task operation must run inside the originating request's association scope"
984+
);
985+
return;
986+
}
987+
}
988+
panic!("task operation did not run");
989+
}
990+
991+
// SEP-2260 (GAP 2): a task spawned outside any request-handler scope must
992+
// not fabricate an association scope.
993+
#[tokio::test]
994+
async fn task_operation_without_originating_request_is_unscoped() {
995+
use crate::service::in_request_handler_scope;
996+
997+
let manager = TaskManager::new();
998+
let observed = Arc::new(Mutex::new(None::<bool>));
999+
let observed_in_task = observed.clone();
1000+
1001+
manager.spawn(TaskOptions::default(), move |_ctx| {
1002+
let observed_in_task = observed_in_task.clone();
1003+
Box::pin(async move {
1004+
*observed_in_task.lock().unwrap() = Some(in_request_handler_scope());
1005+
Ok(ok_result("done"))
1006+
})
1007+
});
1008+
1009+
for _ in 0..100 {
1010+
tokio::time::sleep(std::time::Duration::from_millis(10)).await;
1011+
if let Some(scoped) = *observed.lock().unwrap() {
1012+
assert!(
1013+
!scoped,
1014+
"task operation started without an originating request must remain unscoped"
1015+
);
1016+
return;
1017+
}
1018+
}
1019+
panic!("task operation did not run");
1020+
}
9201021
}

0 commit comments

Comments
 (0)