Skip to content

Commit 5cae935

Browse files
authored
Merge pull request #669 from atesgoral/fix/elicitation-result-validation
fix: validate elicitation replies before returning them
2 parents 2cbc16c + 5c56eb2 commit 5cae935

5 files changed

Lines changed: 253 additions & 5 deletions

File tree

‎docs/_server/elicitation.md‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,12 @@ end
8282

8383
The `requested_schema` must be a flat object schema: a top-level `type: "object"` whose `properties` are limited to
8484
primitive types (`string`, `number`, `integer`, `boolean`). Nested objects and arrays are not allowed, which keeps
85-
the schema simple enough for clients to render as a form. Per the MCP specification, the client validates
86-
the user's input against this schema before returning it, so the `content` of an `accept` response matches the requested shape.
85+
the schema simple enough for clients to render as a form.
86+
87+
On the legacy lifecycle, `create_form_elicitation`, `create_url_elicitation`, and the `input_required` shim validate reply actions.
88+
The SDK checks accepted form content against the requested schema unless `content` is omitted or `nil` (`null` on the wire).
89+
Invalid replies raise `MCP::Server::ValidationError` with a message starting with `Response validation failed:`.
90+
On the 2026-07-28 lifecycle, a retried request's `inputResponses` reach the handler through `server_context.input_response` without this check.
8791

8892
## Default Values and Enums
8993

‎lib/mcp/server_session.rb‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,29 @@ def send_to_transport_request(method, params, related_request_id: nil, timeout:
359359
timeout: timeout,
360360
}.compact
361361

362-
forward_to_transport(@transport.method(:send_request), method, params, kwargs)
362+
result = forward_to_transport(@transport.method(:send_request), method, params, kwargs)
363+
validate_elicitation_result!(result, params || {}) if method == Methods::ELICITATION_CREATE
364+
result
365+
end
366+
367+
# Validate before returning to a handler or replaying a legacy input_required result.
368+
def validate_elicitation_result!(result, params)
369+
raise Server::ValidationError, "Response validation failed: invalid `result`" unless result.is_a?(Hash)
370+
371+
action = result[:action] || result["action"]
372+
raise Server::ValidationError, "Response validation failed: invalid `action`" unless ["accept", "decline", "cancel"].include?(action)
373+
return unless action == "accept"
374+
return if (params[:mode] || params["mode"]) == "url"
375+
376+
requested_schema = params[:requestedSchema] || params["requestedSchema"]
377+
return unless requested_schema
378+
379+
content = result.fetch(:content) { result["content"] }
380+
return if content.nil?
381+
382+
Tool::OutputSchema.new(requested_schema).validate_result(content)
383+
rescue Tool::OutputSchema::ValidationError => e
384+
raise Server::ValidationError, "Response validation failed: #{e.message}"
363385
end
364386

365387
# Calls `transport_method(method, params, **supported)` where `supported` contains only the keys

‎test/mcp/server_cancellation_test.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ def handle_request(request); end
415415
(class << @mock_transport; self; end).define_method(:send_request) do |method, _params = nil, **kwargs|
416416
recorded[:method] = method
417417
recorded[:kwargs] = kwargs
418-
{}
418+
{ action: "accept", content: {} }
419419
end
420420

421421
@session.create_form_elicitation(
Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
# frozen_string_literal: true
2+
3+
require "test_helper"
4+
5+
module MCP
6+
class ServerSessionElicitationResultTest < ActiveSupport::TestCase
7+
SCHEMA = {
8+
type: "object",
9+
properties: {
10+
replicas: { type: "integer", minimum: 1 },
11+
verbose: { type: "boolean" },
12+
environment: { type: "string", enum: ["staging", "production"] },
13+
},
14+
required: ["replicas", "verbose", "environment"],
15+
additionalProperties: false,
16+
}.freeze
17+
CONTENT = { replicas: 2, verbose: false, environment: "staging" }.freeze
18+
19+
class ReplyTransport
20+
attr_accessor :reply
21+
22+
def send_request(_method, _params = nil, **_kwargs)
23+
@reply
24+
end
25+
end
26+
27+
setup do
28+
@server = Server.new(name: "elicitation_test")
29+
@transport = ReplyTransport.new
30+
@session = ServerSession.new(server: @server, transport: @transport)
31+
@session.store_client_info(client: { name: "test_client" }, capabilities: { elicitation: { form: {}, url: {} } })
32+
end
33+
34+
test "accepted form content must match the requested types, required fields and enums" do
35+
invalid_contents = [
36+
CONTENT.merge(replicas: "lots"),
37+
CONTENT.merge(replicas: 1.5),
38+
CONTENT.merge(replicas: 0),
39+
CONTENT.merge(verbose: "false"),
40+
CONTENT.merge(environment: "unlisted"),
41+
CONTENT.reject { |key, _| key == :environment },
42+
CONTENT.merge(unexpected: true),
43+
false,
44+
0,
45+
"invalid",
46+
[],
47+
]
48+
invalid_contents.each do |content|
49+
@transport.reply = { action: "accept", content: content }
50+
51+
error = assert_raises(Server::ValidationError, content.inspect) { create_form }
52+
53+
assert_match(/\AResponse validation failed: Invalid result:/, error.message)
54+
end
55+
end
56+
57+
test "accepted form replies may omit content or return null even when properties are required" do
58+
replies = [
59+
{ action: "accept" },
60+
{ "action" => "accept" },
61+
{ action: "accept", content: nil },
62+
{ "action" => "accept", "content" => nil },
63+
]
64+
replies.each do |reply|
65+
@transport.reply = reply
66+
67+
assert_same(reply, create_form)
68+
assert_same(reply, create_form(schema: { type: "object", properties: {} }))
69+
end
70+
end
71+
72+
test "accepted empty form content is checked against required properties" do
73+
@transport.reply = { action: "accept", content: {} }
74+
75+
assert_raises(Server::ValidationError) { create_form }
76+
assert_same(@transport.reply, create_form(schema: { type: "object", properties: {} }))
77+
end
78+
79+
test "a valid accepted form reply is returned unchanged" do
80+
@transport.reply = { action: "accept", content: CONTENT, _meta: { source: "test" } }
81+
82+
assert_same(@transport.reply, create_form)
83+
end
84+
85+
test "SEP-1330 enum builder content is accepted" do
86+
options = [{ value: "x", title: "X" }]
87+
schema = {
88+
type: "object",
89+
properties: {
90+
single: Elicitation::EnumSchema.untitled_single_select(values: ["x"]).to_h,
91+
titled_single: Elicitation::EnumSchema.titled_single_select(options: options).to_h,
92+
multi: Elicitation::EnumSchema.untitled_multi_select(values: ["x"]).to_h,
93+
titled_multi: Elicitation::EnumSchema.titled_multi_select(options: options).to_h,
94+
},
95+
required: ["single", "titled_single", "multi", "titled_multi"],
96+
}
97+
@transport.reply = { action: "accept", content: { single: "x", titled_single: "x", multi: ["x"], titled_multi: ["x"] } }
98+
99+
assert_same(@transport.reply, create_form(schema: schema))
100+
end
101+
102+
test "string-keyed schemas and replies are supported" do
103+
@transport.reply = { "action" => "accept", "content" => { "replicas" => 2, "verbose" => false, "environment" => "staging" } }
104+
schema = JSON.parse(JSON.generate(SCHEMA))
105+
106+
assert_same(@transport.reply, create_form(schema: schema))
107+
@transport.reply["content"]["replicas"] = "lots"
108+
109+
assert_raises(Server::ValidationError) { create_form(schema: schema) }
110+
end
111+
112+
test "form and URL replies must be objects with a known action" do
113+
[nil, [], "accept", {}, { action: nil }, { action: 1 }, { action: "approve-everything" }].each do |reply|
114+
@transport.reply = reply
115+
116+
[:create_form, :create_url].each do |request|
117+
error = assert_raises(Server::ValidationError, reply.inspect) { send(request) }
118+
119+
detail = reply.is_a?(Hash) ? "invalid `action`" : "invalid `result`"
120+
assert_equal("Response validation failed: #{detail}", error.message)
121+
end
122+
end
123+
end
124+
125+
test "transport errors are not converted to validation errors" do
126+
transport_error = Server::RequestTimeoutError.new("timed out", request_id: "el-1", timeout: 10)
127+
@transport.stubs(:send_request).raises(transport_error)
128+
129+
assert_same(transport_error, assert_raises(Server::RequestTimeoutError) { create_form })
130+
end
131+
132+
test "decline and cancel do not require or validate form content" do
133+
["decline", "cancel"].each do |action|
134+
[{ action: action }, { action: action, content: { replicas: "ignored" } }].each do |reply|
135+
@transport.reply = reply
136+
137+
assert_same(reply, create_form)
138+
assert_same(reply, create_url)
139+
end
140+
end
141+
end
142+
143+
test "URL acceptance does not require form content" do
144+
@transport.reply = { action: "accept" }
145+
146+
assert_same(@transport.reply, create_url)
147+
end
148+
149+
test "fulfilling an embedded form request validates its accepted content" do
150+
@transport.reply = { action: "accept", content: CONTENT.merge(environment: "unlisted") }
151+
152+
assert_raises(Server::ValidationError) do
153+
@session.fulfill_input_request(Methods::ELICITATION_CREATE, { requestedSchema: SCHEMA }, related_request_id: 1)
154+
end
155+
end
156+
157+
test "fulfilling a string-keyed embedded form request validates its accepted content" do
158+
@transport.reply = { action: "accept", content: CONTENT.merge(replicas: "lots") }
159+
160+
assert_raises(Server::ValidationError) do
161+
@session.fulfill_input_request(Methods::ELICITATION_CREATE, { "mode" => "form", "requestedSchema" => SCHEMA }, related_request_id: 1)
162+
end
163+
end
164+
165+
test "fulfilling an embedded URL request validates its action" do
166+
@transport.reply = { action: "approve-everything" }
167+
168+
assert_raises(Server::ValidationError) do
169+
@session.fulfill_input_request(Methods::ELICITATION_CREATE, { mode: "url" }, related_request_id: 1)
170+
end
171+
end
172+
173+
test "fulfilling a valid embedded request returns its reply unchanged" do
174+
@transport.reply = { action: "accept", content: CONTENT }
175+
176+
result = @session.fulfill_input_request(Methods::ELICITATION_CREATE, { mode: "form", requestedSchema: SCHEMA }, related_request_id: 1)
177+
178+
assert_same(@transport.reply, result)
179+
end
180+
181+
test "fulfilling other request methods does not validate them as elicitation results" do
182+
@transport.reply = { roots: [] }
183+
184+
assert_same(@transport.reply, @session.fulfill_input_request(Methods::ROOTS_LIST, nil, related_request_id: 1))
185+
end
186+
187+
test "the legacy shim does not replay invalid form content into the handler" do
188+
calls = 0
189+
delivered = nil
190+
@server.define_tool(name: "collect") do |server_context:|
191+
calls += 1
192+
if server_context.input_responses
193+
delivered = server_context.input_response("settings")
194+
Tool::Response.new([{ type: "text", text: "received" }])
195+
else
196+
Server::InputRequiredResult.new(
197+
input_requests: {
198+
"settings" => { method: Methods::ELICITATION_CREATE, params: { mode: "form", message: "Settings?", requestedSchema: SCHEMA } },
199+
},
200+
)
201+
end
202+
end
203+
@transport.reply = { action: "accept", content: CONTENT.merge(replicas: "lots") }
204+
205+
response = @session.handle({ jsonrpc: "2.0", id: 1, method: Methods::TOOLS_CALL, params: { name: "collect", arguments: {} } })
206+
207+
assert_equal(1, calls)
208+
assert_nil(delivered)
209+
assert_equal(JsonRpcHandler::ErrorCode::INTERNAL_ERROR, response.dig(:error, :code))
210+
end
211+
212+
private
213+
214+
def create_form(schema: SCHEMA)
215+
@session.create_form_elicitation(message: "Settings?", requested_schema: schema, related_request_id: 1)
216+
end
217+
218+
def create_url
219+
@session.create_url_elicitation(message: "Authorize?", url: "https://example.com/authorize", elicitation_id: "el-1", related_request_id: 1)
220+
end
221+
end
222+
end

‎test/mcp/server_session_request_timeout_test.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ def initialize(server)
1818

1919
def send_request(method, params = nil, session_id: nil, related_request_id: nil, parent_cancellation: nil, server_session: nil, timeout: nil)
2020
@requests << { method: method, timeout: timeout }
21-
{}
21+
method == Methods::ELICITATION_CREATE ? { action: "accept", content: {} } : {}
2222
end
2323

2424
def send_response(response); end

0 commit comments

Comments
 (0)